Widen Pointcut for @Retryable to cover multiple methods on same class

Fixes gh-15

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 <code>@Retryable</code> 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 <code>@Retryable</code> 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<Class<? extends Annotation>> retryAnnotationTypes) {
 		ComposablePointcut result = null;
 		for (Class<? extends Annotation> 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<? extends Annotation> annotationType) {
-			super(annotationType, true);
-			this.methodResolver = new AnnotationMethodResolver(annotationType);
+		AnnotationClassOrMethodPointcut(Class<? extends Annotation> 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<? extends Annotation> 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<? extends Annotation> annotationType;
+		
+		public AnnotationMethodsResolver(Class<? extends Annotation> 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();
+		}
+		
+	}
+	
 }

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;