Address review comments

gcloud-java-contrib/gcloud-java-nio/pom.xml

@@ -1,7 +1,6 @@
 <?xml version="1.0"?>
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
-  <groupId>com.google.gcloud</groupId>
   <artifactId>gcloud-java-nio</artifactId>
   <packaging>jar</packaging>
   <name>GCloud Java NIO</name>
@@ -37,12 +36,6 @@
       <artifactId>javax.inject</artifactId>
       <version>1</version>
     </dependency>
-    <dependency>
-      <groupId>com.google.auto.service</groupId>
-      <artifactId>auto-service</artifactId>
-      <version>1.0-rc2</version>
-      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
-    </dependency>
     <dependency>
       <groupId>com.google.auto.value</groupId>
       <artifactId>auto-value</artifactId>
@@ -54,6 +47,28 @@
       <artifactId>auto-factory</artifactId>
       <version>1.0-beta3</version>
       <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <exclusions>
+        <exclusion>
+          <groupId>com.google.guava</groupId>
+          <artifactId>guava</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
+    <dependency>
+      <groupId>com.google.auto.service</groupId>
+      <artifactId>auto-service</artifactId>
+      <version>1.0-rc2</version>
+      <scope>provided</scope> <!-- to leave out of the all-deps jar -->
+      <exclusions>
+        <exclusion>
+          <groupId>com.google.guava</groupId>
+          <artifactId>guava</artifactId>
+        </exclusion>
+        <exclusion>
+          <groupId>com.google.auto</groupId>
+          <artifactId>auto-common</artifactId>
+        </exclusion>
+      </exclusions>
     </dependency>
     <dependency>
       <groupId>junit</groupId>

gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/gcloud/storage/contrib/nio/CloudStorageConfiguration.java

@@ -36,7 +36,7 @@ public abstract class CloudStorageConfiguration {
   /**
    * Returns default GCS NIO configuration.
    */
-  public static CloudStorageConfiguration getDefault() {
+  public static CloudStorageConfiguration defaultInstance() {
     return DEFAULT;
   }
 
@@ -54,9 +54,13 @@ public static Builder builder() {
   }
 
   abstract String workingDirectory();
+
   abstract boolean permitEmptyPathComponents();
+
   abstract boolean stripPrefixSlash();
+
   abstract boolean usePseudoDirectories();
+
   abstract int blockSize();
 
   /**

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

@@ -38,6 +38,7 @@
 import java.nio.file.spi.FileSystemProvider;
 import java.util.Objects;
 import java.util.Set;
+import java.util.logging.Logger;
 
 import javax.annotation.CheckReturnValue;
 import javax.annotation.concurrent.ThreadSafe;
@@ -53,13 +54,15 @@
 @ThreadSafe
 public final class CloudStorageFileSystem extends FileSystem {
 
+  private static final Logger logger = Logger.getLogger(CloudStorageFileSystem.class.getName());
+
   /**
    * Invokes {@link #forBucket(String, CloudStorageConfiguration)} with
-   * {@link CloudStorageConfiguration#getDefault()}.
+   * {@link CloudStorageConfiguration#defaultInstance()}.
    */
   @CheckReturnValue
   public static CloudStorageFileSystem forBucket(String bucket) {
-    return forBucket(bucket, CloudStorageConfiguration.getDefault());
+    return forBucket(bucket, CloudStorageConfiguration.defaultInstance());
   }
 
   /**
@@ -82,16 +85,26 @@ public static CloudStorageFileSystem forBucket(String bucket, CloudStorageConfig
     checkNotNull(config);
     checkArgument(
         !bucket.startsWith(URI_SCHEME + ":"), "Bucket name must not have schema: %s", bucket);
-    return new CloudStorageFileSystem(
-        // XXX: This is a kludge to get the provider instance from the SPI. This is necessary since
-        //      the behavior of NIO changes quite a bit if the provider instances aren't the same.
-        (CloudStorageFileSystemProvider)
-            Iterables.getOnlyElement(
-                Iterables.filter(
-                    FileSystemProvider.installedProviders(),
-                    Predicates.instanceOf(CloudStorageFileSystemProvider.class))),
-        config,
-        bucket);
+    return new CloudStorageFileSystem(getProvider(), config, bucket);
+  }
+
+  private static CloudStorageFileSystemProvider getProvider() {
+    // XXX: This is a kludge to get the provider instance from the SPI. This is necessary since
+    //      the behavior of NIO changes quite a bit if the provider instances aren't the same.
+    //      If the provider can not be found via the SPI, then we fall back to instantiating it
+    //      ourselves. This should safeguard against situations where the weird provider file
+    //      doesn't find its way into the jar.
+    FileSystemProvider provider =
+        Iterables.getOnlyElement(
+            Iterables.filter(
+                FileSystemProvider.installedProviders(),
+                Predicates.instanceOf(CloudStorageFileSystemProvider.class)),
+            null);
+    if (provider != null) {
+      return (CloudStorageFileSystemProvider) provider;
+    }
+    logger.warning("Could not find CloudStorageFileSystemProvider via the SPI");
+    return new CloudStorageFileSystemProvider();
   }
 
   public static final String URI_SCHEME = "gs";
@@ -154,10 +167,7 @@ public CloudStoragePath getPath(String first, String... more) {
    */
   @Override
   public void close() throws IOException {
-    // TODO(jean-philippe-martin,jart): Synchronously close all active channels associated with this
-    //                                  FileSystem instance on close, per NIO documentation. But we
-    //                                  probably shouldn't bother unless a legitimate reason can be
-    //                                  found to implement this behavior.
+    // TODO(#809): Synchronously close all channels associated with this FileSystem instance.
   }
 
   /**
@@ -207,7 +217,7 @@ public Set<String> supportedFileAttributeViews() {
    */
   @Override
   public PathMatcher getPathMatcher(String syntaxAndPattern) {
-    // TODO: Implement me.
+    // TODO(#813): Implement me.
     throw new UnsupportedOperationException();
   }
 

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

@@ -113,11 +113,11 @@ public CloudStorageFileSystem getFileSystem(URI uri) {
    * Returns Cloud Storage file system, provided a URI with no path.
    *
    * <p><b>Note:</b> This method should be invoked indirectly via the SPI by calling
-   * {@link java.nio.file.FileSystems#newFileSystem(URI, Map) FileSystems.newFileSystem()}. However
-   * we recommend that you don't use the SPI if possible; the recommended approach is to write a
-   * dependency injection module that calls the statically-linked type-safe version of this method,
-   * which is: {@link CloudStorageFileSystem#forBucket(String, CloudStorageConfiguration)}. Please
-   * see that method for further documentation on creating GCS file systems.
+   * {@link java.nio.file.FileSystems#newFileSystem(URI, Map) FileSystems.newFileSystem()}; however,
+   * we recommend that you don't use the API if possible. The recommended approach is to write a
+   * dependency injection module that calls the statically-linked, type-safe version of this method:
+   * {@link CloudStorageFileSystem#forBucket(String, CloudStorageConfiguration)}. Please see that
+   * method for further documentation on creating GCS file systems.
    *
    * @param uri bucket and current working directory, e.g. {@code gs://bucket}
    * @param env map of configuration options, whose keys correspond to the method names of
@@ -498,9 +498,9 @@ public <A extends BasicFileAttributes> A readAttributes(
 
   @Override
   public Map<String, Object> readAttributes(Path path, String attributes, LinkOption... options) {
-    // Java 7 NIO defines at least eleven string attributes we'd want to support
-    // (eg. BasicFileAttributeView and PosixFileAttributeView), so rather than a partial
-    // implementation we rely on the other overload for now.
+    // TODO(#811): Java 7 NIO defines at least eleven string attributes we'd want to support (eg.
+    //             BasicFileAttributeView and PosixFileAttributeView), so rather than a partial
+    //             implementation we rely on the other overload for now.
     throw new UnsupportedOperationException();
   }
 
@@ -532,7 +532,7 @@ public void createDirectory(Path dir, FileAttribute<?>... attrs) {
    */
   @Override
   public DirectoryStream<Path> newDirectoryStream(Path dir, Filter<? super Path> filter) {
-    // TODO: Implement me.
+    // TODO(#813): Implement me.
     throw new UnsupportedOperationException();
   }
 
@@ -541,6 +541,7 @@ public DirectoryStream<Path> newDirectoryStream(Path dir, Filter<? super Path> f
    */
   @Override
   public void setAttribute(Path path, String attribute, Object value, LinkOption... options) {
+    // TODO(#811): Implement me.
     throw new CloudStorageObjectImmutableException();
   }
 
@@ -572,7 +573,7 @@ public String toString() {
   private IOException asIOException(StorageException oops) {
     // RPC API can only throw StorageException, but CloudStorageFileSystemProvider
     // can only throw IOException. Square peg, round hole.
-    // TODO: research if other codes should be translated similarly.
+    // TODO(#810): Research if other codes should be translated similarly.
     if (oops.code() == 404) {
       return new NoSuchFileException(oops.reason());
     }

gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/gcloud/storage/contrib/nio/CloudStorageFileAttributesTest.java

@@ -87,8 +87,9 @@ public void testContentDisposition() throws IOException {
       Path path = fs.getPath("/water");
       Files.write(path, HAPPY, withContentDisposition("crash call"));
       assertThat(
-              Files.readAttributes(
-                  path, CloudStorageFileAttributes.class).contentDisposition().get())
+              Files.readAttributes(path, CloudStorageFileAttributes.class)
+                  .contentDisposition()
+                  .get())
           .isEqualTo("crash call");
     }
   }

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

@@ -376,8 +376,8 @@ public void testWrite_absoluteObjectName_prefixSlashGetsRemoved() throws IOExcep
   @Test
   public void testWrite_absoluteObjectName_disableStrip_slashGetsPreserved() throws IOException {
     try (CloudStorageFileSystem fs =
-            helper.forBucket(
-                "greenbean", CloudStorageConfiguration.builder().stripPrefixSlash(false).build())) {
+        helper.forBucket(
+            "greenbean", CloudStorageConfiguration.builder().stripPrefixSlash(false).build())) {
       Path path = fs.getPath("/adipose/yep");
       Files.write(path, FILE_CONTENTS, UTF_8);
       assertThat(Files.readAllLines(path, UTF_8)).isEqualTo(FILE_CONTENTS);

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

@@ -120,7 +120,8 @@ public void testNullness() throws IOException, NoSuchMethodException, SecurityEx
       NullPointerTester tester =
           new NullPointerTester()
               .ignore(CloudStorageFileSystem.class.getMethod("equals", Object.class))
-              .setDefault(CloudStorageConfiguration.class, CloudStorageConfiguration.getDefault());
+              .setDefault(
+                  CloudStorageConfiguration.class, CloudStorageConfiguration.defaultInstance());
       tester.testAllPublicStaticMethods(CloudStorageFileSystem.class);
       tester.testAllPublicInstanceMethods(fs);
     }

gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/gcloud/storage/contrib/nio/CloudStorageOptionsTest.java

@@ -85,7 +85,9 @@ public void testWithContentDisposition() throws IOException {
       Path path = fs.getPath("/path");
       Files.write(path, "(✿◕ ‿◕ )ノ".getBytes(UTF_8), withContentDisposition("bubbly fun"));
       assertThat(
-              Files.readAttributes(path, CloudStorageFileAttributes.class).contentDisposition().get())
+              Files.readAttributes(path, CloudStorageFileAttributes.class)
+                  .contentDisposition()
+                  .get())
           .isEqualTo("bubbly fun");
     }
   }
@@ -95,7 +97,8 @@ public void testWithContentEncoding() throws IOException {
     try (FileSystem fs = helper.forBucket("bucket")) {
       Path path = fs.getPath("/path");
       Files.write(path, "(✿◕ ‿◕ )ノ".getBytes(UTF_8), withContentEncoding("gzip"));
-      assertThat(Files.readAttributes(path, CloudStorageFileAttributes.class).contentEncoding().get())
+      assertThat(
+              Files.readAttributes(path, CloudStorageFileAttributes.class).contentEncoding().get())
           .isEqualTo("gzip");
     }
   }
@@ -110,7 +113,9 @@ public void testWithUserMetadata() throws IOException {
           withUserMetadata("nolo", "contendere"),
           withUserMetadata("eternal", "sadness"));
       assertThat(
-              Files.readAttributes(path, CloudStorageFileAttributes.class).userMetadata().get("nolo"))
+              Files.readAttributes(path, CloudStorageFileAttributes.class)
+                  .userMetadata()
+                  .get("nolo"))
           .isEqualTo("contendere");
       assertThat(
               Files.readAttributes(path, CloudStorageFileAttributes.class)

gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/gcloud/storage/contrib/nio/NioTestHelper.java

@@ -24,7 +24,7 @@ final class NioTestHelper {
   }
 
   CloudStorageFileSystem forBucket(String bucket) {
-    return forBucket(bucket, CloudStorageConfiguration.getDefault());
+    return forBucket(bucket, CloudStorageConfiguration.defaultInstance());
   }
 
   CloudStorageFileSystem forBucket(String bucket, CloudStorageConfiguration config) {
@@ -35,12 +35,12 @@ private static Storage makeStorage(final StorageRpc storageRpc) {
     return StorageOptions.builder()
         .projectId("dummy-project-for-testing")
         .serviceRpcFactory(
-          new ServiceRpcFactory<StorageRpc, StorageOptions>() {
-            @Override
-            public StorageRpc create(StorageOptions options) {
-              return storageRpc;
-            }
-          })
+            new ServiceRpcFactory<StorageRpc, StorageOptions>() {
+              @Override
+              public StorageRpc create(StorageOptions options) {
+                return storageRpc;
+              }
+            })
         .build()
         .service();
   }

gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/gcloud/storage/contrib/nio/it/ITGcsNio.java

@@ -39,32 +39,29 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-
 /**
- * Integration test for gcloud-nio. This test actually talks to GCS (you need an account).
- * Tests both reading and writing.
- *
- * You *must* set the GOOGLE_APPLICATION_CREDENTIALS environment variable
- * for this test to work. It must contain the name of a local file that contains
- * your Service Account JSON Key.
+ * Integration test for gcloud-nio.
  *
- * The instructions for how to get the Service Account JSON Key are
- * at https://cloud.google.com/storage/docs/authentication?hl=en#service_accounts
+ * <p>This test actually talks to GCS (you need an account) and tests both reading and writing. You
+ * *must* set the {@code GOOGLE_APPLICATION_CREDENTIALS} environment variable for this test to work.
+ * It must contain the name of a local file that contains your Service Account JSON Key.
  *
- * The short version is this: go to cloud.google.com/console,
- * select your project, search for "API manager", click "Credentials",
- * click "create credentials/service account key", new service account,
- * JSON. The contents of the file that's sent to your browsers is your
- * "Service Account JSON Key".
+ * <p>The instructions for how to get the Service Account JSON Key are
+ * <a href="https://cloud.google.com/storage/docs/authentication?hl=en#service_accounts">here</a>.
  *
+ * <p>The short version is this: go to <a href="https://cloud.google.com/console">Cloud Console</a>,
+ * select your project, search for "API manager", click "Credentials", click "create
+ * credentials/service account key", new service account, JSON. The contents of the file that's sent
+ * to your browsers is your "Service Account JSON Key".
  */
 @RunWith(JUnit4.class)
 public class ITGcsNio {
 
-  private static final List<String> FILE_CONTENTS = ImmutableList.of(
-      "Tous les êtres humains naissent libres et égaux en dignité et en droits.",
-      "Ils sont doués de raison et de conscience et doivent agir ",
-      "les uns envers les autres dans un esprit de fraternité.");
+  private static final List<String> FILE_CONTENTS =
+      ImmutableList.of(
+          "Tous les êtres humains naissent libres et égaux en dignité et en droits.",
+          "Ils sont doués de raison et de conscience et doivent agir ",
+          "les uns envers les autres dans un esprit de fraternité.");
 
   private static final Logger log = Logger.getLogger(ITGcsNio.class.getName());
   private static final String BUCKET = RemoteGcsHelper.generateBucketName();
@@ -94,9 +91,10 @@ public static void beforeClass() throws IOException {
 
   @AfterClass
   public static void afterClass() throws ExecutionException, InterruptedException {
-    if (storage != null && !RemoteGcsHelper.forceDelete(storage, BUCKET, 5, TimeUnit.SECONDS) &&
-      log.isLoggable(Level.WARNING)) {
-        log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", BUCKET);
+    if (storage != null
+        && !RemoteGcsHelper.forceDelete(storage, BUCKET, 5, TimeUnit.SECONDS)
+        && log.isLoggable(Level.WARNING)) {
+      log.log(Level.WARNING, "Deletion of bucket {0} timed out, bucket is not empty", BUCKET);
     }
   }
 
@@ -335,7 +333,6 @@ private String randomSuffix() {
     return "-" + rnd.nextInt(99999);
   }
 
-
   private CloudStorageFileSystem getTestBucket() throws IOException {
     // in typical usage we use the single-argument version of forBucket
     // and rely on the user being logged into their project with the
@@ -348,5 +345,4 @@ private CloudStorageFileSystem getTestBucket() throws IOException {
     return CloudStorageFileSystem.forBucket(
         BUCKET, CloudStorageConfiguration.DEFAULT, storageOptions);
   }
-
 }