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 4be7503b6f..4d1f0f0d85 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 @@ -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. * *

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 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 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 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 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. - *

If so, put the {@link BeanOverrideHandler} in the early tracking map. - *

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}. + *

If so, register the {@link BeanOverrideHandler} and the corresponding bean + * name in the {@link BeanOverrideRegistry}. + *

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. *

The returned bean definition should not be used to create * a bean instance but rather only for the purpose of having suitable bean - * definition metadata available in the {@link BeanFactory} — for example, + * definition metadata available in the {@code BeanFactory} — for example, * for autowiring candidate resolution. */ private static RootBeanDefinition createPseudoBeanDefinition(BeanOverrideHandler handler) { diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideContextCustomizerFactory.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideContextCustomizerFactory.java index 764f7cedc7..29cc22fc22 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideContextCustomizerFactory.java +++ b/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 configAttributes) { - Set handlers = new HashSet<>(); + Set handlers = new LinkedHashSet<>(); findBeanOverrideHandler(testClass, handlers); if (handlers.isEmpty()) { return null; 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/MockitoBeanDuplicateTypeIntegrationTests.java new file mode 100644 index 0000000000..4ff6f62480 --- /dev/null +++ b/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 gh-34025 + */ +@SpringJUnitConfig +public class MockitoBeanDuplicateTypeIntegrationTests { + + @MockitoBean + ExampleService service1; + + @MockitoBean + ExampleService service2; + + @Autowired + List services; + + + @Test + void duplicateMocksShouldHaveBeenCreated() { + assertThat(service1).isNotSameAs(service2); + assertThat(services).hasSize(2); + } + +} diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanSuperAndSubtypeIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanSuperAndSubtypeIntegrationTests.java new file mode 100644 index 0000000000..c27854b10d --- /dev/null +++ b/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. + * + *

This test class is designed to reproduce scenarios that previously failed + * along the lines of the following. + * + *

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 gh-34025 + */ +@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 supertypes; + + + @Test + void bothMocksShouldHaveBeenCreated() { + assertThat(supertype).isNotSameAs(subtype); + assertThat(supertypes).hasSize(2); + } + + + interface Supertype { + } + + interface Subtype extends Supertype { + } + +}