From ef4f1f0a71d0d1fa453fc21522b889f6ff6dfc15 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sat, 4 Jan 2025 18:19:18 +0200 Subject: [PATCH] =?UTF-8?q?Ensure=20@=E2=81=A0BeanOverride=20in=20subclass?= =?UTF-8?q?=20takes=20precedence=20over=20superclass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit, a @⁠BeanOverride (such as @⁠TestBean) for a specific target bean which was declared in a superclass always took precedence over a bean override for the same target bean in a subclass, thereby rendering the bean override configuration in the subclass useless. In other words, there was no way for a test class to override a bean override declared in a superclass. To address that, this commit switches from direct use of ReflectionUtils.doWithFields() to a custom search algorithm that traverses the class hierarchy using tail recursion for processing @⁠BeanOverride fields (delegating now to ReflectionUtils.doWithLocalFields() in order to continue to benefit from the caching of declared fields in ReflectionUtils). Closes gh-34194 --- .../bean/override/BeanOverrideHandler.java | 21 +++- ... TestBeanInheritanceIntegrationTests.java} | 118 +++++++++++++----- 2 files changed, 103 insertions(+), 36 deletions(-) rename spring-test/src/test/java/org/springframework/test/context/bean/override/convention/{TestBeanForInheritanceIntegrationTests.java => TestBeanInheritanceIntegrationTests.java} (70%) 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 b11081392009..fdfc718385ab 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * 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. @@ -103,10 +103,27 @@ protected BeanOverrideHandler(Field field, ResolvableType beanType, @Nullable St */ public static List forTestClass(Class testClass) { List handlers = new LinkedList<>(); - ReflectionUtils.doWithFields(testClass, field -> processField(field, testClass, handlers)); + findHandlers(testClass, testClass, handlers); return handlers; } + /** + * Find handlers using tail recursion to ensure that "locally declared" + * bean overrides take precedence over inherited bean overrides. + * @since 6.2.2 + */ + private static void findHandlers(Class clazz, Class testClass, List handlers) { + if (clazz == null || clazz == Object.class) { + return; + } + + // 1) Search type hierarchy. + findHandlers(clazz.getSuperclass(), testClass, handlers); + + // 2) Process fields in current class. + ReflectionUtils.doWithLocalFields(clazz, field -> processField(field, testClass, handlers)); + } + private static void processField(Field field, Class testClass, List handlers) { AtomicBoolean overrideAnnotationFound = new AtomicBoolean(); MergedAnnotations.from(field, DIRECT).stream(BeanOverride.class).forEach(mergedAnnotation -> { diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanForInheritanceIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanInheritanceIntegrationTests.java similarity index 70% rename from spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanForInheritanceIntegrationTests.java rename to spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanInheritanceIntegrationTests.java index 5185045b4cb9..9e3890c005b0 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanForInheritanceIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanInheritanceIntegrationTests.java @@ -38,14 +38,21 @@ * @author Sam Brannen * @since 6.2 */ -public class TestBeanForInheritanceIntegrationTests { +@SpringJUnitConfig +public class TestBeanInheritanceIntegrationTests { + + @TestBean + Pojo puzzleBean; + + static Pojo puzzleBean() { + return new FakePojo("puzzle in enclosing class"); + } static Pojo enclosingClassBean() { return new FakePojo("in enclosing test class"); } - @SpringJUnitConfig - abstract static class AbstractTestBeanIntegrationTestCase { + abstract static class AbstractTestCase { @TestBean Pojo someBean; @@ -56,6 +63,9 @@ abstract static class AbstractTestBeanIntegrationTestCase { @TestBean("thirdBean") Pojo anotherBean; + @TestBean + Pojo enigmaBean; + static Pojo otherBean() { return new FakePojo("other in superclass"); } @@ -64,44 +74,18 @@ static Pojo thirdBean() { return new FakePojo("third in superclass"); } - static Pojo commonBean() { - return new FakePojo("common in superclass"); + static Pojo enigmaBean() { + return new FakePojo("enigma in superclass"); } - @Configuration(proxyBeanMethods = false) - static class Config { - - @Bean - Pojo someBean() { - return new ProdPojo(); - } - - @Bean - Pojo otherBean() { - return new ProdPojo(); - } - - @Bean - Pojo thirdBean() { - return new ProdPojo(); - } - - @Bean - Pojo pojo() { - return new ProdPojo(); - } - - @Bean - Pojo pojo2() { - return new ProdPojo(); - } + static Pojo commonBean() { + return new FakePojo("common in superclass"); } - } @Nested @DisplayName("Nested, concrete inherited tests with correct @TestBean setup") - class NestedConcreteTestBeanIntegrationTests extends AbstractTestBeanIntegrationTestCase { + class NestedTests extends AbstractTestCase { @Autowired ApplicationContext ctx; @@ -112,6 +96,21 @@ class NestedConcreteTestBeanIntegrationTests extends AbstractTestBeanIntegration @TestBean(name = "pojo2", methodName = "enclosingClassBean") Pojo pojo2; + @TestBean(methodName = "localEnigmaBean") + Pojo enigmaBean; + + @TestBean + Pojo puzzleBean; + + + static Pojo puzzleBean() { + return new FakePojo("puzzle in nested class"); + } + + static Pojo localEnigmaBean() { + return new FakePojo("enigma in subclass"); + } + static Pojo someBean() { return new FakePojo("someBeanOverride"); } @@ -150,6 +149,57 @@ void fieldInNestedClassWithFactoryMethodInEnclosingClass() { assertThat(ctx.getBean("pojo2")).as("applicationContext").hasToString("in enclosing test class"); assertThat(this.pojo2.value()).as("injection point").isEqualTo("in enclosing test class"); } + + @Test // gh-34194 + void testBeanInSubclassOverridesTestBeanInSuperclass() { + assertThat(ctx.getBean("enigmaBean")).as("applicationContext").hasToString("enigma in subclass"); + assertThat(this.enigmaBean.value()).as("injection point").isEqualTo("enigma in subclass"); + } + + @Test // gh-34194 + void testBeanInNestedClassOverridesTestBeanInEnclosingClass() { + assertThat(ctx.getBean("puzzleBean")).as("applicationContext").hasToString("puzzle in nested class"); + assertThat(this.puzzleBean.value()).as("injection point").isEqualTo("puzzle in nested class"); + } + } + + @Configuration(proxyBeanMethods = false) + static class Config { + + @Bean + Pojo someBean() { + return new ProdPojo(); + } + + @Bean + Pojo otherBean() { + return new ProdPojo(); + } + + @Bean + Pojo thirdBean() { + return new ProdPojo(); + } + + @Bean + Pojo enigmaBean() { + return new ProdPojo(); + } + + @Bean + Pojo puzzleBean() { + return new ProdPojo(); + } + + @Bean + Pojo pojo() { + return new ProdPojo(); + } + + @Bean + Pojo pojo2() { + return new ProdPojo(); + } } interface Pojo {