Update batch delete to return false on not found

gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java

@@ -30,6 +30,7 @@
 import static com.google.gcloud.spi.StorageRpc.Option.PREDEFINED_DEFAULT_OBJECT_ACL;
 import static com.google.gcloud.spi.StorageRpc.Option.PREFIX;
 import static com.google.gcloud.spi.StorageRpc.Option.VERSIONS;
+import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
 
 import com.google.api.client.googleapis.batch.json.JsonBatchCallback;
 import com.google.api.client.googleapis.json.GoogleJsonError;
@@ -94,7 +95,8 @@ public DefaultStorageRpc(StorageOptions options) {
 
   private static StorageException translate(IOException exception) {
     StorageException translated;
-    if (exception instanceof GoogleJsonResponseException) {
+    if (exception instanceof GoogleJsonResponseException
+        && ((GoogleJsonResponseException) exception).getDetails() != null) {
       translated = translate(((GoogleJsonResponseException) exception).getDetails());
     } else {
       translated = new StorageException(0, exception.getMessage(), false);
@@ -191,7 +193,11 @@ public Bucket get(Bucket bucket, Map<Option, ?> options) {
           .setFields(FIELDS.getString(options))
           .execute();
     } catch (IOException ex) {
-      throw translate(ex);
+      StorageException serviceException = translate(ex);
+      if (serviceException.code() == HTTP_NOT_FOUND) {
+        return null;
+      }
+      throw serviceException;
     }
   }
 
@@ -200,7 +206,11 @@ public StorageObject get(StorageObject object, Map<Option, ?> options) {
     try {
       return getRequest(object, options).execute();
     } catch (IOException ex) {
-      throw translate(ex);
+      StorageException serviceException = translate(ex);
+      if (serviceException.code() == HTTP_NOT_FOUND) {
+        return null;
+      }
+      throw serviceException;
     }
   }
 
@@ -265,7 +275,7 @@ public boolean delete(Bucket bucket, Map<Option, ?> options) {
       return true;
     } catch (IOException ex) {
       StorageException serviceException = translate(ex);
-      if (serviceException.code() == 404) {
+      if (serviceException.code() == HTTP_NOT_FOUND) {
         return false;
       }
       throw serviceException;
@@ -279,7 +289,7 @@ public boolean delete(StorageObject blob, Map<Option, ?> options) {
       return true;
     } catch (IOException ex) {
       StorageException serviceException = translate(ex);
-      if (serviceException.code() == 404) {
+      if (serviceException.code() == HTTP_NOT_FOUND) {
         return false;
       }
       throw serviceException;
@@ -368,7 +378,11 @@ public void onSuccess(Void ignore, HttpHeaders responseHeaders) {
 
           @Override
           public void onFailure(GoogleJsonError e, HttpHeaders responseHeaders) {
-            deletes.put(tuple.x(), Tuple.<Boolean, StorageException>of(null, translate(e)));
+            if (e.getCode() == HTTP_NOT_FOUND) {
+              deletes.put(tuple.x(), Tuple.<Boolean, StorageException>of(Boolean.FALSE, null));
+            } else {
+              deletes.put(tuple.x(), Tuple.<Boolean, StorageException>of(null, translate(e)));
+            }
           }
         });
       }
@@ -397,8 +411,13 @@ public void onSuccess(StorageObject storageObject, HttpHeaders responseHeaders)
 
           @Override
           public void onFailure(GoogleJsonError e, HttpHeaders responseHeaders) {
-            gets.put(tuple.x(),
-                Tuple.<StorageObject, StorageException>of(null, translate(e)));
+            if (e.getCode() == HTTP_NOT_FOUND) {
+              gets.put(tuple.x(),
+                  Tuple.<StorageObject, StorageException>of(null, null));
+            } else {
+              gets.put(tuple.x(),
+                  Tuple.<StorageObject, StorageException>of(null, translate(e)));
+            }
           }
         });
       }

gcloud-java-storage/src/main/java/com/google/gcloud/spi/StorageRpc.java

@@ -227,8 +227,18 @@ StorageObject create(StorageObject object, InputStream content, Map<Option, ?> o
   Tuple<String, Iterable<StorageObject>> list(String bucket, Map<Option, ?> options)
       throws StorageException;
 
+  /**
+   * Returns the requested bucket or {@code null} if not found.
+   *
+   * @throws StorageException upon failure
+   */
   Bucket get(Bucket bucket, Map<Option, ?> options) throws StorageException;
 
+  /**
+   * Returns the requested storage object or {@code null} if not found.
+   *
+   * @throws StorageException upon failure
+   */
   StorageObject get(StorageObject object, Map<Option, ?> options)
       throws StorageException;
 

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

@@ -281,7 +281,7 @@ public CopyWriter copyTo(BlobId targetBlob, BlobSourceOption... options) {
    * Deletes this blob.
    *
    * @param options blob delete options
-   * @return true if blob was deleted
+   * @return {@code true} if blob was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
   public boolean delete(BlobSourceOption... options) {
@@ -422,8 +422,8 @@ public Blob apply(BlobInfo f) {
    * @param storage the storage service used to issue the request
    * @param blobs the blobs to delete
    * @return an immutable list of booleans. If a blob has been deleted the corresponding item in the
-   *     list is {@code true}. If deletion failed or access to the resource was denied the item is
-   *     {@code false}.
+   *     list is {@code true}. If a blob was not found, deletion failed or access to the resource
+   *     was denied the corresponding item is {@code false}.
    * @throws StorageException upon failure
    */
   public static List<Boolean> delete(Storage storage, BlobId... blobs) {

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

@@ -271,7 +271,7 @@ public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) {
    * Deletes this bucket.
    *
    * @param options bucket delete options
-   * @return true if bucket was deleted
+   * @return {@code true} if bucket was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
   public boolean delete(BucketSourceOption... options) {

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

@@ -1316,31 +1316,31 @@ private static void checkContentType(BlobInfo blobInfo) throws IllegalArgumentEx
   /**
    * Delete the requested bucket.
    *
-   * @return true if bucket was deleted
+   * @return {@code true} if bucket was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
   boolean delete(String bucket, BucketSourceOption... options);
 
   /**
    * Delete the requested blob.
    *
-   * @return true if blob was deleted
+   * @return {@code true} if blob was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
   boolean delete(String bucket, String blob, BlobSourceOption... options);
 
   /**
    * Delete the requested blob.
    *
-   * @return true if blob was deleted
+   * @return {@code true} if blob was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
   boolean delete(BlobId blob, BlobSourceOption... options);
 
   /**
    * Delete the requested blob.
    *
-   * @return true if blob was deleted
+   * @return {@code true} if blob was deleted, {@code false} if it was not found
    * @throws StorageException upon failure
    */
   boolean delete(BlobId blob);
@@ -1478,8 +1478,8 @@ private static void checkContentType(BlobInfo blobInfo) throws IllegalArgumentEx
    *
    * @param blobIds blobs to delete
    * @return an immutable list of booleans. If a blob has been deleted the corresponding item in the
-   *     list is {@code true}. If deletion failed or access to the resource was denied the item is
-   *     {@code false}.
+   *     list is {@code true}. If a blob was not found, deletion failed or access to the resource
+   *     was denied the corresponding item is {@code false}.
    * @throws StorageException upon failure
    */
   List<Boolean> delete(BlobId... blobIds);

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

@@ -28,7 +28,6 @@
 import static com.google.gcloud.spi.StorageRpc.Option.IF_SOURCE_GENERATION_NOT_MATCH;
 import static com.google.gcloud.spi.StorageRpc.Option.IF_SOURCE_METAGENERATION_MATCH;
 import static com.google.gcloud.spi.StorageRpc.Option.IF_SOURCE_METAGENERATION_NOT_MATCH;
-import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.api.services.storage.model.StorageObject;
@@ -175,14 +174,7 @@ public BucketInfo get(String bucket, BucketGetOption... options) {
           new Callable<com.google.api.services.storage.model.Bucket>() {
             @Override
             public com.google.api.services.storage.model.Bucket call() {
-              try {
-                return storageRpc.get(bucketPb, optionsMap);
-              } catch (StorageException ex) {
-                if (ex.code() == HTTP_NOT_FOUND) {
-                  return null;
-                }
-                throw ex;
-              }
+              return storageRpc.get(bucketPb, optionsMap);
             }
           }, options().retryParams(), EXCEPTION_HANDLER);
       return answer == null ? null : BucketInfo.fromPb(answer);
@@ -204,14 +196,7 @@ public BlobInfo get(BlobId blob, BlobGetOption... options) {
       StorageObject storageObject = runWithRetries(new Callable<StorageObject>() {
         @Override
         public StorageObject call() {
-          try {
-            return storageRpc.get(storedObject, optionsMap);
-          } catch (StorageException ex) {
-            if (ex.code() == HTTP_NOT_FOUND) {
-              return null;
-            }
-            throw ex;
-          }
+          return storageRpc.get(storedObject, optionsMap);
         }
       }, options().retryParams(), EXCEPTION_HANDLER);
       return storageObject == null ? null : BlobInfo.fromPb(storageObject);
@@ -523,28 +508,23 @@ public BatchResponse apply(BatchRequest batchRequest) {
     List<BatchResponse.Result<BlobInfo>> updates = transformBatchResult(
         toUpdate, response.updates, BlobInfo.FROM_PB_FUNCTION);
     List<BatchResponse.Result<BlobInfo>> gets = transformBatchResult(
-        toGet, response.gets, BlobInfo.FROM_PB_FUNCTION, HTTP_NOT_FOUND);
+        toGet, response.gets, BlobInfo.FROM_PB_FUNCTION);
     return new BatchResponse(deletes, updates, gets);
   }
 
   private <I, O extends Serializable> List<BatchResponse.Result<O>> transformBatchResult(
       Iterable<Tuple<StorageObject, Map<StorageRpc.Option, ?>>> request,
-      Map<StorageObject, Tuple<I, StorageException>> results, Function<I, O> transform,
-      int... nullOnErrorCodes) {
-    Set nullOnErrorCodesSet = Sets.newHashSet(Ints.asList(nullOnErrorCodes));
+      Map<StorageObject, Tuple<I, StorageException>> results, Function<I, O> transform) {
     List<BatchResponse.Result<O>> response = Lists.newArrayListWithCapacity(results.size());
     for (Tuple<StorageObject, ?> tuple : request) {
       Tuple<I, StorageException> result = results.get(tuple.x());
-      if (result.x() != null) {
-        response.add(BatchResponse.Result.of(transform.apply(result.x())));
+      I object = result.x();
+      StorageException exception = result.y();
+      if (exception != null) {
+        response.add(new BatchResponse.Result<O>(exception));
       } else {
-        StorageException exception = result.y();
-        if (nullOnErrorCodesSet.contains(exception.code())) {
-          //noinspection unchecked
-          response.add(BatchResponse.Result.<O>empty());
-        } else {
-          response.add(new BatchResponse.Result<O>(exception));
-        }
+        response.add(object != null ?
+            BatchResponse.Result.of(transform.apply(object)) : BatchResponse.Result.<O>empty());
       }
     }
     return response;

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

@@ -644,7 +644,8 @@ public void testBatchRequestFail() {
     assertFalse(batchResponse.gets().get(1).failed());
     assertNull(batchResponse.gets().get(1).get());
     assertTrue(batchResponse.deletes().get(0).failed());
-    assertTrue(batchResponse.deletes().get(1).failed());
+    assertFalse(batchResponse.deletes().get(1).failed());
+    assertFalse(batchResponse.deletes().get(1).get());
     assertTrue(storage.delete(BUCKET, blobName));
   }