GH-299: Fix @Recover with Class Level @Retryable
Resolves https://github.com/spring-projects/spring-retry/issues/299 Caused by the fix for #244, recover methods should not be considered as retryable, with class level annotation. **cherry-pick to 1.3.x** * Fix conflicts after cherry-picking: JUnit 4, proper `MethodCallback` import
This commit is contained in:
committed by
Artem Bilan
parent
db0a050f58
commit
7063d36386
@@ -173,7 +173,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
|
||||
MethodInterceptor interceptor = NULL_INTERCEPTOR;
|
||||
Retryable retryable = AnnotatedElementUtils.findMergedAnnotation(method, Retryable.class);
|
||||
if (retryable == null) {
|
||||
retryable = AnnotatedElementUtils.findMergedAnnotation(method.getDeclaringClass(), Retryable.class);
|
||||
retryable = classLevelAnnotation(method, Retryable.class);
|
||||
}
|
||||
if (retryable == null) {
|
||||
retryable = findAnnotationOnTarget(target, method, Retryable.class);
|
||||
@@ -202,7 +202,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
|
||||
Method targetMethod = target.getClass().getMethod(method.getName(), method.getParameterTypes());
|
||||
A retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod, annotation);
|
||||
if (retryable == null) {
|
||||
retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod.getDeclaringClass(), annotation);
|
||||
retryable = classLevelAnnotation(targetMethod, annotation);
|
||||
}
|
||||
|
||||
return retryable;
|
||||
@@ -212,6 +212,17 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* With a class level annotation, exclude @Recover methods.
|
||||
*/
|
||||
private <A extends Annotation> A classLevelAnnotation(Method method, Class<A> annotation) {
|
||||
A ann = AnnotatedElementUtils.findMergedAnnotation(method.getDeclaringClass(), annotation);
|
||||
if (ann != null && AnnotatedElementUtils.findMergedAnnotation(method, Recover.class) != null) {
|
||||
ann = null;
|
||||
}
|
||||
return ann;
|
||||
}
|
||||
|
||||
private MethodInterceptor getStatelessInterceptor(Object target, Method method, Retryable retryable) {
|
||||
RetryTemplate template = createTemplate(retryable.listeners());
|
||||
template.setRetryPolicy(getRetryPolicy(retryable));
|
||||
@@ -316,9 +327,9 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
|
||||
return (MethodInvocationRecoverer<?>) target;
|
||||
}
|
||||
final AtomicBoolean foundRecoverable = new AtomicBoolean(false);
|
||||
ReflectionUtils.doWithMethods(target.getClass(), new MethodCallback() {
|
||||
ReflectionUtils.doWithMethods(target.getClass(), new ReflectionUtils.MethodCallback() {
|
||||
@Override
|
||||
public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
|
||||
public void doWith(Method method) throws IllegalArgumentException {
|
||||
if (AnnotatedElementUtils.findMergedAnnotation(method, Recover.class) != null) {
|
||||
foundRecoverable.set(true);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
/*
|
||||
* Copyright 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.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.retry.annotation;
|
||||
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.test.context.junit4.SpringRunner;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
/**
|
||||
* @author Gary Russell
|
||||
* @author Artem Bilan
|
||||
* @since 1.3.4
|
||||
*/
|
||||
@RunWith(SpringRunner.class)
|
||||
public class DontRetryRecovererTests {
|
||||
|
||||
@Autowired
|
||||
Service service;
|
||||
|
||||
@Test
|
||||
public void dontRetry() {
|
||||
try {
|
||||
this.service.foo("x");
|
||||
fail("Exception expected");
|
||||
}
|
||||
catch (Exception e) {
|
||||
assertEquals("test", e.getMessage());
|
||||
}
|
||||
|
||||
assertEquals(3, this.service.getCallCount());
|
||||
assertEquals(1, this.service.getRecoverCount());
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@EnableRetry
|
||||
public static class Config {
|
||||
|
||||
@Bean
|
||||
Service service() {
|
||||
return new Service();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Retryable
|
||||
public static class Service {
|
||||
|
||||
int callCount;
|
||||
|
||||
int recoverCount;
|
||||
|
||||
public void foo(String in) {
|
||||
callCount++;
|
||||
throw new RuntimeException();
|
||||
}
|
||||
|
||||
@Recover
|
||||
public void recover(Exception ex, String in) {
|
||||
this.recoverCount++;
|
||||
throw new RuntimeException("test");
|
||||
}
|
||||
|
||||
public int getCallCount() {
|
||||
return callCount;
|
||||
}
|
||||
|
||||
public int getRecoverCount() {
|
||||
return recoverCount;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user