From 949338009bd38ccc3a42baa1b40adde70e3b96e8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 12 Feb 2014 00:01:20 +0100 Subject: [PATCH] Ignore container callback and marker interfaces for auto-proxy decisions Issue: SPR-11416 --- .../autoproxy/AbstractAutoProxyCreator.java | 68 +++++++++++--- .../aop/framework/CglibProxyTests.java | 9 +- .../autoproxy/AutoProxyCreatorTests.java | 92 ++++++++++++++++++- 3 files changed, 149 insertions(+), 20 deletions(-) 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 806b34b526..52f8e4f54a 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 @@ -40,14 +40,18 @@ import org.springframework.aop.framework.adapter.GlobalAdvisorAdapterRegistry; import org.springframework.aop.target.SingletonTargetSource; import org.springframework.beans.BeansException; import org.springframework.beans.PropertyValues; +import org.springframework.beans.factory.Aware; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.config.ConfigurableBeanFactory; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.SmartInstantiationAwareBeanPostProcessor; import org.springframework.core.Ordered; import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; /** * {@link org.springframework.beans.factory.config.BeanPostProcessor} implementation @@ -465,12 +469,12 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig // Copy our properties (proxyTargetClass etc) inherited from ProxyConfig. proxyFactory.copyFrom(this); - if (!shouldProxyTargetClass(beanClass, beanName)) { - // Must allow for introductions; can't just set interfaces to - // the target's interfaces only. - Class[] targetInterfaces = ClassUtils.getAllInterfacesForClass(beanClass, this.proxyClassLoader); - for (Class targetInterface : targetInterfaces) { - proxyFactory.addInterface(targetInterface); + if (!proxyFactory.isProxyTargetClass()) { + if (shouldProxyTargetClass(beanClass, beanName)) { + proxyFactory.setProxyTargetClass(true); + } + else { + evaluateProxyInterfaces(beanClass, proxyFactory); } } @@ -491,10 +495,8 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig } /** - * Determine whether the given bean should be proxied with its target - * class rather than its interfaces. Checks the - * {@link #setProxyTargetClass "proxyTargetClass" setting} as well as the - * {@link AutoProxyUtils#PRESERVE_TARGET_CLASS_ATTRIBUTE "preserveTargetClass" attribute} + * Determine whether the given bean should be proxied with its target class rather than its interfaces. + *

Checks the {@link AutoProxyUtils#PRESERVE_TARGET_CLASS_ATTRIBUTE "preserveTargetClass" attribute} * of the corresponding bean definition. * @param beanClass the class of the bean * @param beanName the name of the bean @@ -502,9 +504,49 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig * @see AutoProxyUtils#shouldProxyTargetClass */ protected boolean shouldProxyTargetClass(Class beanClass, String beanName) { - return (isProxyTargetClass() || - (this.beanFactory instanceof ConfigurableListableBeanFactory && - AutoProxyUtils.shouldProxyTargetClass((ConfigurableListableBeanFactory) this.beanFactory, beanName))); + return (this.beanFactory instanceof ConfigurableListableBeanFactory && + AutoProxyUtils.shouldProxyTargetClass((ConfigurableListableBeanFactory) this.beanFactory, beanName)); + } + + /** + * Check the interfaces on the given bean class and apply them to the ProxyFactory, + * if appropriate. + *

Calls {@link #isConfigurationCallbackInterface} to filter for reasonable + * proxy interfaces, falling back to a target-class proxy otherwise. + * @param beanClass the class of the bean + * @param proxyFactory the ProxyFactory for the bean + */ + private void evaluateProxyInterfaces(Class beanClass, ProxyFactory proxyFactory) { + Class[] targetInterfaces = ClassUtils.getAllInterfacesForClass(beanClass, this.proxyClassLoader); + boolean hasReasonableProxyInterface = false; + for (Class ifc : targetInterfaces) { + if (!isConfigurationCallbackInterface(ifc) && ifc.getMethods().length > 0) { + hasReasonableProxyInterface = true; + break; + } + } + if (hasReasonableProxyInterface) { + // Must allow for introductions; can't just set interfaces to the target's interfaces only. + for (Class ifc : targetInterfaces) { + proxyFactory.addInterface(ifc); + } + } + else { + proxyFactory.setProxyTargetClass(true); + } + } + + /** + * Determine whether the given interface is just a container callback and + * therefore not to be considered as a reasonable proxy interface. + *

If no reasonable proxy interface is found for a given bean, it will get + * proxied with its full target class, assuming that as the user's intention. + * @param ifc the interface to check + * @return whether the given interface is just a container callback + */ + protected boolean isConfigurationCallbackInterface(Class ifc) { + return (ifc.equals(InitializingBean.class) || ifc.equals(DisposableBean.class) || + ObjectUtils.containsElement(ifc.getInterfaces(), Aware.class)); } /** diff --git a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java index ebc0d355dc..8ef8305d9e 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java @@ -158,7 +158,6 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri CglibTestBean proxy = (CglibTestBean) aop.getProxy(); assertNotNull("Proxy should not be null", proxy); assertEquals("Constructor overrode the value of name", "Rob Harrop", proxy.getName()); - } @Test @@ -356,6 +355,13 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri assertEquals(1, advice.getCalls("add")); } + @Test + public void testProxyTargetClassInCaseOfNoInterfaces() throws Exception { + ProxyFactory proxyFactory = new ProxyFactory(new MyBean()); + MyBean proxy = (MyBean) proxyFactory.getProxy(); + assertEquals(4, proxy.add(1, 3)); + } + public static class MyBean { @@ -462,5 +468,4 @@ class UnsupportedInterceptor implements MethodInterceptor { public Object invoke(MethodInvocation mi) throws Throwable { throw new UnsupportedOperationException(mi.getMethod().getName()); } - } 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 d9a838ee2a..36dad8856b 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-2013 the original author or authors. + * Copyright 2002-2014 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,6 +16,7 @@ package org.springframework.aop.framework.autoproxy; +import java.io.Serializable; import java.lang.reflect.Proxy; import org.aopalliance.intercept.MethodInterceptor; @@ -24,17 +25,23 @@ import org.junit.Test; import org.springframework.aop.TargetSource; import org.springframework.aop.support.AopUtils; -import org.springframework.tests.sample.beans.ITestBean; -import org.springframework.tests.sample.beans.IndexedTestBean; import org.springframework.beans.MutablePropertyValues; -import org.springframework.tests.sample.beans.TestBean; -import org.springframework.tests.sample.beans.factory.DummyFactory; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.config.BeanDefinitionHolder; import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.context.MessageSource; import org.springframework.context.support.StaticApplicationContext; import org.springframework.context.support.StaticMessageSource; +import org.springframework.tests.sample.beans.ITestBean; +import org.springframework.tests.sample.beans.IndexedTestBean; +import org.springframework.tests.sample.beans.TestBean; +import org.springframework.tests.sample.beans.factory.DummyFactory; import static org.junit.Assert.*; @@ -133,16 +140,23 @@ public final class AutoProxyCreatorTests { public void testCustomAutoProxyCreator() { StaticApplicationContext sac = new StaticApplicationContext(); sac.registerSingleton("testAutoProxyCreator", TestAutoProxyCreator.class); + sac.registerSingleton("noInterfaces", NoInterfaces.class); + sac.registerSingleton("containerCallbackInterfacesOnly", ContainerCallbackInterfacesOnly.class); sac.registerSingleton("singletonNoInterceptor", TestBean.class); sac.registerSingleton("singletonToBeProxied", TestBean.class); sac.registerPrototype("prototypeToBeProxied", TestBean.class); sac.refresh(); MessageSource messageSource = (MessageSource) sac.getBean("messageSource"); + NoInterfaces noInterfaces = (NoInterfaces) sac.getBean("noInterfaces"); + ContainerCallbackInterfacesOnly containerCallbackInterfacesOnly = + (ContainerCallbackInterfacesOnly) sac.getBean("containerCallbackInterfacesOnly"); ITestBean singletonNoInterceptor = (ITestBean) sac.getBean("singletonNoInterceptor"); ITestBean singletonToBeProxied = (ITestBean) sac.getBean("singletonToBeProxied"); ITestBean prototypeToBeProxied = (ITestBean) sac.getBean("prototypeToBeProxied"); assertFalse(AopUtils.isCglibProxy(messageSource)); + assertTrue(AopUtils.isCglibProxy(noInterfaces)); + assertTrue(AopUtils.isCglibProxy(containerCallbackInterfacesOnly)); assertTrue(AopUtils.isCglibProxy(singletonNoInterceptor)); assertTrue(AopUtils.isCglibProxy(singletonToBeProxied)); assertTrue(AopUtils.isCglibProxy(prototypeToBeProxied)); @@ -157,6 +171,41 @@ public final class AutoProxyCreatorTests { assertEquals(2, tapc.testInterceptor.nrOfInvocations); } + @Test + public void testAutoProxyCreatorWithFallbackToTargetClass() { + StaticApplicationContext sac = new StaticApplicationContext(); + sac.registerSingleton("testAutoProxyCreator", FallbackTestAutoProxyCreator.class); + sac.registerSingleton("noInterfaces", NoInterfaces.class); + sac.registerSingleton("containerCallbackInterfacesOnly", ContainerCallbackInterfacesOnly.class); + sac.registerSingleton("singletonNoInterceptor", TestBean.class); + sac.registerSingleton("singletonToBeProxied", TestBean.class); + sac.registerPrototype("prototypeToBeProxied", TestBean.class); + sac.refresh(); + + MessageSource messageSource = (MessageSource) sac.getBean("messageSource"); + NoInterfaces noInterfaces = (NoInterfaces) sac.getBean("noInterfaces"); + ContainerCallbackInterfacesOnly containerCallbackInterfacesOnly = + (ContainerCallbackInterfacesOnly) sac.getBean("containerCallbackInterfacesOnly"); + ITestBean singletonNoInterceptor = (ITestBean) sac.getBean("singletonNoInterceptor"); + ITestBean singletonToBeProxied = (ITestBean) sac.getBean("singletonToBeProxied"); + ITestBean prototypeToBeProxied = (ITestBean) sac.getBean("prototypeToBeProxied"); + assertFalse(AopUtils.isCglibProxy(messageSource)); + assertTrue(AopUtils.isCglibProxy(noInterfaces)); + assertTrue(AopUtils.isCglibProxy(containerCallbackInterfacesOnly)); + assertFalse(AopUtils.isCglibProxy(singletonNoInterceptor)); + assertFalse(AopUtils.isCglibProxy(singletonToBeProxied)); + assertFalse(AopUtils.isCglibProxy(prototypeToBeProxied)); + + TestAutoProxyCreator tapc = (TestAutoProxyCreator) sac.getBean("testAutoProxyCreator"); + assertEquals(0, tapc.testInterceptor.nrOfInvocations); + singletonNoInterceptor.getName(); + assertEquals(0, tapc.testInterceptor.nrOfInvocations); + singletonToBeProxied.getAge(); + assertEquals(1, tapc.testInterceptor.nrOfInvocations); + prototypeToBeProxied.getSpouse(); + assertEquals(2, tapc.testInterceptor.nrOfInvocations); + } + @Test public void testAutoProxyCreatorWithFactoryBean() { StaticApplicationContext sac = new StaticApplicationContext(); @@ -303,6 +352,14 @@ public final class AutoProxyCreatorTests { } + public static class FallbackTestAutoProxyCreator extends TestAutoProxyCreator { + + public FallbackTestAutoProxyCreator() { + setProxyTargetClass(false); + } + } + + /** * Interceptor that counts the number of non-finalize method calls. */ @@ -319,4 +376,29 @@ public final class AutoProxyCreatorTests { } } + + public static class NoInterfaces { + } + + + public static class ContainerCallbackInterfacesOnly // as well as an empty marker interface + implements BeanFactoryAware, ApplicationContextAware, InitializingBean, DisposableBean, Serializable { + + @Override + public void setBeanFactory(BeanFactory beanFactory) { + } + + @Override + public void setApplicationContext(ApplicationContext applicationContext) { + } + + @Override + public void afterPropertiesSet() { + } + + @Override + public void destroy() { + } + } + }