Improve NIO documentation (#1076)

jean-philippe-martin · mziccard · commit 3b55bfbd5629 · 2016-06-30T09:13:05.000+02:00
Documentation has been added to many methods to clarify the preferred
method for instantiating the library, which is the non-SPI version.
diff --git a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageConfiguration.java
@@ -82,8 +82,9 @@ public static final class Builder {
     private int blockSize = CloudStorageFileSystem.BLOCK_SIZE_DEFAULT;
 
     /**
-     * Changes current working directory for new filesystem. This cannot be changed once it's
-     * been set. You'll need to create another {@link CloudStorageFileSystem} object.
+     * Changes current working directory for new filesystem. This defaults to the root directory.
+     * The working directory cannot be changed once it's been set. You'll need to create another
+     * {@link CloudStorageFileSystem} object.
      *
      * @throws IllegalArgumentException if {@code path} is not absolute.
      */
@@ -95,7 +96,7 @@ public Builder workingDirectory(String path) {
 
     /**
      * Configures whether or not we should throw an exception when encountering object names
-     * containing superfluous slashes, e.g. {@code a//b}
+     * containing superfluous slashes, e.g. {@code a//b}.
      */
     public Builder permitEmptyPathComponents(boolean value) {
       permitEmptyPathComponents = value;
diff --git a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java
@@ -38,6 +38,8 @@
 import javax.annotation.CheckReturnValue;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
+import javax.annotation.CheckReturnValue;
+import javax.annotation.concurrent.ThreadSafe;
 
 /**
  * Google Cloud Storage {@link FileSystem} implementation.
@@ -47,7 +49,7 @@
  * @see <a href="https://developers.google.com/storage/docs/bucketnaming">
  *        Bucket and Object Naming Guidelines</a>
  */
-@Immutable
+@ThreadSafe
 public final class CloudStorageFileSystem extends FileSystem {
 
   /**
@@ -65,6 +67,7 @@ public final class CloudStorageFileSystem extends FileSystem {
    * @see #forBucket(String, CloudStorageConfiguration)
    * @see java.nio.file.FileSystems#getFileSystem(java.net.URI)
    */
+  @CheckReturnValue
   public static CloudStorageFileSystem forBucket(String bucket) {
     return forBucket(bucket, CloudStorageConfiguration.DEFAULT);
   }
@@ -74,6 +77,7 @@ public static CloudStorageFileSystem forBucket(String bucket) {
    *
    * @see #forBucket(String)
    */
+  @CheckReturnValue
   public static CloudStorageFileSystem forBucket(String bucket, CloudStorageConfiguration config) {
     checkArgument(
         !bucket.startsWith(URI_SCHEME + ":"), "Bucket name must not have schema: %s", bucket);
@@ -155,7 +159,9 @@ public CloudStoragePath getPath(String first, String... more) {
   }
 
   /**
-   * Does nothing.
+   * Does nothing currently. This method <i>might</i> be updated in the future to close all channels
+   * associated with this file system object. However it's unlikely that even then, calling this
+   * method will become mandatory.
    */
   @Override
   public void close() throws IOException {
@@ -191,6 +197,9 @@ public Iterable<Path> getRootDirectories() {
     return ImmutableSet.<Path>of(CloudStoragePath.getPath(this, UnixPath.ROOT));
   }
 
+  /**
+   * Returns nothing because GCS doesn't have disk partitions of limited size, or anything similar.
+   */
   @Override
   public Iterable<FileStore> getFileStores() {
     return ImmutableSet.of();
@@ -206,7 +215,7 @@ public Set<String> supportedFileAttributeViews() {
    */
   @Override
   public PathMatcher getPathMatcher(String syntaxAndPattern) {
-    // TODO: Implement me.
+    // TODO(#813): Implement me.
     throw new UnsupportedOperationException();
   }
 
diff --git a/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java b/gcloud-java-contrib/gcloud-java-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemProvider.java
@@ -70,10 +70,16 @@
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
+import javax.inject.Singleton;
 
 /**
  * Google Cloud Storage {@link FileSystemProvider} implementation.
+ *
+ * <p><b>Note:</b> This class should never be used directly. This class is instantiated by the
+ * service loader and called through a standardized API, e.g. {@link java.nio.file.Files}. However
+ * the javadocs in this class serve as useful documentation for the behavior of the GCS NIO library.
  */
+@Singleton
 @ThreadSafe
 @AutoService(FileSystemProvider.class)
 public final class CloudStorageFileSystemProvider extends FileSystemProvider {
@@ -152,6 +158,13 @@ public CloudStorageFileSystem getFileSystem(URI uri) {
 
   /**
    * Returns Cloud Storage file system, provided a URI with no path, e.g. {@code gs://bucket}.
+   *
+   * @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
+   *     {@link CloudStorageConfiguration.Builder}. However you are not allowed to set the working
+   *     directory, as that should be provided in the {@code uri}
+   * @throws IllegalArgumentException if {@code uri} specifies a user, query, fragment, or scheme is
+   *     not {@value CloudStorageFileSystem#URI_SCHEME}
    */
   @Override
   public CloudStorageFileSystem newFileSystem(URI uri, Map<String, ?> env) {
@@ -527,9 +540,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();
   }
 
@@ -611,13 +624,13 @@ 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(#810): Research if other codes should be translated similarly.
     if (oops.code() == 404) {
       return new NoSuchFileException(oops.reason());
     }
-    // TODO: research if other codes should be translated to IOException.
 
-    // RPC API can only throw StorageException, but CloudStorageFileSystemProvider
-    // can only throw IOException. Square peg, round hole.
     Throwable cause = oops.getCause();
     try {
       if (cause instanceof FileAlreadyExistsException) {
diff --git a/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java b/gcloud-java-contrib/gcloud-java-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java
@@ -58,17 +58,16 @@
 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.
+ * Integration test for gcloud-nio.
  *
- * 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.
+ * <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 instructions for how to get the Service Account JSON Key are
+ * <p>The instructions for how to get the Service Account JSON Key are
  * at https://cloud.google.com/storage/docs/authentication?hl=en#service_accounts
  *
- * The short version is this: go to cloud.google.com/console,
+ * <p>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
@@ -78,19 +77,18 @@
 @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 = RemoteStorageHelper.generateBucketName();
   private static final String SML_FILE = "tmp-test-small-file.txt";
   private static final int SML_SIZE = 100;
-  // it's big, relatively speaking.
-  private static final String BIG_FILE = "tmp-test-big-file.txt";
-  // arbitrary size that's not too round.
-  private static final int BIG_SIZE = 2 * 1024 * 1024 - 50;
+  private static final String BIG_FILE = "tmp-test-big-file.txt"; // it's big, relatively speaking.
+  private static final int BIG_SIZE = 2 * 1024 * 1024 - 50; // arbitrary size that's not too round.
   private static final String PREFIX = "tmp-test-file";
   private static Storage storage;
   private static StorageOptions storageOptions;
diff --git a/gcloud-java-storage/src/main/java/com/google/cloud/storage/testing/FakeStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/cloud/storage/testing/FakeStorageRpc.java
@@ -58,14 +58,15 @@
  *   <li>patch
  *   <li>continueRewrite
  *   <li>createBatch
+ *   <li>checksums, etags
  *   </ul>
  * </ul>
  */
 @NotThreadSafe
 public class FakeStorageRpc implements StorageRpc {
 
   // fullname -> metadata
-  Map<String, StorageObject> stuff = new HashMap<>();
+  Map<String, StorageObject> metadata = new HashMap<>();
   // fullname -> contents
   Map<String, byte[]> contents = new HashMap<>();
   // fullname -> future contents that will be visible on close.
@@ -74,15 +75,15 @@ public class FakeStorageRpc implements StorageRpc {
   private final boolean throwIfOption;
 
   /**
-   * @param throwIfOption if true, we throw when given any option.
+   * @param throwIfOption if true, we throw when given any option
    */
   public FakeStorageRpc(boolean throwIfOption) {
     this.throwIfOption = throwIfOption;
   }
 
   // remove all files
   void reset() {
-    stuff = new HashMap<>();
+    metadata = new HashMap<>();
     contents = new HashMap<>();
   }
 
@@ -96,7 +97,7 @@ public StorageObject create(StorageObject object, InputStream content, Map<Optio
       throws StorageException {
     potentiallyThrow(options);
     String key = fullname(object);
-    stuff.put(key, object);
+    metadata.put(key, object);
     try {
       contents.put(key, com.google.common.io.ByteStreams.toByteArray(content));
     } catch (IOException e) {
@@ -138,7 +139,7 @@ public Tuple<String, Iterable<StorageObject>> list(String bucket, Map<Option, ?>
 
     List<StorageObject> values = new ArrayList<>();
     Map<String, StorageObject> folders = new HashMap<>();
-    for (StorageObject so : stuff.values()) {
+    for (StorageObject so : metadata.values()) {
       if (!so.getName().startsWith(prefix)) {
         continue;
       }
@@ -175,8 +176,8 @@ public StorageObject get(StorageObject object, Map<Option, ?> options) throws St
     }
 
     String key = fullname(object);
-    if (stuff.containsKey(key)) {
-      StorageObject ret = stuff.get(key);
+    if (metadata.containsKey(key)) {
+      StorageObject ret = metadata.get(key);
       if (contents.containsKey(key)) {
         ret.setSize(BigInteger.valueOf(contents.get(key).length));
       }
@@ -208,7 +209,7 @@ public boolean delete(Bucket bucket, Map<Option, ?> options) throws StorageExcep
   public boolean delete(StorageObject object, Map<Option, ?> options) throws StorageException {
     String key = fullname(object);
     contents.remove(key);
-    return null != stuff.remove(key);
+    return null != metadata.remove(key);
   }
 
   @Override
@@ -272,10 +273,10 @@ public String open(StorageObject object, Map<Option, ?> options) throws StorageE
         }
       }
     }
-    if (mustNotExist && stuff.containsKey(key)) {
+    if (mustNotExist && metadata.containsKey(key)) {
       throw new StorageException(new FileAlreadyExistsException(key));
     }
-    stuff.put(key, object);
+    metadata.put(key, object);
 
     return fullname(object);
   }
@@ -327,7 +328,7 @@ public RewriteResponse openRewrite(RewriteRequest rewriteRequest) throws Storage
       throw new StorageException(new FileAlreadyExistsException(destKey));
     }
 
-    stuff.put(destKey, rewriteRequest.target);
+    metadata.put(destKey, rewriteRequest.target);
 
     byte[] data = contents.get(sourceKey);
     contents.put(destKey, Arrays.copyOf(data, data.length));
