Merge branch '6.2.x'

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

@@ -18,12 +18,12 @@ package org.springframework.test.context.bean.override;
 
 import java.lang.reflect.Field;
 import java.util.Arrays;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
 
 import org.springframework.aop.scope.ScopedProxyUtils;
 import org.springframework.beans.BeansException;
-import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
@@ -44,7 +44,7 @@ import org.springframework.util.Assert;
 
 /**
  * A {@link BeanFactoryPostProcessor} implementation that processes identified
- * use of {@link BeanOverride @BeanOverride} and adapts the {@link BeanFactory}
+ * use of {@link BeanOverride @BeanOverride} and adapts the {@code BeanFactory}
  * accordingly.
  *
  * <p>For each override, the bean factory is prepared according to the chosen
@@ -94,12 +94,15 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 
 	@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),() -> """
@@ -108,28 +111,43 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 					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.
 		//
-		// 1) JVM runtime
-		// 2) AOT processing
-		// 3) AOT runtime
+		// - JVM runtime
+		// - AOT processing
+		// - AOT runtime
+		//
+		// In addition, this method supports 4 distinct use cases.
+		//
+		// 1) Override existing bean by-type
+		// 2) Create bean by-type, with a generated name
+		// 3) Override existing bean by-name
+		// 4) Create bean by-name, with a provided name
 
 		String beanName = handler.getBeanName();
 		Field field = handler.getField();
 		BeanDefinition existingBeanDefinition = null;
 		if (beanName == null) {
 			beanName = getBeanNameForType(beanFactory, handler, requireExistingBean);
-			if (beanName != null) {
-				// We are overriding an existing bean by-type.
+			// 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
 				// an existing bean definition.
@@ -138,15 +156,16 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 				}
 			}
 			else {
-				// We will later generate a name for the nonexistent bean, but since NullAway
-				// will reject leaving the beanName set to null, we set it to a placeholder.
+				// 2) We are creating a bean by-type, with a generated name.
+				// Since NullAway will reject leaving the beanName set to null,
+				// we set it to a placeholder that will be replaced later.
 				beanName = PSEUDO_BEAN_NAME_PLACEHOLDER;
 			}
 		}
 		else {
 			Set<String> candidates = getExistingBeanNamesByType(beanFactory, handler, false);
 			if (candidates.contains(beanName)) {
-				// We are overriding an existing bean by-name.
+				// 3) We are overriding an existing bean by-name.
 				existingBeanDefinition = beanFactory.getBeanDefinition(beanName);
 			}
 			else if (requireExistingBean) {
@@ -156,6 +175,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 							.formatted(beanName, handler.getBeanType(),
 								field.getDeclaringClass().getSimpleName(), field.getName()));
 			}
+			// 4) We are creating a bean by-name with the provided beanName.
 		}
 
 		if (existingBeanDefinition != null) {
@@ -189,6 +209,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 			// 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);
@@ -214,11 +235,14 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 
 	/**
 	 * Check that a bean with the specified {@link BeanOverrideHandler#getBeanName() name}
-	 * and {@link BeanOverrideHandler#getBeanType() type} is registered.
-	 * <p>If so, put the {@link BeanOverrideHandler} in the early tracking map.
-	 * <p>The map will later be checked to see if a given bean should be wrapped
-	 * upon creation, during the {@link WrapEarlyBeanPostProcessor#getEarlyBeanReference}
-	 * phase.
+	 * or {@link BeanOverrideHandler#getBeanType() type} has already been registered
+	 * in the {@code BeanFactory}.
+	 * <p>If so, register the {@link BeanOverrideHandler} and the corresponding bean
+	 * name in the {@link BeanOverrideRegistry}.
+	 * <p>The registry will later be checked to see if a given bean should be wrapped
+	 * upon creation, during the early bean post-processing phase.
+	 * @see BeanOverrideRegistry#registerBeanOverrideHandler(BeanOverrideHandler, String)
+	 * @see WrapEarlyBeanPostProcessor#getEarlyBeanReference(Object, String)
 	 */
 	private void wrapBean(ConfigurableListableBeanFactory beanFactory, BeanOverrideHandler handler) {
 		String beanName = handler.getBeanName();
@@ -393,7 +417,7 @@ class BeanOverrideBeanFactoryPostProcessor implements BeanFactoryPostProcessor,
 	 * respectively.
 	 * <p>The returned bean definition should <strong>not</strong> be used to create
 	 * a bean instance but rather only for the purpose of having suitable bean
-	 * definition metadata available in the {@link BeanFactory} &mdash; for example,
+	 * definition metadata available in the {@code BeanFactory} &mdash; for example,
 	 * for autowiring candidate resolution.
 	 */
 	private static RootBeanDefinition createPseudoBeanDefinition(BeanOverrideHandler handler) {

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 {
+	}
+
+}