Repository: googleapis/java-storage
Pull request #1454 (2022-06-15 00:04:04)

feat: support formatted bucket name with projectID
1. Added Bucket+ProjectID model in BucketInfo and update setters and getters for Bucket name to use this new model.
1. Added setters and getters for projectID in BucketInfo and Bucket (using BucketName helper)
1. Added unformatted bucket name into property based test "bucket" vs. "projects/project-id/buckets/bucket-id"
1. Support default value when presented with unformatted bucket name to  use "_" for project id
1. Added fall for unformatted into log bucket parser in grpc conversions.
1. Tested unformatted bucket name only for log bucket name (do not do it for BucketInfo)



Patch:
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 6dcb1237..a7206811 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
@@ -518,6 +518,12 @@ public class Bucket extends BucketInfo {
       return this;
     }
 
+    @Override
+    Builder setProject(String project) {
+      infoBuilder.setProject(project);
+      return this;
+    }
+
     @Override
     Builder setGeneratedId(String generatedId) {
       infoBuilder.setGeneratedId(generatedId);
diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
index 77c1b3ee..f5da145e 100644
--- a/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
+++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/BucketInfo.java
@@ -71,7 +71,7 @@ public class BucketInfo implements Serializable {
 
   private static final long serialVersionUID = -4712013629621638459L;
   private final String generatedId;
-  private final String name;
+  private final BucketWithProject bucketWithProject;
   private final Acl.Entity owner;
   private final String selfLink;
   private final Boolean requesterPays;
@@ -332,8 +332,8 @@ public class BucketInfo implements Serializable {
   public static class Logging implements Serializable {
 
     private static final long serialVersionUID = -708892101216778492L;
-    private String logBucket;
-    private String logObjectPrefix;
+    private final BucketWithProject logBucketWithProject;
+    private final String logObjectPrefix;
 
     @Override
     public boolean equals(Object o) {
@@ -344,13 +344,13 @@ public class BucketInfo implements Serializable {
         return false;
       }
       Logging logging = (Logging) o;
-      return Objects.equals(logBucket, logging.logBucket)
+      return Objects.equals(logBucketWithProject, logging.logBucketWithProject)
           && Objects.equals(logObjectPrefix, logging.logObjectPrefix);
     }
 
     @Override
     public int hashCode() {
-      return Objects.hash(logBucket, logObjectPrefix);
+      return Objects.hash(logBucketWithProject, logObjectPrefix);
     }
 
     public static Builder newBuilder() {
@@ -359,13 +359,17 @@ public class BucketInfo implements Serializable {
 
     public Builder toBuilder() {
       Builder builder = new Builder();
-      builder.logBucket = logBucket;
+      builder.logBucketWithProject = new BucketWithProject(logBucketWithProject);
       builder.logObjectPrefix = logObjectPrefix;
       return builder;
     }
 
+    String getLogBucketProject() {
+      return logBucketWithProject.getProject();
+    }
+
     public String getLogBucket() {
-      return logBucket;
+      return logBucketWithProject.getBucketName();
     }
 
     public String getLogObjectPrefix() {
@@ -373,17 +377,26 @@ public class BucketInfo implements Serializable {
     }
 
     private Logging(Builder builder) {
-      this.logBucket = builder.logBucket;
+      this.logBucketWithProject = builder.logBucketWithProject;
       this.logObjectPrefix = builder.logObjectPrefix;
     }
 
     public static class Builder {
-      private String logBucket;
+      private BucketWithProject logBucketWithProject;
       private String logObjectPrefix;
 
+      public Builder() {
+        logBucketWithProject = new BucketWithProject();
+      }
+
+      Builder setLogBucketProject(String project) {
+        this.logBucketWithProject.setProject(project);
+        return this;
+      }
+
       /** The destination bucket where the current bucket's logs should be placed. */
       public Builder setLogBucket(String logBucket) {
-        this.logBucket = logBucket;
+        this.logBucketWithProject.setBucketName(logBucket);
         return this;
       }
 
@@ -1123,6 +1136,8 @@ public class BucketInfo implements Serializable {
   public abstract static class Builder {
     Builder() {}
 
+    abstract Builder setProject(String project);
+
     /** Sets the bucket's name. */
     public abstract Builder setName(String name);
 
@@ -1311,7 +1326,7 @@ public class BucketInfo implements Serializable {
   static final class BuilderImpl extends Builder {
 
     private String generatedId;
-    private String name;
+    private BucketWithProject bucketWithProject;
     private Acl.Entity owner;
     private String selfLink;
     private Boolean requesterPays;
@@ -1340,12 +1355,12 @@ public class BucketInfo implements Serializable {
     private Logging logging;
 
     BuilderImpl(String name) {
-      this.name = name;
+      this.bucketWithProject = new BucketWithProject(name);
     }
 
     BuilderImpl(BucketInfo bucketInfo) {
       generatedId = bucketInfo.generatedId;
-      name = bucketInfo.name;
+      bucketWithProject = bucketInfo.bucketWithProject;
       etag = bucketInfo.etag;
       createTime = bucketInfo.createTime;
       updateTime = bucketInfo.updateTime;
@@ -1376,7 +1391,13 @@ public class BucketInfo implements Serializable {
 
     @Override
     public Builder setName(String name) {
-      this.name = checkNotNull(name);
+      this.bucketWithProject.setBucketName(checkNotNull(name));
+      return this;
+    }
+
+    @Override
+    Builder setProject(String project) {
+      this.bucketWithProject.setProject(project);
       return this;
     }
 
@@ -1642,7 +1663,7 @@ public class BucketInfo implements Serializable {
 
     @Override
     public BucketInfo build() {
-      checkNotNull(name);
+      checkNotNull(bucketWithProject.getBucketName());
       return new BucketInfo(this);
     }
 
@@ -1661,7 +1682,7 @@ public class BucketInfo implements Serializable {
 
   BucketInfo(BuilderImpl builder) {
     generatedId = builder.generatedId;
-    name = builder.name;
+    bucketWithProject = builder.bucketWithProject;
     etag = builder.etag;
     createTime = builder.createTime;
     updateTime = builder.updateTime;
@@ -1690,6 +1711,10 @@ public class BucketInfo implements Serializable {
     logging = builder.logging;
   }
 
+  String getProject() {
+    return bucketWithProject.getProject();
+  }
+
   /** Returns the service-generated id for the bucket. */
   public String getGeneratedId() {
     return generatedId;
@@ -1697,7 +1722,7 @@ public class BucketInfo implements Serializable {
 
   /** Returns the bucket's name. */
   public String getName() {
-    return name;
+    return bucketWithProject.getBucketName();
   }
 
   /** Returns the bucket's owner. This is always the project team's owner group. */
@@ -2015,7 +2040,7 @@ public class BucketInfo implements Serializable {
   public int hashCode() {
     return Objects.hash(
         generatedId,
-        name,
+        bucketWithProject,
         owner,
         selfLink,
         requesterPays,
@@ -2054,7 +2079,7 @@ public class BucketInfo implements Serializable {
     }
     BucketInfo that = (BucketInfo) o;
     return Objects.equals(generatedId, that.generatedId)
-        && Objects.equals(name, that.name)
+        && Objects.equals(bucketWithProject, that.bucketWithProject)
         && Objects.equals(owner, that.owner)
         && Objects.equals(selfLink, that.selfLink)
         && Objects.equals(requesterPays, that.requesterPays)
@@ -2085,7 +2110,9 @@ public class BucketInfo implements Serializable {
 
   @Override
   public String toString() {
-    return MoreObjects.toStringHelper(this).add("name", name).toString();
+    return MoreObjects.toStringHelper(this)
+        .add("name", bucketWithProject.getBucketName())
+        .toString();
   }
 
   /**
@@ -2105,4 +2132,52 @@ public class BucketInfo implements Serializable {
   public static Builder newBuilder(String name) {
     return new BuilderImpl(name);
   }
+
+  private static class BucketWithProject implements Serializable {
+    private static final long serialVersionUID = 3620131016523045533L;
+    private String project = "_";
+    private String bucketName;
+
+    BucketWithProject() {}
+
+    BucketWithProject(String bucketName) {
+      this.bucketName = bucketName;
+    }
+
+    BucketWithProject(BucketWithProject bucketWithProject) {
+      this.project = bucketWithProject.project;
+      this.bucketName = bucketWithProject.bucketName;
+    }
+
+    void setProject(String project) {
+      this.project = project;
+    }
+
+    void setBucketName(String bucketName) {
+      this.bucketName = bucketName;
+    }
+
+    String getProject() {
+      return this.project;
+    }
+
+    String getBucketName() {
+      return this.bucketName;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+      BucketWithProject other = (BucketWithProject) o;
+      return other.project.equals(this.project) && other.bucketName.equals(this.bucketName);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(project, bucketName);
+    }
+  }
 }
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 9175eee7..594df852 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
@@ -34,6 +34,7 @@ import com.google.protobuf.Descriptors;
 import com.google.protobuf.Timestamp;
 import com.google.storage.v2.Bucket;
 import com.google.storage.v2.Bucket.Billing;
+import com.google.storage.v2.BucketName;
 import com.google.storage.v2.HmacKeyMetadata;
 import com.google.storage.v2.Object;
 import com.google.storage.v2.ObjectAccessControl;
@@ -177,7 +178,8 @@ final class GrpcConversions {
   }
 
   private BucketInfo bucketInfoDecode(Bucket from) {
-    BucketInfo.Builder to = BucketInfo.newBuilder(from.getName());
+    BucketInfo.Builder to = BucketInfo.newBuilder(BucketName.parse(from.getName()).getBucket());
+    to.setProject(from.getProject());
     to.setGeneratedId(from.getBucketId());
     if (from.hasRetentionPolicy()) {
       Bucket.RetentionPolicy retentionPolicy = from.getRetentionPolicy();
@@ -225,7 +227,8 @@ final class GrpcConversions {
 
   private Bucket bucketInfoEncode(BucketInfo from) {
     Bucket.Builder to = Bucket.newBuilder();
-    to.setName(from.getName());
+    to.setProject(from.getProject());
+    to.setName(BucketName.format(from.getProject(), from.getName()));
     to.setBucketId(from.getGeneratedId());
     if (from.getRetentionPeriodDuration() != null) {
       Bucket.RetentionPolicy.Builder retentionPolicyBuilder = to.getRetentionPolicyBuilder();
@@ -294,18 +297,35 @@ final class GrpcConversions {
 
   private Bucket.Logging loggingEncode(BucketInfo.Logging from) {
     Bucket.Logging.Builder to = Bucket.Logging.newBuilder();
-    if (from.getLogBucket() != null || from.getLogObjectPrefix() != null) {
-      to.setLogBucket(from.getLogBucket());
+    if (from.getLogObjectPrefix() != null) {
       to.setLogObjectPrefix(from.getLogObjectPrefix());
     }
+    if (from.getLogBucket() != null) {
+      // TODO: Remove unformatted bucket name support when logging bucket only supports formatted
+      // "projects/project/buckets/bucket"
+      if (from.getLogBucketProject().isEmpty()) {
+        to.setLogBucket(from.getLogBucket());
+      } else {
+        to.setLogBucket(BucketName.format(from.getLogBucketProject(), from.getLogBucket()));
+      }
+    }
     return to.build();
   }
 
   private BucketInfo.Logging loggingDecode(Bucket.Logging from) {
-    return BucketInfo.Logging.newBuilder()
-        .setLogBucket(from.getLogBucket())
-        .setLogObjectPrefix(from.getLogObjectPrefix())
-        .build();
+    BucketInfo.Logging.Builder loggingBuilder =
+        BucketInfo.Logging.newBuilder().setLogObjectPrefix(from.getLogObjectPrefix());
+    if (BucketName.isParsableFrom(from.getLogBucket())) {
+      BucketName bucketName = BucketName.parse(from.getLogBucket());
+      loggingBuilder.setLogBucket(bucketName.getBucket());
+      loggingBuilder.setLogBucketProject(bucketName.getProject());
+    } else {
+      // TODO: Remove unformatted bucket name support when logging bucket only supports formatted
+      // "projects/project/buckets/bucket"
+      loggingBuilder.setLogBucket(from.getLogBucket());
+      loggingBuilder.setLogBucketProject("");
+    }
+    return loggingBuilder.build();
   }
 
   private Bucket.Cors corsEncode(Cors from) {


Test patch:
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
index d8e649de..c63d1f64 100644
--- 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
@@ -66,8 +66,9 @@ public final class BucketArbitraryProvider implements ArbitraryProvider {
             .as(
                 (t1, t2, t3) ->
                     Bucket.newBuilder()
-                        .setBucketId(t1.get1().get())
-                        .setName(t1.get2().get())
+                        .setBucketId(t1.get1().getBucket())
+                        .setName(t1.get2().toString())
+                        .setProject(t1.get2().getProject())
                         .setStorageClass(t1.get3())
                         .setLocation(t1.get4())
                         .setLocationType(t1.get5())
diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/ObjectArbitraryProvider.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/ObjectArbitraryProvider.java
index a7eac053..b8df4daa 100644
--- a/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/ObjectArbitraryProvider.java
+++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/jqwik/ObjectArbitraryProvider.java
@@ -72,7 +72,7 @@ public final class ObjectArbitraryProvider implements ArbitraryProvider {
                 (t1, t2, t3) ->
                     Object.newBuilder()
                         .setName(t1.get1())
-                        .setBucket(t1.get2().get())
+                        .setBucket(t1.get2().toString())
                         .setGeneration(t1.get3())
                         .setMetageneration(t1.get4())
                         .setStorageClass(t1.get5())
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
index 4db0b213..b666b4c9 100644
--- 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
@@ -26,6 +26,7 @@ import com.google.storage.v2.Bucket.Encryption;
 import com.google.storage.v2.Bucket.RetentionPolicy;
 import com.google.storage.v2.Bucket.Versioning;
 import com.google.storage.v2.Bucket.Website;
+import com.google.storage.v2.BucketName;
 import com.google.storage.v2.CustomerEncryption;
 import com.google.storage.v2.ObjectAccessControl;
 import com.google.storage.v2.ObjectChecksums;
@@ -84,6 +85,28 @@ public final class StorageArbitraries {
     return Arbitraries.strings().all().ofMinLength(1).ofMaxLength(1024);
   }
 
+  public static Arbitrary<ProjectID> projectID() {
+    return Arbitraries.oneOf(
+        Combinators.combine(
+                // must start with a letter
+                Arbitraries.chars().range('a', 'z'),
+                // can only contain numbers, lowercase letters, and hyphens, and must be 6-30 chars
+                Arbitraries.strings()
+                    .withCharRange('a', 'z')
+                    .numeric()
+                    .withChars('-')
+                    .ofMinLength(4)
+                    .ofMaxLength(28),
+                // must not end with a hyphen
+                Arbitraries.chars().range('a', 'z').numeric())
+            .as(
+                (first, mid, last) -> {
+                  final StringBuilder sb = new StringBuilder();
+                  sb.append(first).append(mid).append(last);
+                  return new ProjectID(sb.toString());
+                }));
+  }
+
   /**
    * Generated bucket name based on the rules outlined in <a target="_blank" rel="noopener
    * noreferrer"
@@ -91,6 +114,10 @@ public final class StorageArbitraries {
    */
   public static Arbitrary<BucketName> bucketName() {
     return Combinators.combine(
+            Arbitraries.oneOf(
+                projectID(),
+                // Global buckets have prefix of projects/_
+                Arbitraries.of(new ProjectID("_"))),
             Arbitraries.oneOf(Arbitraries.chars().alpha(), Arbitraries.chars().numeric()),
             Arbitraries.oneOf(
                     Arbitraries.chars().alpha(),
@@ -101,34 +128,12 @@ public final class StorageArbitraries {
                 .ofMaxSize(61),
             Arbitraries.oneOf(Arbitraries.chars().alpha(), Arbitraries.chars().numeric()))
         .as(
-            (first, mid, last) -> {
+            (p, first, mid, last) -> {
               final StringBuilder sb = new StringBuilder();
               sb.append(first);
               mid.forEach(sb::append);
               sb.append(last);
-
-              return new BucketName(sb.toString());
-            });
-  }
-
-  public static Arbitrary<ProjectID> projectID() {
-    return Combinators.combine(
-            // must start with a letter
-            Arbitraries.chars().range('a', 'z'),
-            // can only contain numbers, lowercase letters, and hyphens, and must be 6-30 chars
-            Arbitraries.strings()
-                .withCharRange('a', 'z')
-                .numeric()
-                .withChars('-')
-                .ofMinLength(4)
-                .ofMaxLength(28),
-            // must not end with a hyphen
-            Arbitraries.chars().range('a', 'z').numeric())
-        .as(
-            (first, mid, last) -> {
-              final StringBuilder sb = new StringBuilder();
-              sb.append(first).append(mid).append(last);
-              return new ProjectID(sb.toString());
+              return BucketName.of(p.get(), sb.toString());
             });
   }
 
@@ -223,10 +228,18 @@ public final class StorageArbitraries {
     public Arbitrary<Bucket.Logging> logging() {
       Arbitrary<BucketName> loggingBucketName = StorageArbitraries.bucketName();
       Arbitrary<String> loggingPrefix = Arbitraries.strings().all().ofMinLength(1).ofMaxLength(10);
-      return Combinators.combine(loggingBucketName, loggingPrefix)
+      return Combinators.combine(loggingBucketName, loggingPrefix, bool())
           .as(
-              (b, p) ->
-                  Bucket.Logging.newBuilder().setLogBucket(b.get()).setLogObjectPrefix(p).build());
+              (b, p, u) -> {
+                Bucket.Logging.Builder loggingBuilder =
+                    Bucket.Logging.newBuilder().setLogObjectPrefix(p);
+                if (u == Boolean.TRUE) {
+                  loggingBuilder.setLogBucket(b.toString());
+                } else {
+                  loggingBuilder.setLogBucket(b.getBucket());
+                }
+                return loggingBuilder.build();
+              });
     }
 
     public ListArbitrary<Bucket.Cors> cors() {
@@ -350,18 +363,6 @@ public final class StorageArbitraries {
     }
   }
 
-  public static final class BucketName {
-    private final String value;
-
-    private BucketName(String value) {
-      this.value = value;
-    }
-
-    public String get() {
-      return value;
-    }
-  }
-
   public static final class ProjectID {
 
     private final String value;