Skip to content

Commit 7063d36

Browse files
garyrussellartembilan
authored andcommitted
GH-299: Fix @recover with Class Level @retryable
Resolves #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
1 parent db0a050 commit 7063d36

File tree

2 files changed

+109
-4
lines changed

2 files changed

+109
-4
lines changed

src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ private MethodInterceptor getDelegate(Object target, Method method) {
173173
MethodInterceptor interceptor = NULL_INTERCEPTOR;
174174
Retryable retryable = AnnotatedElementUtils.findMergedAnnotation(method, Retryable.class);
175175
if (retryable == null) {
176-
retryable = AnnotatedElementUtils.findMergedAnnotation(method.getDeclaringClass(), Retryable.class);
176+
retryable = classLevelAnnotation(method, Retryable.class);
177177
}
178178
if (retryable == null) {
179179
retryable = findAnnotationOnTarget(target, method, Retryable.class);
@@ -202,7 +202,7 @@ private <A extends Annotation> A findAnnotationOnTarget(Object target, Method me
202202
Method targetMethod = target.getClass().getMethod(method.getName(), method.getParameterTypes());
203203
A retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod, annotation);
204204
if (retryable == null) {
205-
retryable = AnnotatedElementUtils.findMergedAnnotation(targetMethod.getDeclaringClass(), annotation);
205+
retryable = classLevelAnnotation(targetMethod, annotation);
206206
}
207207

208208
return retryable;
@@ -212,6 +212,17 @@ private <A extends Annotation> A findAnnotationOnTarget(Object target, Method me
212212
}
213213
}
214214

215+
/*
216+
* With a class level annotation, exclude @Recover methods.
217+
*/
218+
private <A extends Annotation> A classLevelAnnotation(Method method, Class<A> annotation) {
219+
A ann = AnnotatedElementUtils.findMergedAnnotation(method.getDeclaringClass(), annotation);
220+
if (ann != null && AnnotatedElementUtils.findMergedAnnotation(method, Recover.class) != null) {
221+
ann = null;
222+
}
223+
return ann;
224+
}
225+
215226
private MethodInterceptor getStatelessInterceptor(Object target, Method method, Retryable retryable) {
216227
RetryTemplate template = createTemplate(retryable.listeners());
217228
template.setRetryPolicy(getRetryPolicy(retryable));
@@ -316,9 +327,9 @@ private MethodInvocationRecoverer<?> getRecoverer(Object target, Method method)
316327
return (MethodInvocationRecoverer<?>) target;
317328
}
318329
final AtomicBoolean foundRecoverable = new AtomicBoolean(false);
319-
ReflectionUtils.doWithMethods(target.getClass(), new MethodCallback() {
330+
ReflectionUtils.doWithMethods(target.getClass(), new ReflectionUtils.MethodCallback() {
320331
@Override
321-
public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException {
332+
public void doWith(Method method) throws IllegalArgumentException {
322333
if (AnnotatedElementUtils.findMergedAnnotation(method, Recover.class) != null) {
323334
foundRecoverable.set(true);
324335
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* Copyright 2019 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.retry.annotation;
18+
19+
import org.junit.Test;
20+
import org.junit.runner.RunWith;
21+
22+
import org.springframework.beans.factory.annotation.Autowired;
23+
import org.springframework.context.annotation.Bean;
24+
import org.springframework.context.annotation.Configuration;
25+
import org.springframework.test.context.junit4.SpringRunner;
26+
27+
import static org.junit.Assert.assertEquals;
28+
import static org.junit.Assert.fail;
29+
30+
/**
31+
* @author Gary Russell
32+
* @author Artem Bilan
33+
* @since 1.3.4
34+
*/
35+
@RunWith(SpringRunner.class)
36+
public class DontRetryRecovererTests {
37+
38+
@Autowired
39+
Service service;
40+
41+
@Test
42+
public void dontRetry() {
43+
try {
44+
this.service.foo("x");
45+
fail("Exception expected");
46+
}
47+
catch (Exception e) {
48+
assertEquals("test", e.getMessage());
49+
}
50+
51+
assertEquals(3, this.service.getCallCount());
52+
assertEquals(1, this.service.getRecoverCount());
53+
}
54+
55+
@Configuration
56+
@EnableRetry
57+
public static class Config {
58+
59+
@Bean
60+
Service service() {
61+
return new Service();
62+
}
63+
64+
}
65+
66+
@Retryable
67+
public static class Service {
68+
69+
int callCount;
70+
71+
int recoverCount;
72+
73+
public void foo(String in) {
74+
callCount++;
75+
throw new RuntimeException();
76+
}
77+
78+
@Recover
79+
public void recover(Exception ex, String in) {
80+
this.recoverCount++;
81+
throw new RuntimeException("test");
82+
}
83+
84+
public int getCallCount() {
85+
return callCount;
86+
}
87+
88+
public int getRecoverCount() {
89+
return recoverCount;
90+
}
91+
92+
}
93+
94+
}

0 commit comments

Comments
 (0)