From bc547ad96d459d30bc11fd26e7805dc8a2527d83 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Wed, 7 Dec 2022 19:03:52 -0500 Subject: [PATCH] feat: implement GrpcStorageImpl#getDefaultAcl --- .../google/cloud/storage/GrpcConversions.java | 5 ++- .../google/cloud/storage/GrpcStorageImpl.java | 38 ++++++++++++++++++- .../cloud/storage/StorageV2ProtoUtils.java | 10 +++++ .../storage/StorageV2ProtoUtilsTest.java | 16 ++++++++ .../google/cloud/storage/it/ITAccessTest.java | 25 +++++++++++- .../storage/it/ITBucketReadMaskTest.java | 2 +- 6 files changed, 92 insertions(+), 4 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java index a56074e386..c6f5e4dac4 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcConversions.java @@ -473,7 +473,10 @@ private Acl.Entity entityDecode(String from) { private Acl objectAclDecode(ObjectAccessControl from) { Acl.Role role = Acl.Role.valueOf(from.getRole()); Acl.Entity entity = entityCodec.decode(from.getEntity()); - Acl.Builder to = Acl.newBuilder(entity, role).setId(from.getId()); + Acl.Builder to = Acl.newBuilder(entity, role); + if (!from.getId().isEmpty()) { + to.setId(from.getId()); + } if (!from.getEtag().isEmpty()) { to.setEtag(from.getEtag()); } 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 8da0b24a24..b02ec19b8d 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 @@ -19,6 +19,7 @@ import static com.google.cloud.storage.ByteSizeConstants._16MiB; import static com.google.cloud.storage.ByteSizeConstants._256KiB; import static com.google.cloud.storage.GrpcToHttpStatusCodeTranslation.resultRetryAlgorithmToCodes; +import static com.google.cloud.storage.StorageV2ProtoUtils.objectAclEntityOrAltEq; import static com.google.cloud.storage.Utils.bucketNameCodec; import static com.google.cloud.storage.Utils.ifNonNull; import static com.google.cloud.storage.Utils.projectNameCodec; @@ -54,6 +55,7 @@ import com.google.cloud.storage.UnifiedOpts.BucketListOpt; import com.google.cloud.storage.UnifiedOpts.BucketSourceOpt; import com.google.cloud.storage.UnifiedOpts.BucketTargetOpt; +import com.google.cloud.storage.UnifiedOpts.Fields; import com.google.cloud.storage.UnifiedOpts.HmacKeyListOpt; import com.google.cloud.storage.UnifiedOpts.HmacKeySourceOpt; import com.google.cloud.storage.UnifiedOpts.HmacKeyTargetOpt; @@ -89,6 +91,7 @@ import com.google.storage.v2.ListObjectsRequest; import com.google.storage.v2.LockBucketRetentionPolicyRequest; import com.google.storage.v2.Object; +import com.google.storage.v2.ObjectAccessControl; import com.google.storage.v2.ProjectName; import com.google.storage.v2.ReadObjectRequest; import com.google.storage.v2.RewriteObjectRequest; @@ -125,6 +128,7 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.Spliterator; import java.util.Spliterators.AbstractSpliterator; @@ -903,7 +907,39 @@ public List listAcls(String bucket) { @Override public Acl getDefaultAcl(String bucket, Entity entity) { - return throwNotYetImplemented(fmtMethodName("getDefaultAcl", String.class, Entity.class)); + // Specify the read-mask to explicitly include defaultObjectAcl + Fields fields = + UnifiedOpts.fields( + ImmutableSet.of( + BucketField.ACL, // workaround for b/261771961 + BucketField.DEFAULT_OBJECT_ACL)); + GrpcCallContext grpcCallContext = GrpcCallContext.createDefault(); + GetBucketRequest req = + fields + .getBucket() + .apply(GetBucketRequest.newBuilder()) + .setName(bucketNameCodec.encode(bucket)) + .build(); + try { + com.google.storage.v2.Bucket resp = + Retrying.run( + getOptions(), + retryAlgorithmManager.getFor(req), + () -> storageClient.getBucketCallable().call(req, grpcCallContext), + Decoder.identity()); + + Predicate entityPredicate = + objectAclEntityOrAltEq(codecs.entity().encode(entity)); + + //noinspection DataFlowIssue + Optional first = + resp.getDefaultObjectAclList().stream().filter(entityPredicate).findFirst(); + + // HttpStorageRpc defaults to null if Not Found + return first.map(codecs.objectAcl()::decode).orElse(null); + } catch (NotFoundException e) { + throw StorageException.coalesce(e); + } } @Override diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java index 6cd0347936..53d355b4f5 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageV2ProtoUtils.java @@ -22,7 +22,9 @@ import com.google.protobuf.MessageOrBuilder; import com.google.protobuf.util.JsonFormat; import com.google.protobuf.util.JsonFormat.Printer; +import com.google.storage.v2.ObjectAccessControl; import com.google.storage.v2.ReadObjectRequest; +import java.util.function.Predicate; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -82,4 +84,12 @@ static String fmtProto(@NonNull final MessageOrBuilder msg) { throw new RuntimeException(e); } } + + /** + * When evaluating an {@link ObjectAccessControl} entity, look at both {@code entity} (generally + * project number format) and {@code entity_alt} (generally project id format). + */ + static Predicate objectAclEntityOrAltEq(String s) { + return oAcl -> oAcl.getEntity().equals(s) || oAcl.getEntityAlt().equals(s); + } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java index 44f3f3d739..a3b45a0653 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageV2ProtoUtilsTest.java @@ -22,7 +22,9 @@ import static org.junit.Assert.assertThrows; import com.google.cloud.storage.jqwik.StorageArbitraries; +import com.google.storage.v2.ObjectAccessControl; import com.google.storage.v2.ReadObjectRequest; +import java.util.function.Predicate; import net.jqwik.api.Arbitraries; import net.jqwik.api.Arbitrary; import net.jqwik.api.Combinators; @@ -66,6 +68,20 @@ void validation_limit_gteq_0() { () -> seekReadObjectRequest(ReadObjectRequest.getDefaultInstance(), null, -1L)); } + @Example + void objectAclEntityIdOrAltEq() { + String entity = "project-viewer-123123"; + Predicate p = StorageV2ProtoUtils.objectAclEntityOrAltEq(entity); + + ObjectAccessControl inAlt = + ObjectAccessControl.newBuilder().setEntity("something").setEntityAlt(entity).build(); + ObjectAccessControl inPrimary = + ObjectAccessControl.newBuilder().setEntity(entity).setEntityAlt("something-else").build(); + + assertThat(p.test(inAlt)).isTrue(); + assertThat(p.test(inPrimary)).isTrue(); + } + @Property(tries = 100_000) void seek(@ForAll("seekCases") SeekCase srr) { Long offset = srr.offset; 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 3af946275d..75a3d4273a 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 @@ -43,6 +43,7 @@ import com.google.cloud.storage.Storage; import com.google.cloud.storage.Storage.BlobTargetOption; import com.google.cloud.storage.Storage.BucketField; +import com.google.cloud.storage.Storage.BucketGetOption; import com.google.cloud.storage.Storage.BucketSourceOption; import com.google.cloud.storage.Storage.BucketTargetOption; import com.google.cloud.storage.StorageException; @@ -125,11 +126,33 @@ private void testBucketAclRequesterPays( assertNull(storage.getAcl(bucket.getName(), User.ofAllAuthenticatedUsers(), bucketOptions)); } + @Test + public void bucket_defaultAcl_get() { + String bucketName = bucket.getName(); + // lookup an entity from the bucket which is known to exist + Bucket bucketWithAcls = + storage.get( + bucketName, BucketGetOption.fields(BucketField.ACL, BucketField.DEFAULT_OBJECT_ACL)); + + Acl actual = bucketWithAcls.getDefaultAcl().iterator().next(); + + Acl acl = retry429s(() -> storage.getDefaultAcl(bucketName, actual.getEntity()), storage); + + assertThat(acl).isEqualTo(actual); + } + + @Test + public void bucket_defaultAcl_get_notFoundReturnsNull() { + Acl acl = retry429s(() -> storage.getDefaultAcl(bucket.getName(), User.ofAllUsers()), storage); + + assertThat(acl).isNull(); + } + @Test @CrossRun.Ignore(transports = Transport.GRPC) public void testBucketDefaultAcl() { // TODO: break this test up into each of the respective scenarios - // 1. get default ACL for specific entity + // DONE ~1. get default ACL for specific entity~ // 2. Delete a default ACL for a specific entity // 3. Create a default ACL for specific entity // 4. Update default ACL to change role of a specific entity diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBucketReadMaskTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBucketReadMaskTest.java index 62338313af..d3fa6d76be 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBucketReadMaskTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITBucketReadMaskTest.java @@ -113,7 +113,7 @@ public ImmutableList parameters() { BucketField.DEFAULT_OBJECT_ACL, (jsonT, grpcT) -> { assertThat(jsonT.getDefaultAcl()).isNotEmpty(); - assertThat(grpcT.getDefaultAcl()).isNull(); + assertThat(grpcT.getDefaultAcl()).isNull(); // workaround for b/261771961 }), new Args<>(BucketField.ENCRYPTION, LazyAssertion.equal()), new Args<>(