Skip to content

Commit af6bbef

Browse files
author
Ajay Kannan
committed
Add third RetryResult (instead of using null) to denote 'proceed'
1 parent d638926 commit af6bbef

3 files changed

Lines changed: 65 additions & 24 deletions

File tree

‎gcloud-java-core/src/main/java/com/google/gcloud/ExceptionHandler.java‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.google.gcloud;
1818

19-
import static com.google.common.base.MoreObjects.firstNonNull;
2019
import static com.google.common.base.Preconditions.checkNotNull;
2120

2221
import com.google.common.annotations.VisibleForTesting;
@@ -48,18 +47,7 @@ public final class ExceptionHandler implements Serializable {
4847
public interface Interceptor extends Serializable {
4948

5049
enum RetryResult {
51-
52-
RETRY(true), ABORT(false);
53-
54-
private final boolean booleanValue;
55-
56-
RetryResult(boolean booleanValue) {
57-
this.booleanValue = booleanValue;
58-
}
59-
60-
boolean booleanValue() {
61-
return booleanValue;
62-
}
50+
ABORT, RETRY, PROCEED;
6351
}
6452

6553
/**
@@ -68,7 +56,7 @@ boolean booleanValue() {
6856
* @param exception the exception that is being evaluated
6957
* @return {@link RetryResult} to indicate if the exception should be ignored (
7058
* {@link RetryResult#RETRY}), propagated ({@link RetryResult#ABORT}), or evaluation
71-
* should proceed ({@code null}).
59+
* should proceed ({@link RetryResult#PROCEED}).
7260
*/
7361
RetryResult beforeEval(Exception exception);
7462

@@ -79,7 +67,7 @@ boolean booleanValue() {
7967
* @param retryResult the result of the evaluation so far.
8068
* @return {@link RetryResult} to indicate if the exception should be ignored (
8169
* {@link RetryResult#RETRY}), propagated ({@link RetryResult#ABORT}), or evaluation
82-
* should proceed ({@code null}).
70+
* should proceed ({@link RetryResult#PROCEED}).
8371
*/
8472
RetryResult afterEval(Exception exception, RetryResult retryResult);
8573
}
@@ -157,7 +145,7 @@ static final class RetryInfo implements Serializable {
157145

158146
RetryInfo(Class<? extends Exception> exception, Interceptor.RetryResult retry) {
159147
this.exception = checkNotNull(exception);
160-
this.retry = retry;
148+
this.retry = checkNotNull(retry);
161149
}
162150

163151
@Override
@@ -253,18 +241,22 @@ public Set<Class<? extends Exception>> getNonRetriableExceptions() {
253241

254242
boolean shouldRetry(Exception ex) {
255243
for (Interceptor interceptor : interceptors) {
256-
Interceptor.RetryResult retryResult = interceptor.beforeEval(ex);
257-
if (retryResult != null) {
258-
return retryResult.booleanValue();
244+
Interceptor.RetryResult retryResult = checkNotNull(interceptor.beforeEval(ex));
245+
if (retryResult != Interceptor.RetryResult.PROCEED) {
246+
return interceptor.beforeEval(ex) == Interceptor.RetryResult.RETRY;
259247
}
260248
}
261249
RetryInfo retryInfo = findMostSpecificRetryInfo(this.retryInfo, ex.getClass());
262250
Interceptor.RetryResult retryResult =
263251
retryInfo == null ? Interceptor.RetryResult.ABORT : retryInfo.retry;
264252
for (Interceptor interceptor : interceptors) {
265-
retryResult = firstNonNull(interceptor.afterEval(ex, retryResult), retryResult);
253+
Interceptor.RetryResult interceptorRetry =
254+
checkNotNull(interceptor.afterEval(ex, retryResult));
255+
if (interceptorRetry != Interceptor.RetryResult.PROCEED) {
256+
retryResult = interceptorRetry;
257+
}
266258
}
267-
return retryResult.booleanValue();
259+
return retryResult == Interceptor.RetryResult.RETRY;
268260
}
269261

270262
/**

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,11 +158,60 @@ public RetryResult beforeEval(Exception exception) {
158158
assertTrue(handler.shouldRetry(new RuntimeException()));
159159
assertTrue(handler.shouldRetry(new NullPointerException()));
160160

161-
before.set(null);
161+
before.set(RetryResult.PROCEED);
162162
assertFalse(handler.shouldRetry(new IOException()));
163163
assertTrue(handler.shouldRetry(new ClosedByInterruptException()));
164164
assertTrue(handler.shouldRetry(new InterruptedException()));
165165
assertTrue(handler.shouldRetry(new RuntimeException()));
166166
assertFalse(handler.shouldRetry(new NullPointerException()));
167167
}
168+
169+
@Test
170+
public void testNullRetryResult() {
171+
@SuppressWarnings("serial")
172+
Interceptor interceptor1 = new Interceptor() {
173+
174+
@Override
175+
public RetryResult beforeEval(Exception exception) {
176+
return null;
177+
}
178+
179+
@Override
180+
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
181+
return RetryResult.PROCEED;
182+
}
183+
184+
};
185+
186+
@SuppressWarnings("serial")
187+
Interceptor interceptor2 = new Interceptor() {
188+
189+
@Override
190+
public RetryResult beforeEval(Exception exception) {
191+
return RetryResult.PROCEED;
192+
}
193+
194+
@Override
195+
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
196+
return null;
197+
}
198+
199+
};
200+
201+
ExceptionHandler handler1 = ExceptionHandler.builder().interceptor(interceptor1).build();
202+
try {
203+
handler1.shouldRetry(new Exception());
204+
fail("Expected null pointer exception due to null RetryResult from beforeEval");
205+
} catch (NullPointerException e) {
206+
// expected
207+
}
208+
209+
ExceptionHandler handler2 = ExceptionHandler.builder().interceptor(interceptor2).build();
210+
try {
211+
handler2.shouldRetry(new Exception());
212+
fail("Expected null pointer exception due to null RetryResult from afterEval");
213+
} catch (NullPointerException e) {
214+
// expected
215+
}
216+
}
168217
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ final class DatastoreImpl extends BaseService<DatastoreOptions>
5353

5454
@Override
5555
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
56-
return null;
56+
return Interceptor.RetryResult.PROCEED;
5757
}
5858

5959
@Override
@@ -62,7 +62,7 @@ public RetryResult beforeEval(Exception exception) {
6262
boolean retryable = ((DatastoreRpcException) exception).retryable();
6363
return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.ABORT;
6464
}
65-
return null;
65+
return Interceptor.RetryResult.PROCEED;
6666
}
6767
};
6868
private static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder()

0 commit comments

Comments
 (0)