Fix Phantom Read problem for Bean Overrides in the TestContext framework

To make an analogy to read phenomena for transactional databases, this
commit effectively fixes the "Phantom Read" problem for Bean Overrides.

A phantom read occurs when the BeanOverrideBeanFactoryPostProcessor
retrieves a set of bean names by-type twice and a new bean definition
for a compatible type has been created in the BeanFactory by a
BeanOverrideHandler between the first and second retrieval.

Continue reading for the details...

Prior to this commit, the injection of test Bean Overrides (for
example, when using @⁠MockitoBean) could fail in certain scenarios if
overrides were created for nonexistent beans "by type" without an
explicit name or qualifier. Specifically, if an override for a SubType
was created first, and subsequently an attempt was made to create an
override for a SuperType (where SubType extends SuperType), the
override for the SuperType would "override the override" for the
SubType, effectively removing the override for the SubType.
Consequently, injection of the override instance into the SubType field
would fail with an error message similar to the following.

BeanNotOfRequiredTypeException: Bean named 'Subtype#0' is expected to
be of type 'Subtype' but was actually of type 'Supertype$Mock$XHb7Aspo'

This commit addresses this issue by tracking all generated bean names
(in a generatedBeanNames set) and ensuring that a new bean override
instance is created for the current BeanOverrideHandler if a previous
BeanOverrideHandler already created a bean override instance that now
matches the type required by the current BeanOverrideHandler.

In other words, if the generatedBeanNames set already contains the
beanName that we just found by-type, we cannot "override the override",
because we would lose one of the overrides. Instead, we must create a
new override for the current handler. In the example given above, we
must end up with overrides for both SuperType and SubType.

Closes gh-34025

Author: sbrannen

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

@@ -18,6 +18,7 @@
 
 import java.lang.reflect.Field;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
 
@@ -93,12 +94,15 @@ public int getOrder() {
 
 	@Override
 	public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
+		Set<String> generatedBeanNames = new HashSet<>();
 		for (BeanOverrideHandler handler : this.beanOverrideHandlers) {
-			registerBeanOverride(beanFactory, handler);
+			registerBeanOverride(beanFactory, handler, generatedBeanNames);
 		}
 	}
 
-	private void registerBeanOverride(ConfigurableListableBeanFactory beanFactory, BeanOverrideHandler handler) {
+	private void registerBeanOverride(ConfigurableListableBeanFactory beanFactory, BeanOverrideHandler handler,
+			Set<String> generatedBeanNames) {
+
 		String beanName = handler.getBeanName();
 		Field field = handler.getField();
 		Assert.state(!BeanFactoryUtils.isFactoryDereference(beanName),() -> """
@@ -107,14 +111,14 @@ private void registerBeanOverride(ConfigurableListableBeanFactory beanFactory, B
 					beanName, field.getDeclaringClass().getSimpleName(), field.getName()));
 
 		switch (handler.getStrategy()) {
-			case REPLACE -> replaceOrCreateBean(beanFactory, handler, true);
-			case REPLACE_OR_CREATE -> replaceOrCreateBean(beanFactory, handler, false);
+			case REPLACE -> replaceOrCreateBean(beanFactory, handler, generatedBeanNames, true);
+			case REPLACE_OR_CREATE -> replaceOrCreateBean(beanFactory, handler, generatedBeanNames, false);
 			case WRAP -> wrapBean(beanFactory, handler);
 		}
 	}
 
 	private void replaceOrCreateBean(ConfigurableListableBeanFactory beanFactory, BeanOverrideHandler handler,
-			boolean requireExistingBean) {
+			Set<String> generatedBeanNames, boolean requireExistingBean) {
 
 		// NOTE: This method supports 3 distinct scenarios which must be accounted for.
 		//
@@ -134,7 +138,15 @@ private void replaceOrCreateBean(ConfigurableListableBeanFactory beanFactory, Be
 		BeanDefinition existingBeanDefinition = null;
 		if (beanName == null) {
 			beanName = getBeanNameForType(beanFactory, handler, requireExistingBean);
-			if (beanName != null) {
+			// If the generatedBeanNames set already contains the beanName that we
+			// just found by-type, that means we are experiencing a "phantom read"
+			// (i.e., we found a bean that was not previously there). Consequently,
+			// we cannot "override the override", because we would lose one of the
+			// overrides. Instead, we must create a new override for the current
+			// handler. For example, if one handler creates an override for a SubType
+			// and a subsequent handler creates an override for a SuperType of that
+			// SubType, we must end up with overrides for both SuperType and SubType.
+			if (beanName != null && !generatedBeanNames.contains(beanName)) {
 				// 1) We are overriding an existing bean by-type.
 				beanName = BeanFactoryUtils.transformedBeanName(beanName);
 				// If we are overriding a manually registered singleton, we won't find
@@ -197,6 +209,7 @@ else if (Boolean.getBoolean(AbstractAotProcessor.AOT_PROCESSING)) {
 			// Generate a name for the nonexistent bean.
 			if (PSEUDO_BEAN_NAME_PLACEHOLDER.equals(beanName)) {
 				beanName = beanNameGenerator.generateBeanName(pseudoBeanDefinition, registry);
+				generatedBeanNames.add(beanName);
 			}
 
 			registry.registerBeanDefinition(beanName, pseudoBeanDefinition);

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

@@ -16,7 +16,7 @@
 
 package org.springframework.test.context.bean.override;
 
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -42,7 +42,7 @@ class BeanOverrideContextCustomizerFactory implements ContextCustomizerFactory {
 	public BeanOverrideContextCustomizer createContextCustomizer(Class<?> testClass,
 			List<ContextConfigurationAttributes> configAttributes) {
 
-		Set<BeanOverrideHandler> handlers = new HashSet<>();
+		Set<BeanOverrideHandler> handlers = new LinkedHashSet<>();
 		findBeanOverrideHandler(testClass, handlers);
 		if (handlers.isEmpty()) {
 			return null;

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

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2002-2024 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.test.context.bean.override.mockito;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.context.bean.override.example.ExampleService;
+import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Integration tests for {@link MockitoBean @MockitoBean} where duplicate mocks
+ * are created for the same nonexistent type.
+ *
+ * @author Sam Brannen
+ * @since 6.2.1
+ * @see <a href="https://github.com/spring-projects/spring-framework/issues/34025">gh-34025</a>
+ */
+@SpringJUnitConfig
+public class MockitoBeanDuplicateTypeIntegrationTests {
+
+	@MockitoBean
+	ExampleService service1;
+
+	@MockitoBean
+	ExampleService service2;
+
+	@Autowired
+	List<ExampleService> services;
+
+
+	@Test
+	void duplicateMocksShouldHaveBeenCreated() {
+		assertThat(service1).isNotSameAs(service2);
+		assertThat(services).hasSize(2);
+	}
+
+}

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

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2002-2024 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.test.context.bean.override.mockito;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Integration tests for {@link MockitoBean @MockitoBean} where mocks are created
+ * for nonexistent beans for a supertype and subtype of that supertype.
+ *
+ * <p>This test class is designed to reproduce scenarios that previously failed
+ * along the lines of the following.
+ *
+ * <p>BeanNotOfRequiredTypeException: Bean named 'Subtype#0' is expected to be
+ * of type 'Subtype' but was actually of type 'Supertype$MockitoMock$XHb7Aspo'
+ *
+ * @author Sam Brannen
+ * @since 6.2.1
+ * @see <a href="https://github.com/spring-projects/spring-framework/issues/34025">gh-34025</a>
+ */
+@SpringJUnitConfig
+public class MockitoBeanSuperAndSubtypeIntegrationTests {
+
+	// The declaration order of the following fields is intentional, and prior
+	// to fixing gh-34025 this test class consistently failed on JDK 17.
+
+	@MockitoBean
+	Subtype subtype;
+
+	@MockitoBean
+	Supertype supertype;
+
+
+	@Autowired
+	List<Supertype> supertypes;
+
+
+	@Test
+	void bothMocksShouldHaveBeenCreated() {
+		assertThat(supertype).isNotSameAs(subtype);
+		assertThat(supertypes).hasSize(2);
+	}
+
+
+	interface Supertype {
+	}
+
+	interface Subtype extends Supertype {
+	}
+
+}