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

Fix @Recover for Prototype Scoped Beans

Browse Source 
Previously, if a bean annotated with `@Retryable` and `@Recover` was
scoped "prototype", the `@Recover` method in the first instance was
called instead of the method in the same instance as the failed `@Retryable`
method.

This was because the delegate cache in `AnnotationAwareRetryOperationsInterceptor`
was keyed only on the `Method` object.

Change the cache to cache at the `targetObject.method` level.

Also fixes `backoff` javadocs.

Fixes https://github.com/spring-projects/spring-retry/issues/93
This commit is contained in:
Gary Russell
2017-11-28 13:49:43 -05:00
committed by Dave Syer
parent 236d6206d7
commit a22ebe7514
5 changed files with 174 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
pom.xml
View File

@@ -22,7 +22,7 @@
<packaging>jar</packaging>
<properties>
<maven.test.failure.ignore>true</maven.test.failure.ignore>
<spring.framework.version>4.3.9.RELEASE</spring.framework.version>
<spring.framework.version>4.3.13.RELEASE</spring.framework.version>
</properties>
<profiles>
<profile>

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

@@ -1,5 +1,5 @@
/*
* Copyright 2014-2016 the original author or authors.
* Copyright 2014-2017 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.
@@ -80,7 +80,8 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
private final StandardEvaluationContext evaluationContext = new StandardEvaluationContext();
private final Map<Method, MethodInterceptor> delegates = new HashMap<Method, MethodInterceptor>();
private final Map<Object, Map<Method, MethodInterceptor>> delegates =
new HashMap<Object, Map<Method, MethodInterceptor>>();
private RetryContextCache retryContextCache = new MapRetryContextCache();
@@ -157,9 +158,13 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
}
private MethodInterceptor getDelegate(Object target, Method method) {
if (!this.delegates.containsKey(method)) {
if (!this.delegates.containsKey(target) || !this.delegates.get(target).containsKey(method)) {
synchronized (this.delegates) {
if (!this.delegates.containsKey(method)) {
if (!this.delegates.containsKey(target)) {
this.delegates.put(target, new HashMap<Method, MethodInterceptor>());
}
Map<Method, MethodInterceptor> delegatesForTarget = this.delegates.get(target);
if (!delegatesForTarget.containsKey(method)) {
Retryable retryable = AnnotationUtils.findAnnotation(method, Retryable.class);
if (retryable == null) {
retryable = AnnotationUtils.findAnnotation(method.getDeclaringClass(), Retryable.class);
@@ -168,7 +173,7 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
retryable = findAnnotationOnTarget(target, method);
}
if (retryable == null) {
return this.delegates.put(method, null);
return delegatesForTarget.put(method, null);
}
MethodInterceptor delegate;
if (StringUtils.hasText(retryable.interceptor())) {
@@ -180,11 +185,11 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
else {
delegate = getStatelessInterceptor(target, method, retryable);
}
this.delegates.put(method, delegate);
delegatesForTarget.put(method, delegate);
}
}
}
return this.delegates.get(method);
return this.delegates.get(target).get(method);
}
private Retryable findAnnotationOnTarget(Object target, Method method) {

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

@@ -93,9 +93,9 @@ public @interface Retryable {
String maxAttemptsExpression() default "";
/**
* Specify the backoff properties for retrying this operation. The default is no
* backoff, but it can be a good idea to pause between attempts (even at the cost of
* blocking a thread).
* Specify the backoff properties for retrying this operation. The default is a
* simple {@link Backoff} specification with no properties - see it's documentation
* for defaults.
* @return a backoff specification
*/
Backoff backoff() default @Backoff();

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

@@ -16,12 +16,6 @@
package org.springframework.retry.annotation;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.lang.reflect.Method;
import java.util.Map;
import java.util.Properties;
@@ -29,6 +23,7 @@ import java.util.Properties;
import org.aopalliance.intercept.MethodInterceptor;
import org.junit.Test;
import org.springframework.aop.framework.Advised;
import org.springframework.aop.support.AopUtils;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
@@ -41,6 +36,12 @@ import org.springframework.retry.interceptor.RetryInterceptorBuilder;
import org.springframework.retry.policy.SimpleRetryPolicy;
import org.springframework.retry.support.RetryTemplate;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**
* @author Dave Syer
* @author Artem Bilan
@@ -148,7 +149,8 @@ public class EnableRetryTests {
@Test
public void testExternalInterceptor() {
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class);
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
TestConfiguration.class);
InterceptableService service = context.getBean(InterceptableService.class);
service.service();
assertEquals(5, service.getCount());
@@ -157,7 +159,8 @@ public class EnableRetryTests {
@Test
public void testInterface() {
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class);
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
TestConfiguration.class);
TheInterface service = context.getBean(TheInterface.class);
service.service1();
service.service2();
@@ -167,7 +170,8 @@ public class EnableRetryTests {
@Test
public void testExpression() throws Exception {
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(TestConfiguration.class);
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
TestConfiguration.class);
ExpressionService service = context.getBean(ExpressionService.class);
service.service1();
assertEquals(3, service.getCount());
@@ -182,12 +186,12 @@ public class EnableRetryTests {
service.service3();
assertEquals(9, service.getCount());
RetryConfiguration config = context.getBean(RetryConfiguration.class);
AnnotationAwareRetryOperationsInterceptor advice =
(AnnotationAwareRetryOperationsInterceptor) new DirectFieldAccessor(config).getPropertyValue("advice");
AnnotationAwareRetryOperationsInterceptor advice = (AnnotationAwareRetryOperationsInterceptor) new DirectFieldAccessor(
config).getPropertyValue("advice");
@SuppressWarnings("unchecked")
Map<Method, MethodInterceptor> delegates = (Map<Method, MethodInterceptor>) new DirectFieldAccessor(advice)
.getPropertyValue("delegates");
MethodInterceptor interceptor = delegates
Map<Object, Map<Method, MethodInterceptor>> delegates = (Map<Object, Map<Method, MethodInterceptor>>) new DirectFieldAccessor(
advice).getPropertyValue("delegates");
MethodInterceptor interceptor = delegates.get(target(service))
.get(ExpressionService.class.getDeclaredMethod("service3"));
RetryTemplate template = (RetryTemplate) new DirectFieldAccessor(interceptor)
.getPropertyValue("retryOperations");
@@ -197,11 +201,24 @@ public class EnableRetryTests {
assertEquals(1, backOff.getInitialInterval());
assertEquals(5, backOff.getMaxInterval());
assertEquals(1.1, backOff.getMultiplier(), 0.1);
SimpleRetryPolicy retryPolicy = (SimpleRetryPolicy) templateAccessor.getPropertyValue("retryPolicy");
SimpleRetryPolicy retryPolicy = (SimpleRetryPolicy) templateAccessor
.getPropertyValue("retryPolicy");
assertEquals(5, retryPolicy.getMaxAttempts());
context.close();
}
private Object target(Object target) {
if (!AopUtils.isAopProxy(target)) {
return target;
}
try {
return target(((Advised)target).getTargetSource().getTarget());
}
catch (Exception e) {
throw new IllegalStateException(e);
}
}
@Configuration
@EnableRetry(proxyTargetClass = true)
protected static class TestProxyConfiguration {
@@ -271,9 +288,7 @@ public class EnableRetryTests {
@Bean
public MethodInterceptor retryInterceptor() {
return RetryInterceptorBuilder.stateless()
.maxAttempts(5)
.build();
return RetryInterceptorBuilder.stateless().maxAttempts(5).build();
}
@Bean
@@ -448,23 +463,20 @@ public class EnableRetryTests {
private int count = 0;
@Retryable(exceptionExpression="#{message.contains('this can be retried')}")
@Retryable(exceptionExpression = "#{message.contains('this can be retried')}")
public void service1() {
if (count++ < 2) {
throw new RuntimeException("this can be retried");
}
}
@Retryable(exceptionExpression="#{message.contains('this can be retried')}")
@Retryable(exceptionExpression = "#{message.contains('this can be retried')}")
public void service2() {
count++;
throw new RuntimeException("this cannot be retried");
}
@Retryable(exceptionExpression="#{@exceptionChecker.${retryMethod}(#root)}",
maxAttemptsExpression = "#{@integerFiveBean}",
backoff = @Backoff(delayExpression = "#{${one}}", maxDelayExpression = "#{${five}}",
multiplierExpression = "#{${onePointOne}}"))
@Retryable(exceptionExpression = "#{@exceptionChecker.${retryMethod}(#root)}", maxAttemptsExpression = "#{@integerFiveBean}", backoff = @Backoff(delayExpression = "#{${one}}", maxDelayExpression = "#{${five}}", multiplierExpression = "#{${onePointOne}}"))
public void service3() {
if (count++ < 8) {
throw new RuntimeException();

122
src/test/java/org/springframework/retry/annotation/PrototypeBeanTests.java Normal file
View File

@@ -0,0 +1,122 @@
/*
* Copyright 2017 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
*
* http://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 static org.hamcrest.CoreMatchers.equalTo;
import static org.junit.Assert.assertThat;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Scope;
import org.springframework.test.context.junit4.SpringRunner;
/**
* @author Gary Russell
* @since 1.2.2
*
*/
@RunWith(SpringRunner.class)
public class PrototypeBeanTests {
@Autowired
private Bar bar1;
@Autowired
private Bar bar2;
@Autowired
private Foo foo;
@Test
public void testProtoBean() {
this.bar1.foo("one");
this.bar2.foo("two");
assertThat(this.foo.recovered, equalTo("two"));
}
@Configuration
@EnableRetry
public static class Config {
@Bean
public Foo foo() {
return new Foo();
}
@Bean
@Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE)
public Baz baz() {
return new Baz();
}
}
public static class Foo {
private String recovered;
void demoRun(Bar bar) {
throw new RuntimeException();
}
void demoRecover(String instance) {
this.recovered = instance;
}
}
public interface Bar {
@Retryable(backoff = @Backoff(0))
void foo(String instance);
@Recover
void bar();
}
public static class Baz implements Bar {
private String instance;
@Autowired
private Foo foo;
@Override
public void foo(String instance) {
this.instance = instance;
foo.demoRun(this);
}
@Override
public void bar() {
foo.demoRecover(this.instance);
}
@Override
public String toString() {
return "Baz [instance=" + this.instance + "]";
}
}
}