AspectJExpressionPointcut evaluates interface method on proxy as well

Issue: SPR-16803

(cherry picked from commit bba5dca)

Author: jhoeller

spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java

@@ -19,6 +19,7 @@
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Map;
@@ -428,6 +429,9 @@ private void bindParameters(ProxyMethodInvocation invocation, JoinPointMatch jpm
 	private ShadowMatch getTargetShadowMatch(Method method, @Nullable Class<?> targetClass) {
 		Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass);
 		if (targetClass != null && targetMethod.getDeclaringClass().isInterface()) {
+			// Try to build the most specific interface possible for inherited methods to be
+			// considered for sub-interface matches as well, in particular for proxy classes.
+			// Note: AspectJ is only going to take Method.getDeclaringClass() into account.
 			Set<Class<?>> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass);
 			if (ifcs.size() > 1) {
 				Class<?> compositeInterface = ClassUtils.createCompositeInterface(
@@ -465,7 +469,11 @@ private ShadowMatch getShadowMatch(Method targetMethod, Method originalMethod) {
 								fallbackExpression = null;
 							}
 						}
-						if (shadowMatch == null && targetMethod != originalMethod) {
+						if (targetMethod != originalMethod && (shadowMatch == null ||
+								(shadowMatch.neverMatches() && Proxy.isProxyClass(targetMethod.getDeclaringClass())))) {
+							// Fall back to the plain original method in case of no resolvable match or a
+							// negative match on a proxy class (which doesn't carry any annotations on its
+							// redeclared methods).
 							methodToMatch = originalMethod;
 							try {
 								shadowMatch = obtainPointcutExpression().matchesMethodExecution(methodToMatch);

spring-aop/src/test/java/org/springframework/aop/aspectj/TigerAspectJExpressionPointcutTests.java

@@ -26,6 +26,7 @@
 import test.annotation.EmptySpringAnnotation;
 import test.annotation.transaction.Tx;
 
+import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.tests.sample.beans.TestBean;
 
 import static org.junit.Assert.*;
@@ -73,7 +74,7 @@ public void testMatchGenericArgument() {
 	}
 
 	@Test
-	public void testMatchVarargs() throws SecurityException, NoSuchMethodException {
+	public void testMatchVarargs() throws Exception {
 
 		@SuppressWarnings("unused")
 		class MyTemplate {
@@ -99,19 +100,19 @@ public int queryForInt(String sql, Object... params) {
 	}
 
 	@Test
-	public void testMatchAnnotationOnClassWithAtWithin() throws SecurityException, NoSuchMethodException {
+	public void testMatchAnnotationOnClassWithAtWithin() throws Exception {
 		String expression = "@within(test.annotation.transaction.Tx)";
 		testMatchAnnotationOnClass(expression);
 	}
 
 	@Test
-	public void testMatchAnnotationOnClassWithoutBinding() throws SecurityException, NoSuchMethodException {
+	public void testMatchAnnotationOnClassWithoutBinding() throws Exception {
 		String expression = "within(@test.annotation.transaction.Tx *)";
 		testMatchAnnotationOnClass(expression);
 	}
 
 	@Test
-	public void testMatchAnnotationOnClassWithSubpackageWildcard() throws SecurityException, NoSuchMethodException {
+	public void testMatchAnnotationOnClassWithSubpackageWildcard() throws Exception {
 		String expression = "within(@(test.annotation..*) *)";
 		AspectJExpressionPointcut springAnnotatedPc = testMatchAnnotationOnClass(expression);
 		assertFalse(springAnnotatedPc.matches(TestBean.class.getMethod("setName", String.class), TestBean.class));
@@ -123,12 +124,12 @@ public void testMatchAnnotationOnClassWithSubpackageWildcard() throws SecurityEx
 	}
 
 	@Test
-	public void testMatchAnnotationOnClassWithExactPackageWildcard() throws SecurityException, NoSuchMethodException {
+	public void testMatchAnnotationOnClassWithExactPackageWildcard() throws Exception {
 		String expression = "within(@(test.annotation.transaction.*) *)";
 		testMatchAnnotationOnClass(expression);
 	}
 
-	private AspectJExpressionPointcut testMatchAnnotationOnClass(String expression) throws SecurityException, NoSuchMethodException {
+	private AspectJExpressionPointcut testMatchAnnotationOnClass(String expression) throws Exception {
 		AspectJExpressionPointcut ajexp = new AspectJExpressionPointcut();
 		ajexp.setExpression(expression);
 
@@ -141,7 +142,7 @@ private AspectJExpressionPointcut testMatchAnnotationOnClass(String expression)
 	}
 
 	@Test
-	public void testAnnotationOnMethodWithFQN() throws SecurityException, NoSuchMethodException {
+	public void testAnnotationOnMethodWithFQN() throws Exception {
 		String expression = "@annotation(test.annotation.transaction.Tx)";
 		AspectJExpressionPointcut ajexp = new AspectJExpressionPointcut();
 		ajexp.setExpression(expression);
@@ -155,7 +156,31 @@ public void testAnnotationOnMethodWithFQN() throws SecurityException, NoSuchMeth
 	}
 
 	@Test
-	public void testAnnotationOnMethodWithWildcard() throws SecurityException, NoSuchMethodException {
+	public void testAnnotationOnCglibProxyMethod() throws Exception {
+		String expression = "@annotation(test.annotation.transaction.Tx)";
+		AspectJExpressionPointcut ajexp = new AspectJExpressionPointcut();
+		ajexp.setExpression(expression);
+
+		ProxyFactory factory = new ProxyFactory(new BeanA());
+		factory.setProxyTargetClass(true);
+		BeanA proxy = (BeanA) factory.getProxy();
+		assertTrue(ajexp.matches(BeanA.class.getMethod("getAge"), proxy.getClass()));
+	}
+
+	@Test
+	public void testAnnotationOnDynamicProxyMethod() throws Exception {
+		String expression = "@annotation(test.annotation.transaction.Tx)";
+		AspectJExpressionPointcut ajexp = new AspectJExpressionPointcut();
+		ajexp.setExpression(expression);
+
+		ProxyFactory factory = new ProxyFactory(new BeanA());
+		factory.setProxyTargetClass(false);
+		IBeanA proxy = (IBeanA) factory.getProxy();
+		assertTrue(ajexp.matches(IBeanA.class.getMethod("getAge"), proxy.getClass()));
+	}
+
+	@Test
+	public void testAnnotationOnMethodWithWildcard() throws Exception {
 		String expression = "execution(@(test.annotation..*) * *(..))";
 		AspectJExpressionPointcut anySpringMethodAnnotation = new AspectJExpressionPointcut();
 		anySpringMethodAnnotation.setExpression(expression);
@@ -171,7 +196,7 @@ public void testAnnotationOnMethodWithWildcard() throws SecurityException, NoSuc
 	}
 
 	@Test
-	public void testAnnotationOnMethodArgumentsWithFQN() throws SecurityException, NoSuchMethodException {
+	public void testAnnotationOnMethodArgumentsWithFQN() throws Exception {
 		String expression = "@args(*, test.annotation.EmptySpringAnnotation))";
 		AspectJExpressionPointcut takesSpringAnnotatedArgument2 = new AspectJExpressionPointcut();
 		takesSpringAnnotatedArgument2.setExpression(expression);
@@ -201,7 +226,7 @@ ProcessesSpringAnnotatedParameters.class, new TestBean(), new BeanA())
 	}
 
 	@Test
-	public void testAnnotationOnMethodArgumentsWithWildcards() throws SecurityException, NoSuchMethodException {
+	public void testAnnotationOnMethodArgumentsWithWildcards() throws Exception {
 		String expression = "execution(* *(*, @(test..*) *))";
 		AspectJExpressionPointcut takesSpringAnnotatedArgument2 = new AspectJExpressionPointcut();
 		takesSpringAnnotatedArgument2.setExpression(expression);
@@ -266,7 +291,14 @@ public void foo() {
 	}
 
 
-	static class BeanA {
+	interface IBeanA {
+
+		@Tx
+		int getAge();
+	}
+
+
+	static class BeanA implements IBeanA {
 
 		private String name;
 
@@ -277,6 +309,7 @@ public void setName(String name) {
 		}
 
 		@Tx
+		@Override
 		public int getAge() {
 			return age;
 		}