From 9c746905bc38cc78aafc3f4b816e60663b8faca1 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:49:17 +0200 Subject: [PATCH] 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. --- ...OverrideBeanFactoryPostProcessorTests.java | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java index d6fa2383cf0d..abbd216bce4a 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessorTests.java +++ b/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 { + + @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 { + + @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