From c78327717a7936492161ddcc64c86374db72c48c Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 15 Dec 2022 14:29:00 -0500 Subject: [PATCH] feat: implement GrpcStorageImpl#deleteDefaultAcl (#1807) --- .../java/com/google/cloud/storage/Bucket.java | 2 +- .../google/cloud/storage/GrpcStorageImpl.java | 86 ++++++++++++----- .../com/google/cloud/storage/Storage.java | 2 +- .../google/cloud/storage/it/ITAccessTest.java | 92 ++++++++++++------- 4 files changed, 124 insertions(+), 58 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java index 84222eab2..251a74d76 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java +++ b/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); } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java index 1ced5cc8c..42f31ebcd 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java +++ b/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 entityPredicate = objectAclEntityOrAltEq(encode); + + List currentDefaultAcls = resp.getDefaultObjectAclList(); + ImmutableList 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 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 entityPredicate = objectAclEntityOrAltEq(entity); - ImmutableList collect = + ImmutableList 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 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 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 newDefaultAcls, long metageneration) { + com.google.storage.v2.Bucket update = + com.google.storage.v2.Bucket.newBuilder() + .setName(bucketNameCodec.encode(bucket)) + .addAllDefaultObjectAcl(newDefaultAcls) + .build(); + Opts opts = + Opts.from( + UnifiedOpts.fields(ImmutableSet.of(BucketField.DEFAULT_OBJECT_ACL)), + UnifiedOpts.metagenerationMatch(metageneration)); + return opts.updateBucketsRequest() + .apply(UpdateBucketRequest.newBuilder()) + .setBucket(update) + .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 181998983..610a6504e 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 @@ -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); /** diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITAccessTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITAccessTest.java index 9b7a4442e..2efe6a22b 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITAccessTest.java +++ b/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 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 defaultAcls = bucket.getDefaultAcl(); + assertThat(defaultAcls).isNotEmpty(); + + Predicate 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 expectedAcls = + dropEtags( + bucket.getDefaultAcl().stream() + .filter(isProjectEditor.negate()) + .collect(Collectors.toList())); + List actualAcls = dropEtags(bucketUpdated.getDefaultAcl()); + assertThat(actualAcls).containsAtLeastElementsIn(expectedAcls); + Optional 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