AbstractAutoProxyCreator and AbstractAdvisingBeanPostProcessor consistently not considering configuration callbacks and internal language interfaces as reasonable proxy interfaces

Issue: SPR-11715

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
+public abstract class AbstractAdvisingBeanPostProcessor extends ProxyProcessorSupport implements BeanPostProcessor {
-		implements BeanPostProcessor, BeanClassLoaderAware, Ordered {
 
 	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<Class<?>, Boolean> eligibleBeans = new ConcurrentHashMap<Class<?>, 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);
+			ProxyFactory proxyFactory = new ProxyFactory();
-			// Copy our properties (proxyTargetClass etc) inherited from ProxyConfig.
 			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.

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.
+	 * <p>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.
+	 * <p>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.
+	 * <p>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.
+	 * <p>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.
+	 * <p>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");
+	}
+
+}

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
+public abstract class AbstractAutoProxyCreator extends ProxyProcessorSupport
-		implements SmartInstantiationAwareBeanPostProcessor, BeanClassLoaderAware, BeanFactoryAware,
+		implements SmartInstantiationAwareBeanPostProcessor, BeanFactoryAware {
-		Ordered, AopInfrastructureBean {
 
 	/**
 	 * 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<Object, Boolean> advisedBeans = new ConcurrentHashMap<Object, Boolean>(64);
@@ -151,21 +137,6 @@ public abstract class AbstractAutoProxyCreator extends ProxyConfig
 	private final Map<Object, Class<?>> proxyTypes = new ConcurrentHashMap<Object, Class<?>>(16);
 
 
-	/**
-	 * Set the ordering which will apply to this class's implementation
-	 * of Ordered, used when applying multiple BeanPostProcessors.
-	 * <p>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.
-	 * <p>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.
-	 * <p>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.
-	 * <p>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

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<String>(Integer.toString(i));
 		}
+
+		@Override
+		public void destroy() {
+		}
 	}