Skip to content

Commit

Permalink
Fix @recover for Prototype Scoped Beans
Browse files Browse the repository at this point in the history
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 spring-projects#93
  • Loading branch information
garyrussell committed Nov 28, 2017
1 parent f44b3ab commit 079fbc3
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 11 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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<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);
Expand All @@ -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())) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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 interface Bar {

@Retryable(backoff = @Backoff(0))
void foo(String instance);

@Recover
void bar();

}

public static class Foo {

private String recovered;

void demoRun(Bar bar) {
throw new RuntimeException();
}

void demoRecover(String instance) {
this.recovered = instance;
}

}

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 + "]";
}

}

}

0 comments on commit 079fbc3

Please sign in to comment.