From 16c2aef4f09eccee59d1028e3bbf01c65b5982d6 Mon Sep 17 00:00:00 2001
From: Frank Natividad <frankyn@users.noreply.github.com>
Date: Tue, 14 Dec 2021 12:48:14 -0800
Subject: [PATCH] fix: do not cause a failure when encountering no bindings
 (#1177)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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  ☕️
---
 .../java/com/google/cloud/storage/PolicyHelper.java    |  6 ++----
 .../main/java/com/google/cloud/storage/Storage.java    |  8 ++++++++
 .../com/google/cloud/storage/PolicyHelperTest.java     | 10 +++++++---
 3 files changed, 17 insertions(+), 7 deletions(-)

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 e198d853b..ca3e8bc31 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 @@ 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
index 02bdb8dda..211859945 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
@@ -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
index dd5c0f78a..a34538c11 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 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);
   }
 }