Consistent treatment of proxy classes and interfaces for introspection

Issue: SPR-16675
Issue: SPR-16677

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

@@ -192,7 +192,9 @@ public abstract class AopUtils {
 	 * @see org.springframework.util.ClassUtils#getMostSpecificMethod
 	 */
 	public static Method getMostSpecificMethod(Method method, @Nullable Class<?> targetClass) {
-		Method resolvedMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
+		Class<?> specificTargetClass = (targetClass != null && !Proxy.isProxyClass(targetClass) ?
+				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);
 	}
@@ -236,15 +238,18 @@ public abstract class AopUtils {
 			introductionAwareMethodMatcher = (IntroductionAwareMethodMatcher) methodMatcher;
 		}
 
-		Set<Class<?>> classes = new LinkedHashSet<>(ClassUtils.getAllInterfacesForClassAsSet(targetClass));
-		Class<?> userClass = ClassUtils.getUserClass(targetClass);
-		classes.add(userClass);
+		Set<Class<?>> classes = new LinkedHashSet<>();
+		if (!Proxy.isProxyClass(targetClass)) {
+			classes.add(ClassUtils.getUserClass(targetClass));
+		}
+		classes.addAll(ClassUtils.getAllInterfacesForClassAsSet(targetClass));
+
 		for (Class<?> clazz : classes) {
 			Method[] methods = ReflectionUtils.getAllDeclaredMethods(clazz);
 			for (Method method : methods) {
 				if ((introductionAwareMethodMatcher != null &&
-						introductionAwareMethodMatcher.matches(method, userClass, hasIntroductions)) ||
-						methodMatcher.matches(method, userClass)) {
+						introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions)) ||
+						methodMatcher.matches(method, targetClass)) {
 					return true;
 				}
 			}

spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 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.
@@ -24,10 +24,9 @@ import java.util.concurrent.ConcurrentHashMap;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.core.BridgeMethodResolver;
+import org.springframework.aop.support.AopUtils;
 import org.springframework.core.MethodClassKey;
 import org.springframework.lang.Nullable;
-import org.springframework.util.ClassUtils;
 
 /**
  * Abstract implementation of {@link JCacheOperationSource} that caches attributes
@@ -87,9 +86,7 @@ public abstract class AbstractFallbackJCacheOperationSource implements JCacheOpe
 
 		// The method may be on an interface, but we need attributes from the target class.
 		// If the target class is null, the method will be unchanged.
-		Method specificMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
-		// If we are dealing with method with generic parameters, find the original method.
-		specificMethod = BridgeMethodResolver.findBridgedMethod(specificMethod);
+		Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
 
 		// First try is the method in the target class.
 		JCacheOperation<?> operation = findCacheOperation(specificMethod, targetClass);

spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.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.
@@ -26,7 +26,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.core.BridgeMethodResolver;
+import org.springframework.aop.support.AopUtils;
 import org.springframework.core.MethodClassKey;
 import org.springframework.lang.Nullable;
 import org.springframework.util.ClassUtils;
@@ -131,9 +131,7 @@ public abstract class AbstractFallbackCacheOperationSource implements CacheOpera
 
 		// The method may be on an interface, but we need attributes from the target class.
 		// If the target class is null, the method will be unchanged.
-		Method specificMethod = ClassUtils.getMostSpecificMethod(method, targetClass);
-		// If we are dealing with method with generic parameters, find the original method.
-		specificMethod = BridgeMethodResolver.findBridgedMethod(specificMethod);
+		Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
 
 		// First try is the method in the target class.
 		Collection<CacheOperation> opDef = findCacheOperations(specificMethod);

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

@@ -16,6 +16,8 @@
 
 package org.springframework.aop.aspectj.autoproxy;
 
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.lang.reflect.Method;
 
 import org.apache.commons.logging.Log;
@@ -57,7 +59,6 @@ import org.springframework.tests.sample.beans.NestedTestBean;
 import org.springframework.tests.sample.beans.TestBean;
 import org.springframework.util.StopWatch;
 
-import static java.lang.String.format;
 import static org.junit.Assert.*;
 
 /**
@@ -73,15 +74,11 @@ public class AspectJAutoProxyCreatorTests {
 
 	private static final Log factoryLog = LogFactory.getLog(DefaultListableBeanFactory.class);
 
-	private static void assertStopWatchTimeLimit(final StopWatch sw, final long maxTimeMillis) {
-		final long totalTimeMillis = sw.getTotalTimeMillis();
-		assertTrue("'" + sw.getLastTaskName() + "' took too long: expected less than<" + maxTimeMillis
-				+ "> ms, actual<" + totalTimeMillis + "> ms.", totalTimeMillis < maxTimeMillis);
-	}
 
 	@Test
 	public void testAspectsAreApplied() {
 		ClassPathXmlApplicationContext bf = newContext("aspects.xml");
+
 		ITestBean tb = (ITestBean) bf.getBean("adrian");
 		assertEquals(68, tb.getAge());
 		MethodInvokingFactoryBean factoryBean = (MethodInvokingFactoryBean) bf.getBean("&factoryBean");
@@ -92,6 +89,7 @@ public class AspectJAutoProxyCreatorTests {
 	@Test
 	public void testMultipleAspectsWithParameterApplied() {
 		ClassPathXmlApplicationContext bf = newContext("aspects.xml");
+
 		ITestBean tb = (ITestBean) bf.getBean("adrian");
 		tb.setAge(10);
 		assertEquals(20, tb.getAge());
@@ -100,6 +98,7 @@ public class AspectJAutoProxyCreatorTests {
 	@Test
 	public void testAspectsAreAppliedInDefinedOrder() {
 		ClassPathXmlApplicationContext bf = newContext("aspectsWithOrdering.xml");
+
 		ITestBean tb = (ITestBean) bf.getBean("adrian");
 		assertEquals(71, tb.getAge());
 	}
@@ -107,6 +106,7 @@ public class AspectJAutoProxyCreatorTests {
 	@Test
 	public void testAspectsAndAdvisorAreApplied() {
 		ClassPathXmlApplicationContext ac = newContext("aspectsPlusAdvisor.xml");
+
 		ITestBean shouldBeWeaved = (ITestBean) ac.getBean("adrian");
 		doTestAspectsAndAdvisorAreApplied(ac, shouldBeWeaved);
 	}
@@ -115,7 +115,9 @@ public class AspectJAutoProxyCreatorTests {
 	public void testAspectsAndAdvisorAppliedToPrototypeIsFastEnough() {
 		Assume.group(TestGroup.PERFORMANCE);
 		Assume.notLogging(factoryLog);
+
 		ClassPathXmlApplicationContext ac = newContext("aspectsPlusAdvisor.xml");
+
 		StopWatch sw = new StopWatch();
 		sw.start("Prototype Creation");
 		for (int i = 0; i < 10000; i++) {
@@ -135,7 +137,9 @@ public class AspectJAutoProxyCreatorTests {
 	public void testAspectsAndAdvisorNotAppliedToPrototypeIsFastEnough() {
 		Assume.group(TestGroup.PERFORMANCE);
 		Assume.notLogging(factoryLog);
+
 		ClassPathXmlApplicationContext ac = newContext("aspectsPlusAdvisor.xml");
+
 		StopWatch sw = new StopWatch();
 		sw.start("Prototype Creation");
 		for (int i = 0; i < 100000; i++) {
@@ -155,7 +159,9 @@ public class AspectJAutoProxyCreatorTests {
 	public void testAspectsAndAdvisorNotAppliedToManySingletonsIsFastEnough() {
 		Assume.group(TestGroup.PERFORMANCE);
 		Assume.notLogging(factoryLog);
+
 		GenericApplicationContext ac = new GenericApplicationContext();
+
 		new XmlBeanDefinitionReader(ac).loadBeanDefinitions(new ClassPathResource(qName("aspectsPlusAdvisor.xml"),
 				getClass()));
 		for (int i = 0; i < 10000; i++) {
@@ -174,6 +180,7 @@ public class AspectJAutoProxyCreatorTests {
 	@Test
 	public void testAspectsAndAdvisorAreAppliedEvenIfComingFromParentFactory() {
 		ClassPathXmlApplicationContext ac = newContext("aspectsPlusAdvisor.xml");
+
 		GenericApplicationContext childAc = new GenericApplicationContext(ac);
 		// Create a child factory with a bean that should be woven
 		RootBeanDefinition bd = new RootBeanDefinition(TestBean.class);
@@ -336,8 +343,9 @@ public class AspectJAutoProxyCreatorTests {
 	}
 
 	@Test
-	public void testRetryAspect() throws Exception {
+	public void testRetryAspect() {
 		ClassPathXmlApplicationContext bf = newContext("retryAspect.xml");
+
 		UnreliableBean bean = (UnreliableBean) bf.getBean("unreliableBean");
 		RetryAspect aspect = (RetryAspect) bf.getBean("retryAspect");
 		int attempts = bean.unreliable();
@@ -347,6 +355,15 @@ public class AspectJAutoProxyCreatorTests {
 		assertEquals(1, aspect.getCommitCalls());
 	}
 
+	@Test
+	public void testWithBeanNameAutoProxyCreator() {
+		ClassPathXmlApplicationContext bf = newContext("withBeanNameAutoProxyCreator.xml");
+
+		ITestBean tb = (ITestBean) bf.getBean("adrian");
+		assertEquals(68, tb.getAge());
+	}
+
+
 	/**
 	 * Returns a new {@link ClassPathXmlApplicationContext} for the file ending in <var>fileSuffix</var>.
 	 */
@@ -360,7 +377,13 @@ public class AspectJAutoProxyCreatorTests {
 	 * 'AspectJAutoProxyCreatorTests-foo.xml'
 	 */
 	private String qName(String fileSuffix) {
-		return format("%s-%s", getClass().getSimpleName(), fileSuffix);
+		return String.format("%s-%s", getClass().getSimpleName(), fileSuffix);
+	}
+
+	private void assertStopWatchTimeLimit(final StopWatch sw, final long maxTimeMillis) {
+		long totalTimeMillis = sw.getTotalTimeMillis();
+		assertTrue("'" + sw.getLastTaskName() + "' took too long: expected less than<" + maxTimeMillis +
+				"> ms, actual<" + totalTimeMillis + "> ms.", totalTimeMillis < maxTimeMillis);
 	}
 
 }
@@ -409,7 +432,6 @@ class AdviceUsingThisJoinPoint {
 	public void entryTrace(JoinPoint jp) {
 		this.lastEntry = jp.toString();
 	}
-
 }
 
 @Aspect
@@ -419,7 +441,6 @@ class DummyAspect {
 	public Object test(ProceedingJoinPoint pjp) throws Throwable {
 		return pjp.proceed();
 	}
-
 }
 
 @Aspect
@@ -435,7 +456,7 @@ class DummyAspectWithParameter {
 class DummyFactoryBean implements FactoryBean<Object> {
 
 	@Override
-	public Object getObject() throws Exception {
+	public Object getObject() {
 		throw new UnsupportedOperationException();
 	}
 
@@ -460,7 +481,6 @@ class IncreaseReturnValue {
 		int result = (Integer) pjp.proceed();
 		return result + 3;
 	}
-
 }
 
 @Aspect
@@ -484,7 +504,49 @@ class MultiplyReturnValue {
 		int result = (Integer) pjp.proceed();
 		return result * this.multiple;
 	}
+}
 
+@Retention(RetentionPolicy.RUNTIME)
+@interface Marker {
+}
+
+@Aspect
+class MultiplyReturnValueForMarker {
+
+	private int multiple = 2;
+
+	public int invocations;
+
+	public void setMultiple(int multiple) {
+		this.multiple = multiple;
+	}
+
+	public int getMultiple() {
+		return this.multiple;
+	}
+
+	@Around("@annotation(org.springframework.aop.aspectj.autoproxy.Marker)")
+	public Object doubleReturnValue(ProceedingJoinPoint pjp) throws Throwable {
+		++this.invocations;
+		int result = (Integer) pjp.proceed();
+		return result * this.multiple;
+	}
+}
+
+interface IMarkerTestBean extends ITestBean {
+
+	@Marker
+	@Override
+	int getAge();
+}
+
+class MarkerTestBean extends TestBean implements IMarkerTestBean {
+
+	@Marker
+	@Override
+	public int getAge() {
+		return super.getAge();
+	}
 }
 
 @Aspect
@@ -538,7 +600,6 @@ class RetryAspect {
 	public int getRollbackCalls() {
 		return this.rollbackCalls;
 	}
-
 }
 
 @SuppressWarnings("serial")

spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 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.
@@ -16,8 +16,6 @@
 
 package org.springframework.aop.framework.autoproxy;
 
-import java.io.IOException;
-
 import org.junit.Before;
 import org.junit.Test;
 import test.mixin.Lockable;
@@ -45,14 +43,15 @@ public class BeanNameAutoProxyCreatorTests {
 
 	private BeanFactory beanFactory;
 
+
 	@Before
-	public void setUp() throws IOException {
+	public void setup() {
 		// Note that we need an ApplicationContext, not just a BeanFactory,
 		// for post-processing and hence auto-proxying to work.
-		beanFactory =
-			new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass());
+		beanFactory = new ClassPathXmlApplicationContext(getClass().getSimpleName() + "-context.xml", getClass());
 	}
 
+
 	@Test
 	public void testNoProxy() {
 		TestBean tb = (TestBean) beanFactory.getBean("noproxy");
@@ -171,6 +170,7 @@ public class BeanNameAutoProxyCreatorTests {
 		assertTrue(((Advised)testBean).isFrozen());
 	}
 
+
 	private void jdkAssertions(ITestBean tb, int nopInterceptorCount)  {
 		NopInterceptor nop = (NopInterceptor) beanFactory.getBean("nopInterceptor");
 		assertEquals(0, nop.getCount());

spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-aspects.xml

@@ -6,7 +6,7 @@
 				http://www.springframework.org/schema/aop http://www.springframework.org/schema/aop/spring-aop-2.0.xsd">
 
 	<!--
-	<bean class="org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator" />
+	<bean class="org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator"/>
 	-->
 
 	<aop:aspectj-autoproxy/>

spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-withBeanNameAutoProxyCreator.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<beans xmlns="http://www.springframework.org/schema/beans"
+		xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+		xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd">
+
+	<bean class="org.springframework.aop.framework.autoproxy.BeanNameAutoProxyCreator">
+		<property name="beanNames" value="adrian"/>
+		<property name="interceptorNames" value="trace"/>
+		<property name="proxyTargetClass" value="true"/>
+	</bean>
+
+	<bean id="trace" class="org.springframework.aop.interceptor.SimpleTraceInterceptor"/>
+
+	<bean class="org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator">
+		<property name="proxyTargetClass" value="true"/>
+	</bean>
+
+	<bean class="org.springframework.aop.aspectj.autoproxy.MultiplyReturnValueForMarker">
+		<property name="multiple" value="2"/>
+	</bean>
+
+	<bean class="org.springframework.aop.aspectj.autoproxy.DummyAspect"/>
+
+	<bean class="org.springframework.aop.aspectj.autoproxy.DummyAspectWithParameter"/>
+
+	<bean id="adrianParent" abstract="true">
+		<property name="name" value="adrian"/>
+	</bean>
+
+	<bean id="adrian" class="org.springframework.aop.aspectj.autoproxy.MarkerTestBean" parent="adrianParent">
+		<property name="age" value="34"/>
+	</bean>
+
+</beans>

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

@@ -18,7 +18,6 @@ package org.springframework.core;
 
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
-import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -58,10 +57,10 @@ public abstract class MethodIntrospector {
 		Class<?> specificHandlerType = null;
 
 		if (!Proxy.isProxyClass(targetType)) {
-			handlerTypes.add(targetType);
-			specificHandlerType = targetType;
+			specificHandlerType = ClassUtils.getUserClass(targetType);
+			handlerTypes.add(specificHandlerType);
 		}
-		Collections.addAll(handlerTypes, targetType.getInterfaces());
+		handlerTypes.addAll(ClassUtils.getAllInterfacesForClassAsSet(targetType));
 
 		for (Class<?> currentHandlerType : handlerTypes) {
 			final Class<?> targetClass = (specificHandlerType != null ? specificHandlerType : currentHandlerType);

spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.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.
@@ -24,7 +24,7 @@ import java.util.concurrent.ConcurrentHashMap;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
-import org.springframework.core.BridgeMethodResolver;
+import org.springframework.aop.support.AopUtils;
 import org.springframework.core.MethodClassKey;
 import org.springframework.lang.Nullable;
 import org.springframework.util.ClassUtils;
@@ -148,13 +148,9 @@ public abstract class AbstractFallbackTransactionAttributeSource implements Tran
 			return null;
 		}
 
-		// Ignore CGLIB subclasses - introspect the actual user class.
-		Class<?> userClass = (targetClass != null ? ClassUtils.getUserClass(targetClass) : null);
 		// The method may be on an interface, but we need attributes from the target class.
 		// If the target class is null, the method will be unchanged.
-		Method specificMethod = ClassUtils.getMostSpecificMethod(method, userClass);
-		// If we are dealing with method with generic parameters, find the original method.
-		specificMethod = BridgeMethodResolver.findBridgedMethod(specificMethod);
+		Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
 
 		// First try is the method in the target class.
 		TransactionAttribute txAttr = findTransactionAttribute(specificMethod);