From a21c557101ea3454c5fafc2f46ac55ab4d6653f1 Mon Sep 17 00:00:00 2001
From: Sam Brannen <104798+sbrannen@users.noreply.github.com>
Date: Tue, 1 Oct 2024 12:20:15 +0200
Subject: [PATCH] Do not replace existing Bean Override definition with
 pseudo-definition

Prior to this commit, BeanOverrideBeanFactoryPostProcessor replaced
existing bean definitions with "pseudo" bean definitions; however, that
is unnecessary. An existing BeanDefinition is suitable as-is and does
not need to be replaced with a pseudo/fake definition.

To address that, this commit reregisters an existing bean definition
and only registers a new bean definition when creating a bean
definition for a nonexistent bean.

As a side effect, we no longer need to copy primary and fallback flags.

Closes gh-33627
---
 .../BeanOverrideBeanFactoryPostProcessor.java | 25 +++++++------------
 ...OverrideBeanFactoryPostProcessorTests.java |  9 ++++---
 2 files changed, 14 insertions(+), 20 deletions(-)

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 141be2976b..a257a07aba 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
@@ -148,14 +148,17 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 		// Process existing bean definition.
 		if (existingBeanDefinition != null) {
 			validateBeanDefinition(beanFactory, beanName);
-			copyBeanDefinitionProperties(existingBeanDefinition, pseudoBeanDefinition);
+			// Since validation may have registered a singleton as a side effect -- for example,
+			// for a FactoryBean -- we need to remove the bean definition (which removes the
+			// singleton as a side effect) and re-register the bean definition.
 			registry.removeBeanDefinition(beanName);
+			registry.registerBeanDefinition(beanName, existingBeanDefinition);
+		}
+		else {
+			// There was no existing bean definition, so we register the pseudo bean definition
+			// to ensure that a bean definition exists for the given bean name.
+			registry.registerBeanDefinition(beanName, pseudoBeanDefinition);
 		}
-
-		// At this point, we either removed an existing bean definition above, or
-		// there was no bean definition to begin with. So, we register the pseudo bean
-		// definition to ensure that a bean definition exists for the given bean name.
-		registry.registerBeanDefinition(beanName, pseudoBeanDefinition);
 
 		Object override = overrideMetadata.createOverride(beanName, existingBeanDefinition, null);
 		overrideMetadata.track(override, beanFactory);
@@ -294,16 +297,6 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 				() -> "Unable to override bean '" + beanName + "': only singleton beans can be overridden.");
 	}
 
-	/**
-	 * Copy the following properties of the source {@link BeanDefinition} to the
-	 * target: the {@linkplain BeanDefinition#isPrimary() primary flag} and the
-	 * {@linkplain BeanDefinition#isFallback() fallback flag}.
-	 */
-	private static void copyBeanDefinitionProperties(BeanDefinition source, RootBeanDefinition target) {
-		target.setPrimary(source.isPrimary());
-		target.setFallback(source.isFallback());
-	}
-
 
 	static class WrapEarlyBeanPostProcessor implements SmartInstantiationAwareBeanPostProcessor,
 			PriorityOrdered {
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 864d5f2f90..f7802d0fdd 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
@@ -315,7 +315,6 @@ class BeanOverrideBeanFactoryPostProcessorTests {
 
 		assertThatNoException().isThrownBy(context::refresh);
 		assertThat(context.getBeanDefinition("descriptionBean"))
-				.isNotSameAs(definition)
 				.matches(BeanDefinition::isPrimary, "isPrimary")
 				.matches(BeanDefinition::isFallback, "isFallback")
 				.satisfies(d -> assertThat(d.getScope()).isEqualTo(""))
@@ -324,9 +323,8 @@ class BeanOverrideBeanFactoryPostProcessorTests {
 	}
 
 	@Test
-	void qualifiedElementIsSetToBeanOverrideField() {
+	void qualifiedElementIsSetToBeanOverrideFieldForNonexistentBeanDefinition() {
 		AnnotationConfigApplicationContext context = createContext(CaseByNameWithQualifier.class);
-		context.registerBeanDefinition("descriptionBean", new RootBeanDefinition(String.class, () -> "ORIGINAL"));
 
 		assertThatNoException().isThrownBy(context::refresh);
 		assertThat(context.getBeanDefinition("descriptionBean"))
@@ -398,9 +396,12 @@ class BeanOverrideBeanFactoryPostProcessorTests {
 	static class CaseByNameWithQualifier {
 
 		@Qualifier("preferThis")
-		@DummyBean(beanName = "descriptionBean")
+		@TestBean(name = "descriptionBean", enforceOverride = false)
 		private String description;
 
+		static String descriptionBean() {
+			return "overridden";
+		}
 	}
 
 	static class TestFactoryBean implements FactoryBean<Object> {