feat: Store bucket name in URI authority (googleapis#1218) (googleapi…

…s#1219)

google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java

@@ -332,7 +332,9 @@ public int hashCode() {
   @Override
   public String toString() {
     try {
-      return new URI(URI_SCHEME, bucket, null, null).toString();
+      // Store GCS bucket name in the URI authority, see
+      // https://github.com/googleapis/java-storage-nio/issues/1218
+      return new URI(URI_SCHEME, bucket, null, null, null).toString();
     } catch (URISyntaxException e) {
       throw new AssertionError(e);
     }

google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java

@@ -254,8 +254,12 @@ public CloudStorageFileSystem newFileSystem(URI uri, Map<String, ?> env) {
         "Cloud Storage URIs must have '%s' scheme: %s",
         CloudStorageFileSystem.URI_SCHEME,
         uri);
+    // Bucket names may not be compatible with getHost(), see
+    // https://github.com/googleapis/java-storage-nio/issues/1218
+    // However, there may be existing code expecting the exception message to refer to the bucket
+    // name as the "host".
     checkArgument(
-        !isNullOrEmpty(uri.getHost()),
+        !isNullOrEmpty(uri.getAuthority()),
         "%s:// URIs must have a host: %s",
         CloudStorageFileSystem.URI_SCHEME,
         uri);
@@ -266,11 +270,11 @@ && isNullOrEmpty(uri.getFragment())
             && isNullOrEmpty(uri.getUserInfo()),
         "GCS FileSystem URIs mustn't have: port, userinfo, query, or fragment: %s",
         uri);
-    CloudStorageUtil.checkBucket(uri.getHost());
+    CloudStorageUtil.checkBucket(uri.getAuthority());
     initStorage();
     return new CloudStorageFileSystem(
         this,
-        uri.getHost(),
+        uri.getAuthority(),
         CloudStorageConfiguration.fromMap(
             CloudStorageFileSystem.getDefaultCloudStorageConfiguration(), env));
   }

google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStoragePath.java

@@ -368,10 +368,22 @@ public String toString() {
   @Override
   public URI toUri() {
     try {
+      // First try storing GCS bucket name in the hostname for compatibility with earlier behavior.
       return new URI(
           CloudStorageFileSystem.URI_SCHEME, bucket(), path.toAbsolutePath().toString(), null);
     } catch (URISyntaxException e) {
-      throw new AssertionError(e);
+      try {
+        // Store GCS bucket name in the URI authority, see
+        // https://github.com/googleapis/java-storage-nio/issues/1218
+        return new URI(
+            CloudStorageFileSystem.URI_SCHEME,
+            bucket(),
+            path.toAbsolutePath().toString(),
+            null,
+            null);
+      } catch (URISyntaxException unused) {
+        throw new AssertionError(e);
+      }
     }
   }
 

google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageUtil.java

@@ -62,7 +62,14 @@ static URI stripPathFromUri(URI uri) {
           uri.getQuery(),
           uri.getFragment());
     } catch (URISyntaxException e) {
-      throw new IllegalArgumentException(e.getMessage());
+      try {
+        // Store GCS bucket name in the URI authority, see
+        // https://github.com/googleapis/java-storage-nio/issues/1218
+        return new URI(
+            uri.getScheme(), uri.getAuthority(), null, uri.getQuery(), uri.getFragment());
+      } catch (URISyntaxException unused) {
+        throw new IllegalArgumentException(e.getMessage());
+      }
     }
   }
 

google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java

@@ -815,6 +815,50 @@ public void testFromSpace() throws Exception {
     assertThat(path4.toString()).isEqualTo("/with/a%20percent");
   }
 
+  @Test
+  public void testBucketWithHost() {
+    // User should be able to create buckets whose name contains a host name.
+    Path path1 = Paths.get(URI.create("gs://bucket-with-host/path"));
+    CloudStorageFileSystemProvider provider =
+        (CloudStorageFileSystemProvider) path1.getFileSystem().provider();
+
+    Path path2 = provider.getPath("gs://bucket-with-host/path");
+    // Both approaches should be equivalent
+    assertThat(path1.getFileSystem().provider()).isEqualTo(path2.getFileSystem().provider());
+    assertThat(path1.toUri()).isEqualTo(path2.toUri());
+    assertThat(path1.toUri().getHost()).isEqualTo("bucket-with-host");
+    assertThat(path1.toUri().getAuthority()).isEqualTo("bucket-with-host");
+  }
+
+  @Test
+  public void testBucketWithAuthority() {
+    // User should be able to create buckets whose name contains an authority that is not a host.
+    Path path1 = Paths.get(URI.create("gs://bucket_with_authority/path"));
+    CloudStorageFileSystemProvider provider =
+        (CloudStorageFileSystemProvider) path1.getFileSystem().provider();
+
+    Path path2 = provider.getPath("gs://bucket_with_authority/path");
+    // Both approaches should be equivalent
+    assertThat(path1.getFileSystem().provider()).isEqualTo(path2.getFileSystem().provider());
+    assertThat(path1.toUri()).isEqualTo(path2.toUri());
+    assertThat(path1.toUri().getHost()).isNull();
+    assertThat(path1.toUri().getAuthority()).isEqualTo("bucket_with_authority");
+  }
+
+  @Test
+  public void testBucketWithoutAuthority() {
+    Path path1 = Paths.get(URI.create("gs://bucket_with_authority/path"));
+    CloudStorageFileSystemProvider provider =
+        (CloudStorageFileSystemProvider) path1.getFileSystem().provider();
+
+    try {
+      provider.getFileSystem(URI.create("gs:///path"));
+      Assert.fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e.getMessage()).isEqualTo("gs:// URIs must have a host: gs:///path");
+    }
+  }
+
   @Test
   public void testVersion_matchesAcceptablePatterns() {
     String acceptableVersionPattern = "|(?:\\d+\\.\\d+\\.\\d+(?:-.*?)?(?:-SNAPSHOT)?)";

google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java

@@ -139,6 +139,48 @@ public void testGetPath() throws IOException {
     }
   }
 
+  @Test
+  public void testGetHost_valid_dns() throws IOException {
+    try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket-with-host")) {
+      assertThat(fs.getPath("/angel").toUri().getHost()).isEqualTo("bucket-with-host");
+    }
+  }
+
+  @Test
+  public void testGetHost_invalid_dns() throws IOException {
+    try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket_with_authority")) {
+      assertThat(fs.getPath("/angel").toUri().getHost()).isNull();
+    }
+  }
+
+  @Test
+  public void testGetAuthority_valid_dns() throws IOException {
+    try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket-with-host")) {
+      assertThat(fs.getPath("/angel").toUri().getAuthority()).isEqualTo("bucket-with-host");
+    }
+  }
+
+  @Test
+  public void testGetAuthority_invalid_dns() throws IOException {
+    try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket_with_authority")) {
+      assertThat(fs.getPath("/angel").toUri().getAuthority()).isEqualTo("bucket_with_authority");
+    }
+  }
+
+  @Test
+  public void testToString_valid_dns() throws IOException {
+    try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket-with-host")) {
+      assertThat(fs.toString()).isEqualTo("gs://bucket-with-host");
+    }
+  }
+
+  @Test
+  public void testToString_invalid_dns() throws IOException {
+    try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket_with_authority")) {
+      assertThat(fs.toString()).isEqualTo("gs://bucket_with_authority");
+    }
+  }
+
   @Test
   public void testWrite() throws IOException {
     try (FileSystem fs = CloudStorageFileSystem.forBucket("bucket")) {