Fix BlobListOption.recursive, add tests and documentation

Author: mziccard

gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java

@@ -695,6 +695,10 @@ public static BlobListOption prefix(String prefix) {
 
     /**
      * Returns an option to specify whether blob listing should include subdirectories or not.
+     * {@link StorageOptions#pathDelimiter()} is used as a path delimiter. If set to {@code true}
+     * also blobs in subdirectories are listed. If set to {@code false} and used in combination
+     * with {@link #prefix(String)} only blobs in a directory can be listed. If not set also blobs
+     * in subdirectories are listed.
      */
     public static BlobListOption recursive(boolean recursive) {
       return new BlobListOption(StorageRpc.Option.DELIMITER, recursive);

gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java

@@ -668,7 +668,7 @@ private static <T> void addToOptionMap(StorageRpc.Option getOption, StorageRpc.O
       checkArgument(prev == null, "Duplicate option %s", option);
     }
     Boolean value = (Boolean) temp.remove(DELIMITER);
-    if (Boolean.TRUE.equals(value)) {
+    if (Boolean.FALSE.equals(value)) {
       temp.put(DELIMITER, options().pathDelimiter());
     }
     if (useAsSource) {

gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageOptions.java

@@ -107,7 +107,7 @@ protected Set<String> scopes() {
   }
 
   /**
-   * Returns the storage service's path delimiter.
+   * Returns the storage service's path delimiter. If not set, {@code "/"} is used.
    */
   public String pathDelimiter() {
     return pathDelimiter;

gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java

@@ -241,7 +241,7 @@ public long millis() {
   private StorageRpc storageRpcMock;
   private Storage storage;
 
-  private Blob expectedBlob1, expectedBlob2, expectedBlob3;
+  private Blob expectedBlob1, expectedBlob2;
   private Bucket expectedBucket1, expectedBucket2;
 
   @Rule
@@ -286,7 +286,6 @@ private void initializeService() {
   private void initializeServiceDependentObjects() {
     expectedBlob1 = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO1));
     expectedBlob2 = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO2));
-    expectedBlob3 = new Blob(storage, new BlobInfo.BuilderImpl(BLOB_INFO3));
     expectedBucket1 = new Bucket(storage, new BucketInfo.BuilderImpl(BUCKET_INFO1));
     expectedBucket2 = new Bucket(storage, new BucketInfo.BuilderImpl(BUCKET_INFO2));
   }
@@ -659,6 +658,67 @@ public void testListBlobsWithOptions() {
     assertArrayEquals(blobList.toArray(), Iterables.toArray(page.values(), Blob.class));
   }
 
+  @Test
+  public void testListBlobsWithDelimiter() {
+    String cursor = "cursor";
+    Map<StorageRpc.Option, ?> options = ImmutableMap.of(StorageRpc.Option.DELIMITER, "/");
+    ImmutableList<BlobInfo> blobInfoList = ImmutableList.of(BLOB_INFO1, BLOB_INFO2);
+    Tuple<String, Iterable<com.google.api.services.storage.model.StorageObject>> result =
+        Tuple.of(cursor, Iterables.transform(blobInfoList, BlobInfo.INFO_TO_PB_FUNCTION));
+    EasyMock.expect(storageRpcMock.list(BUCKET_NAME1, options)).andReturn(result);
+    EasyMock.replay(storageRpcMock);
+    initializeService();
+    ImmutableList<Blob> blobList = ImmutableList.of(expectedBlob1, expectedBlob2);
+    Page<Blob> page = storage.list(BUCKET_NAME1, Storage.BlobListOption.recursive(false));
+    assertEquals(cursor, page.nextPageCursor());
+    assertArrayEquals(blobList.toArray(), Iterables.toArray(page.values(), Blob.class));
+  }
+
+
+  @Test
+  public void testListBlobsWithNoDelimiter() {
+    String cursor = "cursor";
+    ImmutableList<BlobInfo> blobInfoList = ImmutableList.of(BLOB_INFO1, BLOB_INFO2);
+    Tuple<String, Iterable<com.google.api.services.storage.model.StorageObject>> result =
+        Tuple.of(cursor, Iterables.transform(blobInfoList, BlobInfo.INFO_TO_PB_FUNCTION));
+    EasyMock.expect(storageRpcMock.list(BUCKET_NAME1, EMPTY_RPC_OPTIONS))
+        .andReturn(result);
+    EasyMock.replay(storageRpcMock);
+    initializeService();
+    ImmutableList<Blob> blobList = ImmutableList.of(expectedBlob1, expectedBlob2);
+    Page<Blob> page = storage.list(BUCKET_NAME1, Storage.BlobListOption.recursive(true));
+    assertEquals(cursor, page.nextPageCursor());
+    assertArrayEquals(blobList.toArray(), Iterables.toArray(page.values(), Blob.class));
+  }
+
+  @Test
+  public void testListBlobsWithCustomDelimiter() {
+    StorageRpcFactory factoryMock = EasyMock.createMock(StorageRpcFactory.class);
+    StorageRpc rpcMock = EasyMock.createMock(StorageRpc.class);
+    EasyMock.expect(factoryMock.create(EasyMock.anyObject(StorageOptions.class)))
+        .andReturn(rpcMock);
+    EasyMock.replay(factoryMock);
+    EasyMock.replay(storageRpcMock);
+    initializeService();
+    Storage storage = StorageOptions.builder()
+        .projectId("projectId")
+        .pathDelimiter("-")
+        .clock(TIME_SOURCE)
+        .serviceRpcFactory(factoryMock)
+        .retryParams(RetryParams.noRetries())
+        .build()
+        .service();
+    Map<StorageRpc.Option, ?> options = ImmutableMap.of(StorageRpc.Option.DELIMITER, "-");
+    EasyMock.expect(rpcMock.list(BUCKET_NAME1, options))
+        .andReturn(Tuple.<String, Iterable<com.google.api.services.storage.model.StorageObject>>of(
+            null, null));
+    EasyMock.replay(rpcMock);
+    Page<Blob> page = storage.list(BUCKET_NAME1, Storage.BlobListOption.recursive(false));
+    assertNull(page.nextPageCursor());
+    assertArrayEquals(ImmutableList.of().toArray(), Iterables.toArray(page.values(), Blob.class));
+    EasyMock.verify(factoryMock, rpcMock);
+  }
+
   @Test
   public void testListBlobsWithSelectedFields() {
     String cursor = "cursor";