AopUtils.getMostSpecificMethod exposes dynamic proxy class methods

Includes efficient canApply check for IntroductionAwareMethodMatcher.

Issue: SPR-16757

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

@@ -49,13 +49,13 @@ import org.springframework.aop.ProxyMethodInvocation;
 import org.springframework.aop.framework.autoproxy.ProxyCreationContext;
 import org.springframework.aop.interceptor.ExposeInvocationInterceptor;
 import org.springframework.aop.support.AbstractExpressionPointcut;
+import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
-import org.springframework.core.BridgeMethodResolver;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -426,19 +426,15 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	}
 
 	private ShadowMatch getTargetShadowMatch(Method method, @Nullable Class<?> targetClass) {
-		Method targetMethod = method;
+		Method targetMethod = AopUtils.getMostSpecificMethod(method, targetClass);
-		if (targetClass != null) {
+		if (targetClass != null && targetMethod.getDeclaringClass().isInterface()) {
-			targetMethod = ClassUtils.getMostSpecificMethod(method, ClassUtils.getUserClass(targetClass));
+			Set<Class<?>> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass);
-			if (targetMethod.getDeclaringClass().isInterface()) {
+			if (ifcs.size() > 1) {
-				Set<Class<?>> ifcs = ClassUtils.getAllInterfacesForClassAsSet(targetClass);
+				Class<?> compositeInterface = ClassUtils.createCompositeInterface(
-				if (ifcs.size() > 1) {
+						ClassUtils.toClassArray(ifcs), targetClass.getClassLoader());
-					Class<?> compositeInterface = ClassUtils.createCompositeInterface(
+				targetMethod = ClassUtils.getMostSpecificMethod(targetMethod, compositeInterface);
-							ClassUtils.toClassArray(ifcs), targetClass.getClassLoader());
-					targetMethod = ClassUtils.getMostSpecificMethod(targetMethod, compositeInterface);
-				}
 			}
 		}
-		targetMethod = BridgeMethodResolver.findBridgedMethod(targetMethod);
 		return getShadowMatch(targetMethod, method);
 	}
 

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

@@ -192,8 +192,7 @@ public abstract class AopUtils {
 	 * @see org.springframework.util.ClassUtils#getMostSpecificMethod
 	 */
 	public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {
-		Class<?> specificTargetClass = (targetClass != null && !Proxy.isProxyClass(targetClass) ?
+		Class<?> specificTargetClass = (targetClass != null ? ClassUtils.getUserClass(targetClass) : null);
-				ClassUtils.getUserClass(targetClass) : null);
 		Method resolvedMethod = ClassUtils.getMostSpecificMethod(method, specificTargetClass);
 		// If we are dealing with method with generic parameters, find the original method.
 		return BridgeMethodResolver.findBridgedMethod(resolvedMethod);
@@ -247,8 +246,8 @@ public abstract class AopUtils {
 		for (Class<?> clazz : classes) {
 			Method[] methods = ReflectionUtils.getAllDeclaredMethods(clazz);
 			for (Method method : methods) {
-				if ((introductionAwareMethodMatcher != null &&
+				if (introductionAwareMethodMatcher != null ?
-						introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions)) ||
+						introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions) :
 						methodMatcher.matches(method, targetClass)) {
 					return true;
 				}

spring-aop/src/main/java/org/springframework/aop/support/annotation/AnnotationMethodMatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@ package org.springframework.aop.support.annotation;
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
 
 import org.springframework.aop.support.AopUtils;
 import org.springframework.aop.support.StaticMethodMatcher;
@@ -71,6 +72,10 @@ public class AnnotationMethodMatcher extends StaticMethodMatcher {
 		if (matchesMethod(method)) {
 			return true;
 		}
+		// Proxy classes never have annotations on their redeclared methods.
+		if (targetClass != null && Proxy.isProxyClass(targetClass)) {
+			return false;
+		}
 		// The method may be on an interface, so let's check on the target class as well.
 		Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
 		return (specificMethod != method && matchesMethod(specificMethod));

spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AnnotationPointcutTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,14 +33,16 @@ public class AnnotationPointcutTests {
 
 	private AnnotatedTestBean testBean;
 
+
 	@Before
-	public void setUp() {
+	public void setup() {
 		ClassPathXmlApplicationContext ctx =
-			new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass());
+				new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass());
 
 		testBean = (AnnotatedTestBean) ctx.getBean("testBean");
 	}
 
+
 	@Test
 	public void testAnnotationBindingInAroundAdvice() {
 		assertEquals("this value", testBean.doThis());
@@ -61,4 +63,3 @@ class TestMethodInterceptor implements MethodInterceptor {
 		return "this value";
 	}
 }
-

spring-tx/src/test/java/org/springframework/transaction/annotation/AnnotationTransactionInterceptorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -167,10 +167,13 @@ public class AnnotationTransactionInterceptorTests {
 
 		proxy.doSomething();
 		assertGetTransactionAndCommitCount(4);
+
+		proxy.doSomethingDefault();
+		assertGetTransactionAndCommitCount(5);
 	}
 
 	@Test
-	public void crossClassInterfaceMethodLevelOnJdkProxy() throws Exception {
+	public void crossClassInterfaceMethodLevelOnJdkProxy() {
 		ProxyFactory proxyFactory = new ProxyFactory();
 		proxyFactory.setTarget(new SomeServiceImpl());
 		proxyFactory.addInterface(SomeService.class);
@@ -189,7 +192,7 @@ public class AnnotationTransactionInterceptorTests {
 	}
 
 	@Test
-	public void crossClassInterfaceOnJdkProxy() throws Exception {
+	public void crossClassInterfaceOnJdkProxy() {
 		ProxyFactory proxyFactory = new ProxyFactory();
 		proxyFactory.setTarget(new OtherServiceImpl());
 		proxyFactory.addInterface(OtherService.class);
@@ -201,6 +204,64 @@ public class AnnotationTransactionInterceptorTests {
 		assertGetTransactionAndCommitCount(1);
 	}
 
+	@Test
+	public void withInterfaceOnTargetJdkProxy() {
+		ProxyFactory targetFactory = new ProxyFactory();
+		targetFactory.setTarget(new TestWithInterfaceImpl());
+		targetFactory.addInterface(TestWithInterface.class);
+
+		ProxyFactory proxyFactory = new ProxyFactory();
+		proxyFactory.setTarget(targetFactory.getProxy());
+		proxyFactory.addInterface(TestWithInterface.class);
+		proxyFactory.addAdvice(this.ti);
+
+		TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy();
+
+		proxy.doSomething();
+		assertGetTransactionAndCommitCount(1);
+
+		proxy.doSomethingElse();
+		assertGetTransactionAndCommitCount(2);
+
+		proxy.doSomethingElse();
+		assertGetTransactionAndCommitCount(3);
+
+		proxy.doSomething();
+		assertGetTransactionAndCommitCount(4);
+
+		proxy.doSomethingDefault();
+		assertGetTransactionAndCommitCount(5);
+	}
+
+	@Test
+	public void withInterfaceOnTargetCglibProxy() {
+		ProxyFactory targetFactory = new ProxyFactory();
+		targetFactory.setTarget(new TestWithInterfaceImpl());
+		targetFactory.setProxyTargetClass(true);
+
+		ProxyFactory proxyFactory = new ProxyFactory();
+		proxyFactory.setTarget(targetFactory.getProxy());
+		proxyFactory.addInterface(TestWithInterface.class);
+		proxyFactory.addAdvice(this.ti);
+
+		TestWithInterface proxy = (TestWithInterface) proxyFactory.getProxy();
+
+		proxy.doSomething();
+		assertGetTransactionAndCommitCount(1);
+
+		proxy.doSomethingElse();
+		assertGetTransactionAndCommitCount(2);
+
+		proxy.doSomethingElse();
+		assertGetTransactionAndCommitCount(3);
+
+		proxy.doSomething();
+		assertGetTransactionAndCommitCount(4);
+
+		proxy.doSomethingDefault();
+		assertGetTransactionAndCommitCount(5);
+	}
+
 	private void assertGetTransactionAndCommitCount(int expectedCount) {
 		assertEquals(expectedCount, this.ptm.begun);
 		assertEquals(expectedCount, this.ptm.commits);
@@ -309,13 +370,22 @@ public class AnnotationTransactionInterceptorTests {
 	}
 
 
-	@Transactional
+	public interface BaseInterface {
-	public static interface TestWithInterface {
 
-		public void doSomething();
+		void doSomething();
+	}
+
+
+	@Transactional
+	public interface TestWithInterface extends BaseInterface {
 
 		@Transactional(readOnly = true)
-		public void doSomethingElse();
+		void doSomethingElse();
+
+		default void doSomethingDefault() {
+			assertTrue(TransactionSynchronizationManager.isActualTransactionActive());
+			assertFalse(TransactionSynchronizationManager.isCurrentTransactionReadOnly());
+		}
 	}
 
 
@@ -335,7 +405,7 @@ public class AnnotationTransactionInterceptorTests {
 	}
 
 
-	public static interface SomeService {
+	public interface SomeService {
 
 		void foo();