Repository: googleapis/java-storage
Pull request #1255 (2022-02-15 19:42:50)

fix: cherry-pick #1177

codecov[bot]: # [Codecov](https://codecov.io/gh/googleapis/java-storage/pull/1255?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis) Report
> Merging [#1255](https://codecov.io/gh/googleapis/java-storage/pull/1255?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis) (5cccbb6) into [1.113.14-sp](https://codecov.io/gh/googleapis/java-storage/commit/2084a23cba09c9c1721d0daabd72498af490a874?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis) (2084a23) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/googleapis/java-storage/pull/1255/graphs/tree.svg?width=650&height=150&src=pr&token=ssm139YOdp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis)](https://codecov.io/gh/googleapis/java-storage/pull/1255?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis)

```diff
@@                Coverage Diff                @@
##             1.113.14-sp    #1255      +/-   ##
=================================================
+ Coverage          64.43%   64.47%   +0.04%     
+ Complexity           642      633       -9     
=================================================
  Files                 32       32              
  Lines               5336     5334       -2     
  Branches             526      527       +1     
=================================================
+ Hits                3438     3439       +1     
+ Misses              1733     1730       -3     
  Partials             165      165              
```


| [Impacted Files](https://codecov.io/gh/googleapis/java-storage/pull/1255?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis) | Coverage Δ | |
|---|---|---|
| [...rc/main/java/com/google/cloud/storage/Storage.java](https://codecov.io/gh/googleapis/java-storage/pull/1255/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis#diff-Z29vZ2xlLWNsb3VkLXN0b3JhZ2Uvc3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3N0b3JhZ2UvU3RvcmFnZS5qYXZh) | `80.83% <ø> (ø)` | |
| [...in/java/com/google/cloud/storage/PolicyHelper.java](https://codecov.io/gh/googleapis/java-storage/pull/1255/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis#diff-Z29vZ2xlLWNsb3VkLXN0b3JhZ2Uvc3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3N0b3JhZ2UvUG9saWN5SGVscGVyLmphdmE=) | `62.85% <100.00%> (-2.01%)` | :arrow_down: |
| [...gle/cloud/storage/testing/RemoteStorageHelper.java](https://codecov.io/gh/googleapis/java-storage/pull/1255/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis#diff-Z29vZ2xlLWNsb3VkLXN0b3JhZ2Uvc3JjL21haW4vamF2YS9jb20vZ29vZ2xlL2Nsb3VkL3N0b3JhZ2UvdGVzdGluZy9SZW1vdGVTdG9yYWdlSGVscGVyLmphdmE=) | `64.00% <0.00%> (+2.39%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/googleapis/java-storage/pull/1255?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/googleapis/java-storage/pull/1255?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis). Last update [2084a23...5cccbb6](https://codecov.io/gh/googleapis/java-storage/pull/1255?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=googleapis).


Patch:
diff --git a/google-cloud-storage/clirr-ignored-differences.xml b/google-cloud-storage/clirr-ignored-differences.xml
index 85c4887d..1fe6a791 100644
--- a/google-cloud-storage/clirr-ignored-differences.xml
+++ b/google-cloud-storage/clirr-ignored-differences.xml
@@ -36,4 +36,9 @@
         <method>com.google.api.services.storage.model.StorageObject queryCompletedResumableUpload(java.lang.String, long)</method>
         <differenceType>7012</differenceType>
     </difference>
+    <difference>
+        <className>com/google/cloud/storage/BucketInfo$LifecycleRule$LifecycleAction</className>
+        <method>BucketInfo$LifecycleRule$LifecycleAction()</method>
+        <differenceType>7004</differenceType>
+    </difference>
 </differences>
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
index c1339523..099b50ee 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
@@ -462,8 +462,13 @@ public class BucketInfo implements Serializable {
                   StorageClass.valueOf(action.getStorageClass()));
           break;
         default:
-          throw new UnsupportedOperationException(
-              "The specified lifecycle action " + action.getType() + " is not currently supported");
+          log.warning(
+              "The lifecycle action "
+                  + action.getType()
+                  + " is not supported by this version of the library. "
+                  + "Attempting to update with this rule may cause errors. Please "
+                  + "update to the latest version of google-cloud-storage.");
+          lifecycleAction = LifecycleAction.newLifecycleAction("Unknown action");
       }
 
       Rule.Condition condition = rule.getCondition();
@@ -713,13 +718,21 @@ public class BucketInfo implements Serializable {
     }
 
     /**
-     * Base class for the Action to take when a Lifecycle Condition is met. Specific Actions are
+     * Base class for the Action to take when a Lifecycle Condition is met. Supported Actions are
      * expressed as subclasses of this class, accessed by static factory methods.
      */
-    public abstract static class LifecycleAction implements Serializable {
+    public static class LifecycleAction implements Serializable {
       private static final long serialVersionUID = 5801228724709173284L;
 
-      public abstract String getActionType();
+      private final String actionType;
+
+      public LifecycleAction(String actionType) {
+        this.actionType = actionType;
+      }
+
+      public String getActionType() {
+        return actionType;
+      }
 
       @Override
       public String toString() {
@@ -744,17 +757,24 @@ public class BucketInfo implements Serializable {
           StorageClass storageClass) {
         return new SetStorageClassLifecycleAction(storageClass);
       }
+
+      /**
+       * Creates a new {@code LifecycleAction , with no specific supported action associated with it. This
+       * is only intended as a "backup" for when the library doesn't recognize the type, and should
+       * generally not be used, instead use the supported actions, and upgrade the library if necessary
+       * to get new supported actions.
+       */
+      public static LifecycleAction newLifecycleAction(String actionType) {
+        return new LifecycleAction(actionType);
+      }
     }
 
     public static class DeleteLifecycleAction extends LifecycleAction {
       public static final String TYPE = "Delete";
       private static final long serialVersionUID = -2050986302222644873L;
 
-      private DeleteLifecycleAction() {}
-
-      @Override
-      public String getActionType() {
-        return TYPE;
+      private DeleteLifecycleAction() {
+        super(TYPE);
       }
     }
 
@@ -765,14 +785,10 @@ public class BucketInfo implements Serializable {
       private final StorageClass storageClass;
 
       private SetStorageClassLifecycleAction(StorageClass storageClass) {
+        super(TYPE);
         this.storageClass = storageClass;
       }
 
-      @Override
-      public String getActionType() {
-        return TYPE;
-      }
-
       @Override
       public String toString() {
         return MoreObjects.toStringHelper(this)
@@ -921,7 +937,11 @@ public class BucketInfo implements Serializable {
 
     @Override
     void populateCondition(Rule.Condition condition) {
-      throw new UnsupportedOperationException();
+      log.warning(
+          "The lifecycle condition "
+              + condition
+              + " is not currently supported. Please update to the latest version of google-cloud-java."
+              + " Also, use LifecycleRule rather than the deprecated DeleteRule.");
     }
 
     private void writeObject(ObjectOutputStream out) throws IOException {
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/PolicyHelper.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/PolicyHelper.java
index e198d853..ca3e8bc3 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/PolicyHelper.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/PolicyHelper.java
@@ -34,8 +34,8 @@ class PolicyHelper {
   static Policy convertFromApiPolicy(com.google.api.services.storage.model.Policy apiPolicy) {
     Policy.Builder policyBuilder = Policy.newBuilder();
     List<Bindings> bindings = apiPolicy.getBindings();
+    ImmutableList.Builder<Binding> coreBindings = ImmutableList.builder();
     if (null != bindings && !bindings.isEmpty()) {
-      ImmutableList.Builder<Binding> coreBindings = ImmutableList.builder();
       for (Bindings binding : bindings) {
         Binding.Builder bindingBuilder = Binding.newBuilder();
         bindingBuilder.setRole(binding.getRole());
@@ -49,10 +49,8 @@ class PolicyHelper {
         }
         coreBindings.add(bindingBuilder.build());
       }
-      policyBuilder.setBindings(coreBindings.build());
-    } else {
-      throw new IllegalStateException("Missing required bindings.");
     }
+    policyBuilder.setBindings(coreBindings.build());
     return policyBuilder.setEtag(apiPolicy.getEtag()).setVersion(apiPolicy.getVersion()).build();
   }
 
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
index 08eec26a..d7c6254b 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java
@@ -3516,6 +3516,11 @@ public interface Storage extends Service<StorageOptions> {
   /**
    * Gets the IAM policy for the provided bucket.
    *
+   * <p>It's possible for bindings to be empty and instead have permissions inherited through
+   * Project or Organization IAM Policies. To prevent corrupting policies when you update an IAM
+   * policy with {@code Storage.setIamPolicy}, the ETAG value is used to perform optimistic
+   * concurrency.
+   *
    * <p>Example of getting the IAM policy for a bucket.
    *
    * <pre>{@code
@@ -3532,6 +3537,9 @@ public interface Storage extends Service<StorageOptions> {
   /**
    * Updates the IAM policy on the specified bucket.
    *
+   * <p>To prevent corrupting policies when you update an IAM policy with {@code
+   * Storage.setIamPolicy}, the ETAG value is used to perform optimistic concurrency.
+   *
    * <p>Example of updating the IAM policy on a bucket.
    *
    * <pre>{@code


Test patch:
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java
index d72901c1..9c66533f 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/BucketInfoTest.java
@@ -306,6 +306,10 @@ public class BucketInfoTest {
     for (DeleteRule delRule : rules) {
       assertEquals(delRule, DeleteRule.fromPb(delRule.toPb()));
     }
+    Rule unsupportedRule =
+        new Rule().setAction(new Rule.Action().setType("This action doesn't exist"));
+    DeleteRule.fromPb(
+        unsupportedRule); // if this doesn't throw an exception, unsupported rules work
   }
 
   @Test
@@ -355,6 +359,17 @@ public class BucketInfoTest {
     assertEquals(StorageClass.COLDLINE.toString(), lifecycleRule.getAction().getStorageClass());
     assertEquals(30, lifecycleRule.getCondition().getDaysSinceCustomTime().intValue());
     assertNotNull(lifecycleRule.getCondition().getCustomTimeBefore());
+
+    Rule unsupportedRule =
+        new LifecycleRule(
+                LifecycleAction.newLifecycleAction("This action type doesn't exist"),
+                LifecycleCondition.newBuilder().setAge(10).build())
+            .toPb();
+    unsupportedRule.setAction(
+        unsupportedRule.getAction().setType("This action type also doesn't exist"));
+
+    LifecycleRule.fromPb(
+        unsupportedRule); // If this doesn't throw an exception, unsupported rules are working
   }
 
   @Test
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/PolicyHelperTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/PolicyHelperTest.java
index dd5c0f78..a34538c1 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/PolicyHelperTest.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/PolicyHelperTest.java
@@ -65,11 +65,15 @@ public class PolicyHelperTest {
     assertTrue(new ApiPolicyMatcher(apiPolicy).matches(actualApiPolicy));
   }
 
-  @Test(expected = IllegalStateException.class)
+  @Test
   public void testApiPolicyWithoutBinding() {
     List<Bindings> bindings = null;
     com.google.api.services.storage.model.Policy apiPolicy =
-        new com.google.api.services.storage.model.Policy().setBindings(bindings).setEtag(ETAG);
-    PolicyHelper.convertFromApiPolicy(apiPolicy);
+        new com.google.api.services.storage.model.Policy()
+            .setBindings(bindings)
+            .setEtag(ETAG)
+            .setVersion(1);
+    Policy policy = PolicyHelper.convertFromApiPolicy(apiPolicy);
+    assertEquals(policy.getBindings().size(), 0);
   }
 }