feat: implement GrpcStorageImpl#getDefaultAcl (#1802)

BenWhitehead · web-flow · commit b9b7c49fcfca · 2022-12-08T14:15:27.000-05:00
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
@@ -1241,7 +1241,7 @@ public List<Acl> listAcls() {
    *
    * @throws StorageException upon failure
    */
-  @TransportCompatibility({Transport.HTTP})
+  @TransportCompatibility({Transport.HTTP, Transport.GRPC})
   public Acl getDefaultAcl(Entity entity) {
     return storage.getDefaultAcl(getName(), entity);
   }
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
@@ -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
@@ -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,45 @@ public List<Acl> 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<ObjectAccessControl> entityPredicate =
+          objectAclEntityOrAltEq(codecs.entity().encode(entity));
+
+      //noinspection DataFlowIssue
+      Optional<ObjectAccessControl> 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) {
+      return null;
+    } catch (StorageException se) {
+      if (se.getCode() == 404) {
+        return null;
+      } else {
+        throw se;
+      }
+    }
   }
 
   @Override
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
@@ -3487,7 +3487,7 @@ PostPolicyV4 generateSignedPostPolicyV4(
    *
    * @throws StorageException upon failure
    */
-  @TransportCompatibility({Transport.HTTP})
+  @TransportCompatibility({Transport.HTTP, Transport.GRPC})
   Acl getDefaultAcl(String bucket, Entity entity);
 
   /**
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
@@ -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<ObjectAccessControl> 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
@@ -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<ObjectAccessControl> 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
@@ -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,43 @@ 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);
+  }
+
+  /** When a bucket does exist, but an acl for the specified entity is not defined return null */
+  @Test
+  public void bucket_defaultAcl_get_notFoundReturnsNull() {
+    Acl acl = retry429s(() -> storage.getDefaultAcl(bucket.getName(), User.ofAllUsers()), storage);
+
+    assertThat(acl).isNull();
+  }
+
+  /** When a bucket doesn't exist, return null for the acl value */
+  @Test
+  public void bucket_defaultAcl_get_bucket404() {
+    Acl acl =
+        retry429s(() -> storage.getDefaultAcl(bucket.getName() + "x", 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
@@ -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<>(

Original file line number	Diff line number	Diff line change
`@@ -1241,7 +1241,7 @@ public List<Acl> listAcls() {`
`1241`	`1241`	`*`
`1242`	`1242`	`* @throws StorageException upon failure`
`1243`	`1243`	`*/`
`1244`		`- @TransportCompatibility({Transport.HTTP})`
	`1244`	`+ @TransportCompatibility({Transport.HTTP, Transport.GRPC})`
`1245`	`1245`	`public Acl getDefaultAcl(Entity entity) {`
`1246`	`1246`	`return storage.getDefaultAcl(getName(), entity);`
`1247`	`1247`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3487,7 +3487,7 @@ PostPolicyV4 generateSignedPostPolicyV4(`
`3487`	`3487`	`*`
`3488`	`3488`	`* @throws StorageException upon failure`
`3489`	`3489`	`*/`
`3490`		`- @TransportCompatibility({Transport.HTTP})`
	`3490`	`+ @TransportCompatibility({Transport.HTTP, Transport.GRPC})`
`3491`	`3491`	`Acl getDefaultAcl(String bucket, Entity entity);`
`3492`	`3492`
`3493`	`3493`	`/**`