From 81d89f478aa2441ecfa90907ba21b7cc20152059 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 28 Oct 2024 11:42:56 +0100 Subject: [PATCH] Relax singleton enforcement for Bean Overrides in the TestContext framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In gh-33602, we introduced strict singleton enforcement for bean overrides -- for example, for @⁠MockitoBean, @⁠TestBean, etc. However, the use of BeanFactory#isSingleton(beanName) can result in a BeanCreationException for certain beans, such as a Spring Data JPA FactoryBean for a JpaRepository. In light of that, this commit relaxes the singleton enforcement in BeanOverrideBeanFactoryPostProcessor by only checking the result of BeanDefinition#isSingleton() for existing bean definitions. This commit also updates the Javadoc and reference documentation to reflect the status quo. See gh-33602 Closes gh-33800 --- .../annotation-mockitobean.adoc | 13 ++++++++++-- .../annotation-testbean.adoc | 13 +++++++++--- .../bean-overriding.adoc | 16 ++++++++++++--- .../BeanOverrideBeanFactoryPostProcessor.java | 15 ++++++++++---- .../bean/override/convention/TestBean.java | 5 ++++- .../bean/override/mockito/MockitoBean.java | 7 +++++-- .../bean/override/mockito/MockitoSpyBean.java | 7 +++++-- ...OverrideBeanFactoryPostProcessorTests.java | 20 +++++++++---------- 8 files changed, 69 insertions(+), 27 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-mockitobean.adoc b/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-mockitobean.adoc index 8c87c43c77..2056f6454f 100644 --- a/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-mockitobean.adoc +++ b/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-mockitobean.adoc @@ -39,8 +39,17 @@ xref:testing/testcontext-framework/bean-overriding.adoc#testcontext-bean-overrid and the original instance is wrapped in a Mockito spy. This strategy requires that exactly one candidate bean exists. -NOTE: Only _singleton_ beans can be overridden. Any attempt to override a non-singleton -bean will result in an exception. +[TIP] +==== +Only _singleton_ beans can be overridden. Any attempt to override a non-singleton bean +will result in an exception. + +When using `@MockitoBean` to mock a bean created by a `FactoryBean`, the `FactoryBean` +will be replaced with a singleton mock of the type of object created by the `FactoryBean`. + +When using `@MockitoSpyBean` to create a spy for a `FactoryBean`, a spy will be created +for the object created by the `FactoryBean`, not for the `FactoryBean` itself. +==== The following example shows how to use the default behavior of the `@MockitoBean` annotation: diff --git a/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-testbean.adoc b/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-testbean.adoc index cf76494332..6da8abed74 100644 --- a/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-testbean.adoc +++ b/framework-docs/modules/ROOT/pages/testing/annotations/integration-spring/annotation-testbean.adoc @@ -82,7 +82,7 @@ Java:: <2> The result of this static method will be used as the instance and injected into the field. ====== -[NOTE] +[TIP] ==== Spring searches for the factory method to invoke in the test class, in the test class hierarchy, and in the enclosing class hierarchy for a `@Nested` test class. @@ -92,5 +92,12 @@ fully-qualified method name following the syntax `#< – for example, `methodName = "org.example.TestUtils#createCustomService"`. ==== -NOTE: Only _singleton_ beans can be overridden. Any attempt to override a non-singleton -bean will result in an exception. +[TIP] +==== +Only _singleton_ beans can be overridden. Any attempt to override a non-singleton bean +will result in an exception. + +When overriding a bean created by a `FactoryBean`, the `FactoryBean` will be replaced +with a singleton bean corresponding to the value returned from the `@TestBean` factory +method. +==== diff --git a/framework-docs/modules/ROOT/pages/testing/testcontext-framework/bean-overriding.adoc b/framework-docs/modules/ROOT/pages/testing/testcontext-framework/bean-overriding.adoc index 5af9e033e0..b35ee1dd3a 100644 --- a/framework-docs/modules/ROOT/pages/testing/testcontext-framework/bean-overriding.adoc +++ b/framework-docs/modules/ROOT/pages/testing/testcontext-framework/bean-overriding.adoc @@ -57,6 +57,19 @@ defined by the corresponding `BeanOverrideStrategy`: `WRAP`:: Retrieves the original bean and wraps it. +[TIP] +==== +Only _singleton_ beans can be overridden. Any attempt to override a non-singleton bean +will result in an exception. + +When replacing a bean created by a `FactoryBean`, the `FactoryBean` itself will be +replaced with a singleton bean corresponding to bean override instance created by the +applicable `BeanOverrideHandler`. + +When wrapping a bean created by a `FactoryBean`, the object created by the `FactoryBean` +will be wrapped, not the `FactoryBean` itself. +==== + [NOTE] ==== In contrast to Spring's autowiring mechanism (for example, resolution of an `@Autowired` @@ -71,6 +84,3 @@ Alternatively, the user can directly provide the bean name in the custom annotat `BeanOverrideProcessor` implementations may also internally compute a bean name based on a convention or some other method. ==== - -NOTE: Only _singleton_ beans can be overridden. Any attempt to override a non-singleton -bean will result in an exception. diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java index f146060800..ba86ecd1fc 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideBeanFactoryPostProcessor.java @@ -197,8 +197,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, // Now we have an instance (the override) that we can manually register as a singleton. // // However, we need to remove any existing singleton instance -- for example, a - // manually registered singleton or a singleton that was registered as a side effect - // of the isSingleton() check in validateBeanDefinition(). + // manually registered singleton. // // As a bonus, by manually registering a singleton during "AOT processing", we allow // GenericApplicationContext's preDetermineBeanType() method to transparently register @@ -334,10 +333,18 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor, /** * Validate that the {@link BeanDefinition} for the supplied bean name is suitable * for being replaced by a bean override. + *

If there is no registered {@code BeanDefinition} for the supplied bean name, + * no validation is performed. */ private static void validateBeanDefinition(ConfigurableListableBeanFactory beanFactory, String beanName) { - Assert.state(beanFactory.isSingleton(beanName), - () -> "Unable to override bean '" + beanName + "': only singleton beans can be overridden."); + // Due to https://github.com/spring-projects/spring-framework/issues/33800, we do NOT invoke + // beanFactory.isSingleton(beanName), since doing so can result in a BeanCreationException for + // certain beans -- for example, a Spring Data FactoryBean for a JpaRepository. + if (beanFactory.containsBeanDefinition(beanName)) { + BeanDefinition beanDefinition = beanFactory.getBeanDefinition(beanName); + Assert.state(beanDefinition.isSingleton(), + () -> "Unable to override bean '" + beanName + "': only singleton beans can be overridden."); + } } private static void destroySingleton(ConfigurableListableBeanFactory beanFactory, String beanName) { diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java index 064075ab1f..5c22a4b0c3 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java @@ -98,7 +98,10 @@ import org.springframework.test.context.bean.override.BeanOverride; * } * *

NOTE: Only singleton beans can be overridden. - * Any attempt to override a non-singleton bean will result in an exception. + * Any attempt to override a non-singleton bean will result in an exception. When + * overriding a bean created by a {@link org.springframework.beans.factory.FactoryBean + * FactoryBean}, the {@code FactoryBean} will be replaced with a singleton bean + * corresponding to the value returned from the {@code @TestBean} factory method. * * @author Simon Baslé * @author Stephane Nicoll diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBean.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBean.java index e3cf968b27..12dcf0a151 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBean.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoBean.java @@ -52,8 +52,11 @@ import org.springframework.test.context.bean.override.BeanOverride; * registered directly}) will not be found, and a mocked bean will be added to * the context alongside the existing dependency. * - *

NOTE: Only singleton beans can be overridden. - * Any attempt to mock a non-singleton bean will result in an exception. + *

NOTE: Only singleton beans can be mocked. + * Any attempt to mock a non-singleton bean will result in an exception. When + * mocking a bean created by a {@link org.springframework.beans.factory.FactoryBean + * FactoryBean}, the {@code FactoryBean} will be replaced with a singleton mock + * of the type of object created by the {@code FactoryBean}. * * @author Simon Baslé * @author Sam Brannen diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBean.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBean.java index 9732f995bb..359579d639 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBean.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBean.java @@ -43,8 +43,11 @@ import org.springframework.test.context.bean.override.BeanOverride; * {@link org.springframework.beans.factory.config.ConfigurableListableBeanFactory#registerResolvableDependency(Class, Object) * registered directly} as resolvable dependencies. * - *

NOTE: Only singleton beans can be spied. - * Any attempt to create a spy for a non-singleton bean will result in an exception. + *

NOTE: Only singleton beans can be spied. Any attempt + * to create a spy for a non-singleton bean will result in an exception. When + * creating a spy for a {@link org.springframework.beans.factory.FactoryBean FactoryBean}, + * a spy will be created for the object created by the {@code FactoryBean}, not + * for the {@code FactoryBean} itself. * * @author Simon Baslé * @author Sam Brannen 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 4b14b200e4..6f52030cd8 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 @@ -237,16 +237,16 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThat(context.getBean(beanName)).isEqualTo("overridden"); } - @Test - void replaceBeanByNameWithMatchingBeanDefinitionForClassBasedNonSingletonFactoryBeanFails() { + @Test // gh-33800 + void replaceBeanByNameWithMatchingBeanDefinitionForClassBasedNonSingletonFactoryBean() { String beanName = "descriptionBean"; AnnotationConfigApplicationContext context = createContext(CaseByName.class); RootBeanDefinition factoryBeanDefinition = new RootBeanDefinition(NonSingletonStringFactoryBean.class); context.registerBeanDefinition(beanName, factoryBeanDefinition); - assertThatIllegalStateException() - .isThrownBy(context::refresh) - .withMessage("Unable to override bean 'descriptionBean': only singleton beans can be overridden."); + assertThatNoException().isThrownBy(context::refresh); + assertThat(context.isSingleton(beanName)).as("isSingleton").isTrue(); + assertThat(context.getBean(beanName)).isEqualTo("overridden"); } @Test @@ -261,16 +261,16 @@ class BeanOverrideBeanFactoryPostProcessorTests { assertThat(context.getBean(beanName, MessageService.class).getMessage()).isEqualTo("overridden"); } - @Test - void replaceBeanByNameWithMatchingBeanDefinitionForInterfaceBasedNonSingletonFactoryBeanFails() { + @Test // gh-33800 + void replaceBeanByNameWithMatchingBeanDefinitionForInterfaceBasedNonSingletonFactoryBean() { String beanName = "messageServiceBean"; AnnotationConfigApplicationContext context = createContext(MessageServiceTestCase.class); RootBeanDefinition factoryBeanDefinition = new RootBeanDefinition(NonSingletonMessageServiceFactoryBean.class); context.registerBeanDefinition(beanName, factoryBeanDefinition); - assertThatIllegalStateException() - .isThrownBy(context::refresh) - .withMessage("Unable to override bean 'messageServiceBean': only singleton beans can be overridden."); + assertThatNoException().isThrownBy(context::refresh); + assertThat(context.isSingleton(beanName)).as("isSingleton").isTrue(); + assertThat(context.getBean(beanName, MessageService.class).getMessage()).isEqualTo("overridden"); } @Test