Fix @EnableRetry do not proxy all beans
Fixes https://github.com/spring-projects/spring-retry/issues/13 Previously the `@EnableRetry` caused to proxy **all** beans in the context, because of `IntroductionAdvisor` nature in the `AopUtils` logic and simple `ClassFilter.TRUE` in that case. In the end it just skipped `MethodMatcher` and applied `ProxyFactory` for any bean. Since we can't avoid `IntroductionAdvisor` because of `getInterfaces()` introduction, provide a new internal `AnnotationClassOrMethodFilter` to apply both class and method level annotation filter at once. Polishing for the `AnnotationAwareRetryOperationsInterceptor` to skip non-`@Retryable` methods and just call `invocation.proceed()` Fixes gh-13, fixes gh-14
This commit is contained in:
@@ -114,7 +114,12 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
|
||||
@Override
|
||||
public Object invoke(MethodInvocation invocation) throws Throwable {
|
||||
MethodInterceptor delegate = getDelegate(invocation.getThis(), invocation.getMethod());
|
||||
return delegate.invoke(invocation);
|
||||
if (delegate != null) {
|
||||
return delegate.invoke(invocation);
|
||||
}
|
||||
else {
|
||||
return invocation.proceed();
|
||||
}
|
||||
}
|
||||
|
||||
private MethodInterceptor getDelegate(Object target, Method method) {
|
||||
@@ -125,6 +130,9 @@ public class AnnotationAwareRetryOperationsInterceptor implements IntroductionIn
|
||||
if (retryable == null) {
|
||||
retryable = AnnotationUtils.findAnnotation(method.getDeclaringClass(), Retryable.class);
|
||||
}
|
||||
if (retryable == null) {
|
||||
return this.delegates.put(method, null);
|
||||
}
|
||||
MethodInterceptor delegate;
|
||||
if (StringUtils.hasText(retryable.interceptor())) {
|
||||
delegate = this.beanFactory.getBean(retryable.interceptor(), MethodInterceptor.class);
|
||||
|
||||
@@ -29,10 +29,13 @@ import org.springframework.aop.IntroductionAdvisor;
|
||||
import org.springframework.aop.Pointcut;
|
||||
import org.springframework.aop.support.AbstractPointcutAdvisor;
|
||||
import org.springframework.aop.support.ComposablePointcut;
|
||||
import org.springframework.aop.support.annotation.AnnotationClassFilter;
|
||||
import org.springframework.aop.support.annotation.AnnotationMatchingPointcut;
|
||||
import org.springframework.beans.factory.BeanFactory;
|
||||
import org.springframework.beans.factory.BeanFactoryAware;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.classify.util.AnnotationMethodResolver;
|
||||
import org.springframework.classify.util.MethodResolver;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.retry.backoff.Sleeper;
|
||||
import org.springframework.retry.interceptor.MethodArgumentsKeyGenerator;
|
||||
@@ -145,16 +148,32 @@ public class RetryConfiguration extends AbstractPointcutAdvisor implements
|
||||
protected Pointcut buildPointcut(Set<Class<? extends Annotation>> retryAnnotationTypes) {
|
||||
ComposablePointcut result = null;
|
||||
for (Class<? extends Annotation> retryAnnotationType : retryAnnotationTypes) {
|
||||
Pointcut cpc = new AnnotationMatchingPointcut(retryAnnotationType, true);
|
||||
ClassFilter filter = new AnnotationClassOrMethodFilter(retryAnnotationType);
|
||||
Pointcut mpc = AnnotationMatchingPointcut.forMethodAnnotation(retryAnnotationType);
|
||||
if (result == null) {
|
||||
result = new ComposablePointcut(cpc).union(mpc);
|
||||
result = new ComposablePointcut(filter);
|
||||
}
|
||||
else {
|
||||
result.union(cpc).union(mpc);
|
||||
result.union(filter);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private final class AnnotationClassOrMethodFilter extends AnnotationClassFilter {
|
||||
|
||||
private final MethodResolver methodResolver;
|
||||
|
||||
AnnotationClassOrMethodFilter(Class<? extends Annotation> annotationType) {
|
||||
super(annotationType, true);
|
||||
this.methodResolver = new AnnotationMethodResolver(annotationType);
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean matches(Class<?> clazz) {
|
||||
return super.matches(clazz) || this.methodResolver.findMethod(clazz) != null;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
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;
|
||||
@@ -43,6 +44,8 @@ public class EnableRetryTests {
|
||||
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
|
||||
TestConfiguration.class);
|
||||
Service service = context.getBean(Service.class);
|
||||
Foo foo = context.getBean(Foo.class);
|
||||
assertFalse(AopUtils.isAopProxy(foo));
|
||||
service.service();
|
||||
assertEquals(3, service.getCount());
|
||||
context.close();
|
||||
@@ -145,7 +148,7 @@ public class EnableRetryTests {
|
||||
protected static class TestConfiguration {
|
||||
|
||||
@Bean
|
||||
public Sleeper sleper() {
|
||||
public Sleeper sleeper() {
|
||||
return new Sleeper() {
|
||||
@Override
|
||||
public void sleep(long period) throws InterruptedException {
|
||||
@@ -189,6 +192,12 @@ public class EnableRetryTests {
|
||||
public InterceptableService serviceWithExternalInterceptor() {
|
||||
return new InterceptableService();
|
||||
}
|
||||
|
||||
@Bean
|
||||
public Foo foo() {
|
||||
return new Foo();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
protected static class Service {
|
||||
@@ -302,4 +311,9 @@ public class EnableRetryTests {
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private static class Foo {
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user