diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideRegistry.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideRegistry.java index aead53904d9..fa465e50915 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideRegistry.java +++ b/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 @@ import org.springframework.util.StringUtils; */ class BeanOverrideRegistry { - private final Map handlerToBeanNameMap = new HashMap<>(); + private static final Log logger = LogFactory.getLog(BeanOverrideRegistry.class); + - private final Map wrappingBeanOverrideHandlers = new HashMap<>(); + private final Map handlerToBeanNameMap = new LinkedHashMap<>(); + + private final Map 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 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); } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeCreationIntegrationTests.java similarity index 76% rename from spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeIntegrationTests.java rename to spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeCreationIntegrationTests.java index d00ead41963..695e10a5b80 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeCreationIntegrationTests.java @@ -25,25 +25,26 @@ 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.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 gh-34025 + * @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 services; @@ -51,8 +52,10 @@ public class MockitoBeanDuplicateTypeIntegrationTests { @Test void duplicateMocksShouldHaveBeenCreated() { - assertThat(service1).isNotSameAs(service2); - assertThat(services).hasSize(2); + assertThat(services).containsExactly(mock1, mock2); + assertThat(mock1).isNotSameAs(mock2); + assertIsMock(mock1); + assertIsMock(mock2); } } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeReplacementIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanDuplicateTypeReplacementIntegrationTests.java new file mode 100644 index 00000000000..76124c07383 --- /dev/null +++ b/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. + * + *

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 gh-34056 + * @see MockitoBeanDuplicateTypeCreationIntegrationTests + * @see MockitoSpyBeanDuplicateTypeIntegrationTests + */ +@SpringJUnitConfig +public class MockitoBeanDuplicateTypeReplacementIntegrationTests { + + @MockitoBean(reset = AFTER) + ExampleService mock1; + + @MockitoBean(reset = BEFORE) + ExampleService mock2; + + @Autowired + List 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. + *

This method therefore asserts the status quo in terms of behavior. + *

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"; + } + } + +} diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverridesTestBeanIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanOverridesTestBeanIntegrationTests.java new file mode 100644 index 00000000000..9a52589f0ef --- /dev/null +++ b/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. + * + *

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 gh-34056 + * @see MockitoBeanDuplicateTypeCreationIntegrationTests + * @see MockitoBeanDuplicateTypeReplacementIntegrationTests + */ +@SpringJUnitConfig +public class MockitoBeanOverridesTestBeanIntegrationTests { + + @TestBean + ExampleService testService; + + @MockitoBean + ExampleService mockService; + + @Autowired + List 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. + *

This method therefore asserts the status quo in terms of behavior. + *

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"; + } + } + +} diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanDuplicateTypeIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanDuplicateTypeIntegrationTests.java index 35a81384746..772fab5e2c4 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanDuplicateTypeIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoSpyBeanDuplicateTypeIntegrationTests.java @@ -36,28 +36,34 @@ import static org.springframework.test.mockito.MockitoAssertions.assertIsSpy; * * @author Sam Brannen * @since 6.2.1 - * @see MockitoBeanDuplicateTypeIntegrationTests + * @see gh-34056 + * @see MockitoBeanDuplicateTypeCreationIntegrationTests * @see MockitoSpyBeanDuplicateTypeAndNameIntegrationTests */ @SpringJUnitConfig public class MockitoSpyBeanDuplicateTypeIntegrationTests { @MockitoSpyBean - ExampleService service1; + ExampleService spy1; @MockitoSpyBean - ExampleService service2; + ExampleService spy2; @Autowired List 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); }