Skip to content

Commit 48458d3

Browse files
aozarovaozarov
committed
Merge pull request #181 from ajkannan/add-retry-result-value
Add third RetryResult (instead of using null) to denote 'proceed'
2 parents 0a71721 + 800b709 commit 48458d3

File tree

4 files changed

+75
-34
lines changed

4 files changed

+75
-34
lines changed

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

Lines changed: 17 additions & 25 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,27 +47,16 @@ 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+
NO_RETRY, RETRY, CONTINUE_EVALUATION;
6351
}
6452

6553
/**
6654
* This method is called before exception evaluation and could short-circuit the process.
6755
*
6856
* @param exception the exception that is being evaluated
6957
* @return {@link RetryResult} to indicate if the exception should be ignored (
70-
* {@link RetryResult#RETRY}), propagated ({@link RetryResult#ABORT}), or evaluation
71-
* should proceed ({@code null}).
58+
* {@link RetryResult#RETRY}), propagated ({@link RetryResult#NO_RETRY}), or evaluation
59+
* should proceed ({@link RetryResult#CONTINUE_EVALUATION}).
7260
*/
7361
RetryResult beforeEval(Exception exception);
7462

@@ -78,8 +66,8 @@ boolean booleanValue() {
7866
* @param exception the exception that is being evaluated
7967
* @param retryResult the result of the evaluation so far.
8068
* @return {@link RetryResult} to indicate if the exception should be ignored (
81-
* {@link RetryResult#RETRY}), propagated ({@link RetryResult#ABORT}), or evaluation
82-
* should proceed ({@code null}).
69+
* {@link RetryResult#RETRY}), propagated ({@link RetryResult#NO_RETRY}), or evaluation
70+
* should proceed ({@link RetryResult#CONTINUE_EVALUATION}).
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
@@ -189,7 +177,7 @@ private ExceptionHandler(Builder builder) {
189177
addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.RETRY), retryInfo);
190178
}
191179
for (Class<? extends Exception> exception : nonRetriableExceptions) {
192-
addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.ABORT), retryInfo);
180+
addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.NO_RETRY), retryInfo);
193181
}
194182
}
195183

@@ -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.CONTINUE_EVALUATION) {
246+
return retryResult == Interceptor.RetryResult.RETRY;
259247
}
260248
}
261249
RetryInfo retryInfo = findMostSpecificRetryInfo(this.retryInfo, ex.getClass());
262250
Interceptor.RetryResult retryResult =
263-
retryInfo == null ? Interceptor.RetryResult.ABORT : retryInfo.retry;
251+
retryInfo == null ? Interceptor.RetryResult.NO_RETRY : 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.CONTINUE_EVALUATION) {
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: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import com.google.gcloud.ExceptionHandler.Interceptor;
2424
import com.google.gcloud.ExceptionHandler.Interceptor.RetryResult;
2525

26+
import org.junit.rules.ExpectedException;
27+
import org.junit.Rule;
2628
import org.junit.Test;
2729

2830
import java.io.FileNotFoundException;
@@ -36,6 +38,9 @@
3638
*/
3739
public class ExceptionHandlerTest {
3840

41+
@Rule
42+
public ExpectedException thrown = ExpectedException.none();
43+
3944
@Test
4045
public void testVerifyCaller() {
4146
class A implements Callable<Object> {
@@ -128,13 +133,13 @@ public void testShouldTry() {
128133
assertFalse(handler.shouldRetry(new RuntimeException()));
129134
assertTrue(handler.shouldRetry(new NullPointerException()));
130135

131-
final AtomicReference<RetryResult> before = new AtomicReference<>(RetryResult.ABORT);
136+
final AtomicReference<RetryResult> before = new AtomicReference<>(RetryResult.NO_RETRY);
132137
@SuppressWarnings("serial")
133138
Interceptor interceptor = new Interceptor() {
134139

135140
@Override
136141
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
137-
return retryResult == RetryResult.ABORT ? RetryResult.RETRY : RetryResult.ABORT;
142+
return retryResult == RetryResult.NO_RETRY ? RetryResult.RETRY : RetryResult.NO_RETRY;
138143
}
139144

140145
@Override
@@ -158,11 +163,55 @@ public RetryResult beforeEval(Exception exception) {
158163
assertTrue(handler.shouldRetry(new RuntimeException()));
159164
assertTrue(handler.shouldRetry(new NullPointerException()));
160165

161-
before.set(null);
166+
before.set(RetryResult.CONTINUE_EVALUATION);
162167
assertFalse(handler.shouldRetry(new IOException()));
163168
assertTrue(handler.shouldRetry(new ClosedByInterruptException()));
164169
assertTrue(handler.shouldRetry(new InterruptedException()));
165170
assertTrue(handler.shouldRetry(new RuntimeException()));
166171
assertFalse(handler.shouldRetry(new NullPointerException()));
167172
}
173+
174+
@Test
175+
public void testNullRetryResultFromBeforeEval() {
176+
@SuppressWarnings("serial")
177+
Interceptor interceptor = new Interceptor() {
178+
179+
@Override
180+
public RetryResult beforeEval(Exception exception) {
181+
return null;
182+
}
183+
184+
@Override
185+
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
186+
return RetryResult.CONTINUE_EVALUATION;
187+
}
188+
189+
};
190+
191+
ExceptionHandler handler = ExceptionHandler.builder().interceptor(interceptor).build();
192+
thrown.expect(NullPointerException.class);
193+
handler.shouldRetry(new Exception());
194+
}
195+
196+
@Test
197+
public void testNullRetryResultFromAfterEval() {
198+
@SuppressWarnings("serial")
199+
Interceptor interceptor = new Interceptor() {
200+
201+
@Override
202+
public RetryResult beforeEval(Exception exception) {
203+
return RetryResult.CONTINUE_EVALUATION;
204+
}
205+
206+
@Override
207+
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
208+
return null;
209+
}
210+
211+
};
212+
213+
ExceptionHandler handler = ExceptionHandler.builder().interceptor(interceptor).build();
214+
thrown.expect(NullPointerException.class);
215+
handler.shouldRetry(new Exception());
216+
}
168217
}

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

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

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

5959
@Override
6060
public RetryResult beforeEval(Exception exception) {
6161
if (exception instanceof DatastoreRpcException) {
6262
boolean retryable = ((DatastoreRpcException) exception).retryable();
63-
return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.ABORT;
63+
return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY;
6464
}
65-
return null;
65+
return Interceptor.RetryResult.CONTINUE_EVALUATION;
6666
}
6767
};
6868
private static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder()

‎gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,16 @@ final class StorageImpl extends BaseService<StorageOptions> implements Storage {
7272

7373
@Override
7474
public RetryResult afterEval(Exception exception, RetryResult retryResult) {
75-
return null;
75+
return Interceptor.RetryResult.CONTINUE_EVALUATION;
7676
}
7777

7878
@Override
7979
public RetryResult beforeEval(Exception exception) {
8080
if (exception instanceof StorageException) {
8181
boolean retriable = ((StorageException) exception).retryable();
82-
return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.ABORT;
82+
return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY;
8383
}
84-
return null;
84+
return Interceptor.RetryResult.CONTINUE_EVALUATION;
8585
}
8686
};
8787
static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder()

0 commit comments

Comments
 (0)