From 305686dbf756513ab3e075501bc5faa12c38f056 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sat, 8 Feb 2025 13:30:22 +0100 Subject: [PATCH] =?UTF-8?q?Ensure=20Bean=20Overrides=20are=20discovered=20?= =?UTF-8?q?once=20in=20@=E2=81=A0Nested=20hierarchies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes made to the Bean Override search algorithms in commit 9181cce65f resulted in a regression that caused tests to start failing due to duplicate BeanOverrideHandlers under the following circumstances. - An enclosing class (typically a top-level test class) declares a @⁠BeanOverride such as @⁠MockitoBean. - An inner class is declared in that enclosing class. - A @⁠Nested test class which extends that inner class is declared in the same enclosing class. The reason for the duplicate detection is that the current search algorithm visits the common enclosing class twice. To address that, this commit revises the search algorithm in BeanOverrideHandler so that enclosing classes are only visited once. See gh-33925 Closes gh-34324 --- .../bean/override/BeanOverrideHandler.java | 14 ++-- ...kitoBeanNestedAndTypeHierarchiesTests.java | 76 +++++++++++++++++++ .../MockitoBeansByTypeIntegrationTests.java | 2 +- 3 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesTests.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 db89dd6e185..06eeaaf1fa9 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 @@ -132,7 +132,7 @@ public abstract class BeanOverrideHandler { private static List findHandlers(Class testClass, boolean localFieldsOnly) { List handlers = new ArrayList<>(); - findHandlers(testClass, testClass, handlers, localFieldsOnly); + findHandlers(testClass, testClass, handlers, localFieldsOnly, new HashSet<>()); return handlers; } @@ -146,26 +146,30 @@ 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 * @since 6.2.2 */ private static void findHandlers(Class clazz, Class testClass, List handlers, - boolean localFieldsOnly) { + boolean localFieldsOnly, Set> visitedEnclosingClasses) { // 1) Search enclosing class hierarchy. if (!localFieldsOnly && TestContextAnnotationUtils.searchEnclosingClass(clazz)) { - findHandlers(clazz.getEnclosingClass(), testClass, handlers, localFieldsOnly); + Class enclosingClass = clazz.getEnclosingClass(); + if (visitedEnclosingClasses.add(enclosingClass)) { + findHandlers(enclosingClass, testClass, handlers, localFieldsOnly, visitedEnclosingClasses); + } } // 2) Search class hierarchy. Class superclass = clazz.getSuperclass(); if (superclass != null && superclass != Object.class) { - findHandlers(superclass, testClass, handlers, localFieldsOnly); + findHandlers(superclass, testClass, handlers, localFieldsOnly, visitedEnclosingClasses); } if (!localFieldsOnly) { // 3) Search interfaces. for (Class ifc : clazz.getInterfaces()) { - findHandlers(ifc, testClass, handlers, localFieldsOnly); + findHandlers(ifc, testClass, handlers, localFieldsOnly, visitedEnclosingClasses); } // 4) Process current class. 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/MockitoBeanNestedAndTypeHierarchiesTests.java new file mode 100644 index 00000000000..72babe68674 --- /dev/null +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/MockitoBeanNestedAndTypeHierarchiesTests.java @@ -0,0 +1,76 @@ +/* + * 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 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 + * {@code @MockitoBean} fields are not discovered more than once when searching + * intertwined enclosing class hierarchies and type hierarchies. + * + * @author Sam Brannen + * @since 6.2.3 + * @see gh-34324 + */ +@ExtendWith(SpringExtension.class) +class MockitoBeanNestedAndTypeHierarchiesTests { + + @Autowired + ApplicationContext enclosingContext; + + @MockitoBean + ExampleService service; + + + @Test + void topLevelTest() { + assertIsMock(service); + + // The following are prerequisites for the reported regression. + assertThat(NestedTests.class.getSuperclass()) + .isEqualTo(AbstractBaseClassForNestedTests.class); + assertThat(NestedTests.class.getEnclosingClass()) + .isEqualTo(AbstractBaseClassForNestedTests.class.getEnclosingClass()) + .isEqualTo(getClass()); + } + + + abstract class AbstractBaseClassForNestedTests { + + @Test + void nestedTest(ApplicationContext nestedContext) { + assertIsMock(service); + assertThat(enclosingContext).isSameAs(nestedContext); + } + } + + @Nested + class NestedTests extends AbstractBaseClassForNestedTests { + } + +} diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/mockbeans/MockitoBeansByTypeIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/mockbeans/MockitoBeansByTypeIntegrationTests.java index c70fc89f612..e58b2e888ce 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/mockbeans/MockitoBeansByTypeIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/mockito/mockbeans/MockitoBeansByTypeIntegrationTests.java @@ -90,7 +90,7 @@ class MockitoBeansByTypeIntegrationTests implements TestInterface01 { @MockitoBean(types = Service09.class) - static class BaseTestCase implements TestInterface08 { + class BaseTestCase implements TestInterface08 { @Autowired Service08 service08;