diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/ExceptionHandler.java b/gcloud-java-core/src/main/java/com/google/gcloud/ExceptionHandler.java index 412462ae156e..a0fab3dca566 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/ExceptionHandler.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/ExceptionHandler.java @@ -16,7 +16,6 @@ package com.google.gcloud; -import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; @@ -48,18 +47,7 @@ public final class ExceptionHandler implements Serializable { public interface Interceptor extends Serializable { enum RetryResult { - - RETRY(true), ABORT(false); - - private final boolean booleanValue; - - RetryResult(boolean booleanValue) { - this.booleanValue = booleanValue; - } - - boolean booleanValue() { - return booleanValue; - } + NO_RETRY, RETRY, CONTINUE_EVALUATION; } /** @@ -67,8 +55,8 @@ boolean booleanValue() { * * @param exception the exception that is being evaluated * @return {@link RetryResult} to indicate if the exception should be ignored ( - * {@link RetryResult#RETRY}), propagated ({@link RetryResult#ABORT}), or evaluation - * should proceed ({@code null}). + * {@link RetryResult#RETRY}), propagated ({@link RetryResult#NO_RETRY}), or evaluation + * should proceed ({@link RetryResult#CONTINUE_EVALUATION}). */ RetryResult beforeEval(Exception exception); @@ -78,8 +66,8 @@ boolean booleanValue() { * @param exception the exception that is being evaluated * @param retryResult the result of the evaluation so far. * @return {@link RetryResult} to indicate if the exception should be ignored ( - * {@link RetryResult#RETRY}), propagated ({@link RetryResult#ABORT}), or evaluation - * should proceed ({@code null}). + * {@link RetryResult#RETRY}), propagated ({@link RetryResult#NO_RETRY}), or evaluation + * should proceed ({@link RetryResult#CONTINUE_EVALUATION}). */ RetryResult afterEval(Exception exception, RetryResult retryResult); } @@ -157,7 +145,7 @@ static final class RetryInfo implements Serializable { RetryInfo(Class exception, Interceptor.RetryResult retry) { this.exception = checkNotNull(exception); - this.retry = retry; + this.retry = checkNotNull(retry); } @Override @@ -189,7 +177,7 @@ private ExceptionHandler(Builder builder) { addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.RETRY), retryInfo); } for (Class exception : nonRetriableExceptions) { - addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.ABORT), retryInfo); + addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.NO_RETRY), retryInfo); } } @@ -253,18 +241,22 @@ public Set> getNonRetriableExceptions() { boolean shouldRetry(Exception ex) { for (Interceptor interceptor : interceptors) { - Interceptor.RetryResult retryResult = interceptor.beforeEval(ex); - if (retryResult != null) { - return retryResult.booleanValue(); + Interceptor.RetryResult retryResult = checkNotNull(interceptor.beforeEval(ex)); + if (retryResult != Interceptor.RetryResult.CONTINUE_EVALUATION) { + return retryResult == Interceptor.RetryResult.RETRY; } } RetryInfo retryInfo = findMostSpecificRetryInfo(this.retryInfo, ex.getClass()); Interceptor.RetryResult retryResult = - retryInfo == null ? Interceptor.RetryResult.ABORT : retryInfo.retry; + retryInfo == null ? Interceptor.RetryResult.NO_RETRY : retryInfo.retry; for (Interceptor interceptor : interceptors) { - retryResult = firstNonNull(interceptor.afterEval(ex, retryResult), retryResult); + Interceptor.RetryResult interceptorRetry = + checkNotNull(interceptor.afterEval(ex, retryResult)); + if (interceptorRetry != Interceptor.RetryResult.CONTINUE_EVALUATION) { + retryResult = interceptorRetry; + } } - return retryResult.booleanValue(); + return retryResult == Interceptor.RetryResult.RETRY; } /** diff --git a/gcloud-java-core/src/test/java/com/google/gcloud/ExceptionHandlerTest.java b/gcloud-java-core/src/test/java/com/google/gcloud/ExceptionHandlerTest.java index 3844f9de36d7..5ce05ad900a8 100644 --- a/gcloud-java-core/src/test/java/com/google/gcloud/ExceptionHandlerTest.java +++ b/gcloud-java-core/src/test/java/com/google/gcloud/ExceptionHandlerTest.java @@ -23,6 +23,8 @@ import com.google.gcloud.ExceptionHandler.Interceptor; import com.google.gcloud.ExceptionHandler.Interceptor.RetryResult; +import org.junit.rules.ExpectedException; +import org.junit.Rule; import org.junit.Test; import java.io.FileNotFoundException; @@ -36,6 +38,9 @@ */ public class ExceptionHandlerTest { + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Test public void testVerifyCaller() { class A implements Callable { @@ -128,13 +133,13 @@ public void testShouldTry() { assertFalse(handler.shouldRetry(new RuntimeException())); assertTrue(handler.shouldRetry(new NullPointerException())); - final AtomicReference before = new AtomicReference<>(RetryResult.ABORT); + final AtomicReference before = new AtomicReference<>(RetryResult.NO_RETRY); @SuppressWarnings("serial") Interceptor interceptor = new Interceptor() { @Override public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return retryResult == RetryResult.ABORT ? RetryResult.RETRY : RetryResult.ABORT; + return retryResult == RetryResult.NO_RETRY ? RetryResult.RETRY : RetryResult.NO_RETRY; } @Override @@ -158,11 +163,55 @@ public RetryResult beforeEval(Exception exception) { assertTrue(handler.shouldRetry(new RuntimeException())); assertTrue(handler.shouldRetry(new NullPointerException())); - before.set(null); + before.set(RetryResult.CONTINUE_EVALUATION); assertFalse(handler.shouldRetry(new IOException())); assertTrue(handler.shouldRetry(new ClosedByInterruptException())); assertTrue(handler.shouldRetry(new InterruptedException())); assertTrue(handler.shouldRetry(new RuntimeException())); assertFalse(handler.shouldRetry(new NullPointerException())); } + + @Test + public void testNullRetryResultFromBeforeEval() { + @SuppressWarnings("serial") + Interceptor interceptor = new Interceptor() { + + @Override + public RetryResult beforeEval(Exception exception) { + return null; + } + + @Override + public RetryResult afterEval(Exception exception, RetryResult retryResult) { + return RetryResult.CONTINUE_EVALUATION; + } + + }; + + ExceptionHandler handler = ExceptionHandler.builder().interceptor(interceptor).build(); + thrown.expect(NullPointerException.class); + handler.shouldRetry(new Exception()); + } + + @Test + public void testNullRetryResultFromAfterEval() { + @SuppressWarnings("serial") + Interceptor interceptor = new Interceptor() { + + @Override + public RetryResult beforeEval(Exception exception) { + return RetryResult.CONTINUE_EVALUATION; + } + + @Override + public RetryResult afterEval(Exception exception, RetryResult retryResult) { + return null; + } + + }; + + ExceptionHandler handler = ExceptionHandler.builder().interceptor(interceptor).build(); + thrown.expect(NullPointerException.class); + handler.shouldRetry(new Exception()); + } } 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 6f2454c62167..c12ffc8a032d 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 @@ -53,16 +53,16 @@ final class DatastoreImpl extends BaseService @Override public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return null; + 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.ABORT; + return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; } - return null; + return Interceptor.RetryResult.CONTINUE_EVALUATION; } }; private static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java index f59c6c670969..403f8a7ab78a 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java @@ -72,16 +72,16 @@ final class StorageImpl extends BaseService implements Storage { @Override public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return null; + return Interceptor.RetryResult.CONTINUE_EVALUATION; } @Override public RetryResult beforeEval(Exception exception) { if (exception instanceof StorageException) { boolean retriable = ((StorageException) exception).retryable(); - return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.ABORT; + return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; } - return null; + return Interceptor.RetryResult.CONTINUE_EVALUATION; } }; static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder()