fix: update bucket creation to set project as bucket.project (#1912)

BenWhitehead · web-flow · commit 65993c061498 · 2023-03-28T13:45:00.000-04:00
Creation of a bucket is modifying which field is used to specify the project a bucket
is associated with. Changing from `parent` to `bucket.project`. This change updates
our handling to use this new field.

The existing `parent` is set to the sentinel value `projects/_` which was previously
only implicitly found in the bucket name.

Update testbench tag to v0.35.0 which includes the new field behavior.

ApiaryConversions have been updated to track the project attribute when cross converting
as the property `.x_project`; this value should avoid any possible collision if the field
name project is supported in the future while the client is operating in this intervening
time. If/when StorageObject receives its own project field we should switch to using it.

Fix associated with b/254678990
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryConversions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/ApiaryConversions.java
@@ -21,6 +21,7 @@
 import static com.google.cloud.storage.Utils.ifNonNull;
 import static com.google.cloud.storage.Utils.lift;
 import static com.google.cloud.storage.Utils.nullableDateTimeCodec;
+import static com.google.cloud.storage.Utils.projectNameCodec;
 import static com.google.common.base.MoreObjects.firstNonNull;
 
 import com.google.api.client.util.Data;
@@ -84,6 +85,10 @@
 @InternalApi
 final class ApiaryConversions {
   static final ApiaryConversions INSTANCE = new ApiaryConversions();
+  // gRPC has a Bucket.project property that apiary doesn't have yet.
+  // when converting from gRPC to apiary or vice-versa we want to preserve this property. Until
+  // such a time as the apiary model has a project field, we manually apply it with this name.
+  private static final String PROJECT_ID_FIELD_NAME = "x_project";
 
   private final Codec<Entity, String> entityCodec =
       Codec.of(this::entityEncode, this::entityDecode);
@@ -323,6 +328,7 @@ private CustomerEncryption customerEncryptionDecode(StorageObject.CustomerEncryp
 
   private Bucket bucketInfoEncode(BucketInfo from) {
     Bucket to = new Bucket();
+    ifNonNull(from.getProject(), projectNameCodec::encode, p -> to.set(PROJECT_ID_FIELD_NAME, p));
     ifNonNull(from.getAcl(), toListOf(bucketAcl()::encode), to::setAcl);
     ifNonNull(from.getCors(), toListOf(cors()::encode), to::setCors);
     ifNonNull(from.getCreateTimeOffsetDateTime(), dateTimeCodec::encode, to::setTimeCreated);
@@ -395,6 +401,10 @@ private Bucket bucketInfoEncode(BucketInfo from) {
   @SuppressWarnings("deprecation")
   private BucketInfo bucketInfoDecode(com.google.api.services.storage.model.Bucket from) {
     BucketInfo.Builder to = new BucketInfo.BuilderImpl(from.getName());
+    ifNonNull(
+        from.get(PROJECT_ID_FIELD_NAME),
+        lift(String.class::cast).andThen(projectNameCodec::decode),
+        to::setProject);
     ifNonNull(from.getAcl(), toListOf(bucketAcl()::decode), to::setAcl);
     ifNonNull(from.getCors(), toListOf(cors()::decode), to::setCors);
     ifNonNull(from.getDefaultObjectAcl(), toListOf(objectAcl()::decode), to::setDefaultAcl);
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
@@ -209,7 +209,7 @@ Codec<Policy, com.google.iam.v1.Policy> policyCodec() {
 
   private BucketInfo bucketInfoDecode(Bucket from) {
     BucketInfo.Builder to = new BucketInfo.BuilderImpl(bucketNameCodec.decode(from.getName()));
-    to.setProject(from.getProject());
+    to.setProject(projectNameCodec.decode(from.getProject()));
     to.setGeneratedId(from.getBucketId());
     maybeDecodeRetentionPolicy(from, to);
     ifNonNull(from.getLocation(), to::setLocation);
@@ -302,6 +302,7 @@ private BucketInfo bucketInfoDecode(Bucket from) {
   private Bucket bucketInfoEncode(BucketInfo from) {
     Bucket.Builder to = Bucket.newBuilder();
     to.setName(bucketNameCodec.encode(from.getName()));
+    ifNonNull(from.getProject(), projectNameCodec::encode, to::setProject);
     ifNonNull(from.getGeneratedId(), to::setBucketId);
     maybeEncodeRetentionPolicy(from, to);
     ifNonNull(from.getLocation(), to::setLocation);
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
@@ -99,7 +99,6 @@
 import com.google.storage.v2.Object;
 import com.google.storage.v2.ObjectAccessControl;
 import com.google.storage.v2.ObjectChecksums;
-import com.google.storage.v2.ProjectName;
 import com.google.storage.v2.ReadObjectRequest;
 import com.google.storage.v2.RewriteObjectRequest;
 import com.google.storage.v2.RewriteResponse;
@@ -190,12 +189,15 @@ public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) {
     Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucketInfo).prepend(defaultOpts);
     GrpcCallContext grpcCallContext =
         opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
+    if (bucketInfo.getProject() == null || bucketInfo.getProject().trim().isEmpty()) {
+      bucketInfo = bucketInfo.toBuilder().setProject(getOptions().getProjectId()).build();
+    }
     com.google.storage.v2.Bucket bucket = codecs.bucketInfo().encode(bucketInfo);
     CreateBucketRequest.Builder builder =
         CreateBucketRequest.newBuilder()
             .setBucket(bucket)
             .setBucketId(bucketInfo.getName())
-            .setParent(ProjectName.format(getOptions().getProjectId()));
+            .setParent("projects/_");
     CreateBucketRequest req = opts.createBucketsRequest().apply(builder).build();
     return Retrying.run(
         getOptions(),
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/runner/registry/TestBench.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/runner/registry/TestBench.java
@@ -413,7 +413,7 @@ static final class Builder {
     private static final String DEFAULT_GRPC_BASE_URI = "http://localhost:9005";
     private static final String DEFAULT_IMAGE_NAME =
         "gcr.io/cloud-devrel-public-resources/storage-testbench";
-    private static final String DEFAULT_IMAGE_TAG = "v0.33.0";
+    private static final String DEFAULT_IMAGE_TAG = "v0.35.0";
     private static final String DEFAULT_CONTAINER_NAME = "default";
 
     private boolean ignorePullError;
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/BucketArbitraryProvider.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/BucketArbitraryProvider.java
@@ -18,8 +18,10 @@
 
 import static com.google.cloud.storage.PackagePrivateMethodWorkarounds.ifNonNull;
 
+import com.google.cloud.storage.jqwik.StorageArbitraries.ProjectID;
 import com.google.storage.v2.Bucket;
 import com.google.storage.v2.BucketName;
+import com.google.storage.v2.ProjectName;
 import java.util.Collections;
 import java.util.Set;
 import javax.annotation.ParametersAreNonnullByDefault;
@@ -72,9 +74,15 @@ public Set<Arbitrary<?>> provideFor(TypeUsage targetType, SubtypeProvider subtyp
                         StorageArbitraries.buckets().iamConfig().injectNull(0.5),
                         StorageArbitraries.buckets().labels(),
                         StorageArbitraries.etag())
+                    .as(Tuple::of),
+                Combinators.combine(
+                        StorageArbitraries.projectID().map(ProjectID::toProjectName),
+                        StorageArbitraries
+                            .alnum() // ignored for now, tuples can't be a single element
+                        )
                     .as(Tuple::of))
             .as(
-                (t1, t2, t3) -> {
+                (t1, t2, t3, t4) -> {
                   Bucket.Builder b = Bucket.newBuilder();
                   ifNonNull(t1.get1(), BucketName::getBucket, b::setBucketId);
                   ifNonNull(t1.get2(), BucketName::toString, b::setName);
@@ -100,6 +108,7 @@ public Set<Arbitrary<?>> provideFor(TypeUsage targetType, SubtypeProvider subtyp
                   ifNonNull(t3.get6(), b::setIamConfig);
                   ifNonNull(t3.get7(), b::putAllLabels);
                   ifNonNull(t3.get8(), b::setEtag);
+                  ifNonNull(t4.get1(), ProjectName::toString, b::setProject);
                   // TODO: add CustomPlacementConfig
                   return b.build();
                 });
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/StorageArbitraries.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/StorageArbitraries.java
@@ -37,6 +37,7 @@
 import com.google.storage.v2.ObjectAccessControl;
 import com.google.storage.v2.ObjectChecksums;
 import com.google.storage.v2.Owner;
+import com.google.storage.v2.ProjectName;
 import com.google.storage.v2.ProjectTeam;
 import com.google.type.Date;
 import java.math.BigInteger;
@@ -437,6 +438,10 @@ private ProjectID(String value) {
     public String get() {
       return value;
     }
+
+    public ProjectName toProjectName() {
+      return ProjectName.of(value);
+    }
   }
 
   public static Objects objects() {
