Add third RetryResult (instead of using null) to denote 'proceed'

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;
-      }
+      ABORT, RETRY, PROCEED;
     }
 
     /**
@@ -68,7 +56,7 @@ 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}).
+     *         should proceed ({@link RetryResult#PROCEED}).
      */
     RetryResult beforeEval(Exception exception);
 
@@ -79,7 +67,7 @@ boolean booleanValue() {
      * @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}).
+     *         should proceed ({@link RetryResult#PROCEED}).
      */
     RetryResult afterEval(Exception exception, RetryResult retryResult);
   }
@@ -157,7 +145,7 @@ static final class RetryInfo implements Serializable {
 
     RetryInfo(Class<? extends Exception> exception, Interceptor.RetryResult retry) {
       this.exception = checkNotNull(exception);
-      this.retry = retry;
+      this.retry = checkNotNull(retry);
     }
 
     @Override
@@ -253,18 +241,22 @@ public Set<Class<? extends Exception>> 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.PROCEED) {
+        return interceptor.beforeEval(ex) == Interceptor.RetryResult.RETRY;
       }
     }
     RetryInfo retryInfo = findMostSpecificRetryInfo(this.retryInfo, ex.getClass());
     Interceptor.RetryResult retryResult =
         retryInfo == null ? Interceptor.RetryResult.ABORT : 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.PROCEED) {
+        retryResult = interceptorRetry;
+      }
     }
-    return retryResult.booleanValue();
+    return retryResult == Interceptor.RetryResult.RETRY;
   }
 
   /**

gcloud-java-core/src/test/java/com/google/gcloud/ExceptionHandlerTest.java

@@ -158,11 +158,60 @@ public RetryResult beforeEval(Exception exception) {
     assertTrue(handler.shouldRetry(new RuntimeException()));
     assertTrue(handler.shouldRetry(new NullPointerException()));
 
-    before.set(null);
+    before.set(RetryResult.PROCEED);
     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 testNullRetryResult() {
+    @SuppressWarnings("serial")
+    Interceptor interceptor1 = new Interceptor() {
+
+      @Override
+      public RetryResult beforeEval(Exception exception) {
+        return null;
+      }
+
+      @Override
+      public RetryResult afterEval(Exception exception, RetryResult retryResult) {
+        return RetryResult.PROCEED;
+      }
+
+    };
+
+    @SuppressWarnings("serial")
+    Interceptor interceptor2 = new Interceptor() {
+
+      @Override
+      public RetryResult beforeEval(Exception exception) {
+        return RetryResult.PROCEED;
+      }
+
+      @Override
+      public RetryResult afterEval(Exception exception, RetryResult retryResult) {
+        return null;
+      }
+
+    };
+
+    ExceptionHandler handler1 = ExceptionHandler.builder().interceptor(interceptor1).build();
+    try {
+      handler1.shouldRetry(new Exception());
+      fail("Expected null pointer exception due to null RetryResult from beforeEval");
+    } catch (NullPointerException e) {
+      // expected
+    }
+
+    ExceptionHandler handler2 = ExceptionHandler.builder().interceptor(interceptor2).build();
+    try {
+      handler2.shouldRetry(new Exception());
+      fail("Expected null pointer exception due to null RetryResult from afterEval");
+    } catch (NullPointerException e) {
+      // expected
+    }
+  }
 }

gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java

@@ -53,7 +53,7 @@ final class DatastoreImpl extends BaseService<DatastoreOptions>
 
         @Override
         public RetryResult afterEval(Exception exception, RetryResult retryResult) {
-          return null;
+          return Interceptor.RetryResult.PROCEED;
         }
 
         @Override
@@ -62,7 +62,7 @@ public RetryResult beforeEval(Exception exception) {
             boolean retryable = ((DatastoreRpcException) exception).retryable();
             return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.ABORT;
           }
-          return null;
+          return Interceptor.RetryResult.PROCEED;
         }
       };
   private static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder()