Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 26, 2015
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af6bbef
Add third RetryResult (instead of using null) to denote 'proceed'
Sep 25, 2015
800b709
Rename RetryResult enum values, and update datastore and storage inte…
Sep 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 17 additions & 25 deletions gcloud-java-core/src/main/java/com/google/gcloud/ExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -48,27 +47,16 @@ 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;
}

/**
* This method is called before exception evaluation and could short-circuit the process.
*
* @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);

Expand All @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -189,7 +177,7 @@ private ExceptionHandler(Builder builder) {
addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.RETRY), retryInfo);
}
for (Class<? extends Exception> exception : nonRetriableExceptions) {
addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.ABORT), retryInfo);
addRetryInfo(new RetryInfo(exception, Interceptor.RetryResult.NO_RETRY), retryInfo);
}
}

Expand Down Expand Up @@ -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.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;
}

/**
Expand Down
55 changes: 52 additions & 3 deletions gcloud-java-core/src/test/java/com/google/gcloud/ExceptionHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,6 +38,9 @@
*/
public class ExceptionHandlerTest {

@Rule
public ExpectedException thrown = ExpectedException.none();

@Test
public void testVerifyCaller() {
class A implements Callable<Object> {
Expand Down Expand Up @@ -128,13 +133,13 @@ public void testShouldTry() {
assertFalse(handler.shouldRetry(new RuntimeException()));
assertTrue(handler.shouldRetry(new NullPointerException()));

final AtomicReference<RetryResult> before = new AtomicReference<>(RetryResult.ABORT);
final AtomicReference<RetryResult> 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
Expand All @@ -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());
}
}
6 changes: 3 additions & 3 deletions gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,16 @@ final class DatastoreImpl extends BaseService<DatastoreOptions>

@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()
Expand Down
6 changes: 3 additions & 3 deletions gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,16 @@ final class StorageImpl extends BaseService<StorageOptions> 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()
Expand Down