From 7c00859806109615277f69dfd4f85b1a9cfb61cb Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 28 Nov 2017 13:49:43 -0500 Subject: [PATCH] Fix @Recover for Prototype Scoped Beans 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 --- pom.xml | 2 +- ...tationAwareRetryOperationsInterceptor.java | 19 ++- .../retry/annotation/Retryable.java | 6 +- .../retry/annotation/PrototypeBeanTests.java | 122 ++++++++++++++++++ 4 files changed, 138 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/springframework/retry/annotation/PrototypeBeanTests.java diff --git a/pom.xml b/pom.xml index 8f2fa7a5..855344ad 100644 --- a/pom.xml +++ b/pom.xml @@ -22,7 +22,7 @@ jar true - 4.3.9.RELEASE + 4.3.13.RELEASE diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index 406d0063..26b53003 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -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 delegates = new HashMap(); + private final Map> delegates = + new HashMap>(); private RetryContextCache retryContextCache = new MapRetryContextCache(); @@ -157,9 +158,13 @@ public Object invoke(MethodInvocation invocation) throws Throwable { } 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()); + } + Map 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 @@ private MethodInterceptor getDelegate(Object target, Method method) { 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 @@ else if (retryable.stateful()) { 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) { diff --git a/src/main/java/org/springframework/retry/annotation/Retryable.java b/src/main/java/org/springframework/retry/annotation/Retryable.java index 82753206..ccd04f79 100644 --- a/src/main/java/org/springframework/retry/annotation/Retryable.java +++ b/src/main/java/org/springframework/retry/annotation/Retryable.java @@ -93,9 +93,9 @@ 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(); diff --git a/src/test/java/org/springframework/retry/annotation/PrototypeBeanTests.java b/src/test/java/org/springframework/retry/annotation/PrototypeBeanTests.java new file mode 100644 index 00000000..5398dc69 --- /dev/null +++ b/src/test/java/org/springframework/retry/annotation/PrototypeBeanTests.java @@ -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 + "]"; + } + + } + +}