From e8f873a3493bf845ee54e42b4b9135d0fbf60a41 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 1 May 2025 17:36:16 +0200 Subject: [PATCH] Ensure Bean Overrides are discovered once in hierarchies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit, bean overrides (such as @⁠MockitoBean, etc.) were discovered multiple times if they were declared: - at the type-level on an interface that is implemented at more than one level in the type hierarchy, the enclosing class hierarchy, or a combination of the type and enclosing class hierarchies. or - on a field declared in a class which can be reached multiple times while traversing the type and enclosing class hierarchies in scenarios such as the following: the class (X) in which the field is declared is a supertype of an enclosing type of the test class, and X is also an enclosing type of a supertype of the test class. Such scenarios resulted in an IllegalStateException stating that a duplicate BeanOverrideHandler was discovered. To address that, this commit revises the search algorithm in BeanOverrideHandler so that all types (superclasses, enclosing classes, and implemented interfaces) are only visited once while traversing the type and enclosing class hierarchies in search of bean override handlers. See gh-33925 See gh-34324 Closes gh-34844 --- .../bean/override/BeanOverrideHandler.java | 18 ++--- ...rchiesWithSuperclassPresentTwiceTests.java | 69 +++++++++++++++++++ ...sWithEnclosingClassPresentTwiceTests.java} | 9 ++- ...rchiesWithSuperclassPresentTwiceTests.java | 59 ++++++++++++++++ ...itoBeanWithInterfacePresentTwiceTests.java | 66 ++++++++++++++++++ 5 files changed, 211 insertions(+), 10 deletions(-) create mode 100644 spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java rename spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/{MockitoBeanNestedAndTypeHierarchiesTests.java => MockitoBeanNestedAndTypeHierarchiesWithEnclosingClassPresentTwiceTests.java} (81%) create mode 100644 spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java create mode 100644 spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanWithInterfacePresentTwiceTests.java diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideHandler.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideHandler.java index 4fb5b3c2704..63a34510033 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideHandler.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideHandler.java @@ -184,30 +184,32 @@ public abstract class BeanOverrideHandler { * @param testClass the original test class * @param handlers the list of handlers found * @param localFieldsOnly whether to search only on local fields within the type hierarchy - * @param visitedEnclosingClasses the set of enclosing classes already visited + * @param visitedTypes the set of types already visited * @since 6.2.2 */ private static void findHandlers(Class clazz, Class testClass, List handlers, - boolean localFieldsOnly, Set> visitedEnclosingClasses) { + boolean localFieldsOnly, Set> visitedTypes) { + + // 0) Ensure that we do not process the same class or interface multiple times. + if (!visitedTypes.add(clazz)) { + return; + } // 1) Search enclosing class hierarchy. if (!localFieldsOnly && TestContextAnnotationUtils.searchEnclosingClass(clazz)) { - Class enclosingClass = clazz.getEnclosingClass(); - if (visitedEnclosingClasses.add(enclosingClass)) { - findHandlers(enclosingClass, testClass, handlers, localFieldsOnly, visitedEnclosingClasses); - } + findHandlers(clazz.getEnclosingClass(), testClass, handlers, localFieldsOnly, visitedTypes); } // 2) Search class hierarchy. Class superclass = clazz.getSuperclass(); if (superclass != null && superclass != Object.class) { - findHandlers(superclass, testClass, handlers, localFieldsOnly, visitedEnclosingClasses); + findHandlers(superclass, testClass, handlers, localFieldsOnly, visitedTypes); } if (!localFieldsOnly) { // 3) Search interfaces. for (Class ifc : clazz.getInterfaces()) { - findHandlers(ifc, testClass, handlers, localFieldsOnly, visitedEnclosingClasses); + findHandlers(ifc, testClass, handlers, localFieldsOnly, visitedTypes); } // 4) Process current class. diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java new file mode 100644 index 00000000000..84fd8be1433 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/AbstractMockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java @@ -0,0 +1,69 @@ +/* + * Copyright 2002-2025 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 org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContext; +import org.springframework.test.context.bean.override.example.ExampleService; +import org.springframework.test.context.junit.jupiter.SpringExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.mockito.MockitoAssertions.assertIsMock; + +/** + * Abstract top-level class and abstract inner class for integration tests for + * {@link MockitoBean @MockitoBean} which verify that {@code @MockitoBean} fields + * are not discovered more than once when searching intertwined enclosing class + * hierarchies and type hierarchies, when a superclass is present twice + * in the intertwined hierarchies. + * + * @author Sam Brannen + * @since 6.2.7 + * @see MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests + * @see gh-34844 + */ +@ExtendWith(SpringExtension.class) +abstract class AbstractMockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests { + + @Autowired + ApplicationContext enclosingContext; + + @MockitoBean + ExampleService service; + + + @Test + void topLevelTest() { + assertIsMock(service); + assertThat(enclosingContext.getBeanNamesForType(ExampleService.class)).hasSize(1); + } + + + abstract class AbstractBaseClassForNestedTests { + + @Test + void nestedTest(ApplicationContext nestedContext) { + assertIsMock(service); + assertThat(enclosingContext).isSameAs(nestedContext); + assertThat(enclosingContext.getBeanNamesForType(ExampleService.class)).hasSize(1); + } + } + +} diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesWithEnclosingClassPresentTwiceTests.java similarity index 81% rename from spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesTests.java rename to spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesWithEnclosingClassPresentTwiceTests.java index 72babe68674..7934e07f7bd 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesWithEnclosingClassPresentTwiceTests.java @@ -31,14 +31,17 @@ import static org.springframework.test.mockito.MockitoAssertions.assertIsMock; /** * Integration tests for {@link MockitoBean @MockitoBean} which verify that * {@code @MockitoBean} fields are not discovered more than once when searching - * intertwined enclosing class hierarchies and type hierarchies. + * intertwined enclosing class hierarchies and type hierarchies, when an enclosing + * class is present twice in the intertwined hierarchies. * * @author Sam Brannen * @since 6.2.3 + * @see MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests + * @see MockitoBeanWithInterfacePresentTwiceTests * @see gh-34324 */ @ExtendWith(SpringExtension.class) -class MockitoBeanNestedAndTypeHierarchiesTests { +class MockitoBeanNestedAndTypeHierarchiesWithEnclosingClassPresentTwiceTests { @Autowired ApplicationContext enclosingContext; @@ -50,6 +53,7 @@ class MockitoBeanNestedAndTypeHierarchiesTests { @Test void topLevelTest() { assertIsMock(service); + assertThat(enclosingContext.getBeanNamesForType(ExampleService.class)).hasSize(1); // The following are prerequisites for the reported regression. assertThat(NestedTests.class.getSuperclass()) @@ -66,6 +70,7 @@ class MockitoBeanNestedAndTypeHierarchiesTests { void nestedTest(ApplicationContext nestedContext) { assertIsMock(service); assertThat(enclosingContext).isSameAs(nestedContext); + assertThat(enclosingContext.getBeanNamesForType(ExampleService.class)).hasSize(1); } } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java new file mode 100644 index 00000000000..bc43837b244 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests.java @@ -0,0 +1,59 @@ +/* + * Copyright 2002-2025 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 org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests for {@link MockitoBean @MockitoBean} which verify that + * {@code @MockitoBean} fields are not discovered more than once when searching + * intertwined enclosing class hierarchies and type hierarchies, when a superclass + * is present twice in the intertwined hierarchies. + * + * @author Sam Brannen + * @since 6.2.7 + * @see MockitoBeanNestedAndTypeHierarchiesWithEnclosingClassPresentTwiceTests + * @see MockitoBeanWithInterfacePresentTwiceTests + * @see gh-34844 + */ +class MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests + extends AbstractMockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests { + + @Test + @Override + void topLevelTest() { + super.topLevelTest(); + + // The following are prerequisites for the reported regression. + assertThat(NestedTests.class.getSuperclass()) + .isEqualTo(AbstractBaseClassForNestedTests.class); + assertThat(NestedTests.class.getEnclosingClass()) + .isEqualTo(getClass()); + assertThat(NestedTests.class.getEnclosingClass().getSuperclass()) + .isEqualTo(AbstractBaseClassForNestedTests.class.getEnclosingClass()) + .isEqualTo(getClass().getSuperclass()); + } + + + @Nested + class NestedTests extends AbstractBaseClassForNestedTests { + } + +} diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanWithInterfacePresentTwiceTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanWithInterfacePresentTwiceTests.java new file mode 100644 index 00000000000..95d4ac84585 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanWithInterfacePresentTwiceTests.java @@ -0,0 +1,66 @@ +/* + * Copyright 2002-2025 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 org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationContext; +import org.springframework.test.context.bean.override.example.ExampleService; +import org.springframework.test.context.junit.jupiter.SpringExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.mockito.MockitoAssertions.assertIsMock; + +/** + * Integration tests for {@link MockitoBean @MockitoBean} which verify that type-level + * {@code @MockitoBean} declarations are not discovered more than once when searching + * a type hierarchy, when an interface is present twice in the hierarchy. + * + * @author Sam Brannen + * @since 6.2.7 + * @see MockitoBeanNestedAndTypeHierarchiesWithEnclosingClassPresentTwiceTests + * @see MockitoBeanNestedAndTypeHierarchiesWithSuperclassPresentTwiceTests + * @see gh-34844 + */ +class MockitoBeanWithInterfacePresentTwiceTests extends AbstractMockitoBeanWithInterfacePresentTwiceTests + implements MockConfigInterface { + + @Test + void test(ApplicationContext context) { + assertIsMock(service); + assertThat(context.getBeanNamesForType(ExampleService.class)).hasSize(1); + + // The following are prerequisites for the tested scenario. + assertThat(getClass().getInterfaces()).containsExactly(MockConfigInterface.class); + assertThat(getClass().getSuperclass().getInterfaces()).containsExactly(MockConfigInterface.class); + } + +} + +@MockitoBean(types = ExampleService.class) +interface MockConfigInterface { +} + +@ExtendWith(SpringExtension.class) +abstract class AbstractMockitoBeanWithInterfacePresentTwiceTests implements MockConfigInterface { + + @Autowired + ExampleService service; + +}