From 55839cefbba081596e77079c3d537642b26bb144 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 19 Sep 2014 10:24:54 +0100 Subject: [PATCH] Widen Pointcut for @Retryable to cover multiple methods on same class Fixes gh-15 --- .../retry/annotation/RetryConfiguration.java | 90 ++++++++++++++----- .../retry/annotation/EnableRetryTests.java | 42 +++++++++ 2 files changed, 110 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java b/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java index 37b1d73..d65ec62 100644 --- a/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java +++ b/src/main/java/org/springframework/retry/annotation/RetryConfiguration.java @@ -17,35 +17,40 @@ package org.springframework.retry.annotation; import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.util.LinkedHashSet; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.PostConstruct; import org.aopalliance.aop.Advice; import org.springframework.aop.ClassFilter; import org.springframework.aop.IntroductionAdvisor; +import org.springframework.aop.MethodMatcher; import org.springframework.aop.Pointcut; import org.springframework.aop.support.AbstractPointcutAdvisor; import org.springframework.aop.support.ComposablePointcut; +import org.springframework.aop.support.StaticMethodMatcherPointcut; import org.springframework.aop.support.annotation.AnnotationClassFilter; +import org.springframework.aop.support.annotation.AnnotationMethodMatcher; 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.core.annotation.AnnotationUtils; import org.springframework.retry.backoff.Sleeper; import org.springframework.retry.interceptor.MethodArgumentsKeyGenerator; import org.springframework.retry.interceptor.NewMethodArgumentsIdentifier; import org.springframework.retry.policy.RetryContextCache; +import org.springframework.util.ReflectionUtils; +import org.springframework.util.ReflectionUtils.MethodCallback; /** - * Basic configuration for @Retryable processing. For stateful - * retry, if there is a unique bean elsewhere in the context of type - * {@link RetryContextCache}, {@link MethodArgumentsKeyGenerator} or - * {@link NewMethodArgumentsIdentifier} it will be used by the corresponding - * retry interceptor (otherwise sensible defaults are adopted). + * Basic configuration for @Retryable processing. For stateful retry, if there is a unique bean elsewhere + * in the context of type {@link RetryContextCache}, {@link MethodArgumentsKeyGenerator} or + * {@link NewMethodArgumentsIdentifier} it will be used by the corresponding retry interceptor (otherwise sensible + * defaults are adopted). * * @author Dave Syer * @author Artem Bilan @@ -54,8 +59,7 @@ import org.springframework.retry.policy.RetryContextCache; */ @SuppressWarnings("serial") @Configuration -public class RetryConfiguration extends AbstractPointcutAdvisor implements - IntroductionAdvisor, BeanFactoryAware { +public class RetryConfiguration extends AbstractPointcutAdvisor implements IntroductionAdvisor, BeanFactoryAware { private Advice advice; @@ -87,8 +91,7 @@ public class RetryConfiguration extends AbstractPointcutAdvisor implements } /** - * Set the {@code BeanFactory} to be used when looking up executors by - * qualifier. + * Set the {@code BeanFactory} to be used when looking up executors by qualifier. */ @Override public void setBeanFactory(BeanFactory beanFactory) { @@ -102,7 +105,7 @@ public class RetryConfiguration extends AbstractPointcutAdvisor implements @Override public Class[] getInterfaces() { - return new Class[] {org.springframework.retry.interceptor.Retryable.class}; + return new Class[] { org.springframework.retry.interceptor.Retryable.class }; } @Override @@ -139,14 +142,13 @@ public class RetryConfiguration extends AbstractPointcutAdvisor implements /** * Calculate a pointcut for the given retry annotation types, if any. * - * @param retryAnnotationTypes - * the retry annotation types to introspect + * @param retryAnnotationTypes the retry annotation types to introspect * @return the applicable Pointcut object, or {@code null} if none */ protected Pointcut buildPointcut(Set> retryAnnotationTypes) { ComposablePointcut result = null; for (Class retryAnnotationType : retryAnnotationTypes) { - ClassFilter filter = new AnnotationClassOrMethodFilter(retryAnnotationType); + Pointcut filter = new AnnotationClassOrMethodPointcut(retryAnnotationType); if (result == null) { result = new ComposablePointcut(filter); } @@ -157,20 +159,64 @@ public class RetryConfiguration extends AbstractPointcutAdvisor implements return result; } - private final class AnnotationClassOrMethodFilter extends AnnotationClassFilter { + private final class AnnotationClassOrMethodPointcut extends StaticMethodMatcherPointcut { - private final MethodResolver methodResolver; + private final MethodMatcher methodResolver; - AnnotationClassOrMethodFilter(Class annotationType) { - super(annotationType, true); - this.methodResolver = new AnnotationMethodResolver(annotationType); + AnnotationClassOrMethodPointcut(Class annotationType) { + this.methodResolver = new AnnotationMethodMatcher(annotationType); + setClassFilter(new AnnotationClassOrMethodFilter(annotationType)); } @Override - public boolean matches(Class clazz) { - return super.matches(clazz) || this.methodResolver.findMethod(clazz) != null; + public boolean matches(Method method, Class targetClass) { + return getClassFilter().matches(targetClass) || this.methodResolver.matches(method, targetClass); } } + private final class AnnotationClassOrMethodFilter extends AnnotationClassFilter { + + private final AnnotationMethodsResolver methodResolver; + + AnnotationClassOrMethodFilter(Class annotationType) { + super(annotationType, true); + this.methodResolver = new AnnotationMethodsResolver(annotationType); + } + + @Override + public boolean matches(Class clazz) { + return super.matches(clazz) || this.methodResolver.hasAnnotatedMethods(clazz); + } + + } + + private static class AnnotationMethodsResolver { + + private Class annotationType; + + public AnnotationMethodsResolver(Class annotationType) { + this.annotationType = annotationType; + } + + public boolean hasAnnotatedMethods(Class clazz) { + final AtomicBoolean found = new AtomicBoolean(false); + ReflectionUtils.doWithMethods(clazz, + new MethodCallback() { + @Override + public void doWith(Method method) throws IllegalArgumentException, + IllegalAccessException { + if (found.get()) { + return; + } + Annotation annotation = AnnotationUtils.findAnnotation(method, + annotationType); + if (annotation != null) { found.set(true); } + } + }); + return found.get(); + } + + } + } diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index d81b24b..72d6235 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -46,11 +46,24 @@ public class EnableRetryTests { Service service = context.getBean(Service.class); Foo foo = context.getBean(Foo.class); assertFalse(AopUtils.isAopProxy(foo)); + assertTrue(AopUtils.isAopProxy(service)); service.service(); assertEquals(3, service.getCount()); context.close(); } + @Test + public void multipleMethods() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + TestConfiguration.class); + MultiService service = context.getBean(MultiService.class); + service.service(); + assertEquals(3, service.getCount()); + service.other(); + assertEquals(4, service.getCount()); + context.close(); + } + @Test public void proxyTargetClass() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( @@ -161,6 +174,11 @@ public class EnableRetryTests { return new Service(); } + @Bean + public MultiService multiService() { + return new MultiService(); + } + @Bean public RecoverableService recoverable() { return new RecoverableService(); @@ -217,6 +235,30 @@ public class EnableRetryTests { } + protected static class MultiService { + + private int count = 0; + + @Retryable(RuntimeException.class) + public void service() { + if (count++ < 2) { + throw new RuntimeException("Planned"); + } + } + + @Retryable(RuntimeException.class) + public void other() { + if (count++ < 3) { + throw new RuntimeException("Other"); + } + } + + public int getCount() { + return count; + } + + } + protected static class RecoverableService { private int count = 0;