diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java index b0473cf1a7..5f3a68e4ca 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java @@ -19,6 +19,7 @@ package org.springframework.aop.aspectj; 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 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut 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> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass); if (ifcs.size() > 1) { Class compositeInterface = ClassUtils.createCompositeInterface( @@ -465,7 +469,11 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut 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); diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/TigerAspectJExpressionPointcutTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/TigerAspectJExpressionPointcutTests.java index c74a767d5c..ab2fc39b97 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/TigerAspectJExpressionPointcutTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/TigerAspectJExpressionPointcutTests.java @@ -26,6 +26,7 @@ import org.junit.Test; 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 class TigerAspectJExpressionPointcutTests { } @Test - public void testMatchVarargs() throws SecurityException, NoSuchMethodException { + public void testMatchVarargs() throws Exception { @SuppressWarnings("unused") class MyTemplate { @@ -99,19 +100,19 @@ public class TigerAspectJExpressionPointcutTests { } @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 class TigerAspectJExpressionPointcutTests { } @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 @@ public class TigerAspectJExpressionPointcutTests { } @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 class TigerAspectJExpressionPointcutTests { } @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 class TigerAspectJExpressionPointcutTests { } @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 @@ public class TigerAspectJExpressionPointcutTests { } @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 class TigerAspectJExpressionPointcutTests { } - static class BeanA { + interface IBeanA { + + @Tx + int getAge(); + } + + + static class BeanA implements IBeanA { private String name; @@ -277,6 +309,7 @@ public class TigerAspectJExpressionPointcutTests { } @Tx + @Override public int getAge() { return age; }