diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java index cd0933426756..b7f7803d982d 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java @@ -16,26 +16,80 @@ package com.google.gcloud; +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.api.client.googleapis.json.GoogleJsonResponseException; + +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.util.Collections; +import java.util.Set; + /** * Base class for all service exceptions. */ public class BaseServiceException extends RuntimeException { private static final long serialVersionUID = 5028833760039966178L; + public static final int UNKNOWN_CODE = 0; private final int code; private final boolean retryable; + private final boolean idempotent; + + public BaseServiceException(IOException exception, boolean idempotent) { + super(exception.getMessage(), exception); + this.code = statusCode(exception); + this.idempotent = idempotent; + this.retryable = idempotent && isRetryable(exception); + } + + public BaseServiceException(GoogleJsonError error, boolean idempotent) { + super(error.getMessage()); + this.code = error.getCode(); + this.idempotent = idempotent; + this.retryable = idempotent && isRetryable(error); + } - public BaseServiceException(int code, String message, boolean retryable) { + public BaseServiceException(int code, String message, boolean retryable, boolean idempotent) { super(message); this.code = code; - this.retryable = retryable; + this.idempotent = idempotent; + this.retryable = idempotent && retryable; } - public BaseServiceException(int code, String message, boolean retryable, Exception cause) { + public BaseServiceException(int code, String message, boolean retryable, boolean idempotent, + Exception cause) { super(message, cause); this.code = code; - this.retryable = retryable; + this.idempotent = idempotent; + this.retryable = idempotent && retryable; + } + + protected Set retryableCodes() { + return Collections.emptySet(); + } + + protected int statusCode(IOException exception) { + if (exception instanceof GoogleJsonResponseException) { + if (((GoogleJsonResponseException) exception).getDetails() != null) { + return ((GoogleJsonResponseException) exception).getDetails().getCode(); + } + } + return UNKNOWN_CODE; + } + + protected boolean isRetryable(GoogleJsonError error) { + return error != null && retryableCodes().contains(error.getCode()); + } + + protected boolean isRetryable(IOException exception) { + if (exception instanceof GoogleJsonResponseException) { + return isRetryable(((GoogleJsonResponseException) exception).getDetails()); + } + if (exception instanceof SocketTimeoutException) { + return true; + } + return false; } /** @@ -51,4 +105,22 @@ public int code() { public boolean retryable() { return retryable; } + + /** + * Returns {@code true} when the operation that caused this exception had no side effects. + */ + public boolean idempotent() { + return idempotent; + } + + protected static T translateAndThrow( + RetryHelper.RetryHelperException ex) { + if (ex.getCause() instanceof BaseServiceException) { + throw (T) ex.getCause(); + } + if (ex instanceof RetryHelper.RetryInterruptedException) { + RetryHelper.RetryInterruptedException.propagate(); + } + return null; + } } diff --git a/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java b/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java index f30fd3abfb79..abba26ae62f7 100644 --- a/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java +++ b/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java @@ -16,30 +16,91 @@ package com.google.gcloud; +import com.google.api.client.googleapis.json.GoogleJsonError; + +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; import org.junit.Test; +import java.io.IOException; +import java.net.SocketTimeoutException; + /** * Tests for {@link BaseServiceException}. */ public class BaseServiceExceptionTest { - private final int code = 1; - private final String message = "some message"; - private final boolean retryable = true; + private static final int CODE = 1; + private static final String MESSAGE = "some message"; + private static final boolean RETRYABLE = true; + private static final boolean IDEMPOTENT = true; @Test public void testBaseServiceException() { - BaseServiceException serviceException = new BaseServiceException(code, message, retryable); - assertEquals(serviceException.code(), code); - assertEquals(serviceException.getMessage(), message); - assertEquals(serviceException.getCause(), null); + BaseServiceException serviceException = new BaseServiceException(CODE, MESSAGE, RETRYABLE, + IDEMPOTENT); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(RETRYABLE, serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + assertEquals(null, serviceException.getCause()); + + serviceException = new BaseServiceException(CODE, MESSAGE, RETRYABLE, false); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(false, serviceException.retryable()); + assertEquals(false, serviceException.idempotent()); + assertEquals(null, serviceException.getCause()); Exception cause = new RuntimeException(); - serviceException = new BaseServiceException(code, message, retryable, cause); - assertEquals(serviceException.code(), code); - assertEquals(serviceException.getMessage(), message); - assertEquals(serviceException.getCause(), cause); + serviceException = new BaseServiceException(CODE, MESSAGE, RETRYABLE, IDEMPOTENT, cause); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(RETRYABLE, serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + assertEquals(cause, serviceException.getCause()); + + serviceException = new BaseServiceException(CODE, MESSAGE, RETRYABLE, false, cause); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(false, serviceException.retryable()); + assertEquals(false, serviceException.idempotent()); + + IOException exception = new SocketTimeoutException(); + serviceException = new BaseServiceException(exception, true); + assertEquals(true, serviceException.retryable()); + assertEquals(true, serviceException.idempotent()); + assertEquals(exception, serviceException.getCause()); + + GoogleJsonError error = new GoogleJsonError(); + error.setCode(CODE); + error.setMessage(MESSAGE); + serviceException = new BaseServiceException(error, true); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(false, serviceException.retryable()); + assertEquals(true, serviceException.idempotent()); + } + + @Test + public void testTranslateAndThrow() throws Exception { + BaseServiceException cause = new BaseServiceException(CODE, MESSAGE, RETRYABLE, IDEMPOTENT); + RetryHelper.RetryHelperException exceptionMock = createMock(RetryHelper.RetryHelperException.class); + expect(exceptionMock.getCause()).andReturn(cause).times(2); + replay(exceptionMock); + try { + BaseServiceException.translateAndThrow(exceptionMock); + } catch (BaseServiceException ex) { + assertEquals(CODE, ex.code()); + assertEquals(MESSAGE, ex.getMessage()); + assertEquals(RETRYABLE, ex.retryable()); + assertEquals(IDEMPOTENT, ex.idempotent()); + } finally { + verify(exceptionMock); + } } } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseDatastoreBatchWriter.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseDatastoreBatchWriter.java index 7eaf5c535f26..9ffd3a124b27 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseDatastoreBatchWriter.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseDatastoreBatchWriter.java @@ -16,6 +16,9 @@ package com.google.gcloud.datastore; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + import com.google.api.services.datastore.DatastoreV1; import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; @@ -64,10 +67,8 @@ public final void addWithDeferredIdAllocation(FullEntity... entities) { private void addInternal(FullEntity entity) { Key key = entity.key(); - if (toAdd.containsKey(key) || toUpdate.containsKey(key) || toPut.containsKey(key)) { - throw newInvalidRequest("Entity with the key %s was already added or updated in this %s", - entity.key(), name); - } + checkArgument(!toAdd.containsKey(key) && !toUpdate.containsKey(key) && !toPut.containsKey(key), + "Entity with the key %s was already added or updated in this %s", entity.key(), name); if (toDelete.remove(key)) { toPut.put(key, entity); } else { @@ -120,10 +121,8 @@ public final void update(Entity... entities) { validateActive(); for (Entity entity : entities) { Key key = entity.key(); - if (toDelete.contains(key)) { - throw newInvalidRequest("Entity with the key %s was already deleted in this %s", - entity.key(), name); - } + checkArgument(!toDelete.contains(key), + "Entity with the key %s was already deleted in this %s", entity.key(), name); if (toAdd.remove(key) != null || toPut.containsKey(key)) { toPut.put(key, entity); } else { @@ -190,13 +189,7 @@ protected void deactivate() { } protected void validateActive() { - if (!active) { - throw newInvalidRequest("%s is no longer active", name); - } - } - - protected DatastoreException newInvalidRequest(String msg, Object... params) { - return DatastoreException.throwInvalidRequest(String.format(msg, params)); + checkState(active, "%s is no longer active", name); } protected DatastoreV1.Mutation.Builder toMutationPb() { diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseEntity.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseEntity.java index 3a79f3053a1e..47cec40ec42a 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseEntity.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/BaseEntity.java @@ -16,6 +16,7 @@ package com.google.gcloud.datastore; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.gcloud.datastore.BlobValue.of; import static com.google.gcloud.datastore.BooleanValue.of; import static com.google.gcloud.datastore.DateTimeValue.of; @@ -248,9 +249,7 @@ public boolean contains(String name) { public > V getValue(String name) { @SuppressWarnings("unchecked") V property = (V) properties.get(name); - if (property == null) { - throw DatastoreException.throwInvalidRequest("No such property %s", name); - } + checkArgument(property != null, "No such property %s", name); return property; } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java index dded1d11875e..c33cd1d65f61 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java @@ -16,140 +16,100 @@ package com.google.gcloud.datastore; -import com.google.common.base.MoreObjects; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.gcloud.BaseServiceException; -import com.google.gcloud.RetryHelper; import com.google.gcloud.RetryHelper.RetryHelperException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; +import com.google.gcloud.RetryHelper.RetryInterruptedException; -import java.util.HashMap; -import java.util.Map; +import java.io.IOException; +import java.util.Set; public class DatastoreException extends BaseServiceException { - private static final long serialVersionUID = -2336749234060754893L; - private static final ImmutableMap REASON_TO_ERROR; - private static final ImmutableMap HTTP_TO_ERROR; - - private final DatastoreError error; - /** - * Represents Datastore errors. + * Possible reasons for a Google Cloud Datastore exception. * - * @see Google Cloud - * Datastore error codes + * @see Error + * Codes */ - public enum DatastoreError { - - ABORTED(Reason.ABORTED), - DEADLINE_EXCEEDED(Reason.DEADLINE_EXCEEDED), - UNAVAILABLE(Reason.UNAVAILABLE), - FAILED_PRECONDITION(Reason.FAILED_PRECONDITION), - INVALID_ARGUMENT(Reason.INVALID_ARGUMENT), - PERMISSION_DENIED(Reason.PERMISSION_DENIED), - UNAUTHORIZED(false, "Unauthorized", 401), - INTERNAL(Reason.INTERNAL), - RESOURCE_EXHAUSTED(Reason.RESOURCE_EXHAUSTED), - UNKNOWN(false, "Unknown failure", -1); - - private final boolean retryable; - private final String description; - private final int httpStatus; - - DatastoreError(Reason reason) { - this(reason.retryable(), reason.description(), reason.httpStatus()); - } - - DatastoreError(boolean retryable, String description, int httpStatus) { - this.retryable = retryable; - this.description = description; - this.httpStatus = httpStatus; - } - - String description() { - return description; - } - - int httpStatus() { - return httpStatus; + public enum Reason { + ABORTED, + DEADLINE_EXCEEDED, + UNAVAILABLE, + FAILED_PRECONDITION, + INVALID_ARGUMENT, + PERMISSION_DENIED, + UNAUTHORIZED, + INTERNAL, + RESOURCE_EXHAUSTED, + UNKNOWN; + + public static Reason of(String reason) { + try { + return Reason.valueOf(reason); + } catch (Exception ex) { + return Reason.UNKNOWN; + } } + } - boolean retryable() { - return retryable; - } + private static final Set RETRYABLE_CODES = ImmutableSet.of(409, 403, 503); + private static final Set RETRYABLE_REASONS = ImmutableSet.of(Reason.ABORTED, + Reason.DEADLINE_EXCEEDED, Reason.UNAVAILABLE); + private static final long serialVersionUID = -2336749234060754893L; - DatastoreException translate(DatastoreRpcException exception, String message) { - return new DatastoreException(this, message, exception); - } - } + private final Reason reason; - static { - ImmutableMap.Builder builder = ImmutableMap.builder(); - Map httpCodes = new HashMap<>(); - for (DatastoreError error : DatastoreError.values()) { - builder.put(error.name(), error); - httpCodes.put(error.httpStatus(), error); - } - REASON_TO_ERROR = builder.build(); - HTTP_TO_ERROR = ImmutableMap.copyOf(httpCodes); + public DatastoreException(Reason reason, int status, String message, Exception cause) { + super(status, message, RETRYABLE_CODES.contains(status) && RETRYABLE_REASONS.contains(reason), + true, cause); + this.reason = reason; } - public DatastoreException(DatastoreError error, String message, Exception cause) { - super(error.httpStatus(), MoreObjects.firstNonNull(message, error.description), - error.retryable(), cause); - this.error = error; + public DatastoreException(Reason reason, int status, String message) { + super(status, message, RETRYABLE_CODES.contains(status) && RETRYABLE_REASONS.contains(reason), + true); + this.reason = reason; } - public DatastoreException(DatastoreError error, String message) { - this(error, message, null); + public DatastoreException(IOException exception) { + super(exception, true); + this.reason = Reason.UNKNOWN; } - /** - * Returns the DatastoreError associated with this exception. - */ - public DatastoreError datastoreError() { - return error; + @Override + protected Set retryableCodes() { + return RETRYABLE_CODES; } - static DatastoreException translateAndThrow(RetryHelperException ex) { - if (ex.getCause() instanceof DatastoreRpcException) { - return translateAndThrow((DatastoreRpcException) ex.getCause()); - } - if (ex instanceof RetryHelper.RetryInterruptedException) { - RetryHelper.RetryInterruptedException.propagate(); - } - throw new DatastoreException(DatastoreError.UNKNOWN, ex.getMessage(), ex); + protected Set retryableReasons() { + return RETRYABLE_REASONS; } /** - * Translate DatastoreRpcExceptions to DatastoreExceptions based on their - * HTTP error codes. This method will always throw a new DatastoreException. + * Translate RetryHelperException to the DatastoreException that caused the error. This method + * will always throw an exception. * - * @throws DatastoreException every time + * @throws DatastoreException when {@code ex} was caused by a {@code DatastoreException} + * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - static DatastoreException translateAndThrow(DatastoreRpcException exception) { - String message = exception.getMessage(); - DatastoreError error = REASON_TO_ERROR.get(exception.reason()); - if (error == null) { - error = MoreObjects.firstNonNull( - HTTP_TO_ERROR.get(exception.httpStatus()), DatastoreError.UNKNOWN); - } - throw error.translate(exception, message); + public static DatastoreException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndThrow(ex); + throw new DatastoreException(Reason.UNKNOWN, UNKNOWN_CODE, ex.getMessage()); + } + + static DatastoreException propagateUserException(Exception ex) { + throw new DatastoreException( + Reason.UNKNOWN, BaseServiceException.UNKNOWN_CODE, ex.getMessage(), ex); } /** - * Throw a DatastoreException with {@code FAILED_PRECONDITION} error and the {@code message} - * in a nested exception. + * Returns the reason of the exception. * - * @throws DatastoreException every time + * @see Error + * Codes */ - static DatastoreException throwInvalidRequest(String massage, Object... params) { - throw new DatastoreException(DatastoreError.FAILED_PRECONDITION, String.format(massage, params)); - } - - static DatastoreException propagateUserException(Exception ex) { - throw new DatastoreException(DatastoreError.UNKNOWN, ex.getMessage(), ex); + public Reason reason() { + return reason; } } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java index 43fd75396538..9ec4deb489cc 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java @@ -16,9 +16,10 @@ package com.google.gcloud.datastore; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.api.services.datastore.DatastoreV1; import com.google.common.base.MoreObjects; -import com.google.common.base.Preconditions; import com.google.common.collect.AbstractIterator; import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; @@ -29,7 +30,6 @@ import com.google.gcloud.RetryHelper.RetryHelperException; import com.google.gcloud.RetryParams; import com.google.gcloud.spi.DatastoreRpc; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; import com.google.protobuf.ByteString; import java.util.Arrays; @@ -42,32 +42,28 @@ import java.util.Set; import java.util.concurrent.Callable; +final class DatastoreImpl extends BaseService implements Datastore { -final class DatastoreImpl extends BaseService - implements Datastore { - - private static final Interceptor EXCEPTION_HANDLER_INTERCEPTOR = - new Interceptor() { + private static final Interceptor EXCEPTION_HANDLER_INTERCEPTOR = new Interceptor() { - private static final long serialVersionUID = 6911242958397733203L; + private static final long serialVersionUID = 6911242958397733203L; - @Override - public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } + @Override + public RetryResult afterEval(Exception exception, RetryResult retryResult) { + return Interceptor.RetryResult.CONTINUE_EVALUATION; + } - @Override - public RetryResult beforeEval(Exception exception) { - if (exception instanceof DatastoreRpcException) { - boolean retryable = ((DatastoreRpcException) exception).retryable(); - return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; - } - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - }; - private static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() - .abortOn(RuntimeException.class, DatastoreRpcException.class) - .interceptor(EXCEPTION_HANDLER_INTERCEPTOR).build(); + @Override + public RetryResult beforeEval(Exception exception) { + if (exception instanceof DatastoreException) { + boolean retriable = ((DatastoreException) exception).retryable(); + return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; + } + return Interceptor.RetryResult.CONTINUE_EVALUATION; + } + }; + static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() + .abortOn(RuntimeException.class).interceptor(EXCEPTION_HANDLER_INTERCEPTOR).build(); private final DatastoreRpc datastoreRpc; private final RetryParams retryParams; @@ -105,7 +101,7 @@ QueryResults run(DatastoreV1.ReadOptions readOptionsPb, Query query) { DatastoreV1.RunQueryResponse runQuery(final DatastoreV1.RunQueryRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.RunQueryResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.RunQueryResponse call() throws DatastoreException { return datastoreRpc.runQuery(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -139,7 +135,7 @@ public List allocateId(IncompleteKey... keys) { DatastoreV1.AllocateIdsResponse allocateIds(final DatastoreV1.AllocateIdsRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.AllocateIdsResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.AllocateIdsResponse call() throws DatastoreException { return datastoreRpc.allocateIds(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -174,13 +170,11 @@ public List add(FullEntity... entities) { completeEntity = Entity.convert((FullEntity) entity); } if (completeEntity != null) { - if (completeEntities.put(completeEntity.key(), completeEntity) != null) { - throw DatastoreException.throwInvalidRequest( - "Duplicate entity with the key %s", entity.key()); - } + Object prev = completeEntities.put(completeEntity.key(), completeEntity); + checkArgument(prev == null, "Duplicate entity with the key %s", entity.key()); mutationPb.addInsert(completeEntity.toPb()); } else { - Preconditions.checkArgument(entity.hasKey(), "entity %s is missing a key", entity); + checkArgument(entity.hasKey(), "entity %s is missing a key", entity); mutationPb.addInsertAutoId(entity.toPb()); } } @@ -263,7 +257,7 @@ protected Entity computeNext() { DatastoreV1.LookupResponse lookup(final DatastoreV1.LookupRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.LookupResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.LookupResponse call() throws DatastoreException { return datastoreRpc.lookup(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -334,7 +328,7 @@ private DatastoreV1.CommitResponse commitMutation(DatastoreV1.Mutation.Builder m DatastoreV1.CommitResponse commit(final DatastoreV1.CommitRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.CommitResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.CommitResponse call() throws DatastoreException { return datastoreRpc.commit(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -352,7 +346,7 @@ DatastoreV1.BeginTransactionResponse beginTransaction( try { return RetryHelper.runWithRetries(new Callable() { @Override - public DatastoreV1.BeginTransactionResponse call() throws DatastoreRpcException { + public DatastoreV1.BeginTransactionResponse call() throws DatastoreException { return datastoreRpc.beginTransaction(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -370,7 +364,7 @@ void rollbackTransaction(ByteString transaction) { void rollback(final DatastoreV1.RollbackRequest requestPb) { try { RetryHelper.runWithRetries(new Callable() { - @Override public Void call() throws DatastoreRpcException { + @Override public Void call() throws DatastoreException { datastoreRpc.rollback(requestPb); return null; } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java index ee15393a8048..148a5e2727b1 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java @@ -25,7 +25,6 @@ import com.google.common.collect.Iterables; import com.google.gcloud.ServiceOptions; import com.google.gcloud.spi.DatastoreRpc; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; import com.google.gcloud.spi.DatastoreRpcFactory; import com.google.gcloud.spi.DefaultDatastoreRpc; @@ -126,20 +125,16 @@ private DatastoreOptions normalize() { .addPathElement(DatastoreV1.Key.PathElement.newBuilder().setKind("__foo__").setName("bar")) .build(); requestPb.addKey(key); - try { - LookupResponse responsePb = rpc().lookup(requestPb.build()); - if (responsePb.getDeferredCount() > 0) { - key = responsePb.getDeferred(0); - } else { - Iterator combinedIter = - Iterables.concat(responsePb.getMissingList(), responsePb.getFoundList()).iterator(); - key = combinedIter.next().getEntity().getKey(); - } - builder.projectId(key.getPartitionId().getDatasetId()); - return new DatastoreOptions(builder); - } catch (DatastoreRpcException e) { - throw DatastoreException.translateAndThrow(e); + LookupResponse responsePb = rpc().lookup(requestPb.build()); + if (responsePb.getDeferredCount() > 0) { + key = responsePb.getDeferred(0); + } else { + Iterator combinedIter = + Iterables.concat(responsePb.getMissingList(), responsePb.getFoundList()).iterator(); + key = combinedIter.next().getEntity().getKey(); } + builder.projectId(key.getPartitionId().getDatasetId()); + return new DatastoreOptions(builder); } @Override diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/ListValue.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/ListValue.java index 41c7e82788b5..a8f75da93beb 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/ListValue.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/ListValue.java @@ -17,6 +17,7 @@ package com.google.gcloud.datastore; import static com.google.api.services.datastore.DatastoreV1.Value.LIST_VALUE_FIELD_NUMBER; +import static com.google.common.base.Preconditions.checkArgument; import com.google.api.services.datastore.DatastoreV1; import com.google.common.base.Preconditions; @@ -72,7 +73,7 @@ private Builder() { public Builder addValue(Value value) { // see datastore_v1.proto definition for list_value - Preconditions.checkArgument(value.type() != ValueType.LIST, "Cannot contain another list"); + checkArgument(value.type() != ValueType.LIST, "Cannot contain another list"); listBuilder.add(value); return this; } @@ -87,8 +88,8 @@ public Builder addValue(Value first, Value... other) { @Override public Builder indexed(boolean indexed) { - // see issue #26 - throw DatastoreException.throwInvalidRequest("ListValue can't specify index"); + // todo() see issue #26 + throw new UnsupportedOperationException("ListValue can't specify index"); } /** diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java index dffcc3f0e16f..31acfaafc07e 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java @@ -27,92 +27,23 @@ import com.google.api.services.datastore.DatastoreV1.RollbackResponse; import com.google.api.services.datastore.DatastoreV1.RunQueryRequest; import com.google.api.services.datastore.DatastoreV1.RunQueryResponse; +import com.google.gcloud.datastore.DatastoreException; /** * Provides access to the remote Datastore service. */ public interface DatastoreRpc { - public class DatastoreRpcException extends Exception { - - /** - * The reason for the exception. - * - * @see Google - * Cloud Datastore error codes - */ - public enum Reason { - - ABORTED(true, "Request aborted", 409), - DEADLINE_EXCEEDED(true, "Deadline exceeded", 403), - FAILED_PRECONDITION(false, "Invalid request", 412), - INTERNAL(false, "Server returned an error", 500), - INVALID_ARGUMENT(false, "Request parameter has an invalid value", 400), - PERMISSION_DENIED(false, "Unauthorized request", 403), - RESOURCE_EXHAUSTED(false, "Quota exceeded", 402), - UNAVAILABLE(true, "Could not reach service", 503); - - private final boolean retryable; - private final String description; - private final int httpStatus; - - private Reason(boolean retryable, String description, int httpStatus) { - this.retryable = retryable; - this.description = description; - this.httpStatus = httpStatus; - } - - public boolean retryable() { - return retryable; - } - - public String description() { - return description; - } - - public int httpStatus() { - return httpStatus; - } - } - - private final String reason; - private final int httpStatus; - private final boolean retryable; - - public DatastoreRpcException(Reason reason) { - this(reason.name(), reason.httpStatus, reason.retryable, reason.description); - } - - public DatastoreRpcException(String reason, int httpStatus, boolean retryable, String message) { - super(message); - this.reason = reason; - this.httpStatus = httpStatus; - this.retryable = retryable; - } - - public String reason() { - return reason; - } - - public int httpStatus() { - return httpStatus; - } - - public boolean retryable() { - return retryable; - } - } - - AllocateIdsResponse allocateIds(AllocateIdsRequest request) throws DatastoreRpcException; + AllocateIdsResponse allocateIds(AllocateIdsRequest request) throws DatastoreException; BeginTransactionResponse beginTransaction(BeginTransactionRequest request) - throws DatastoreRpcException; + throws DatastoreException; - CommitResponse commit(CommitRequest request) throws DatastoreRpcException; + CommitResponse commit(CommitRequest request) throws DatastoreException; - LookupResponse lookup(LookupRequest request) throws DatastoreRpcException; + LookupResponse lookup(LookupRequest request) throws DatastoreException; - RollbackResponse rollback(RollbackRequest request) throws DatastoreRpcException; + RollbackResponse rollback(RollbackRequest request) throws DatastoreException; - RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreRpcException; + RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreException; } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java index 028027f4bc33..3b703f58d628 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java @@ -29,42 +29,24 @@ import com.google.api.services.datastore.DatastoreV1.RunQueryRequest; import com.google.api.services.datastore.DatastoreV1.RunQueryResponse; import com.google.api.services.datastore.client.Datastore; -import com.google.api.services.datastore.client.DatastoreException; import com.google.api.services.datastore.client.DatastoreFactory; import com.google.api.services.datastore.client.DatastoreOptions.Builder; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; +import com.google.gcloud.datastore.DatastoreException; import com.google.gcloud.datastore.DatastoreOptions; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; import org.json.JSONException; import org.json.JSONObject; import org.json.JSONTokener; +import java.io.IOException; import java.net.InetAddress; -import java.net.SocketTimeoutException; import java.net.URL; -import java.util.HashMap; -import java.util.Map; public class DefaultDatastoreRpc implements DatastoreRpc { private final Datastore client; - private static final ImmutableMap STR_TO_REASON; - private static final ImmutableMap HTTP_STATUS_TO_REASON; - - static { - ImmutableMap.Builder builder = ImmutableMap.builder(); - Map httpCodes = new HashMap<>(); - for (Reason reason : Reason.values()) { - builder.put(reason.name(), reason); - httpCodes.put(reason.httpStatus(), reason); - } - STR_TO_REASON = builder.build(); - HTTP_STATUS_TO_REASON = ImmutableMap.copyOf(httpCodes); - } - public DefaultDatastoreRpc(DatastoreOptions options) { String normalizedHost = normalizeHost(options.host()); client = DatastoreFactory.get().create( @@ -105,88 +87,81 @@ private static boolean includesScheme(String url) { return url.startsWith("http://") || url.startsWith("https://"); } - private static DatastoreRpcException translate(DatastoreException exception) { + private static DatastoreException translate( + com.google.api.services.datastore.client.DatastoreException exception) { String message = exception.getMessage(); - String reasonStr = ""; + int code = exception.getCode(); + String reason = ""; if (message != null) { try { JSONObject json = new JSONObject(new JSONTokener(message)); JSONObject error = json.getJSONObject("error").getJSONArray("errors").getJSONObject(0); - reasonStr = error.getString("reason"); + reason = error.getString("reason"); message = error.getString("message"); } catch (JSONException ignore) { // ignore - will be converted to unknown } } - Reason reason = STR_TO_REASON.get(reasonStr); if (reason == null) { - reason = HTTP_STATUS_TO_REASON.get(exception.getCode()); - } - if (reason != null) { - return new DatastoreRpcException(reason); - } else { - boolean retryable = false; - reasonStr = "Unknown"; - if (exception.getCause() instanceof SocketTimeoutException) { - retryable = true; - reasonStr = "Request timeout"; + if (exception.getCause() instanceof IOException) { + return new DatastoreException((IOException) exception.getCause()); } - return new DatastoreRpcException(reasonStr, exception.getCode(), retryable, message); } + return new DatastoreException(DatastoreException.Reason.of(reason), code, message); } @Override public AllocateIdsResponse allocateIds(AllocateIdsRequest request) - throws DatastoreRpcException { + throws DatastoreException { try { return client.allocateIds(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override public BeginTransactionResponse beginTransaction(BeginTransactionRequest request) - throws DatastoreRpcException { + throws DatastoreException { try { return client.beginTransaction(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public CommitResponse commit(CommitRequest request) throws DatastoreRpcException { + public CommitResponse commit(CommitRequest request) throws DatastoreException { try { return client.commit(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public LookupResponse lookup(LookupRequest request) throws DatastoreRpcException { + public LookupResponse lookup(LookupRequest request) throws DatastoreException { try { return client.lookup(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public RollbackResponse rollback(RollbackRequest request) throws DatastoreRpcException { + public RollbackResponse rollback(RollbackRequest request) throws DatastoreException { try { return client.rollback(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreRpcException { + public RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreException { try { return client.runQuery(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseDatastoreBatchWriterTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseDatastoreBatchWriterTest.java index ad8e0cee266a..7ca1f11ef04a 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseDatastoreBatchWriterTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseDatastoreBatchWriterTest.java @@ -109,25 +109,25 @@ public void testAddAfterDelete() throws Exception { assertEquals(pb, batchWriter.toMutationPb().build()); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalArgumentException.class) public void testAddDuplicate() throws Exception { batchWriter.add(ENTITY1); batchWriter.add(ENTITY1); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalArgumentException.class) public void testAddAfterPut() throws Exception { batchWriter.put(ENTITY1); batchWriter.add(ENTITY1); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalArgumentException.class) public void testAddAfterUpdate() throws Exception { batchWriter.update(ENTITY1); batchWriter.add(ENTITY1); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalStateException.class) public void testAddWhenNotActive() throws Exception { batchWriter.deactivate(); batchWriter.add(ENTITY1); @@ -145,7 +145,7 @@ public void testAddWithDeferredAllocation() throws Exception { assertEquals(pb, batchWriter.toMutationPb().build()); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalStateException.class) public void testAddWithDeferredAllocationWhenNotActive() throws Exception { batchWriter.deactivate(); batchWriter.addWithDeferredIdAllocation(INCOMPLETE_ENTITY_1); @@ -196,13 +196,13 @@ public void testUpdateAfterPut() throws Exception { assertEquals(pb, batchWriter.toMutationPb().build()); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalArgumentException.class) public void testUpdateAfterDelete() throws Exception { batchWriter.delete(KEY1); batchWriter.update(ENTITY1, ENTITY2); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalStateException.class) public void testUpdateWhenNotActive() throws Exception { batchWriter.deactivate(); batchWriter.update(ENTITY1); @@ -264,7 +264,7 @@ public void testPutAfterDelete() throws Exception { assertEquals(pb, batchWriter.toMutationPb().build()); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalStateException.class) public void testPutWhenNotActive() throws Exception { batchWriter.deactivate(); batchWriter.put(ENTITY1); @@ -314,7 +314,7 @@ public void testDeleteAfterPut() throws Exception { assertEquals(pb, batchWriter.toMutationPb().build()); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalStateException.class) public void testDeleteWhenNotActive() throws Exception { batchWriter.deactivate(); batchWriter.delete(KEY1); diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseEntityTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseEntityTest.java index 5ece01508d3a..0b434eaa5adf 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseEntityTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/BaseEntityTest.java @@ -84,7 +84,7 @@ public void testGetValue() throws Exception { assertEquals(BlobValue.of(BLOB), entity.getValue("blob")); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalArgumentException.class) public void testGetValueNotFound() throws Exception { BaseEntity entity = builder.clear().build(); entity.getValue("blob"); @@ -99,7 +99,7 @@ public void testIsNull() throws Exception { assertTrue(entity.isNull("blob")); } - @Test(expected = DatastoreException.class) + @Test(expected = IllegalArgumentException.class) public void testIsNullNotFound() throws Exception { BaseEntity entity = builder.clear().build(); entity.isNull("null"); diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java index 9ad836b15a4e..483a242b1689 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java @@ -16,50 +16,102 @@ package com.google.gcloud.datastore; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import com.google.gcloud.datastore.DatastoreException.DatastoreError; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; +import com.google.common.collect.ImmutableSet; +import com.google.gcloud.BaseServiceException; +import com.google.gcloud.RetryHelper.RetryHelperException; +import com.google.gcloud.datastore.DatastoreException.Reason; import org.junit.Test; +import java.util.Set; + public class DatastoreExceptionTest { + private static final Reason RETRYABLE_REASON = Reason.ABORTED; + private static final Reason NON_RETRYABLE_REASON = Reason.PERMISSION_DENIED; + private static final String MESSAGE = "message"; + private static final int NON_RETRYABLE_CODE = 500; + private static final int RETRYABLE_CODE = 403; + private static final Set RETRYABLE_CODES = ImmutableSet.of(409, 403, 503); + @Test - public void testDatastoreError() throws Exception { - for (Reason reason : Reason.values()) { - DatastoreError error = DatastoreError.valueOf(reason.name()); - assertEquals(reason.retryable(), error.retryable()); - assertEquals(reason.description(), error.description()); - assertEquals(reason.httpStatus(), error.httpStatus()); + public void testConstructor() { + DatastoreException exception; + for (int code : RETRYABLE_CODES) { + exception = new DatastoreException(RETRYABLE_REASON, code, MESSAGE); + assertTrue(exception.idempotent()); + assertTrue(exception.retryable()); + assertEquals(code, exception.code()); + assertEquals(RETRYABLE_REASON, exception.reason()); + assertEquals(MESSAGE, exception.getMessage()); + } + exception = new DatastoreException(RETRYABLE_REASON, NON_RETRYABLE_CODE, MESSAGE); + assertTrue(exception.idempotent()); + assertFalse(exception.retryable()); + assertEquals(NON_RETRYABLE_CODE, exception.code()); + assertEquals(RETRYABLE_REASON, exception.reason()); + assertEquals(MESSAGE, exception.getMessage()); + exception = new DatastoreException(NON_RETRYABLE_REASON, RETRYABLE_CODE, MESSAGE); + assertTrue(exception.idempotent()); + assertFalse(exception.retryable()); + assertEquals(RETRYABLE_CODE, exception.code()); + assertEquals(NON_RETRYABLE_REASON, exception.reason()); + assertEquals(MESSAGE, exception.getMessage()); + Exception cause = new Exception(); + for (int code : RETRYABLE_CODES) { + exception = new DatastoreException(RETRYABLE_REASON, code, MESSAGE, cause); + assertTrue(exception.idempotent()); + assertTrue(exception.retryable()); + assertEquals(code, exception.code()); + assertEquals(RETRYABLE_REASON, exception.reason()); + assertEquals(MESSAGE, exception.getMessage()); + assertSame(cause, exception.getCause()); } - - DatastoreException exception = new DatastoreException(DatastoreError.ABORTED, "bla"); - assertEquals(DatastoreError.ABORTED, exception.datastoreError()); } @Test - public void testTranslateAndThrow() throws Exception { - for (Reason reason : Reason.values()) { - try { - DatastoreException.translateAndThrow(new DatastoreRpcException(reason)); - fail("Exception expected"); - } catch (DatastoreException ex) { - assertEquals(reason.name(), ex.datastoreError().name()); - } + public void testPropagateUserException() throws Exception { + Exception cause = new Exception(MESSAGE); + try { + DatastoreException.propagateUserException(cause); + fail("DatastoreException expected"); + } catch (DatastoreException ex) { + assertEquals(Reason.UNKNOWN, ex.reason()); + assertEquals(BaseServiceException.UNKNOWN_CODE, ex.code()); + assertEquals(MESSAGE, ex.getMessage()); + assertEquals(false, ex.retryable()); + assertEquals(true, ex.idempotent()); + assertSame(cause, ex.getCause()); } } @Test - public void testThrowInvalidRequest() throws Exception { + public void testTranslateAndThrow() throws Exception { + Exception cause = new Exception(); + RetryHelperException exceptionMock = createMock(RetryHelperException.class); + expect(exceptionMock.getCause()).andReturn(cause); + expect(exceptionMock.getMessage()).andReturn(MESSAGE); + replay(exceptionMock); try { - DatastoreException.throwInvalidRequest("message %s %d", "a", 1); - fail("Exception expected"); + DatastoreException.translateAndThrow(exceptionMock); } catch (DatastoreException ex) { - assertEquals(DatastoreError.FAILED_PRECONDITION, ex.datastoreError()); - assertEquals("message a 1", ex.getMessage()); + assertEquals(Reason.UNKNOWN, ex.reason()); + assertEquals(BaseServiceException.UNKNOWN_CODE, ex.code()); + assertEquals(MESSAGE, ex.getMessage()); + assertEquals(false, ex.retryable()); + assertEquals(true, ex.idempotent()); + } finally { + verify(exceptionMock); } } } diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java index 162104c0dd84..90356d6bfaeb 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java @@ -29,14 +29,13 @@ import com.google.api.services.datastore.DatastoreV1.EntityResult; import com.google.common.collect.Iterators; import com.google.gcloud.RetryParams; +import com.google.gcloud.datastore.DatastoreException.Reason; import com.google.gcloud.datastore.Query.ResultType; import com.google.gcloud.datastore.StructuredQuery.OrderBy; import com.google.gcloud.datastore.StructuredQuery.Projection; import com.google.gcloud.datastore.StructuredQuery.PropertyFilter; import com.google.gcloud.datastore.testing.LocalGcdHelper; import com.google.gcloud.spi.DatastoreRpc; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; import com.google.gcloud.spi.DatastoreRpcFactory; import org.easymock.EasyMock; @@ -166,14 +165,14 @@ public void testNewTransactionCommit() { try { transaction.commit(); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } try { transaction.rollback(); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } @@ -197,7 +196,7 @@ public void testTransactionWithRead() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(DatastoreException.DatastoreError.ABORTED, expected.datastoreError()); + assertEquals(Reason.ABORTED, expected.reason()); } } @@ -225,7 +224,7 @@ public void testTransactionWithQuery() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(DatastoreException.DatastoreError.ABORTED, expected.datastoreError()); + assertEquals(Reason.ABORTED, expected.reason()); } } @@ -243,7 +242,7 @@ public void testNewTransactionRollback() { try { transaction.commit(); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } @@ -260,28 +259,28 @@ private void verifyNotUsable(DatastoreWriter writer) { try { writer.add(ENTITY3); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } try { writer.put(ENTITY3); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } try { writer.update(ENTITY3); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } try { writer.delete(ENTITY3.key()); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } } @@ -326,7 +325,7 @@ public void testNewBatch() { try { batch.submit(); fail("Expecting a failure"); - } catch (DatastoreException ex) { + } catch (IllegalStateException ex) { // expected to fail } verifyNotUsable(batch); @@ -546,13 +545,13 @@ public void testGetArrayNoDeferredResults() { try { entity3.getString("str"); fail("Expecting a failure"); - } catch (DatastoreException expected) { + } catch (IllegalArgumentException expected) { // expected - no such property } assertFalse(result.hasNext()); } - public void testGetArrayDeferredResults() throws DatastoreRpcException { + public void testGetArrayDeferredResults() throws DatastoreException { Set requestedKeys = new HashSet<>(); requestedKeys.add(KEY1); requestedKeys.add(KEY2); @@ -567,7 +566,7 @@ public void testGetArrayDeferredResults() throws DatastoreRpcException { assertEquals(requestedKeys, keysOfFoundEntities); } - public void testFetchArrayDeferredResults() throws DatastoreRpcException { + public void testFetchArrayDeferredResults() throws DatastoreException { List foundEntities = createDatastoreForDeferredLookup().fetch(KEY1, KEY2, KEY3, KEY4, KEY5); assertEquals(foundEntities.get(0).key(), KEY1); @@ -578,7 +577,7 @@ public void testFetchArrayDeferredResults() throws DatastoreRpcException { assertEquals(foundEntities.size(), 5); } - private Datastore createDatastoreForDeferredLookup() throws DatastoreRpcException { + private Datastore createDatastoreForDeferredLookup() throws DatastoreException { List keysPb = new ArrayList<>(); keysPb.add(KEY1.toPb()); keysPb.add(KEY2.toPb()); @@ -734,7 +733,7 @@ public void testRetryableException() throws Exception { EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(DatastoreOptions.class))) .andReturn(rpcMock); EasyMock.expect(rpcMock.lookup(requestPb)) - .andThrow(new DatastoreRpc.DatastoreRpcException(Reason.UNAVAILABLE)) + .andThrow(new DatastoreException(Reason.UNAVAILABLE, 503, "Server returned an error.")) .andReturn(responsePb); EasyMock.replay(rpcFactoryMock, rpcMock); DatastoreOptions options = this.options.toBuilder() @@ -756,7 +755,8 @@ public void testNonRetryableException() throws Exception { EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(DatastoreOptions.class))) .andReturn(rpcMock); EasyMock.expect(rpcMock.lookup(requestPb)) - .andThrow(new DatastoreRpc.DatastoreRpcException(Reason.PERMISSION_DENIED)) + .andThrow(new DatastoreException(Reason.PERMISSION_DENIED, 403, + "Indicates that the user was not authorized to make the request.")) .times(1); EasyMock.replay(rpcFactoryMock, rpcMock); RetryParams retryParams = RetryParams.builder().retryMinAttempts(2).build(); @@ -766,7 +766,7 @@ public void testNonRetryableException() throws Exception { .build(); Datastore datastore = options.service(); thrown.expect(DatastoreException.class); - thrown.expectMessage(Reason.PERMISSION_DENIED.description()); + thrown.expectMessage("Indicates that the user was not authorized to make the request."); datastore.get(KEY1); EasyMock.verify(rpcFactoryMock, rpcMock); } diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/ListValueTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/ListValueTest.java index 7af82c29901d..25ddd4fd167c 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/ListValueTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/ListValueTest.java @@ -46,7 +46,7 @@ public void testOf() throws Exception { assertFalse(value.hasMeaning()); } - @Test(expected = DatastoreException.class) + @Test(expected = UnsupportedOperationException.class) public void testIndexedCannotBeSpecified() { ListValue.builder().indexed(false); } diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java index d874f99ebb4c..ec326edbe75f 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java @@ -34,7 +34,6 @@ import com.google.api.client.googleapis.batch.json.JsonBatchCallback; import com.google.api.client.googleapis.json.GoogleJsonError; -import com.google.api.client.googleapis.json.GoogleJsonResponseException; import com.google.api.client.http.ByteArrayContent; import com.google.api.client.http.GenericUrl; import com.google.api.client.http.HttpHeaders; @@ -59,7 +58,6 @@ import com.google.api.services.storage.model.StorageObject; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gcloud.storage.StorageException; @@ -68,12 +66,10 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.net.SocketTimeoutException; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; public class DefaultStorageRpc implements StorageRpc { @@ -82,7 +78,6 @@ public class DefaultStorageRpc implements StorageRpc { private final Storage storage; // see: https://cloud.google.com/storage/docs/concepts-techniques#practices - private static final Set RETRYABLE_CODES = ImmutableSet.of(504, 503, 502, 500, 429, 408); private static final long MEGABYTE = 1024L * 1024L; private static final int MAX_BATCH_DELETES = 100; @@ -97,25 +92,11 @@ public DefaultStorageRpc(StorageOptions options) { } private static StorageException translate(IOException exception) { - StorageException translated; - if (exception instanceof GoogleJsonResponseException - && ((GoogleJsonResponseException) exception).getDetails() != null) { - translated = translate(((GoogleJsonResponseException) exception).getDetails()); - } else { - boolean retryable = false; - if (exception instanceof SocketTimeoutException) { - retryable = true; - } - translated = new StorageException(0, exception.getMessage(), retryable); - } - translated.initCause(exception); - return translated; + return new StorageException(exception); } - private static StorageException translate(GoogleJsonError exception) { - boolean retryable = RETRYABLE_CODES.contains(exception.getCode()) - || "InternalError".equals(exception.getMessage()); - return new StorageException(exception.getCode(), exception.getMessage(), retryable); + private static StorageException translate(GoogleJsonError error) { + return new StorageException(error); } @Override diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannelImpl.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannelImpl.java index 8fe6eae66d8f..b11f443a3a66 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannelImpl.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannelImpl.java @@ -20,6 +20,7 @@ import com.google.api.services.storage.model.StorageObject; import com.google.common.base.MoreObjects; +import com.google.gcloud.BaseServiceException; import com.google.gcloud.RestorableState; import com.google.gcloud.RetryHelper; import com.google.gcloud.spi.StorageRpc; @@ -128,7 +129,7 @@ public Tuple call() { if (lastEtag != null && !Objects.equals(result.x(), lastEtag)) { StringBuilder messageBuilder = new StringBuilder(); messageBuilder.append("Blob ").append(blob).append(" was updated while reading"); - throw new StorageException(0, messageBuilder.toString(), false); + throw new StorageException(BaseServiceException.UNKNOWN_CODE, messageBuilder.toString()); } lastEtag = result.x(); buffer = result.y(); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java index 6e1c33b137fd..c8951e5396ce 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/Storage.java @@ -33,7 +33,6 @@ import java.io.InputStream; import java.io.Serializable; import java.net.URL; -import java.nio.ByteBuffer; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java index c1075ae28c8b..5ff1f22d3107 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java @@ -16,11 +16,15 @@ package com.google.gcloud.storage; +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.common.collect.ImmutableSet; import com.google.gcloud.BaseServiceException; -import com.google.gcloud.RetryHelper; import com.google.gcloud.RetryHelper.RetryHelperException; import com.google.gcloud.RetryHelper.RetryInterruptedException; +import java.io.IOException; +import java.util.Set; + /** * Storage service exception. * @@ -29,11 +33,29 @@ */ public class StorageException extends BaseServiceException { + private static final Set RETRYABLE_CODES = ImmutableSet.of(504, 503, 502, 500, 429, 408); private static final long serialVersionUID = 8088235105953640145L; - private static final int UNKNOWN_CODE = -1; - public StorageException(int code, String message, boolean retryable) { - super(code, message, retryable); + public StorageException(int code, String message) { + super(code, message, RETRYABLE_CODES.contains(code), true); + } + + public StorageException(IOException exception) { + super(exception, true); + } + + public StorageException(GoogleJsonError error) { + super(error, true); + } + + @Override + protected Set retryableCodes() { + return RETRYABLE_CODES; + } + + @Override + protected boolean isRetryable(GoogleJsonError error) { + return super.isRetryable(error) || "InternalError".equals(error.getMessage()); } /** @@ -43,13 +65,8 @@ public StorageException(int code, String message, boolean retryable) { * @throws StorageException when {@code ex} was caused by a {@code StorageException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - static StorageException translateAndThrow(RetryHelperException ex) { - if (ex.getCause() instanceof StorageException) { - throw (StorageException) ex.getCause(); - } - if (ex instanceof RetryHelper.RetryInterruptedException) { - RetryHelper.RetryInterruptedException.propagate(); - } - throw new StorageException(UNKNOWN_CODE, ex.getMessage(), false); + public static StorageException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndThrow(ex); + throw new StorageException(UNKNOWN_CODE, ex.getMessage()); } } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java index 3c3d1aebb3df..2a6a65178803 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java @@ -73,8 +73,8 @@ public class RemoteGcsHelperTest { private static final List BLOB_LIST = ImmutableList.of( BlobInfo.builder(BUCKET_NAME, "n1").build(), BlobInfo.builder(BUCKET_NAME, "n2").build()); - private static final StorageException RETRYABLE_EXCEPTION = new StorageException(409, "", true); - private static final StorageException FATAL_EXCEPTION = new StorageException(500, "", false); + private static final StorageException RETRYABLE_EXCEPTION = new StorageException(409, ""); + private static final StorageException FATAL_EXCEPTION = new StorageException(501, ""); private static final Page BLOB_PAGE = new Page() { @Override diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java index 42671d37c60c..a0a93526310e 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java @@ -1271,7 +1271,7 @@ public Tuple apply(StorageObject f) { public void testRetryableException() { BlobId blob = BlobId.of(BUCKET_NAME1, BLOB_NAME1); EasyMock.expect(storageRpcMock.get(blob.toPb(), EMPTY_RPC_OPTIONS)) - .andThrow(new StorageException(500, "InternalError", true)) + .andThrow(new StorageException(500, "InternalError")) .andReturn(BLOB_INFO1.toPb()); EasyMock.replay(storageRpcMock); storage = options.toBuilder().retryParams(RetryParams.defaultInstance()).build().service(); @@ -1284,7 +1284,7 @@ public void testNonRetryableException() { BlobId blob = BlobId.of(BUCKET_NAME1, BLOB_NAME1); String exceptionMessage = "Not Implemented"; EasyMock.expect(storageRpcMock.get(blob.toPb(), EMPTY_RPC_OPTIONS)) - .andThrow(new StorageException(501, exceptionMessage, false)); + .andThrow(new StorageException(501, exceptionMessage)); EasyMock.replay(storageRpcMock); storage = options.toBuilder().retryParams(RetryParams.defaultInstance()).build().service(); thrown.expect(StorageException.class);