address comments

neozwu · neozwu · commit 0625f801d1cd · 2017-04-17T15:26:29.000-07:00
diff --git a/google-cloud-core/src/main/java/com/google/cloud/BaseService.java b/google-cloud-core/src/main/java/com/google/cloud/BaseService.java
@@ -39,7 +39,7 @@ public RetryResult afterEval(Exception exception, RetryResult retryResult) {
     public RetryResult beforeEval(Exception exception) {
       if (exception instanceof BaseServiceException) {
         boolean retriable = ((BaseServiceException) exception).isRetryable();
-        return retriable ? Interceptor.RetryResult.RETRY : RetryResult.CONTINUE_EVALUATION;
+        return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.CONTINUE_EVALUATION;
       }
       return Interceptor.RetryResult.CONTINUE_EVALUATION;
     }
diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/TransactionExceptionHandler.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/TransactionExceptionHandler.java
@@ -22,12 +22,14 @@
 public class TransactionExceptionHandler {
   public static final Interceptor TRANSACTION_EXCEPTION_HANDLER_INTERCEPTOR =
       new Interceptor() {
+        private final int ABORTED_CODE = 10;
+
         @Override
         public RetryResult beforeEval(Exception exception) {
           if (exception instanceof DatastoreException) {
             DatastoreException e = getInnerException((DatastoreException) exception);
-            if (e.getCode() == 10 || e.getReason() == "ABORTED") {
-              return RetryResult.RETRY;
+            if (e.getCode() == ABORTED_CODE || e.getReason() == "ABORTED") {
+              return Interceptor.RetryResult.RETRY;
             }
           }
           return Interceptor.RetryResult.CONTINUE_EVALUATION;
@@ -53,4 +55,4 @@ public static ExceptionHandler build() {
             DatastoreImpl.EXCEPTION_HANDLER_INTERCEPTOR, TRANSACTION_EXCEPTION_HANDLER_INTERCEPTOR)
         .build();
   }
-}
+}
diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/TransactionExceptionHandlerTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/TransactionExceptionHandlerTest.java
@@ -57,4 +57,4 @@ public void testShouldTry() {
     assertTrue(transactionHandler.accept(nestedDatastoreException));
     assertFalse(handler.accept(nestedDatastoreException));
   }
-}
+}
diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java
@@ -733,19 +733,43 @@ public void testDelete() {
 
   @Test
   public void testRunInTransaction() {
-    Datastore.TransactionCallable<Integer> callable = new Datastore.TransactionCallable<Integer>() {
-      private Integer attempts = 0;
-      public Integer run(DatastoreReaderWriter transaction) {
-        transaction.get(KEY1);
-        if (attempts < 1) {
-          ++attempts;
-          throw new DatastoreException(10,"","ABORTED",false,null);
-        }
-
-        return attempts;
-      }
-    };
-    int result = DATASTORE.runInTransaction(callable);
-    assertTrue(result == 1);
+    Datastore.TransactionCallable<Integer> callable1 =
+        new Datastore.TransactionCallable<Integer>() {
+          private Integer attempts = 0;
+
+          public Integer run(DatastoreReaderWriter transaction) {
+            transaction.get(KEY1);
+            if (attempts < 1) {
+              ++attempts;
+              throw new DatastoreException(10, "", "ABORTED", false, null);
+            }
+
+            return attempts;
+          }
+        };
+
+    int result = DATASTORE.runInTransaction(callable1);
+    assertEquals(result, 1);
+
+    Datastore.TransactionCallable<Integer> callable2 =
+        new Datastore.TransactionCallable<Integer>() {
+          private Integer attempts = 0;
+
+          public Integer run(DatastoreReaderWriter transaction) {
+            transaction.get(KEY1);
+            if (attempts < 1) {
+              ++attempts;
+              throw new DatastoreException(4, "", "DEADLINE_EXCEEDED", false, null);
+            }
+            return attempts;
+          }
+        };
+
+    try {
+      DATASTORE.runInTransaction(callable2);
+      fail("Expecting a failure");
+    } catch (DatastoreException expected) {
+      assertEquals(((DatastoreException) expected.getCause()).getCode(), 4);
+    }
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public RetryResult afterEval(Exception exception, RetryResult retryResult) {`
`39`	`39`	`public RetryResult beforeEval(Exception exception) {`
`40`	`40`	`if (exception instanceof BaseServiceException) {`
`41`	`41`	`boolean retriable = ((BaseServiceException) exception).isRetryable();`
`42`		`- return retriable ? Interceptor.RetryResult.RETRY : RetryResult.CONTINUE_EVALUATION;`
	`42`	`+ return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.CONTINUE_EVALUATION;`
`43`	`43`	`}`
`44`	`44`	`return Interceptor.RetryResult.CONTINUE_EVALUATION;`
`45`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -57,4 +57,4 @@ public void testShouldTry() {`
`57`	`57`	`assertTrue(transactionHandler.accept(nestedDatastoreException));`
`58`	`58`	`assertFalse(handler.accept(nestedDatastoreException));`
`59`	`59`	`}`
`60`		`-}`
	`60`	`+}`