From 999112216d46d95119b520dcef928cb7924370dc Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 14 Apr 2016 21:46:25 +0200 Subject: [PATCH] AopUtils.canApply properly matches package-visible methods (aligned with advice matching within proxies) Also, AbstractAutoProxyCreator considers Pointcut as infrastructure class, analogous to Advice and Advisor. Issue: SPR-14174 --- .../autoproxy/AbstractAutoProxyCreator.java | 4 +- .../springframework/aop/support/AopUtils.java | 9 +++- .../AdvisorAutoProxyCreatorTests.java | 36 ++++++++++++---- .../autoproxy/AutoProxyCreatorTests.java | 21 ++++++++- .../autoproxy/PackageVisibleMethod.java | 24 +++++++++++ ...oProxyCreatorTests-common-interceptors.xml | 43 ++++++++++--------- 6 files changed, 102 insertions(+), 35 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/aop/framework/autoproxy/PackageVisibleMethod.java diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java index eb476b4ec40..729343083bb 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -31,6 +31,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.aop.Advisor; +import org.springframework.aop.Pointcut; import org.springframework.aop.TargetSource; import org.springframework.aop.framework.AopInfrastructureBean; import org.springframework.aop.framework.ProxyFactory; @@ -370,6 +371,7 @@ public abstract class AbstractAutoProxyCreator extends ProxyProcessorSupport */ protected boolean isInfrastructureClass(Class beanClass) { boolean retVal = Advice.class.isAssignableFrom(beanClass) || + Pointcut.class.isAssignableFrom(beanClass) || Advisor.class.isAssignableFrom(beanClass) || AopInfrastructureBean.class.isAssignableFrom(beanClass); if (retVal && logger.isTraceEnabled()) { 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 184b8ef7e2f..4c8273ec39e 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -222,6 +222,11 @@ public abstract class AopUtils { } MethodMatcher methodMatcher = pc.getMethodMatcher(); + if (methodMatcher == MethodMatcher.TRUE) { + // No need to iterate the methods if we're matching any method anyway... + return true; + } + IntroductionAwareMethodMatcher introductionAwareMethodMatcher = null; if (methodMatcher instanceof IntroductionAwareMethodMatcher) { introductionAwareMethodMatcher = (IntroductionAwareMethodMatcher) methodMatcher; @@ -230,7 +235,7 @@ public abstract class AopUtils { Set> classes = new LinkedHashSet>(ClassUtils.getAllInterfacesForClassAsSet(targetClass)); classes.add(targetClass); for (Class clazz : classes) { - Method[] methods = clazz.getMethods(); + Method[] methods = ReflectionUtils.getAllDeclaredMethods(clazz); for (Method method : methods) { if ((introductionAwareMethodMatcher != null && introductionAwareMethodMatcher.matches(method, targetClass, hasIntroductions)) || diff --git a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java index 5c7dfce742d..0170cfd7287 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -21,6 +21,7 @@ import java.io.IOException; import org.junit.Test; import test.mixin.Lockable; +import org.springframework.aop.Advisor; import org.springframework.aop.framework.Advised; import org.springframework.aop.framework.autoproxy.target.AbstractBeanFactoryBasedTargetSourceCreator; import org.springframework.aop.support.AopUtils; @@ -48,7 +49,7 @@ import static org.junit.Assert.*; * @author Chris Beams */ @SuppressWarnings("resource") -public final class AdvisorAutoProxyCreatorTests { +public class AdvisorAutoProxyCreatorTests { private static final Class CLASS = AdvisorAutoProxyCreatorTests.class; private static final String CLASSNAME = CLASS.getSimpleName(); @@ -59,6 +60,7 @@ public final class AdvisorAutoProxyCreatorTests { private static final String QUICK_TARGETSOURCE_CONTEXT = CLASSNAME + "-quick-targetsource.xml"; private static final String OPTIMIZED_CONTEXT = CLASSNAME + "-optimized.xml"; + /** * Return a bean factory with attributes and EnterpriseServices configured. */ @@ -66,6 +68,7 @@ public final class AdvisorAutoProxyCreatorTests { return new ClassPathXmlApplicationContext(DEFAULT_CONTEXT, CLASS); } + /** * Check that we can provide a common interceptor that will * appear in the chain before "specific" interceptors, @@ -78,8 +81,8 @@ public final class AdvisorAutoProxyCreatorTests { assertTrue(AopUtils.isAopProxy(test1)); Lockable lockable1 = (Lockable) test1; - NopInterceptor nop = (NopInterceptor) bf.getBean("nopInterceptor"); - assertEquals(0, nop.getCount()); + NopInterceptor nop1 = (NopInterceptor) bf.getBean("nopInterceptor"); + NopInterceptor nop2 = (NopInterceptor) bf.getBean("pointcutAdvisor", Advisor.class).getAdvice(); ITestBean test2 = (ITestBean) bf.getBean("test2"); Lockable lockable2 = (Lockable) test2; @@ -87,14 +90,28 @@ public final class AdvisorAutoProxyCreatorTests { // Locking should be independent; nop is shared assertFalse(lockable1.locked()); assertFalse(lockable2.locked()); - // equals 2 calls on shared nop, because it's first - // and sees calls against the Lockable interface introduced - // by the specific advisor - assertEquals(2, nop.getCount()); + // equals 2 calls on shared nop, because it's first and sees calls + // against the Lockable interface introduced by the specific advisor + assertEquals(2, nop1.getCount()); + assertEquals(0, nop2.getCount()); lockable1.lock(); assertTrue(lockable1.locked()); assertFalse(lockable2.locked()); - assertEquals(5, nop.getCount()); + assertEquals(5, nop1.getCount()); + assertEquals(0, nop2.getCount()); + + PackageVisibleMethod packageVisibleMethod = (PackageVisibleMethod) bf.getBean("packageVisibleMethod"); + assertEquals(5, nop1.getCount()); + assertEquals(0, nop2.getCount()); + packageVisibleMethod.doSomething(); + assertEquals(6, nop1.getCount()); + assertEquals(1, nop2.getCount()); + assertTrue(packageVisibleMethod instanceof Lockable); + Lockable lockable3 = (Lockable) packageVisibleMethod; + lockable3.lock(); + assertTrue(lockable3.locked()); + lockable3.unlock(); + assertFalse(lockable3.locked()); } /** @@ -202,6 +219,7 @@ public final class AdvisorAutoProxyCreatorTests { } + class SelectivePrototypeTargetSourceCreator extends AbstractBeanFactoryBasedTargetSourceCreator { @Override diff --git a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AutoProxyCreatorTests.java index acaa8b72f66..5ed2091c24c 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/AutoProxyCreatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -56,7 +56,7 @@ import static org.junit.Assert.*; * @since 09.12.2003 */ @SuppressWarnings("resource") -public final class AutoProxyCreatorTests { +public class AutoProxyCreatorTests { @Test public void testBeanNameAutoProxyCreator() { @@ -252,6 +252,23 @@ public final class AutoProxyCreatorTests { assertEquals(2, tapc.testInterceptor.nrOfInvocations); } + @Test + public void testAutoProxyCreatorWithPackageVisibleMethod() { + StaticApplicationContext sac = new StaticApplicationContext(); + sac.registerSingleton("testAutoProxyCreator", TestAutoProxyCreator.class); + sac.registerSingleton("packageVisibleMethodToBeProxied", PackageVisibleMethod.class); + sac.refresh(); + + TestAutoProxyCreator tapc = (TestAutoProxyCreator) sac.getBean("testAutoProxyCreator"); + tapc.testInterceptor.nrOfInvocations = 0; + + PackageVisibleMethod tb = (PackageVisibleMethod) sac.getBean("packageVisibleMethodToBeProxied"); + assertTrue(AopUtils.isCglibProxy(tb)); + assertEquals(0, tapc.testInterceptor.nrOfInvocations); + tb.doSomething(); + assertEquals(1, tapc.testInterceptor.nrOfInvocations); + } + @Test public void testAutoProxyCreatorWithFactoryBean() { StaticApplicationContext sac = new StaticApplicationContext(); diff --git a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/PackageVisibleMethod.java b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/PackageVisibleMethod.java new file mode 100644 index 00000000000..cf5e5a39328 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/PackageVisibleMethod.java @@ -0,0 +1,24 @@ +/* + * Copyright 2002-2016 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.aop.framework.autoproxy; + +public class PackageVisibleMethod { + + void doSomething() { + } + +} diff --git a/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml b/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml index 08486fba19f..cfcc828592c 100644 --- a/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml +++ b/spring-context/src/test/resources/org/springframework/aop/framework/autoproxy/AdvisorAutoProxyCreatorTests-common-interceptors.xml @@ -10,37 +10,38 @@ Matches all Advisors in the factory: we don't use a prefix - - - + + nopInterceptor - + - - + + + + + + + + + + + + + - - 4 + + - - 4 + + + - +