Log warning when one Bean Override overrides another Bean Override

It is currently possible for one Bean Override to override another
logically equivalent Bean Override.

For example, a @⁠TestBean can override a @⁠MockitoBean, and vice versa.

In fact, it's also possible for a @⁠MockitoBean to override another
@⁠MockitoBean, for a @⁠TestBean to override a @⁠TestBean, etc.

However, there may be viable use cases for one override overriding
another override. For example, one may have a need to spy on a bean
created by a @⁠TestBean factory method.

In light of that, we do not prohibit one Bean Override from overriding
another Bean Override; however, with this commit we now log a warning
to help developers diagnose issues in case such an override is
unintentional.

For example, given the following test class, where TestConfig registers
a single bean of type MyService named "myService"...

@⁠SpringJUnitConfig(TestConfig.class)
class MyTests {

  @⁠TestBean(methodName = "example.TestUtils#createMyService")
  MyService testService;

  @⁠MockitoBean
  MyService mockService;

  @⁠Test
  void test() {
    // ...
  }
}

... running that test class results in a log message similar to the
following, which has been formatted for readability.

WARN - Bean with name 'myService' was overridden by multiple handlers:
[
 [TestBeanOverrideHandler@44b21f9f
  field = example.MyService example.MyTests.testService,
  beanType = example.MyService,
  beanName = [null],
  strategy = REPLACE_OR_CREATE
 ],
 [MockitoBeanOverrideHandler@7ee8130e
  field = example.MyService example.MyTests.mockService,
  beanType = example.MyService,
  beanName = [null],
  strategy = REPLACE_OR_CREATE,
  reset = AFTER,
  extraInterfaces = set[[empty]],
  answers = RETURNS_DEFAULTS, serializable = false
 ]
]

NOTE: The last registered BeanOverrideHandler wins. In the above
example, that means that @⁠MockitoBean overrides @⁠TestBean, resulting
in a Mockito mock for the MyService bean in the test's
ApplicationContext.

Closes gh-34056

Author: sbrannen

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

@@ -17,8 +17,13 @@
 package org.springframework.test.context.bean.override;
 
 import java.lang.reflect.Field;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
@@ -37,9 +42,12 @@
  */
 class BeanOverrideRegistry {
 
-	private final Map<BeanOverrideHandler, String> handlerToBeanNameMap = new HashMap<>();
+	private static final Log logger = LogFactory.getLog(BeanOverrideRegistry.class);
+
 
-	private final Map<String, BeanOverrideHandler> wrappingBeanOverrideHandlers = new HashMap<>();
+	private final Map<BeanOverrideHandler, String> handlerToBeanNameMap = new LinkedHashMap<>();
+
+	private final Map<String, BeanOverrideHandler> wrappingBeanOverrideHandlers = new LinkedHashMap<>();
 
 	private final ConfigurableBeanFactory beanFactory;
 
@@ -57,7 +65,25 @@ class BeanOverrideRegistry {
 	 * bean via {@link #wrapBeanIfNecessary(Object, String)}.
 	 */
 	void registerBeanOverrideHandler(BeanOverrideHandler handler, String beanName) {
+		Assert.state(!this.handlerToBeanNameMap.containsKey(handler), () ->
+				"Cannot register BeanOverrideHandler for bean with name '%s'; detected multiple registrations for %s"
+					.formatted(beanName, handler));
+
+		// Check if beanName was already registered, before adding the new mapping.
+		boolean beanNameAlreadyRegistered = this.handlerToBeanNameMap.containsValue(beanName);
+		// Add new mapping before potentially logging a warning, to ensure that
+		// the current handler is logged as well.
 		this.handlerToBeanNameMap.put(handler, beanName);
+
+		if (beanNameAlreadyRegistered && logger.isWarnEnabled()) {
+			List<BeanOverrideHandler> competingHandlers = this.handlerToBeanNameMap.entrySet().stream()
+					.filter(entry -> entry.getValue().equals(beanName))
+					.map(Entry::getKey)
+					.toList();
+			logger.warn("Bean with name '%s' was overridden by multiple handlers: %s"
+					.formatted(beanName, competingHandlers));
+		}
+
 		if (handler.getStrategy() == BeanOverrideStrategy.WRAP) {
 			this.wrappingBeanOverrideHandlers.put(beanName, handler);
 		}

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

@@ -25,34 +25,37 @@
 import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.springframework.test.mockito.MockitoAssertions.assertIsMock;
 
 /**
  * Integration tests for {@link MockitoBean @MockitoBean} where duplicate mocks
- * are created for the same nonexistent type.
+ * are created for the same nonexistent type, selected by-type.
  *
  * @author Sam Brannen
  * @since 6.2.1
  * @see <a href="https://github.com/spring-projects/spring-framework/issues/34025">gh-34025</a>
+ * @see MockitoBeanDuplicateTypeReplacementIntegrationTests
  * @see MockitoSpyBeanDuplicateTypeIntegrationTests
- * @see MockitoSpyBeanDuplicateTypeAndNameIntegrationTests
  */
 @SpringJUnitConfig
-public class MockitoBeanDuplicateTypeIntegrationTests {
+public class MockitoBeanDuplicateTypeCreationIntegrationTests {
 
 	@MockitoBean
-	ExampleService service1;
+	ExampleService mock1;
 
 	@MockitoBean
-	ExampleService service2;
+	ExampleService mock2;
 
 	@Autowired
 	List<ExampleService> services;
 
 
 	@Test
 	void duplicateMocksShouldHaveBeenCreated() {
-		assertThat(service1).isNotSameAs(service2);
-		assertThat(services).hasSize(2);
+		assertThat(services).containsExactly(mock1, mock2);
+		assertThat(mock1).isNotSameAs(mock2);
+		assertIsMock(mock1);
+		assertIsMock(mock2);
 	}
 
 }

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

@@ -0,0 +1,97 @@
+/*
+ * 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.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+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;
+import static org.mockito.BDDMockito.given;
+import static org.springframework.test.context.bean.override.mockito.MockReset.AFTER;
+import static org.springframework.test.context.bean.override.mockito.MockReset.BEFORE;
+import static org.springframework.test.mockito.MockitoAssertions.assertIsMock;
+
+/**
+ * Integration tests for {@link MockitoBean @MockitoBean} where duplicate mocks
+ * are created to replace the same existing bean, selected by-type.
+ *
+ * <p>In other words, this test class demonstrates how one {@code @MockitoBean}
+ * can silently override another {@code @MockitoBean}.
+ *
+ * @author Sam Brannen
+ * @since 6.2.1
+ * @see <a href="https://github.com/spring-projects/spring-framework/issues/34056">gh-34056</a>
+ * @see MockitoBeanDuplicateTypeCreationIntegrationTests
+ * @see MockitoSpyBeanDuplicateTypeIntegrationTests
+ */
+@SpringJUnitConfig
+public class MockitoBeanDuplicateTypeReplacementIntegrationTests {
+
+	@MockitoBean(reset = AFTER)
+	ExampleService mock1;
+
+	@MockitoBean(reset = BEFORE)
+	ExampleService mock2;
+
+	@Autowired
+	List<ExampleService> services;
+
+	/**
+	 * One could argue that we would ideally expect an exception to be thrown when
+	 * two competing mocks are created to replace the same existing bean; however,
+	 * we currently only log a warning in such cases.
+	 * <p>This method therefore asserts the status quo in terms of behavior.
+	 * <p>And the log can be manually checked to verify that an appropriate
+	 * warning was logged.
+	 */
+	@Test
+	void onlyOneMockShouldHaveBeenCreated() {
+		// Currently logs something similar to the following.
+		//
+		// WARN - Bean with name 'exampleService' was overridden by multiple handlers:
+		// [MockitoBeanOverrideHandler@5478ce1e ..., MockitoBeanOverrideHandler@5edc70ed ...]
+
+		// Last one wins: there's only one physical mock
+		assertThat(services).containsExactly(mock2);
+		assertThat(mock1).isSameAs(mock2);
+
+		assertIsMock(mock2);
+		assertThat(MockReset.get(mock2)).as("MockReset").isEqualTo(BEFORE);
+
+		assertThat(mock2.greeting()).isNull();
+		given(mock2.greeting()).willReturn("mocked");
+		assertThat(mock2.greeting()).isEqualTo("mocked");
+	}
+
+
+	@Configuration
+	static class Config {
+
+		@Bean
+		ExampleService exampleService() {
+			return () -> "@Bean";
+		}
+	}
+
+}

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

@@ -0,0 +1,103 @@
+/*
+ * 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.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.test.context.bean.override.convention.TestBean;
+import org.springframework.test.context.bean.override.example.ExampleService;
+import org.springframework.test.context.bean.override.example.RealExampleService;
+import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.springframework.test.mockito.MockitoAssertions.assertIsMock;
+
+/**
+ * Integration tests for Bean Overrides where a {@link MockitoBean @MockitoBean}
+ * overrides a {@link TestBean @TestBean} when trying to replace the same existing
+ * bean, selected by-type.
+ *
+ * <p>In other words, this test class demonstrates how one Bean Override can
+ * silently override another Bean Override.
+ *
+ * @author Sam Brannen
+ * @since 6.2.1
+ * @see <a href="https://github.com/spring-projects/spring-framework/issues/34056">gh-34056</a>
+ * @see MockitoBeanDuplicateTypeCreationIntegrationTests
+ * @see MockitoBeanDuplicateTypeReplacementIntegrationTests
+ */
+@SpringJUnitConfig
+public class MockitoBeanOverridesTestBeanIntegrationTests {
+
+	@TestBean
+	ExampleService testService;
+
+	@MockitoBean
+	ExampleService mockService;
+
+	@Autowired
+	List<ExampleService> services;
+
+
+	static ExampleService testService() {
+		return new RealExampleService("@TestBean");
+	}
+
+
+	/**
+	 * One could argue that we would ideally expect an exception to be thrown when
+	 * two competing overrides are created to replace the same existing bean; however,
+	 * we currently only log a warning in such cases.
+	 * <p>This method therefore asserts the status quo in terms of behavior.
+	 * <p>And the log can be manually checked to verify that an appropriate
+	 * warning was logged.
+	 */
+	@Test
+	void mockitoBeanShouldOverrideTestBean() {
+		// Currently logs something similar to the following.
+		//
+		// WARN - Bean with name 'exampleService' was overridden by multiple handlers:
+		// [TestBeanOverrideHandler@770beef5 ..., MockitoBeanOverrideHandler@6dd1f638 ...]
+
+		// Last override wins...
+		assertThat(services).containsExactly(mockService);
+		assertThat(testService).isSameAs(mockService);
+
+		assertIsMock(mockService);
+
+		assertThat(mockService.greeting()).isNull();
+		given(mockService.greeting()).willReturn("mocked");
+		assertThat(mockService.greeting()).isEqualTo("mocked");
+	}
+
+
+	@Configuration
+	static class Config {
+
+		@Bean
+		ExampleService exampleService() {
+			return () -> "@Bean";
+		}
+	}
+
+}

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

@@ -36,28 +36,34 @@
  *
  * @author Sam Brannen
  * @since 6.2.1
- * @see MockitoBeanDuplicateTypeIntegrationTests
+ * @see <a href="https://github.com/spring-projects/spring-framework/issues/34056">gh-34056</a>
+ * @see MockitoBeanDuplicateTypeCreationIntegrationTests
  * @see MockitoSpyBeanDuplicateTypeAndNameIntegrationTests
  */
 @SpringJUnitConfig
 public class MockitoSpyBeanDuplicateTypeIntegrationTests {
 
 	@MockitoSpyBean
-	ExampleService service1;
+	ExampleService spy1;
 
 	@MockitoSpyBean
-	ExampleService service2;
+	ExampleService spy2;
 
 	@Autowired
 	List<ExampleService> services;
 
 
 	@Test
-	void test() {
-		assertThat(service1).isSameAs(service2);
-		assertThat(services).containsExactly(service1);
+	void onlyOneSpyShouldHaveBeenCreated() {
+		// Currently logs something similar to the following.
+		//
+		// WARN - Bean with name 'exampleService' was overridden by multiple handlers:
+		// [MockitoSpyBeanOverrideHandler@1d269ed7 ..., MockitoSpyBeanOverrideHandler@437ebf59 ...]
 
-		assertIsSpy(service1, "service1");
+		assertThat(services).containsExactly(spy2);
+		assertThat(spy1).isSameAs(spy2);
+
+		assertIsSpy(spy2);
 	}