From 20d703410a81a06dc6f74ec9f97692b847112790 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 6 Aug 2014 18:18:42 +0300 Subject: [PATCH] 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 --- ...tationAwareRetryOperationsInterceptor.java | 10 +++++++- .../retry/annotation/RetryConfiguration.java | 25 ++++++++++++++++--- .../retry/annotation/EnableRetryTests.java | 16 +++++++++++- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index bc2fca2..6c46121 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -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); diff --git a/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java b/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java index ea2c045..8096f9c 100644 --- a/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java +++ b/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java @@ -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> retryAnnotationTypes) { ComposablePointcut result = null; for (Class 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 annotationType) { + super(annotationType, true); + this.methodResolver = new AnnotationMethodResolver(annotationType); + } + + @Override + public boolean matches(Class clazz) { + return super.matches(clazz) || this.methodResolver.findMethod(clazz) != null; + } + + } + } diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index b6cc391..d81b24b 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -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 { + + } + }