Do not proxy test instances based on "original instance" convention

Issue: SPR-17137

Author: jhoeller

spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java

@@ -385,14 +385,17 @@ protected boolean isInfrastructureClass(Class<?> beanClass) {
 	/**
 	 * Subclasses should override this method to return {@code true} if the
 	 * given bean should not be considered for auto-proxying by this post-processor.
-	 * <p>Sometimes we need to be able to avoid this happening if it will lead to
-	 * a circular reference. This implementation returns {@code false}.
+	 * <p>Sometimes we need to be able to avoid this happening, e.g. if it will lead to
+	 * a circular reference or if the existing target instance needs to be preserved.
+	 * This implementation returns {@code false} unless the bean name indicates an
+	 * "original instance" according to {@code AutowireCapableBeanFactory} conventions.
 	 * @param beanClass the class of the bean
 	 * @param beanName the name of the bean
 	 * @return whether to skip the given bean
+	 * @see org.springframework.beans.factory.config.AutowireCapableBeanFactory#ORIGINAL_INSTANCE_SUFFIX
 	 */
 	protected boolean shouldSkip(Class<?> beanClass, String beanName) {
-		return false;
+		return AutoProxyUtils.isOriginalInstance(beanName, beanClass);
 	}
 
 	/**

spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractBeanFactoryAwareAdvisingPostProcessor.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.
@@ -64,4 +64,10 @@ protected ProxyFactory prepareProxyFactory(Object bean, String beanName) {
 		return proxyFactory;
 	}
 
+	@Override
+	protected boolean isEligible(Object bean, String beanName) {
+		return (!AutoProxyUtils.isOriginalInstance(beanName, bean.getClass()) &&
+				super.isEligible(bean, beanName));
+	}
+
 }

spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AutoProxyUtils.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.
@@ -16,10 +16,12 @@
 
 package org.springframework.aop.framework.autoproxy;
 
+import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.core.Conventions;
 import org.springframework.lang.Nullable;
+import org.springframework.util.StringUtils;
 
 /**
  * Utilities for auto-proxy aware components.
@@ -63,7 +65,9 @@ public abstract class AutoProxyUtils {
 	 * @param beanName the name of the bean
 	 * @return whether the given bean should be proxied with its target class
 	 */
-	public static boolean shouldProxyTargetClass(ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) {
+	public static boolean shouldProxyTargetClass(
+			ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) {
+
 		if (beanName != null && beanFactory.containsBeanDefinition(beanName)) {
 			BeanDefinition bd = beanFactory.getBeanDefinition(beanName);
 			return Boolean.TRUE.equals(bd.getAttribute(PRESERVE_TARGET_CLASS_ATTRIBUTE));
@@ -81,7 +85,9 @@ public static boolean shouldProxyTargetClass(ConfigurableListableBeanFactory bea
 	 * @see org.springframework.beans.factory.BeanFactory#getType(String)
 	 */
 	@Nullable
-	public static Class<?> determineTargetClass(ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) {
+	public static Class<?> determineTargetClass(
+			ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) {
+
 		if (beanName == null) {
 			return null;
 		}
@@ -102,12 +108,30 @@ public static Class<?> determineTargetClass(ConfigurableListableBeanFactory bean
 	 * @param targetClass the corresponding target class
 	 * @since 4.2.3
 	 */
-	static void exposeTargetClass(ConfigurableListableBeanFactory beanFactory, @Nullable String beanName,
-			Class<?> targetClass) {
+	static void exposeTargetClass(
+			ConfigurableListableBeanFactory beanFactory, @Nullable String beanName, Class<?> targetClass) {
 
 		if (beanName != null && beanFactory.containsBeanDefinition(beanName)) {
 			beanFactory.getMergedBeanDefinition(beanName).setAttribute(ORIGINAL_TARGET_CLASS_ATTRIBUTE, targetClass);
 		}
 	}
 
+	/**
+	 * Determine whether the given bean name indicates an "original instance"
+	 * according to {@link AutowireCapableBeanFactory#ORIGINAL_INSTANCE_SUFFIX},
+	 * skipping any proxy attempts for it.
+	 * @param beanName the name of the bean
+	 * @param beanClass the corresponding bean class
+	 * @since 5.1
+	 * @see AutowireCapableBeanFactory#ORIGINAL_INSTANCE_SUFFIX
+	 */
+	static boolean isOriginalInstance(String beanName, Class<?> beanClass) {
+		if (!StringUtils.hasLength(beanName) || beanName.length() !=
+				beanClass.getName().length() + AutowireCapableBeanFactory.ORIGINAL_INSTANCE_SUFFIX.length()) {
+			return false;
+		}
+		return (beanName.startsWith(beanClass.getName()) &&
+				beanName.endsWith(AutowireCapableBeanFactory.ORIGINAL_INSTANCE_SUFFIX));
+	}
+
 }

spring-beans/src/main/java/org/springframework/beans/factory/config/AutowireCapableBeanFactory.java

@@ -107,6 +107,18 @@ public interface AutowireCapableBeanFactory extends BeanFactory {
 	@Deprecated
 	int AUTOWIRE_AUTODETECT = 4;
 
+	/**
+	 * Suffix for the "original instance" convention when initializing an existing
+	 * bean instance: to be appended to the fully-qualified bean class name,
+	 * e.g. "com.mypackage.MyClass.ORIGINAL", in order to enforce the given instance
+	 * to be returned, i.e. no proxies etc.
+	 * @since 5.1
+	 * @see #initializeBean(Object, String)
+	 * @see #applyBeanPostProcessorsBeforeInitialization(Object, String)
+	 * @see #applyBeanPostProcessorsAfterInitialization(Object, String)
+	 */
+	String ORIGINAL_INSTANCE_SUFFIX = ".ORIGINAL";
+
 
 	//-------------------------------------------------------------------------
 	// Typical methods for creating and populating external bean instances
@@ -264,9 +276,12 @@ void autowireBeanProperties(Object existingBean, int autowireMode, boolean depen
 	 * for callbacks but not checked against the registered bean definitions.
 	 * @param existingBean the existing bean instance
 	 * @param beanName the name of the bean, to be passed to it if necessary
-	 * (only passed to {@link BeanPostProcessor BeanPostProcessors})
+	 * (only passed to {@link BeanPostProcessor BeanPostProcessors};
+	 * can follow the {@link #ORIGINAL_INSTANCE_SUFFIX} convention in order to
+	 * enforce the given instance to be returned, i.e. no proxies etc)
 	 * @return the bean instance to use, either the original or a wrapped one
 	 * @throws BeansException if the initialization failed
+	 * @see #ORIGINAL_INSTANCE_SUFFIX
 	 */
 	Object initializeBean(Object existingBean, String beanName) throws BeansException;
 
@@ -275,10 +290,13 @@ void autowireBeanProperties(Object existingBean, int autowireMode, boolean depen
 	 * instance, invoking their {@code postProcessBeforeInitialization} methods.
 	 * The returned bean instance may be a wrapper around the original.
 	 * @param existingBean the new bean instance
-	 * @param beanName the name of the bean
+	 * (only passed to {@link BeanPostProcessor BeanPostProcessors};
+	 * can follow the {@link #ORIGINAL_INSTANCE_SUFFIX} convention in order to
+	 * enforce the given instance to be returned, i.e. no proxies etc)
 	 * @return the bean instance to use, either the original or a wrapped one
 	 * @throws BeansException if any post-processing failed
 	 * @see BeanPostProcessor#postProcessBeforeInitialization
+	 * @see #ORIGINAL_INSTANCE_SUFFIX
 	 */
 	Object applyBeanPostProcessorsBeforeInitialization(Object existingBean, String beanName)
 			throws BeansException;
@@ -288,10 +306,13 @@ Object applyBeanPostProcessorsBeforeInitialization(Object existingBean, String b
 	 * instance, invoking their {@code postProcessAfterInitialization} methods.
 	 * The returned bean instance may be a wrapper around the original.
 	 * @param existingBean the new bean instance
-	 * @param beanName the name of the bean
+	 * (only passed to {@link BeanPostProcessor BeanPostProcessors};
+	 * can follow the {@link #ORIGINAL_INSTANCE_SUFFIX} convention in order to
+	 * enforce the given instance to be returned, i.e. no proxies etc)
 	 * @return the bean instance to use, either the original or a wrapped one
 	 * @throws BeansException if any post-processing failed
 	 * @see BeanPostProcessor#postProcessAfterInitialization
+	 * @see #ORIGINAL_INSTANCE_SUFFIX
 	 */
 	Object applyBeanPostProcessorsAfterInitialization(Object existingBean, String beanName)
 			throws BeansException;

spring-test/src/main/java/org/springframework/test/context/support/DependencyInjectionTestExecutionListener.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.
@@ -49,7 +49,7 @@ public class DependencyInjectionTestExecutionListener extends AbstractTestExecut
 	 * <p>Permissible values include {@link Boolean#TRUE} and {@link Boolean#FALSE}.
 	 */
 	public static final String REINJECT_DEPENDENCIES_ATTRIBUTE = Conventions.getQualifiedAttributeName(
-		DependencyInjectionTestExecutionListener.class, "reinjectDependencies");
+			DependencyInjectionTestExecutionListener.class, "reinjectDependencies");
 
 	private static final Log logger = LogFactory.getLog(DependencyInjectionTestExecutionListener.class);
 
@@ -76,7 +76,7 @@ public final int getOrder() {
 	 * from the test context, regardless of its value.
 	 */
 	@Override
-	public void prepareTestInstance(final TestContext testContext) throws Exception {
+	public void prepareTestInstance(TestContext testContext) throws Exception {
 		if (logger.isDebugEnabled()) {
 			logger.debug("Performing dependency injection for test context [" + testContext + "].");
 		}
@@ -91,7 +91,7 @@ public void prepareTestInstance(final TestContext testContext) throws Exception
 	 * otherwise, this method will have no effect.
 	 */
 	@Override
-	public void beforeTestMethod(final TestContext testContext) throws Exception {
+	public void beforeTestMethod(TestContext testContext) throws Exception {
 		if (Boolean.TRUE.equals(testContext.getAttribute(REINJECT_DEPENDENCIES_ATTRIBUTE))) {
 			if (logger.isDebugEnabled()) {
 				logger.debug("Reinjecting dependencies for test context [" + testContext + "].");
@@ -112,11 +112,12 @@ public void beforeTestMethod(final TestContext testContext) throws Exception {
 	 * @see #prepareTestInstance(TestContext)
 	 * @see #beforeTestMethod(TestContext)
 	 */
-	protected void injectDependencies(final TestContext testContext) throws Exception {
+	protected void injectDependencies(TestContext testContext) throws Exception {
 		Object bean = testContext.getTestInstance();
+		Class<?> clazz = testContext.getTestClass();
 		AutowireCapableBeanFactory beanFactory = testContext.getApplicationContext().getAutowireCapableBeanFactory();
 		beanFactory.autowireBeanProperties(bean, AutowireCapableBeanFactory.AUTOWIRE_NO, false);
-		beanFactory.initializeBean(bean, testContext.getTestClass().getName());
+		beanFactory.initializeBean(bean, clazz.getName() + AutowireCapableBeanFactory.ORIGINAL_INSTANCE_SUFFIX);
 		testContext.removeAttribute(REINJECT_DEPENDENCIES_ATTRIBUTE);
 	}
 

spring-test/src/test/java/org/springframework/test/context/groovy/GroovySpringContextTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 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.
@@ -43,13 +43,6 @@
 @ContextConfiguration("context.groovy")
 public class GroovySpringContextTests implements BeanNameAware, InitializingBean {
 
-	private boolean beanInitialized = false;
-
-	private String beanName = "replace me with [" + getClass().getName() + "]";
-
-	@Autowired
-	private ApplicationContext applicationContext;
-
 	private Employee employee;
 
 	@Autowired
@@ -63,60 +56,68 @@ public class GroovySpringContextTests implements BeanNameAware, InitializingBean
 
 	protected String bar;
 
+	@Autowired
+	private ApplicationContext applicationContext;
+
+	private String beanName;
+
+	private boolean beanInitialized = false;
+
 
 	@Autowired
-	protected final void setEmployee(final Employee employee) {
+	protected void setEmployee(Employee employee) {
 		this.employee = employee;
 	}
 
 	@Resource
-	protected final void setBar(final String bar) {
+	protected void setBar(String bar) {
 		this.bar = bar;
 	}
 
 	@Override
-	public final void setBeanName(final String beanName) {
+	public void setBeanName(String beanName) {
 		this.beanName = beanName;
 	}
 
 	@Override
-	public final void afterPropertiesSet() throws Exception {
+	public void afterPropertiesSet() {
 		this.beanInitialized = true;
 	}
 
+
 	@Test
-	public final void verifyBeanInitialized() {
-		assertTrue("This test bean should have been initialized due to InitializingBean semantics.",
-			this.beanInitialized);
+	public void verifyBeanNameSet() {
+		assertTrue("The bean name of this test instance should have been set to the fully qualified class name " +
+				"due to BeanNameAware semantics.", this.beanName.startsWith(getClass().getName()));
 	}
 
 	@Test
-	public final void verifyBeanNameSet() {
-		assertEquals("The bean name of this test instance should have been set to the fully qualified class name "
-				+ "due to BeanNameAware semantics.", getClass().getName(), this.beanName);
+	public void verifyBeanInitialized() {
+		assertTrue("This test bean should have been initialized due to InitializingBean semantics.",
+				this.beanInitialized);
 	}
 
 	@Test
-	public final void verifyAnnotationAutowiredFields() {
+	public void verifyAnnotationAutowiredFields() {
 		assertNull("The nonrequiredLong property should NOT have been autowired.", this.nonrequiredLong);
 		assertNotNull("The application context should have been autowired.", this.applicationContext);
 		assertNotNull("The pet field should have been autowired.", this.pet);
 		assertEquals("Dogbert", this.pet.getName());
 	}
 
 	@Test
-	public final void verifyAnnotationAutowiredMethods() {
+	public void verifyAnnotationAutowiredMethods() {
 		assertNotNull("The employee setter method should have been autowired.", this.employee);
 		assertEquals("Dilbert", this.employee.getName());
 	}
 
 	@Test
-	public final void verifyResourceAnnotationWiredFields() {
+	public void verifyResourceAnnotationWiredFields() {
 		assertEquals("The foo field should have been wired via @Resource.", "Foo", this.foo);
 	}
 
 	@Test
-	public final void verifyResourceAnnotationWiredMethods() {
+	public void verifyResourceAnnotationWiredMethods() {
 		assertEquals("The bar method should have been wired via @Resource.", "Bar", this.bar);
 	}