Spring/spring-retry
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "GH-264: Add retrhro to @Retryable"

Browse Source 
This reverts commit ec245fd1ba.
This commit is contained in:
Gary Russell
2022-04-14 16:51:13 -04:00
parent e1ebf233d7
commit fdcfb4d2d2
9 changed files with 45 additions and 179 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java
View File

@@ -217,16 +217,13 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
RetryTemplate template = createTemplate(retryable.listeners());
template.setRetryPolicy(getRetryPolicy(retryable));
template.setBackOffPolicy(getBackoffPolicy(retryable.backoff()));
template.setNoRecoveryForNotRetryable(retryable.rethrow());
return RetryInterceptorBuilder.stateless().retryOperations(template).label(retryable.label())
.recoverer(getRecoverer(target, method, retryable.rethrow())).build();
.recoverer(getRecoverer(target, method)).build();
}
private MethodInterceptor getStatefulInterceptor(Object target, Method method, Retryable retryable) {
boolean rethrow = retryable.rethrow();
RetryTemplate template = createTemplate(retryable.listeners());
template.setRetryContextCache(this.retryContextCache);
template.setNoRecoveryForNotRetryable(rethrow);
CircuitBreaker circuit = AnnotatedElementUtils.findMergedAnnotation(method, CircuitBreaker.class);
if (circuit == null) {
@@ -244,7 +241,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
label = method.toGenericString();
}
return RetryInterceptorBuilder.circuitBreaker().keyGenerator(new FixedKeyGenerator("circuit"))
.retryOperations(template).recoverer(getRecoverer(target, method, rethrow)).label(label).build();
.retryOperations(template).recoverer(getRecoverer(target, method)).label(label).build();
}
RetryPolicy policy = getRetryPolicy(retryable);
template.setRetryPolicy(policy);
@@ -252,7 +249,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
String label = retryable.label();
return RetryInterceptorBuilder.stateful().keyGenerator(this.methodArgumentsKeyGenerator)
.newMethodArgumentsIdentifier(this.newMethodArgumentsIdentifier).retryOperations(template).label(label)
.recoverer(getRecoverer(target, method, rethrow)).build();
.recoverer(getRecoverer(target, method)).build();
}
private long getOpenTimeout(CircuitBreaker circuit) {
@@ -296,7 +293,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
return listeners;
}
private MethodInvocationRecoverer<?> getRecoverer(Object target, Method method, boolean rethrow) {
private MethodInvocationRecoverer<?> getRecoverer(Object target, Method method) {
if (target instanceof MethodInvocationRecoverer) {
return (MethodInvocationRecoverer<?>) target;
}
@@ -313,9 +310,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
if (!foundRecoverable.get()) {
return null;
}
RecoverAnnotationRecoveryHandler recoveryHandler = new RecoverAnnotationRecoveryHandler<Object>(target, method);
recoveryHandler.setThrowLastExceptionWhenNoRecoverMethod(rethrow);
return recoveryHandler;
return new RecoverAnnotationRecoveryHandler<Object>(target, method);
}
private RetryPolicy getRetryPolicy(Annotation retryable) {

11
src/main/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandler.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2022 the original author or authors.
* Copyright 2013-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -64,23 +64,16 @@ public class RecoverAnnotationRecoveryHandler<T> implements MethodInvocationReco
private String recoverMethodName;
private boolean throwLastExceptionWhenNoRecoverMethod;
public RecoverAnnotationRecoveryHandler(Object target, Method method) {
this.target = target;
init(target, method);
}
public void setThrowLastExceptionWhenNoRecoverMethod(boolean throwLastExceptionWhenNoRecoverMethod) {
this.throwLastExceptionWhenNoRecoverMethod = throwLastExceptionWhenNoRecoverMethod;
}
@Override
public T recover(Object[] args, Throwable cause) {
Method method = findClosestMatch(args, cause.getClass());
if (method == null) {
throw throwLastExceptionWhenNoRecoverMethod && cause instanceof RuntimeException ? (RuntimeException) cause
: new ExhaustedRetryException("Cannot locate recovery method", cause);
throw new ExhaustedRetryException("Cannot locate recovery method", cause);
}
SimpleMetadata meta = this.methods.get(method);
Object[] argsToUse = meta.getArgs(cause, args);

10
src/main/java/org/springframework/retry/annotation/Retryable.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2022 the original author or authors.
* Copyright 2014-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -130,12 +130,4 @@ public @interface Retryable {
*/
String[] listeners() default {};
/**
* When true raw exceptions are thrown without being wrapped and no recovery is
* performed for not-retryable exceptions.
* @return true if to rethrow raw exceptions, default false
* @since 1.3.3
*/
boolean rethrow() default false;
}

21
src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2006-2022 the original author or authors.
* Copyright 2006-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -199,16 +199,6 @@ public class SimpleRetryPolicy implements RetryPolicy {
return new SimpleRetryContext(parent);
}
/**
* Delegates to an exception classifier.
* @param ex
* @return true if this exception or its ancestors have been registered as retryable.
* @since 1.3.3
*/
public boolean retryForException(Throwable ex) {
return this.retryableClassifier.classify(ex);
}
private static class SimpleRetryContext extends RetryContextSupport {
public SimpleRetryContext(RetryContext parent) {
@@ -217,6 +207,15 @@ public class SimpleRetryPolicy implements RetryPolicy {
}
/**
* Delegates to an exception classifier.
* @param ex
* @return true if this exception or its ancestors have been registered as retryable.
*/
private boolean retryForException(Throwable ex) {
return this.retryableClassifier.classify(ex);
}
@Override
public String toString() {
return ClassUtils.getShortName(getClass()) + "[maxAttempts=" + this.maxAttempts + "]";

29
src/main/java/org/springframework/retry/support/RetryTemplate.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2006-2022 the original author or authors.
* Copyright 2006-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -96,8 +96,6 @@ public class RetryTemplate implements RetryOperations {
private boolean throwLastExceptionOnExhausted;
private boolean noRecoveryForNotRetryable;
/**
* Main entry point to configure RetryTemplate using fluent API. See
* {@link RetryTemplateBuilder} for usage examples and details.
@@ -126,14 +124,6 @@ public class RetryTemplate implements RetryOperations {
this.throwLastExceptionOnExhausted = throwLastExceptionOnExhausted;
}
/**
* @param noRecoveryForNotRetryable the noRecoveryForNotRetryable to set
* @since 1.3.3
*/
public void setNoRecoveryForNotRetryable(boolean noRecoveryForNotRetryable) {
this.noRecoveryForNotRetryable = noRecoveryForNotRetryable;
}
/**
* Public setter for the {@link RetryContextCache}.
* @param retryContextCache the {@link RetryContextCache} to set.
@@ -376,6 +366,7 @@ public class RetryTemplate implements RetryOperations {
}
throw RetryTemplate.<E>wrapIfNecessary(e);
}
}
/*
@@ -393,7 +384,7 @@ public class RetryTemplate implements RetryOperations {
}
exhausted = true;
return handleRetryExhausted(recoveryCallback, context, state, retryPolicy);
return handleRetryExhausted(recoveryCallback, context, state);
}
catch (Throwable e) {
@@ -471,6 +462,7 @@ public class RetryTemplate implements RetryOperations {
* was encountered
*/
protected RetryContext open(RetryPolicy retryPolicy, RetryState state) {
if (state == null) {
return doOpenInternal(retryPolicy);
}
@@ -504,6 +496,7 @@ public class RetryTemplate implements RetryOperations {
context.removeAttribute(RetryContext.EXHAUSTED);
context.removeAttribute(RetryContext.RECOVERED);
return context;
}
private RetryContext doOpenInternal(RetryPolicy retryPolicy, RetryState state) {
@@ -536,16 +529,12 @@ public class RetryTemplate implements RetryOperations {
* @return T the payload to return
* @throws Throwable if there is an error
*/
protected <T> T handleRetryExhausted(RecoveryCallback<T> recoveryCallback, RetryContext context, RetryState state,
RetryPolicy retryPolicy) throws Throwable {
protected <T> T handleRetryExhausted(RecoveryCallback<T> recoveryCallback, RetryContext context, RetryState state)
throws Throwable {
context.setAttribute(RetryContext.EXHAUSTED, true);
if (state != null && !context.hasAttribute(GLOBAL_STATE)) {
this.retryContextCache.remove(state.getKey());
}
if (this.noRecoveryForNotRetryable && retryPolicy instanceof SimpleRetryPolicy
&& !((SimpleRetryPolicy) retryPolicy).retryForException(context.getLastThrowable())) {
throw context.getLastThrowable();
}
if (recoveryCallback != null) {
T recovered = recoveryCallback.recover(context);
context.setAttribute(RetryContext.RECOVERED, true);
@@ -559,7 +548,7 @@ public class RetryTemplate implements RetryOperations {
}
protected <E extends Throwable> void rethrow(RetryContext context, String message) throws E {
if (this.throwLastExceptionOnExhausted || this.noRecoveryForNotRetryable) {
if (this.throwLastExceptionOnExhausted) {
@SuppressWarnings("unchecked")
E rethrow = (E) context.getLastThrowable();
throw rethrow;
@@ -583,6 +572,7 @@ public class RetryTemplate implements RetryOperations {
}
private <T, E extends Throwable> boolean doOpenInterceptors(RetryCallback<T, E> callback, RetryContext context) {
boolean result = true;
for (RetryListener listener : this.listeners) {
@@ -590,6 +580,7 @@ public class RetryTemplate implements RetryOperations {
}
return result;
}
private <T, E extends Throwable> void doCloseInterceptors(RetryCallback<T, E> callback, RetryContext context,

40
src/test/java/org/springframework/retry/annotation/EnableRetryTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2022 the original author or authors.
* Copyright 2014-2021 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -255,22 +255,6 @@ public class EnableRetryTests {
context.close();
}
@Test
public void rethrow() {
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class);
RethrowService service = context.getBean(RethrowService.class);
for (int i = 0; i < 3; i++) {
try {
service.service();
}
catch (RuntimeException e) {
assertEquals("Planned", e.getMessage());
}
}
assertEquals(3, service.getCount());
context.close();
}
private Object target(Object target) {
if (!AopUtils.isAopProxy(target)) {
return target;
@@ -450,11 +434,6 @@ public class EnableRetryTests {
return new ExcludesOnlyService();
}
@Bean
public RethrowService rethrowService() {
return new RethrowService();
}
@Bean
public MethodInterceptor retryInterceptor() {
return RetryInterceptorBuilder.stateless().maxAttempts(5).build();
@@ -717,23 +696,6 @@ public class EnableRetryTests {
}
private static class RethrowService {
private int count = 0;
@Retryable(include = IllegalArgumentException.class, rethrow = true)
public void service() {
if (this.count++ < 2) {
throw new RuntimeException("Planned");
}
}
public int getCount() {
return this.count;
}
}
public static class ExceptionChecker {
public boolean shouldRetry(Throwable t) {

25
src/test/java/org/springframework/retry/annotation/RecoverAnnotationRecoveryHandlerTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2022 the original author or authors.
* Copyright 2013-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -74,16 +74,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<Integer>(
new SpecificException(), ReflectionUtils.findMethod(SpecificException.class, "foo", String.class));
this.expected.expect(ExhaustedRetryException.class);
handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned"));
}
@Test
public void noMatchWithRethrow() {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<Integer>(
new SpecificException(), ReflectionUtils.findMethod(SpecificException.class, "foo", String.class));
handler.setThrowLastExceptionWhenNoRecoverMethod(true);
this.expected.expect(IllegalArgumentException.class);
handler.recover(new Object[] { "Dave" }, new IllegalArgumentException("Planned"));
handler.recover(new Object[] { "Dave" }, new Error("Planned"));
}
@Test
@@ -99,6 +90,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<Integer>(
new InAccessibleRecover(), foo);
assertEquals(1, handler.recover(new Object[] { "Dave" }, new RuntimeException("Planned")));
}
@Test
@@ -122,6 +114,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
@Test
public void genericReturnStringValueTypeParentThrowableRecoverMethod() {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<List<String>>(
new GenericReturnTypeRecover(),
ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class));
@@ -135,6 +128,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
@Test
public void genericReturnStringValueTypeChildThrowableRecoverMethod() {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<List<String>>(
new GenericReturnTypeRecover(),
ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "foo", String.class));
@@ -148,6 +142,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
@Test
public void genericReturnOneValueTypeRecoverMethod() {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<List<String>>(
new GenericReturnTypeRecover(),
ReflectionUtils.findMethod(GenericReturnTypeRecover.class, "bar", String.class));
@@ -214,6 +209,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
Map<String, Map<String, Map<Number, String>>> recoverResponseMapRe = (Map<String, Map<String, Map<Number, String>>>) barHandler
.recover(new Object[] { "Aldo" }, new RuntimeException("Planned"));
assertEquals("barRecoverNumberValue", recoverResponseMapRe.get("bar").get("bar").get(0.0));
}
@Test
@@ -222,6 +218,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<Integer>(
new MultipleQualifyingRecovers(), foo);
assertEquals(1, handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned")));
}
@Test
@@ -230,6 +227,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<Integer>(
new MultipleQualifyingRecovers(), foo);
assertEquals(1, handler.recover(new Object[] { null }, new RuntimeException("Planned")));
}
@Test
@@ -238,6 +236,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<Integer>(
new MultipleQualifyingRecoversNoThrowable(), foo);
assertEquals(1, handler.recover(new Object[] { null }, new RuntimeException("Planned")));
}
@Test
@@ -246,6 +245,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
RecoverAnnotationRecoveryHandler<?> handler = new RecoverAnnotationRecoveryHandler<Integer>(
new MultipleQualifyingRecoversReOrdered(), foo);
assertEquals(3, handler.recover(new Object[] { "Randell" }, new RuntimeException("Planned")));
}
@Test
@@ -255,6 +255,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
new MultipleQualifyingRecoversExtendsThrowable(), foo);
assertEquals(2, handler.recover(new Object[] { "Kevin" }, new IllegalArgumentException("Planned")));
assertEquals(3, handler.recover(new Object[] { "Kevin" }, new UnsupportedOperationException("Planned")));
}
@Test
@@ -370,7 +371,7 @@ public class RecoverAnnotationRecoveryHandlerTests {
}
@Recover
public int bar(IllegalStateException e, String name) {
public int bar(RuntimeException e, String name) {
return 1;
}

22
src/test/java/org/springframework/retry/policy/SimpleRetryPolicyTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2006-2022 the original author or authors.
* Copyright 2006-2013 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -137,24 +137,4 @@ public class SimpleRetryPolicyTests {
assertSame(context, child.getParent());
}
@Test
public void testRetryForException() {
Map<Class<? extends Throwable>, Boolean> map = new HashMap<Class<? extends Throwable>, Boolean>();
map.put(RuntimeException.class, true);
SimpleRetryPolicy policy = new SimpleRetryPolicy(3, map, true);
RetryContext context = policy.open(null);
assertNotNull(context);
assertTrue(policy.retryForException(new RuntimeException()));
}
@Test
public void testNoRetryForException() {
Map<Class<? extends Throwable>, Boolean> map = new HashMap<Class<? extends Throwable>, Boolean>();
map.put(IllegalArgumentException.class, true);
SimpleRetryPolicy policy = new SimpleRetryPolicy(3, map, true);
RetryContext context = policy.open(null);
assertNotNull(context);
assertFalse(policy.retryForException(new RuntimeException()));
}
}

51
src/test/java/org/springframework/retry/support/RetryTemplateTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2006-2022 the original author or authors.
* Copyright 2006-2007 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -346,6 +346,7 @@ public class RetryTemplateTests {
*/
@Test
public void testNoBackOffForRethrownException() throws Throwable {
RetryTemplate tested = new RetryTemplate();
tested.setRetryPolicy(new SimpleRetryPolicy(1));
@@ -383,54 +384,6 @@ public class RetryTemplateTests {
verify(bop);
}
@Test
public void testRethrowForNotRetryable() throws Throwable {
SimpleRetryPolicy policy = new SimpleRetryPolicy(1,
Collections.<Class<? extends Throwable>, Boolean>singletonMap(IllegalArgumentException.class, true));
RetryTemplate retryTemplate = new RetryTemplate();
retryTemplate.setRetryPolicy(policy);
retryTemplate.setNoRecoveryForNotRetryable(true);
try {
retryTemplate.execute(new RetryCallback<Object, Exception>() {
@Override
public Object doWithRetry(RetryContext context) throws Exception {
throw new RuntimeException("Realllly bad!");
}
}, new RecoveryCallback<Object>() {
@Override
public Object recover(RetryContext context) throws Exception {
return new Object();
}
});
fail("Expected RuntimeException");
}
catch (RuntimeException e) {
assertEquals("Realllly bad!", e.getMessage());
}
}
@Test
public void testRethrowForRetryable() throws Throwable {
SimpleRetryPolicy policy = new SimpleRetryPolicy(1,
Collections.<Class<? extends Throwable>, Boolean>singletonMap(RuntimeException.class, true));
RetryTemplate retryTemplate = new RetryTemplate();
retryTemplate.setRetryPolicy(policy);
retryTemplate.setNoRecoveryForNotRetryable(true);
final Object value = new Object();
Object result = retryTemplate.execute(new RetryCallback<Object, Exception>() {
@Override
public Object doWithRetry(RetryContext context) throws Exception {
throw new RuntimeException("Will be recovered");
}
}, new RecoveryCallback<Object>() {
@Override
public Object recover(RetryContext context) throws Exception {
return value;
}
});
assertEquals(value, result);
}
private static class MockRetryCallback implements RetryCallback<Object, Exception> {
private int attempts;