Repository: googleapis/java-storage-nio
Pull request #1219 (2023-07-14 17:54:27)

feat: Store bucket name in URI authority
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/java-storage-nio/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [ ] Ensure the tests and linter pass
- [x] Code coverage does not decrease (if any source code was changed)
- [x] Appropriate docs were updated (if necessary)

Fixes https://github.com/googleapis/java-storage-nio/issues/1218 ☕️


google-cla[bot]: Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this [failed invocation](https://github.com/googleapis/java-storage-nio/pull/1219/checks?check_run_id=15053327819) of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.
conventional-commit-lint-gcf[bot]: <!-- probot comment [1356997]-->
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use `automerge` label. Good luck human!

 -- conventional-commit-lint bot
https://conventionalcommits.org/

Patch:
diff --git a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java
index 8f923c5..b1cc374 100644
--- a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java
+++ b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java
@@ -332,7 +332,9 @@ public final class CloudStorageFileSystem extends FileSystem {
   @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);
     }
diff --git a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
index bba5d87..823e99e 100644
--- a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
+++ b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
@@ -254,8 +254,12 @@ public final class CloudStorageFileSystemProvider extends FileSystemProvider {
         "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 @@ public final class CloudStorageFileSystemProvider extends FileSystemProvider {
             && 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));
   }
diff --git a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStoragePath.java b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStoragePath.java
index 29b3222..1c49f8e 100644
--- a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStoragePath.java
+++ b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStoragePath.java
@@ -368,10 +368,22 @@ public final class CloudStoragePath implements Path {
   @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);
+      }
     }
   }
 
diff --git a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageUtil.java b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageUtil.java
index eda81fc..3802fef 100644
--- a/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageUtil.java
+++ b/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageUtil.java
@@ -62,7 +62,14 @@ final class CloudStorageUtil {
           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());
+      }
     }
   }
 
diff --git a/pom.xml b/pom.xml
index f46cdb8..dc21a66 100644
--- a/pom.xml
+++ b/pom.xml
@@ -14,7 +14,7 @@
   <parent>
     <groupId>com.google.cloud</groupId>
     <artifactId>google-cloud-shared-config</artifactId>
-    <version>1.5.6</version>
+    <version>1.5.7</version>
   </parent>
 
   <developers>
diff --git a/samples/snippets/pom.xml b/samples/snippets/pom.xml
index c4f04e9..65980fd 100644
--- a/samples/snippets/pom.xml
+++ b/samples/snippets/pom.xml
@@ -30,7 +30,7 @@
       <dependency>
         <groupId>com.google.cloud</groupId>
         <artifactId>libraries-bom</artifactId>
-        <version>26.18.0</version>
+        <version>26.19.0</version>
         <type>pom</type>
         <scope>import</scope>
       </dependency>


Test patch:
diff --git a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java
index f44a932..7f6bc0d 100644
--- a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java
+++ b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProviderTest.java
@@ -815,6 +815,50 @@ public class CloudStorageFileSystemProviderTest {
     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)?)";
diff --git a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java
index 4033e78..6c63a7e 100644
--- a/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java
+++ b/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java
@@ -139,6 +139,48 @@ public class CloudStorageFileSystemTest {
     }
   }
 
+  @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")) {