Test status quo for Bean Override singleton semantics

The tests introduced in this commit reveal the following issues in our
Bean Override support.

- If a FactoryBean signals it does not manage a singleton, the Bean
  Override support silently replaces it with a singleton.

- An attempt to override a prototype-scoped bean or a bean configured
  with a custom scope results in one of the following.

  - If the bean type of the original bean definition is a concrete
    class, an attempt will be made to invoke the default constructor
    which will either succeed with undesirable results or fail with an
    exception if the bean type does not have a default constructor.

  - If the bean type of the original bean definition is an interface or
    a FactoryBean that claims to create a bean of a certain interface
    type, an attempt will be made to instantiate the interface which
    will always fail with a BeanCreationException.

Author: sbrannen

spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java

@@ -25,6 +25,7 @@
 import org.junit.jupiter.api.Test;
 
 import org.springframework.beans.BeanWrapper;
+import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.beans.factory.config.BeanDefinition;
@@ -37,10 +38,12 @@
 import org.springframework.core.Ordered;
 import org.springframework.core.ResolvableType;
 import org.springframework.test.context.MergedContextConfiguration;
+import org.springframework.test.context.bean.override.convention.TestBean;
 import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.Assert;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.mockito.Mockito.mock;
@@ -212,6 +215,130 @@ void replaceBeanByNameWithMatchingBeanDefinitionWithExplicitSingletonScope() {
 		assertThat(context.getBean("descriptionBean")).isEqualTo("overridden");
 	}
 
+	@Test
+	void replaceBeanByNameWithMatchingBeanDefinitionForClassBasedSingletonFactoryBean() {
+		String beanName = "descriptionBean";
+		AnnotationConfigApplicationContext context = createContext(CaseByName.class);
+		RootBeanDefinition factoryBeanDefinition = new RootBeanDefinition(SingletonStringFactoryBean.class);
+		context.registerBeanDefinition(beanName, factoryBeanDefinition);
+
+		assertThatNoException().isThrownBy(context::refresh);
+		assertThat(context.isSingleton(beanName)).as("isSingleton").isTrue();
+		assertThat(context.getBean(beanName)).isEqualTo("overridden");
+	}
+
+	@Test
+	void replaceBeanByNameWithMatchingBeanDefinitionForClassBasedNonSingletonFactoryBean() {
+		String beanName = "descriptionBean";
+		AnnotationConfigApplicationContext context = createContext(CaseByName.class);
+		RootBeanDefinition factoryBeanDefinition = new RootBeanDefinition(NonSingletonStringFactoryBean.class);
+		context.registerBeanDefinition(beanName, factoryBeanDefinition);
+
+		assertThatNoException().isThrownBy(context::refresh);
+		// Even though the FactoryBean signals it does not manage a singleton,
+		// the Bean Override support currently replaces it with a singleton.
+		assertThat(context.isSingleton(beanName)).as("isSingleton").isTrue();
+		assertThat(context.getBean(beanName)).isEqualTo("overridden");
+	}
+
+	@Test
+	void replaceBeanByNameWithMatchingBeanDefinitionForInterfaceBasedSingletonFactoryBean() {
+		String beanName = "messageServiceBean";
+		AnnotationConfigApplicationContext context = createContext(MessageServiceTestCase.class);
+		RootBeanDefinition factoryBeanDefinition = new RootBeanDefinition(SingletonMessageServiceFactoryBean.class);
+		context.registerBeanDefinition(beanName, factoryBeanDefinition);
+
+		assertThatNoException().isThrownBy(context::refresh);
+		assertThat(context.isSingleton(beanName)).as("isSingleton").isTrue();
+		assertThat(context.getBean(beanName, MessageService.class).getMessage()).isEqualTo("overridden");
+	}
+
+	@Test
+	void replaceBeanByNameWithMatchingBeanDefinitionForInterfaceBasedNonSingletonFactoryBean() {
+		String beanName = "messageServiceBean";
+		AnnotationConfigApplicationContext context = createContext(MessageServiceTestCase.class);
+		RootBeanDefinition factoryBeanDefinition = new RootBeanDefinition(NonSingletonMessageServiceFactoryBean.class);
+		context.registerBeanDefinition(beanName, factoryBeanDefinition);
+
+		assertThatNoException().isThrownBy(context::refresh);
+		// Even though the FactoryBean signals it does not manage a singleton,
+		// the Bean Override support currently replaces it with a singleton.
+		assertThat(context.isSingleton(beanName)).as("isSingleton").isTrue();
+		assertThat(context.getBean(beanName, MessageService.class).getMessage()).isEqualTo("overridden");
+	}
+
+	@Test
+	void replaceBeanByNameWithMatchingBeanDefinitionWithPrototypeScope() {
+		String beanName = "descriptionBean";
+
+		AnnotationConfigApplicationContext context = createContext(CaseByName.class);
+		RootBeanDefinition definition = new RootBeanDefinition(String.class, () -> "ORIGINAL");
+		definition.setScope(BeanDefinition.SCOPE_PROTOTYPE);
+		context.registerBeanDefinition(beanName, definition);
+
+		assertThatNoException().isThrownBy(context::refresh);
+		// The Bean Override support currently creates a "dummy" BeanDefinition that
+		// retains the prototype scope of the original BeanDefinition.
+		assertThat(context.isSingleton(beanName)).as("isSingleton").isFalse();
+		assertThat(context.isPrototype(beanName)).as("isPrototype").isTrue();
+		// Since the "dummy" BeanDefinition has prototype scope, a manual singleton
+		// is not registered, and the "dummy" BeanDefinition is used to create a
+		// new java.lang.String using the default constructor, which results in an
+		// empty string instead of "overridden". In other words, the bean is not
+		// actually overridden as expected, and no exception is thrown which
+		// silently masks the issue.
+		assertThat(context.getBean(beanName)).isEqualTo("");
+	}
+
+	@Test
+	void replaceBeanByNameWithMatchingBeanDefinitionWithCustomScope() {
+		String beanName = "descriptionBean";
+		String scope = "customScope";
+
+		AnnotationConfigApplicationContext context = createContext(CaseByName.class);
+		ConfigurableListableBeanFactory beanFactory = context.getBeanFactory();
+		beanFactory.registerScope(scope, new SimpleThreadScope());
+		RootBeanDefinition definition = new RootBeanDefinition(String.class, () -> "ORIGINAL");
+		definition.setScope(scope);
+		context.registerBeanDefinition(beanName, definition);
+
+		assertThatNoException().isThrownBy(context::refresh);
+		// The Bean Override support currently creates a "dummy" BeanDefinition that
+		// retains the custom scope of the original BeanDefinition.
+		assertThat(context.isSingleton(beanName)).as("isSingleton").isFalse();
+		assertThat(context.isPrototype(beanName)).as("isPrototype").isFalse();
+		assertThat(beanFactory.getBeanDefinition(beanName).getScope()).isEqualTo(scope);
+		// Since the "dummy" BeanDefinition has a custom scope, a manual singleton
+		// is not registered, and the "dummy" BeanDefinition is used to create a
+		// new java.lang.String using the default constructor, which results in an
+		// empty string instead of "overridden". In other words, the bean is not
+		// actually overridden as expected, and no exception is thrown which
+		// silently masks the issue.
+		assertThat(context.getBean(beanName)).isEqualTo("");
+	}
+
+	@Test
+	void replaceBeanByNameWithMatchingBeanDefinitionForPrototypeScopedFactoryBean() {
+		String beanName = "messageServiceBean";
+		AnnotationConfigApplicationContext context = createContext(MessageServiceTestCase.class);
+		RootBeanDefinition factoryBeanDefinition = new RootBeanDefinition(SingletonMessageServiceFactoryBean.class);
+		factoryBeanDefinition.setScope(BeanDefinition.SCOPE_PROTOTYPE);
+		context.registerBeanDefinition(beanName, factoryBeanDefinition);
+
+		assertThatNoException().isThrownBy(context::refresh);
+		// The Bean Override support currently creates a "dummy" BeanDefinition that
+		// retains the prototype scope of the original BeanDefinition.
+		assertThat(context.isSingleton(beanName)).as("isSingleton").isFalse();
+		assertThat(context.isPrototype(beanName)).as("isPrototype").isTrue();
+		// Since the "dummy" BeanDefinition has prototype scope, a manual singleton
+		// is not registered, and the "dummy" BeanDefinition is used to create a
+		// new MessageService using the default constructor, which results in an
+		// error since MessageService is an interface.
+		assertThatExceptionOfType(BeanCreationException.class)
+				.isThrownBy(() -> context.getBean(beanName))
+				.withMessageContaining("Specified class is an interface");
+	}
+
 	@Test
 	void replaceBeanByNameWithMatchingBeanDefinitionRetainsPrimaryFallbackAndScopeProperties() {
 		AnnotationConfigApplicationContext context = createContext(CaseByName.class);
@@ -330,6 +457,63 @@ public boolean isSingleton() {
 		}
 	}
 
+	static class SingletonStringFactoryBean implements FactoryBean<String> {
+
+		@Override
+		public String getObject() {
+			return "test";
+		}
+
+		@Override
+		public Class<?> getObjectType() {
+			return String.class;
+		}
+	}
+
+	static class NonSingletonStringFactoryBean extends SingletonStringFactoryBean {
+
+		@Override
+		public boolean isSingleton() {
+			return false;
+		}
+	}
+
+	static class SingletonMessageServiceFactoryBean implements FactoryBean<MessageService> {
+
+		@Override
+		public MessageService getObject() {
+			return () -> "test";
+		}
+
+		@Override
+		public Class<?> getObjectType() {
+			return MessageService.class;
+		}
+	}
+
+	static class NonSingletonMessageServiceFactoryBean extends SingletonMessageServiceFactoryBean {
+
+		@Override
+		public boolean isSingleton() {
+			return false;
+		}
+	}
+
+	@FunctionalInterface
+	interface MessageService {
+		String getMessage();
+	}
+
+	static class MessageServiceTestCase {
+
+		@TestBean(name = "messageServiceBean")
+		MessageService messageService;
+
+		static MessageService messageService() {
+			return () -> "overridden";
+		}
+	}
+
 	static class FactoryBeanRegisteringPostProcessor implements BeanFactoryPostProcessor, Ordered {
 
 		@Override