Consistent validation of annotated methods behind AOP proxies

Issue: SPR-13816

Author: jhoeller

spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java

@@ -18,6 +18,7 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.Proxy;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
@@ -34,6 +35,7 @@
 import org.springframework.aop.SpringProxy;
 import org.springframework.aop.TargetClassAware;
 import org.springframework.core.BridgeMethodResolver;
+import org.springframework.core.MethodIntrospector;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ReflectionUtils;
@@ -112,6 +114,30 @@ public static Class<?> getTargetClass(Object candidate) {
 		return result;
 	}
 
+	/**
+	 * Select an invocable method on the target type: either the given method itself
+	 * if actually exposed on the target type, or otherwise a corresponding method
+	 * on one of the target type's interfaces or on the target type itself.
+	 * @param method the method to check
+	 * @param targetType the target type to search methods on (typically an AOP proxy)
+	 * @return a corresponding invocable method on the target type
+	 * @throws IllegalStateException if the given method is not invocable on the given
+	 * target type (typically due to a proxy mismatch)
+	 * @since 4.3
+	 * @see MethodIntrospector#selectInvocableMethod(Method, Class)
+	 */
+	public static Method selectInvocableMethod(Method method, Class<?> targetType) {
+		Method methodToUse = MethodIntrospector.selectInvocableMethod(method, targetType);
+		if (Modifier.isPrivate(methodToUse.getModifiers()) && !Modifier.isStatic(methodToUse.getModifiers()) &&
+				SpringProxy.class.isAssignableFrom(targetType)) {
+			throw new IllegalStateException(String.format(
+					"Need to invoke method '%s' found on proxy for target class '%s' but cannot " +
+					"be delegated to target bean. Switch its visibility to package or protected.",
+					method.getName(), method.getDeclaringClass().getSimpleName()));
+		}
+		return methodToUse;
+	}
+
 	/**
 	 * Determine whether the given method is an "equals" method.
 	 * @see java.lang.Object#equals

spring-context/src/main/java/org/springframework/context/event/EventListenerMethodProcessor.java

@@ -30,6 +30,7 @@
 import org.springframework.aop.framework.autoproxy.AutoProxyUtils;
 import org.springframework.aop.scope.ScopedObject;
 import org.springframework.aop.scope.ScopedProxyUtils;
+import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanInitializationException;
 import org.springframework.beans.factory.SmartInitializingSingleton;
@@ -142,7 +143,7 @@ public EventListener inspect(Method method) {
 				for (Method method : annotatedMethods.keySet()) {
 					for (EventListenerFactory factory : factories) {
 						if (factory.supportsMethod(method)) {
-							Method methodToUse = MethodIntrospector.selectInvocableMethod(
+							Method methodToUse = AopUtils.selectInvocableMethod(
 									method, this.applicationContext.getType(beanName));
 							ApplicationListener<?> applicationListener =
 									factory.createApplicationListener(beanName, targetType, methodToUse);

spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java

@@ -17,7 +17,6 @@
 package org.springframework.scheduling.annotation;
 
 import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
@@ -53,7 +52,6 @@
 import org.springframework.scheduling.support.CronTrigger;
 import org.springframework.scheduling.support.ScheduledMethodRunnable;
 import org.springframework.util.Assert;
-import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.util.StringValueResolver;
 
@@ -274,35 +272,8 @@ protected void processScheduled(Scheduled scheduled, Method method, Object bean)
 			Assert.isTrue(method.getParameterTypes().length == 0,
 					"Only no-arg methods may be annotated with @Scheduled");
 
-			if (AopUtils.isJdkDynamicProxy(bean)) {
-				try {
-					// Found a @Scheduled method on the target class for this JDK proxy ->
-					// is it also present on the proxy itself?
-					method = bean.getClass().getMethod(method.getName(), method.getParameterTypes());
-				}
-				catch (SecurityException ex) {
-					ReflectionUtils.handleReflectionException(ex);
-				}
-				catch (NoSuchMethodException ex) {
-					throw new IllegalStateException(String.format(
-							"@Scheduled method '%s' found on bean target class '%s' but not " +
-							"found in any interface(s) for a dynamic proxy. Either pull the " +
-							"method up to a declared interface or switch to subclass (CGLIB) " +
-							"proxies by setting proxy-target-class/proxyTargetClass to 'true'.",
-							method.getName(), method.getDeclaringClass().getSimpleName()));
-				}
-			}
-			else if (AopUtils.isCglibProxy(bean)) {
-				// Common problem: private methods end up in the proxy instance, not getting delegated.
-				if (Modifier.isPrivate(method.getModifiers())) {
-					throw new IllegalStateException(String.format(
-							"@Scheduled method '%s' found on CGLIB proxy for target class '%s' but cannot " +
-							"be delegated to target bean. Switch its visibility to package or protected.",
-							method.getName(), method.getDeclaringClass().getSimpleName()));
-				}
-			}
-
-			Runnable runnable = new ScheduledMethodRunnable(bean, method);
+			Method invocableMethod = AopUtils.selectInvocableMethod(method, bean.getClass());
+			Runnable runnable = new ScheduledMethodRunnable(bean, invocableMethod);
 			boolean processedSchedule = false;
 			String errorMessage =
 					"Exactly one of the 'cron', 'fixedDelay(String)', or 'fixedRate(String)' attributes is required";

spring-context/src/test/java/org/springframework/context/event/AnnotationDrivenEventListenerTests.java

@@ -284,6 +284,17 @@ public void eventListenerWorksWithCglibProxy() throws Exception {
 		this.eventCollector.assertTotalEventsCount(1);
 	}
 
+	@Test
+	public void privateMethodOnCglibProxyFails() throws Exception {
+		try {
+			load(CglibProxyWithPrivateMethod.class);
+			fail("Should have thrown BeanInitializationException");
+		}
+		catch (BeanInitializationException ex) {
+			assertTrue(ex.getCause() instanceof IllegalStateException);
+		}
+	}
+
 	@Test
 	public void eventListenerWorksWithCustomScope() throws Exception {
 		load(CustomScopeTestBean.class);
@@ -790,6 +801,17 @@ public void handleIt(TestEvent event) {
 	}
 
 
+	@Component
+	@Scope(proxyMode = ScopedProxyMode.TARGET_CLASS)
+	static class CglibProxyWithPrivateMethod extends AbstractTestEventListener {
+
+		@EventListener
+		private void handleIt(TestEvent event) {
+			collectEvent(event);
+		}
+	}
+
+
 	@Component
 	@Scope(scopeName = "custom", proxyMode = ScopedProxyMode.TARGET_CLASS)
 	static class CustomScopeTestBean extends AbstractTestEventListener {

spring-core/src/main/java/org/springframework/core/MethodIntrospector.java

@@ -111,22 +111,26 @@ public Boolean inspect(Method method) {
 	 * @param targetType the target type to search methods on
 	 * (typically an interface-based JDK proxy)
 	 * @return a corresponding invocable method on the target type
+	 * @throws IllegalStateException if the given method is not invocable on the given
+	 * target type (typically due to a proxy mismatch)
 	 */
 	public static Method selectInvocableMethod(Method method, Class<?> targetType) {
 		if (method.getDeclaringClass().isAssignableFrom(targetType)) {
 			return method;
 		}
 		try {
+			String methodName = method.getName();
+			Class<?>[] parameterTypes = method.getParameterTypes();
 			for (Class<?> ifc : targetType.getInterfaces()) {
 				try {
-					return ifc.getMethod(method.getName(), method.getParameterTypes());
+					return ifc.getMethod(methodName, parameterTypes);
 				}
 				catch (NoSuchMethodException ex) {
 					// Alright, not on this interface then...
 				}
 			}
 			// A final desperate attempt on the proxy class itself...
-			return targetType.getMethod(method.getName(), method.getParameterTypes());
+			return targetType.getMethod(methodName, parameterTypes);
 		}
 		catch (NoSuchMethodException ex) {
 			throw new IllegalStateException(String.format(

spring-jms/src/main/java/org/springframework/jms/annotation/JmsListenerAnnotationBeanPostProcessor.java

@@ -236,7 +236,7 @@ public Set<JmsListener> inspect(Method method) {
 	 * @see JmsListenerEndpointRegistrar#registerEndpoint
 	 */
 	protected void processJmsListener(JmsListener jmsListener, Method mostSpecificMethod, Object bean) {
-		Method invocableMethod = MethodIntrospector.selectInvocableMethod(mostSpecificMethod, bean.getClass());
+		Method invocableMethod = AopUtils.selectInvocableMethod(mostSpecificMethod, bean.getClass());
 
 		MethodJmsListenerEndpoint endpoint = createMethodJmsListenerEndpoint();
 		endpoint.setBean(bean);