diff --git a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java b/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java index 05e5327abec..a6f87f573e3 100644 --- a/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java +++ b/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> classes = new LinkedHashSet<>(ClassUtils.getAllInterfacesForClassAsSet(targetClass)); - Class userClass = ClassUtils.getUserClass(targetClass); - classes.add(userClass); + Set> 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; } } diff --git a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java b/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java index 56233656e19..3ca53b2aebe 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/jcache/interceptor/AbstractFallbackJCacheOperationSource.java +++ b/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); diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java b/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java index aaf09ab849b..4b5cb59614c 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/AbstractFallbackCacheOperationSource.java +++ b/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 opDef = findCacheOperations(specificMethod); diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java index 799d0eba059..58f55bd6ad8 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java +++ b/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 fileSuffix. */ @@ -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 { @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") diff --git a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java index e65ea69f1e0..0b2786b2e85 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/BeanNameAutoProxyCreatorTests.java +++ b/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()); diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-aspects.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-aspects.xml index 406422efc22..b96cfef03fd 100644 --- a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-aspects.xml +++ b/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"> diff --git a/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-withBeanNameAutoProxyCreator.xml b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-withBeanNameAutoProxyCreator.xml new file mode 100644 index 00000000000..7c29e5f5d0c --- /dev/null +++ b/spring-context/src/test/resources/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests-withBeanNameAutoProxyCreator.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-core/src/main/java/org/springframework/core/MethodIntrospector.java b/spring-core/src/main/java/org/springframework/core/MethodIntrospector.java index d220814653c..be7ea23a9ab 100644 --- a/spring-core/src/main/java/org/springframework/core/MethodIntrospector.java +++ b/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); diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.java index 7846f6f2d71..f9360ee5cf6 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/AbstractFallbackTransactionAttributeSource.java +++ b/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);