fix: do not cause a failure when encountering no bindings (#1177)

frankyn · web-flow · commit 16c2aef4f09e · 2021-12-14T20:48:14.000Z
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/java-storage/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Appropriate docs were updated (if necessary) Fixes #1175 ☕️
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
@@ -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 @@ static Policy convertFromApiPolicy(com.google.api.services.storage.model.Policy
         }
         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
@@ -3540,6 +3540,11 @@ HmacKeyMetadata updateHmacKeyState(
   /**
    * 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
@@ -3556,6 +3561,9 @@ HmacKeyMetadata updateHmacKeyState(
   /**
    * 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
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
@@ -65,11 +65,15 @@ public void testEquivalence() {
     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);
   }
 }
