From 0c7bc232d67ff12e6c98ab803dc83ba3ef0e405b Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 7 Apr 2025 13:42:11 +0200 Subject: [PATCH 1/2] Redesign BeanOverrideRegistry internals --- .../bean/override/BeanOverrideRegistry.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) 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 3afc7c885af..0ff3fbc3327 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 @@ -27,9 +27,9 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.config.ConfigurableBeanFactory; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; -import org.springframework.util.StringUtils; /** * An internal class used to track {@link BeanOverrideHandler}-related state after @@ -110,14 +110,13 @@ class BeanOverrideRegistry { void inject(Object target, BeanOverrideHandler handler) { Field field = handler.getField(); Assert.notNull(field, () -> "BeanOverrideHandler must have a non-null field: " + handler); - String beanName = this.handlerToBeanNameMap.get(handler); - Assert.state(StringUtils.hasLength(beanName), () -> "No bean found for BeanOverrideHandler: " + handler); - inject(field, target, beanName); + Object bean = getBeanForHandler(handler, field.getType()); + Assert.state(bean != null, () -> "No bean found for BeanOverrideHandler: " + handler); + inject(field, target, bean); } - private void inject(Field field, Object target, String beanName) { + private void inject(Field field, Object target, Object bean) { try { - Object bean = this.beanFactory.getBean(beanName, field.getType()); ReflectionUtils.makeAccessible(field); ReflectionUtils.setField(field, target, bean); } @@ -126,4 +125,13 @@ class BeanOverrideRegistry { } } + @Nullable + private Object getBeanForHandler(BeanOverrideHandler handler, Class requiredType) { + String beanName = this.handlerToBeanNameMap.get(handler); + if (beanName != null) { + return this.beanFactory.getBean(beanName, requiredType); + } + return null; + } + } From 63f4ba4b2acc80106c7b08a9f431eb4af5c0be0e Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 7 Apr 2025 14:03:50 +0200 Subject: [PATCH 2/2] Move field injection logic to BeanOverrideTestExecutionListener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For bean override support (@⁠MockitoBean, @⁠TestBean, etc.), the logic for field injection previously resided in the BeanOverrideRegistry which resulted in a strange mixture of concerns. To address that, this commit moves the field injection logic to the BeanOverrideTestExecutionListener, and the BeanOverrideRegistry now serves a single role, namely the role of a registry. Closes gh-34726 --- .../bean/override/BeanOverrideRegistry.java | 37 +++++++------------ .../BeanOverrideTestExecutionListener.java | 20 +++++++++- 2 files changed, 32 insertions(+), 25 deletions(-) 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 0ff3fbc3327..d9c6deb6447 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 @@ -16,7 +16,6 @@ package org.springframework.test.context.bean.override; -import java.lang.reflect.Field; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -25,16 +24,14 @@ 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; import org.springframework.lang.Nullable; import org.springframework.util.Assert; -import org.springframework.util.ReflectionUtils; /** * An internal class used to track {@link BeanOverrideHandler}-related state after - * the bean factory has been processed and to provide field injection utilities - * for test execution listeners. + * the bean factory has been processed and to provide lookup facilities to test + * execution listeners. * * @author Simon Baslé * @author Sam Brannen @@ -63,6 +60,7 @@ class BeanOverrideRegistry { *

Also associates a {@linkplain BeanOverrideStrategy#WRAP "wrapping"} handler * with the given {@code beanName}, allowing for subsequent wrapping of the * bean via {@link #wrapBeanIfNecessary(Object, String)}. + * @see #getBeanForHandler(BeanOverrideHandler, Class) */ void registerBeanOverrideHandler(BeanOverrideHandler handler, String beanName) { Assert.state(!this.handlerToBeanNameMap.containsKey(handler), () -> @@ -107,26 +105,17 @@ class BeanOverrideRegistry { return handler.createOverrideInstance(beanName, null, bean, this.beanFactory); } - void inject(Object target, BeanOverrideHandler handler) { - Field field = handler.getField(); - Assert.notNull(field, () -> "BeanOverrideHandler must have a non-null field: " + handler); - Object bean = getBeanForHandler(handler, field.getType()); - Assert.state(bean != null, () -> "No bean found for BeanOverrideHandler: " + handler); - inject(field, target, bean); - } - - private void inject(Field field, Object target, Object bean) { - try { - ReflectionUtils.makeAccessible(field); - ReflectionUtils.setField(field, target, bean); - } - catch (Throwable ex) { - throw new BeanCreationException("Could not inject field '" + field + "'", ex); - } - } - + /** + * Get the bean instance that was created by the provided {@link BeanOverrideHandler}. + * @param handler the {@code BeanOverrideHandler} that created the bean + * @param requiredType the required bean type + * @return the bean instance, or {@code null} if the provided handler is not + * registered in this registry + * @since 6.2.6 + * @see #registerBeanOverrideHandler(BeanOverrideHandler, String) + */ @Nullable - private Object getBeanForHandler(BeanOverrideHandler handler, Class requiredType) { + Object getBeanForHandler(BeanOverrideHandler handler, Class requiredType) { String beanName = this.handlerToBeanNameMap.get(handler); if (beanName != null) { return this.beanFactory.getBean(beanName, requiredType); diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideTestExecutionListener.java index 736223358cc..ca0499c875d 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/BeanOverrideTestExecutionListener.java @@ -16,11 +16,15 @@ package org.springframework.test.context.bean.override; +import java.lang.reflect.Field; import java.util.List; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.test.context.TestContext; import org.springframework.test.context.support.AbstractTestExecutionListener; import org.springframework.test.context.support.DependencyInjectionTestExecutionListener; +import org.springframework.util.Assert; +import org.springframework.util.ReflectionUtils; /** * {@code TestExecutionListener} that enables {@link BeanOverride @BeanOverride} @@ -94,9 +98,23 @@ public class BeanOverrideTestExecutionListener extends AbstractTestExecutionList .getBean(BeanOverrideContextCustomizer.REGISTRY_BEAN_NAME, BeanOverrideRegistry.class); for (BeanOverrideHandler handler : handlers) { - beanOverrideRegistry.inject(testInstance, handler); + Field field = handler.getField(); + Assert.state(field != null, () -> "BeanOverrideHandler must have a non-null field: " + handler); + Object bean = beanOverrideRegistry.getBeanForHandler(handler, field.getType()); + Assert.state(bean != null, () -> "No bean found for BeanOverrideHandler: " + handler); + injectField(field, testInstance, bean); } } } + private static void injectField(Field field, Object target, Object bean) { + try { + ReflectionUtils.makeAccessible(field); + ReflectionUtils.setField(field, target, bean); + } + catch (Throwable ex) { + throw new BeanCreationException("Could not inject field '" + field + "'", ex); + } + } + }