feat: implement GrpcStorageImpl#deleteDefaultAcl (#1807)

Author: BenWhitehead

google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java

@@ -1266,7 +1266,7 @@ public Acl getDefaultAcl(Entity entity) {
    * @return {@code true} if the ACL was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
-  @TransportCompatibility({Transport.HTTP})
+  @TransportCompatibility({Transport.HTTP, Transport.GRPC})
   public boolean deleteDefaultAcl(Entity entity) {
     return storage.deleteDefaultAcl(getName(), entity);
   }

google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java

@@ -932,7 +932,40 @@ public Acl getDefaultAcl(String bucket, Entity entity) {
 
   @Override
   public boolean deleteDefaultAcl(String bucket, Entity entity) {
-    return throwNotYetImplemented(fmtMethodName("deleteDefaultAcl", String.class, Entity.class));
+    try {
+      com.google.storage.v2.Bucket resp = getBucketDefaultAcls(bucket);
+      String encode = codecs.entity().encode(entity);
+
+      Predicate<ObjectAccessControl> entityPredicate = objectAclEntityOrAltEq(encode);
+
+      List<ObjectAccessControl> currentDefaultAcls = resp.getDefaultObjectAclList();
+      ImmutableList<ObjectAccessControl> newDefaultAcls =
+          currentDefaultAcls.stream()
+              .filter(entityPredicate.negate())
+              .collect(ImmutableList.toImmutableList());
+      if (newDefaultAcls.equals(currentDefaultAcls)) {
+        // we didn't actually filter anything out, no need to send an RPC, simply return false
+        return false;
+      }
+      long metageneration = resp.getMetageneration();
+
+      UpdateBucketRequest req = createUpdateRequest(bucket, newDefaultAcls, metageneration);
+
+      com.google.storage.v2.Bucket updateResult = updateBucket(req);
+      // read the response to ensure there is no longer an acl for the specified entity
+      Optional<ObjectAccessControl> first =
+          updateResult.getDefaultObjectAclList().stream().filter(entityPredicate).findFirst();
+      return !first.isPresent();
+    } catch (NotFoundException e) {
+      // HttpStorageRpc returns false if the bucket doesn't exist :(
+      return false;
+    } catch (StorageException se) {
+      if (se.getCode() == 404) {
+        return false;
+      } else {
+        throw se;
+      }
+    }
   }
 
   @Override
@@ -949,36 +982,17 @@ public Acl updateDefaultAcl(String bucket, Acl acl) {
 
       Predicate<ObjectAccessControl> entityPredicate = objectAclEntityOrAltEq(entity);
 
-      ImmutableList<ObjectAccessControl> collect =
+      ImmutableList<ObjectAccessControl> newDefaultAcls =
           Streams.concat(
                   resp.getDefaultObjectAclList().stream().filter(entityPredicate.negate()),
                   Stream.of(encode))
               .collect(ImmutableList.toImmutableList());
 
-      com.google.storage.v2.Bucket update =
-          com.google.storage.v2.Bucket.newBuilder()
-              .setName(bucketNameCodec.encode(bucket))
-              .addAllDefaultObjectAcl(collect)
-              .build();
-      Opts<BucketTargetOpt> opts =
-          Opts.from(
-              UnifiedOpts.fields(ImmutableSet.of(BucketField.DEFAULT_OBJECT_ACL)),
-              UnifiedOpts.metagenerationMatch(resp.getMetageneration()));
       UpdateBucketRequest req =
-          opts.updateBucketsRequest()
-              .apply(UpdateBucketRequest.newBuilder())
-              .setBucket(update)
-              .build();
+          createUpdateRequest(bucket, newDefaultAcls, resp.getMetageneration());
 
-      GrpcCallContext grpcCallContext = GrpcCallContext.createDefault();
-      com.google.storage.v2.Bucket updateResult =
-          Retrying.run(
-              getOptions(),
-              retryAlgorithmManager.getFor(req),
-              () -> storageClient.updateBucketCallable().call(req, grpcCallContext),
-              Decoder.identity());
+      com.google.storage.v2.Bucket updateResult = updateBucket(req);
 
-      //noinspection DataFlowIssue
       Optional<Acl> first =
           updateResult.getDefaultObjectAclList().stream()
               .filter(entityPredicate)
@@ -1488,4 +1502,30 @@ private com.google.storage.v2.Bucket getBucketDefaultAcls(String bucketName) {
         () -> storageClient.getBucketCallable().call(req, grpcCallContext),
         Decoder.identity());
   }
+
+  private com.google.storage.v2.Bucket updateBucket(UpdateBucketRequest req) {
+    GrpcCallContext grpcCallContext = GrpcCallContext.createDefault();
+    return Retrying.run(
+        getOptions(),
+        retryAlgorithmManager.getFor(req),
+        () -> storageClient.updateBucketCallable().call(req, grpcCallContext),
+        Decoder.identity());
+  }
+
+  private static UpdateBucketRequest createUpdateRequest(
+      String bucket, ImmutableList<ObjectAccessControl> newDefaultAcls, long metageneration) {
+    com.google.storage.v2.Bucket update =
+        com.google.storage.v2.Bucket.newBuilder()
+            .setName(bucketNameCodec.encode(bucket))
+            .addAllDefaultObjectAcl(newDefaultAcls)
+            .build();
+    Opts<BucketTargetOpt> opts =
+        Opts.from(
+            UnifiedOpts.fields(ImmutableSet.of(BucketField.DEFAULT_OBJECT_ACL)),
+            UnifiedOpts.metagenerationMatch(metageneration));
+    return opts.updateBucketsRequest()
+        .apply(UpdateBucketRequest.newBuilder())
+        .setBucket(update)
+        .build();
+  }
 }

google-cloud-storage/src/main/java/com/google/cloud/storage/Storage.java

@@ -3511,7 +3511,7 @@ PostPolicyV4 generateSignedPostPolicyV4(
    * @return {@code true} if the ACL was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
-  @TransportCompatibility({Transport.HTTP})
+  @TransportCompatibility({Transport.HTTP, Transport.GRPC})
   boolean deleteDefaultAcl(String bucket, Entity entity);
 
   /**

google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITAccessTest.java

@@ -70,6 +70,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.function.Predicate;
@@ -286,40 +287,65 @@ public void bucket_defaultAcl_update_bucket404() {
   }
 
   @Test
-  @CrossRun.Ignore(transports = Transport.GRPC)
-  public void testBucketDefaultAcl() {
-    // TODO: break this test up into each of the respective scenarios
-    //   2. Delete a default ACL for a specific entity
-    //   4. Update default ACL to change role of a specific entity
-
-    // according to https://cloud.google.com/storage/docs/access-control/lists#default
-    // it can take up to 30 seconds for default acl updates to propagate
-    // Since this test is performing so many mutations to default acls there are several calls
-    // that are otherwise non-idempotent wrapped with retries.
-    assertNull(
-        retry429s(
-            () -> storage.getDefaultAcl(bucket.getName(), User.ofAllAuthenticatedUsers()),
-            storage));
-    assertFalse(
-        retry429s(
-            () -> storage.deleteDefaultAcl(bucket.getName(), User.ofAllAuthenticatedUsers()),
-            storage));
-    Acl acl = Acl.of(User.ofAllAuthenticatedUsers(), Role.READER);
-    assertNotNull(retry429s(() -> storage.createDefaultAcl(bucket.getName(), acl), storage));
-    Acl updatedAcl =
-        retry429s(
-            () ->
-                storage.updateDefaultAcl(
-                    bucket.getName(), acl.toBuilder().setRole(Role.OWNER).build()),
-            storage);
-    assertEquals(Role.OWNER, updatedAcl.getRole());
-    Set<Acl> acls = new HashSet<>(storage.listDefaultAcls(bucket.getName()));
-    assertTrue(acls.contains(updatedAcl));
-    assertTrue(
+  public void bucket_defaultAcl_delete() throws Exception {
+    BucketInfo bucketInfo = BucketInfo.newBuilder(generator.randomBucketName()).build();
+    try (TemporaryBucket tempB =
+        TemporaryBucket.newBuilder().setBucketInfo(bucketInfo).setStorage(storage).build()) {
+      BucketInfo bucket = tempB.getBucket();
+
+      List<Acl> defaultAcls = bucket.getDefaultAcl();
+      assertThat(defaultAcls).isNotEmpty();
+
+      Predicate<Acl> isProjectEditor = hasProjectRole(ProjectRole.VIEWERS);
+
+      //noinspection OptionalGetWithoutIsPresent
+      Acl projectViewerAsReader =
+          defaultAcls.stream().filter(hasRole(Role.READER).and(isProjectEditor)).findFirst().get();
+
+      Entity entity = projectViewerAsReader.getEntity();
+
+      boolean actual = retry429s(() -> storage.deleteDefaultAcl(bucket.getName(), entity), storage);
+
+      assertThat(actual).isTrue();
+
+      Bucket bucketUpdated =
+          storage.get(bucket.getName(), BucketGetOption.fields(BucketField.values()));
+      assertThat(bucketUpdated.getMetageneration()).isNotEqualTo(bucket.getMetageneration());
+
+      // etags change when deletes happen, drop before our comparison
+      List<Acl> expectedAcls =
+          dropEtags(
+              bucket.getDefaultAcl().stream()
+                  .filter(isProjectEditor.negate())
+                  .collect(Collectors.toList()));
+      List<Acl> actualAcls = dropEtags(bucketUpdated.getDefaultAcl());
+      assertThat(actualAcls).containsAtLeastElementsIn(expectedAcls);
+      Optional<Entity> search =
+          actualAcls.stream().map(Acl::getEntity).filter(e -> e.equals(entity)).findAny();
+      assertThat(search.isPresent()).isFalse();
+    }
+  }
+
+  @Test
+  public void bucket_defaultAcl_delete_bucket404() {
+    boolean actual =
         retry429s(
-            () -> storage.deleteDefaultAcl(bucket.getName(), User.ofAllAuthenticatedUsers()),
-            storage));
-    assertNull(storage.getDefaultAcl(bucket.getName(), User.ofAllAuthenticatedUsers()));
+            () -> storage.deleteDefaultAcl(bucket.getName() + "x", User.ofAllUsers()), storage);
+
+    assertThat(actual).isEqualTo(false);
+  }
+
+  @Test
+  public void bucket_defaultAcl_delete_noExistingAcl() throws Exception {
+    BucketInfo bucketInfo = BucketInfo.newBuilder(generator.randomBucketName()).build();
+    try (TemporaryBucket tempB =
+        TemporaryBucket.newBuilder().setBucketInfo(bucketInfo).setStorage(storage).build()) {
+      BucketInfo bucket = tempB.getBucket();
+      boolean actual =
+          retry429s(() -> storage.deleteDefaultAcl(bucket.getName(), User.ofAllUsers()), storage);
+
+      assertThat(actual).isEqualTo(false);
+    }
   }
 
   @Test