From 1b2014a52de2cd84d554bc0acfc5d412228bf6da Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 29 Apr 2014 19:07:29 +0200 Subject: [PATCH] AbstractAutoProxyCreator and AbstractAdvisingBeanPostProcessor consistently not considering configuration callbacks and internal language interfaces as reasonable proxy interfaces Issue: SPR-11715 --- .../AbstractAdvisingBeanPostProcessor.java | 37 +---- .../aop/framework/ProxyProcessorSupport.java | 145 ++++++++++++++++++ .../autoproxy/AbstractAutoProxyCreator.java | 99 +----------- .../annotation/AsyncExecutionTests.java | 10 +- 4 files changed, 165 insertions(+), 126 deletions(-) create mode 100644 spring-aop/src/main/java/org/springframework/aop/framework/ProxyProcessorSupport.java diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java b/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java index fb59086b2a..bc79dee31e 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AbstractAdvisingBeanPostProcessor.java @@ -21,10 +21,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.springframework.aop.Advisor; import org.springframework.aop.support.AopUtils; -import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.beans.factory.config.BeanPostProcessor; -import org.springframework.core.Ordered; -import org.springframework.util.ClassUtils; /** * Base class for {@link BeanPostProcessor} implementations that apply a @@ -34,21 +31,12 @@ import org.springframework.util.ClassUtils; * @since 3.2 */ @SuppressWarnings("serial") -public abstract class AbstractAdvisingBeanPostProcessor extends ProxyConfig - implements BeanPostProcessor, BeanClassLoaderAware, Ordered { +public abstract class AbstractAdvisingBeanPostProcessor extends ProxyProcessorSupport implements BeanPostProcessor { protected Advisor advisor; protected boolean beforeExistingAdvisors = false; - private ClassLoader beanClassLoader = ClassUtils.getDefaultClassLoader(); - - /** - * This should run after all other post-processors, so that it can just add - * an advisor to existing proxies rather than double-proxy. - */ - private int order = Ordered.LOWEST_PRECEDENCE; - private final Map, Boolean> eligibleBeans = new ConcurrentHashMap, Boolean>(64); @@ -65,20 +53,6 @@ public abstract class AbstractAdvisingBeanPostProcessor extends ProxyConfig this.beforeExistingAdvisors = beforeExistingAdvisors; } - @Override - public void setBeanClassLoader(ClassLoader beanClassLoader) { - this.beanClassLoader = beanClassLoader; - } - - public void setOrder(int order) { - this.order = order; - } - - @Override - public int getOrder() { - return this.order; - } - @Override public Object postProcessBeforeInitialization(Object bean, String beanName) { @@ -107,11 +81,14 @@ public abstract class AbstractAdvisingBeanPostProcessor extends ProxyConfig } if (isEligible(bean, beanName)) { - ProxyFactory proxyFactory = new ProxyFactory(bean); - // Copy our properties (proxyTargetClass etc) inherited from ProxyConfig. + ProxyFactory proxyFactory = new ProxyFactory(); proxyFactory.copyFrom(this); + proxyFactory.setTarget(bean); + if (!proxyFactory.isProxyTargetClass()) { + evaluateProxyInterfaces(bean.getClass(), proxyFactory); + } proxyFactory.addAdvisor(this.advisor); - return proxyFactory.getProxy(this.beanClassLoader); + return proxyFactory.getProxy(getProxyClassLoader()); } // No async proxy needed. diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/ProxyProcessorSupport.java b/spring-aop/src/main/java/org/springframework/aop/framework/ProxyProcessorSupport.java new file mode 100644 index 0000000000..45b2c3dd58 --- /dev/null +++ b/spring-aop/src/main/java/org/springframework/aop/framework/ProxyProcessorSupport.java @@ -0,0 +1,145 @@ +/* + * 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. + * 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; + +import org.springframework.beans.factory.Aware; +import org.springframework.beans.factory.BeanClassLoaderAware; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.core.Ordered; +import org.springframework.util.ClassUtils; +import org.springframework.util.ObjectUtils; + +/** + * Base class with common functionality for proxy processors, in particular + * ClassLoader management and the {@link #evaluateProxyInterfaces} algorithm. + * + * @author Juergen Hoeller + * @since 4.1 + * @see AbstractAdvisingBeanPostProcessor + * @see org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator + */ +@SuppressWarnings("serial") +public class ProxyProcessorSupport extends ProxyConfig implements Ordered, BeanClassLoaderAware, AopInfrastructureBean { + + /** + * This should run after all other processors, so that it can just add + * an advisor to existing proxies rather than double-proxy. + */ + private int order = Ordered.LOWEST_PRECEDENCE; + + private ClassLoader proxyClassLoader = ClassUtils.getDefaultClassLoader(); + + private boolean classLoaderConfigured = false; + + + /** + * Set the ordering which will apply to this class's implementation + * of Ordered, used when applying multiple processors. + *

Default value is {@code Integer.MAX_VALUE}, meaning that it's non-ordered. + * @param order ordering value + */ + public void setOrder(int order) { + this.order = order; + } + + @Override + public int getOrder() { + return this.order; + } + + /** + * Set the ClassLoader to generate the proxy class in. + *

Default is the bean ClassLoader, i.e. the ClassLoader used by the + * containing BeanFactory for loading all bean classes. This can be + * overridden here for specific proxies. + */ + public void setProxyClassLoader(ClassLoader classLoader) { + this.proxyClassLoader = classLoader; + this.classLoaderConfigured = (classLoader != null); + } + + /** + * Return the configured proxy ClassLoader for this processor. + */ + protected ClassLoader getProxyClassLoader() { + return this.proxyClassLoader; + } + + @Override + public void setBeanClassLoader(ClassLoader classLoader) { + if (!this.classLoaderConfigured) { + this.proxyClassLoader = classLoader; + } + } + + + /** + * Check the interfaces on the given bean class and apply them to the ProxyFactory, + * if appropriate. + *

Calls {@link #isConfigurationCallbackInterface} and {@link #isInternalLanguageInterface} + * 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 + */ + protected void evaluateProxyInterfaces(Class beanClass, ProxyFactory proxyFactory) { + Class[] targetInterfaces = ClassUtils.getAllInterfacesForClass(beanClass, getProxyClassLoader()); + boolean hasReasonableProxyInterface = false; + for (Class ifc : targetInterfaces) { + if (!isConfigurationCallbackInterface(ifc) && !isInternalLanguageInterface(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)); + } + + /** + * Determine whether the given interface is a well-known internal language interface + * 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 an internal language interface + */ + protected boolean isInternalLanguageInterface(Class ifc) { + return ifc.getName().equals("groovy.lang.GroovyObject"); + } + +} 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 52f8e4f54a..2199bd5105 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-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. @@ -33,25 +33,19 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.Advisor; import org.springframework.aop.TargetSource; import org.springframework.aop.framework.AopInfrastructureBean; -import org.springframework.aop.framework.ProxyConfig; import org.springframework.aop.framework.ProxyFactory; +import org.springframework.aop.framework.ProxyProcessorSupport; import org.springframework.aop.framework.adapter.AdvisorAdapterRegistry; 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 @@ -94,9 +88,8 @@ import org.springframework.util.ObjectUtils; * @see DefaultAdvisorAutoProxyCreator */ @SuppressWarnings("serial") -public abstract class AbstractAutoProxyCreator extends ProxyConfig - implements SmartInstantiationAwareBeanPostProcessor, BeanClassLoaderAware, BeanFactoryAware, - Ordered, AopInfrastructureBean { +public abstract class AbstractAutoProxyCreator extends ProxyProcessorSupport + implements SmartInstantiationAwareBeanPostProcessor, BeanFactoryAware { /** * Convenience constant for subclasses: Return value for "do not proxy". @@ -115,9 +108,6 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig /** Logger available to subclasses */ protected final Log logger = LogFactory.getLog(getClass()); - /** Default value is same as non-ordered */ - private int order = Ordered.LOWEST_PRECEDENCE; - /** Default is global AdvisorAdapterRegistry */ private AdvisorAdapterRegistry advisorAdapterRegistry = GlobalAdvisorAdapterRegistry.getInstance(); @@ -134,10 +124,6 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig private TargetSourceCreator[] customTargetSourceCreators; - private ClassLoader proxyClassLoader = ClassUtils.getDefaultClassLoader(); - - private boolean classLoaderConfigured = false; - private BeanFactory beanFactory; private final Map advisedBeans = new ConcurrentHashMap(64); @@ -151,21 +137,6 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig private final Map> proxyTypes = new ConcurrentHashMap>(16); - /** - * Set the ordering which will apply to this class's implementation - * of Ordered, used when applying multiple BeanPostProcessors. - *

Default value is {@code Integer.MAX_VALUE}, meaning that it's non-ordered. - * @param order ordering value - */ - public final void setOrder(int order) { - this.order = order; - } - - @Override - public final int getOrder() { - return this.order; - } - /** * Set whether or not the proxy should be frozen, preventing advice * from being added to it once it is created. @@ -228,24 +199,6 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig this.applyCommonInterceptorsFirst = applyCommonInterceptorsFirst; } - /** - * Set the ClassLoader to generate the proxy class in. - *

Default is the bean ClassLoader, i.e. the ClassLoader used by the - * containing BeanFactory for loading all bean classes. This can be - * overridden here for specific proxies. - */ - public void setProxyClassLoader(ClassLoader classLoader) { - this.proxyClassLoader = classLoader; - this.classLoaderConfigured = (classLoader != null); - } - - @Override - public void setBeanClassLoader(ClassLoader classLoader) { - if (!this.classLoaderConfigured) { - this.proxyClassLoader = classLoader; - } - } - @Override public void setBeanFactory(BeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -466,7 +419,6 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig Class beanClass, String beanName, Object[] specificInterceptors, TargetSource targetSource) { ProxyFactory proxyFactory = new ProxyFactory(); - // Copy our properties (proxyTargetClass etc) inherited from ProxyConfig. proxyFactory.copyFrom(this); if (!proxyFactory.isProxyTargetClass()) { @@ -491,7 +443,7 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig proxyFactory.setPreFiltered(true); } - return proxyFactory.getProxy(this.proxyClassLoader); + return proxyFactory.getProxy(getProxyClassLoader()); } /** @@ -508,47 +460,6 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig 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)); - } - /** * Return whether the Advisors returned by the subclass are pre-filtered * to match the bean's target class already, allowing the ClassFilter check diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/AsyncExecutionTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/AsyncExecutionTests.java index bcc22879a0..2587439fc6 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/AsyncExecutionTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/annotation/AsyncExecutionTests.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.scheduling.annotation; +import java.io.Serializable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.HashMap; @@ -28,6 +29,7 @@ import org.junit.Test; import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.framework.autoproxy.DefaultAdvisorAutoProxyCreator; import org.springframework.aop.support.DefaultIntroductionAdvisor; +import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.ApplicationEvent; @@ -397,7 +399,7 @@ public class AsyncExecutionTests { @Async - public static class AsyncClassBean { + public static class AsyncClassBean implements Serializable, DisposableBean { public void doSomething(int i) { assertTrue(!Thread.currentThread().getName().equals(originalThreadName)); @@ -407,6 +409,10 @@ public class AsyncExecutionTests { assertTrue(!Thread.currentThread().getName().equals(originalThreadName)); return new AsyncResult(Integer.toString(i)); } + + @Override + public void destroy() { + } }