Skip to content

Commit

Permalink
Merge pull request #380 from mziccard/fix-delete-batch
Browse files Browse the repository at this point in the history 
Update batch delete to return false on not found
  • Loading branch information
@aozarov
aozarov committed Nov 18, 2015
2 parents 101048e + 1e7fe54 commit a4c4273
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 49 deletions.
35 changes: 27 additions & 8 deletions gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)));
}
}
});
}
Expand Down Expand Up @@ -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)));
}
}
});
}
Expand Down
10 changes: 10 additions & 0 deletions gcloud-java-storage/src/main/java/com/google/gcloud/spi/StorageRpc.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/Blob.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion gcloud-java-storage/src/main/java/com/google/gcloud/storage/Bucket.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 6 additions & 6 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
40 changes: 10 additions & 30 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -177,14 +176,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);
Expand All @@ -206,14 +198,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);
Expand Down Expand Up @@ -525,28 +510,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;
Expand Down
3 changes: 2 additions & 1 deletion gcloud-java-storage/src/test/java/com/google/gcloud/storage/ITStorageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down

0 comments on commit a4c4273

Please sign in to comment.