From d6422d368a7b0099219c9bbbc3196b27cccfe843 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 13 Mar 2024 14:28:27 +0100 Subject: [PATCH] =?UTF-8?q?Revise=20@=E2=81=A0TestBean=20support?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See gh-29917 --- .../bean/override/convention/TestBean.java | 55 ++++++++------ .../convention/TestBeanOverrideProcessor.java | 66 +++++++++------- .../TestBeanOverrideProcessorTests.java | 75 ++++++++++++------- 3 files changed, 124 insertions(+), 72 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java index d78a5b03846..d9afaab178c 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java @@ -22,23 +22,24 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import org.springframework.core.annotation.AliasFor; import org.springframework.test.context.bean.override.BeanOverride; /** * Mark a field to override a bean instance in the {@code BeanFactory}. * - *

The instance is created from a no-arg static method in the declaring + *

The instance is created from a no-arg static factory method in the test * class whose return type is compatible with the annotated field. The method - * is deduced as follows: + * is deduced as follows. *

* - *

Consider the following example: + *

Consider the following example. * * 


  * class CustomerServiceTests {
@@ -54,13 +55,13 @@ import org.springframework.test.context.bean.override.BeanOverride;
  * }
* *

In the example above, the {@code repository} bean is replaced by the - * instance generated by the {@code repositoryTestOverride} method. Not only - * the overridden instance is injected in the {@code repository} field, but it + * instance generated by the {@code repositoryTestOverride()} method. Not only + * is the overridden instance injected into the {@code repository} field, but it * is also replaced in the {@code BeanFactory} so that other injection points - * for that bean use the override. + * for that bean use the overridden bean instance. * *

To make things more explicit, the method name can be set, as shown in the - * following example: + * following example. * * 


  * class CustomerServiceTests {
@@ -75,11 +76,13 @@ import org.springframework.test.context.bean.override.BeanOverride;
  *     }
  * }
* - *

By default, the name of the bean is inferred from the name of the annotated - * field. To use a different bean name, set the {@link #name()} property. + *

By default, the name of the bean to override is inferred from the name of + * the annotated field. To use a different bean name, set the {@link #name()} + * attribute. * * @author Simon Baslé * @author Stephane Nicoll + * @author Sam Brannen * @since 6.2 * @see TestBeanOverrideProcessor */ @@ -90,24 +93,32 @@ import org.springframework.test.context.bean.override.BeanOverride; public @interface TestBean { /** - * Required suffix for a method that overrides a bean instance that is - * detected by convention. + * Required suffix for a factory method that overrides a bean instance that + * is detected by convention. */ String CONVENTION_SUFFIX = "TestOverride"; + /** - * Name of a static method to look for in the test, which will be used to - * instantiate the bean to override. - *

Default to {@code ""} (the empty String), which detects the method - * to us by convention. + * Alias for {@link #name()}. */ - String methodName() default ""; + @AliasFor("name") + String value() default ""; /** * Name of the bean to override. - *

Default to {@code ""} (the empty String) to use the name of the - * annotated field. + *

Defaults to {@code ""} (the empty String) to signal that the name of + * the annotated field should be used as the bean name. */ + @AliasFor("value") String name() default ""; + /** + * Name of a static factory method to look for in the test class, which will + * be used to instantiate the bean to override. + *

Defaults to {@code ""} (the empty String) to signal that the factory + * method should be detected based on convention. + */ + String methodName() default ""; + } diff --git a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java index 4e5bb1d7745..0ee2c171f88 100644 --- a/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java +++ b/spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessor.java @@ -33,40 +33,58 @@ import org.springframework.test.context.bean.override.BeanOverrideProcessor; import org.springframework.test.context.bean.override.BeanOverrideStrategy; import org.springframework.test.context.bean.override.OverrideMetadata; import org.springframework.util.Assert; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** * {@link BeanOverrideProcessor} implementation primarily made to work with - * {@link TestBean @TestBean}, but can work with arbitrary override annotations - * provided the annotated class has a relevant method according to the - * convention documented in {@link TestBean}. + * fields annotated with {@link TestBean @TestBean}, but can also work with + * arbitrary test bean override annotations provided the annotated field's + * declaring class declares an appropriate test bean factory method according + * to the conventions documented in {@link TestBean}. * * @author Simon Baslé + * @author Sam Brannen * @since 6.2 */ public class TestBeanOverrideProcessor implements BeanOverrideProcessor { /** - * Ensure the given {@code enclosingClass} has a static, no-arguments method - * with the given {@code expectedMethodReturnType} and exactly one of the - * {@code expectedMethodNames}. + * Find a test bean factory {@link Method} in the given {@link Class} which + * meets the following criteria. + *

+ * @param clazz the class in which to search for the factory method + * @param methodReturnType the return type for the factory method + * @param methodNames a set of supported names for the factory method + * @return the corresponding factory method + * @throws IllegalStateException if a single matching factory method cannot + * be found */ - public static Method ensureMethod(Class enclosingClass, Class expectedMethodReturnType, - String... expectedMethodNames) { - - Assert.isTrue(expectedMethodNames.length > 0, "At least one expectedMethodName is required"); - Set expectedNames = new LinkedHashSet<>(Arrays.asList(expectedMethodNames)); - List found = Arrays.stream(enclosingClass.getDeclaredMethods()) + public static Method findTestBeanFactoryMethod(Class clazz, Class methodReturnType, String... methodNames) { + Assert.isTrue(methodNames.length > 0, "At least one candidate method name is required"); + Set supportedNames = new LinkedHashSet<>(Arrays.asList(methodNames)); + List methods = Arrays.stream(clazz.getDeclaredMethods()) .filter(method -> Modifier.isStatic(method.getModifiers()) && - expectedNames.contains(method.getName()) && - expectedMethodReturnType.isAssignableFrom(method.getReturnType())) + supportedNames.contains(method.getName()) && + methodReturnType.isAssignableFrom(method.getReturnType())) .toList(); - Assert.state(found.size() == 1, () -> "Found " + found.size() + " static methods " + - "instead of exactly one, matching a name in " + expectedNames + " with return type " + - expectedMethodReturnType.getName() + " on class " + enclosingClass.getName()); + Assert.state(!methods.isEmpty(), () -> """ + Failed to find a static test bean factory method in %s with return type %s \ + whose name matches one of the supported candidates %s""".formatted( + clazz.getName(), methodReturnType.getName(), supportedNames)); + + Assert.state(methods.size() == 1, () -> """ + Found %d competing static test bean factory methods in %s with return type %s \ + whose name matches one of the supported candidates %s""".formatted( + methods.size(), clazz.getName(), methodReturnType.getName(), supportedNames)); - return found.get(0); + return methods.get(0); } @Override @@ -77,7 +95,7 @@ public class TestBeanOverrideProcessor implements BeanOverrideProcessor { Method overrideMethod = null; String beanName = null; if (!testBeanAnnotation.methodName().isBlank()) { - overrideMethod = ensureMethod(declaringClass, field.getType(), testBeanAnnotation.methodName()); + overrideMethod = findTestBeanFactoryMethod(declaringClass, field.getType(), testBeanAnnotation.methodName()); } if (!testBeanAnnotation.name().isBlank()) { beanName = testBeanAnnotation.name(); @@ -89,6 +107,7 @@ public class TestBeanOverrideProcessor implements BeanOverrideProcessor { return new MethodConventionOverrideMetadata(field, null, null, overrideAnnotation, typeToOverride); } + static final class MethodConventionOverrideMetadata extends OverrideMetadata { @Nullable @@ -124,22 +143,19 @@ public class TestBeanOverrideProcessor implements BeanOverrideProcessor { Method methodToInvoke = this.overrideMethod; if (methodToInvoke == null) { - methodToInvoke = ensureMethod(field().getDeclaringClass(), field().getType(), + methodToInvoke = findTestBeanFactoryMethod(field().getDeclaringClass(), field().getType(), beanName + TestBean.CONVENTION_SUFFIX, field().getName() + TestBean.CONVENTION_SUFFIX); } - methodToInvoke.setAccessible(true); - Object override; try { - override = methodToInvoke.invoke(null); + ReflectionUtils.makeAccessible(methodToInvoke); + return methodToInvoke.invoke(null); } catch (IllegalAccessException | InvocationTargetException ex) { throw new IllegalArgumentException("Could not invoke bean overriding method " + methodToInvoke.getName() + "; a static method with no formal parameters is expected", ex); } - - return override; } } diff --git a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessorTests.java b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessorTests.java index ab4c912e617..29032d7d72a 100644 --- a/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessorTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessorTests.java @@ -18,6 +18,7 @@ package org.springframework.test.context.bean.override.convention; import java.lang.reflect.Field; import java.lang.reflect.Method; +import java.util.List; import java.util.Objects; import org.junit.jupiter.api.Test; @@ -31,74 +32,98 @@ import org.springframework.test.context.bean.override.example.FailingExampleServ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.springframework.test.context.bean.override.convention.TestBeanOverrideProcessor.findTestBeanFactoryMethod; /** * Tests for {@link TestBeanOverrideProcessor}. * * @author Simon Baslé + * @author Sam Brannen * @since 6.2 */ class TestBeanOverrideProcessorTests { @Test - void ensureMethodFindsFromList() { - Method method = TestBeanOverrideProcessor.ensureMethod(MethodConventionConf.class, ExampleService.class, - "example1", "example2", "example3"); + void findTestBeanFactoryMethodFindsFromCandidateNames() { + Class clazz = MethodConventionConf.class; + Class returnType = ExampleService.class; + + Method method = findTestBeanFactoryMethod(clazz, returnType, "example1", "example2", "example3"); assertThat(method.getName()).isEqualTo("example2"); } @Test - void ensureMethodNotFound() { + void findTestBeanFactoryMethodNotFound() { + Class clazz = MethodConventionConf.class; + Class returnType = ExampleService.class; + assertThatIllegalStateException() - .isThrownBy(() -> TestBeanOverrideProcessor.ensureMethod(MethodConventionConf.class, ExampleService.class, - "example1", "example3")) - .withMessage("Found 0 static methods instead of exactly one, matching a name in [example1, example3] with return type " + - ExampleService.class.getName() + " on class " + MethodConventionConf.class.getName()); + .isThrownBy(() -> findTestBeanFactoryMethod(clazz, returnType, "example1", "example3")) + .withMessage(""" + Failed to find a static test bean factory method in %s with return type %s \ + whose name matches one of the supported candidates %s""", + clazz.getName(), returnType.getName(), List.of("example1", "example3")); } @Test - void ensureMethodTwoFound() { + void findTestBeanFactoryMethodTwoFound() { + Class clazz = MethodConventionConf.class; + Class returnType = ExampleService.class; + assertThatIllegalStateException() - .isThrownBy(() -> TestBeanOverrideProcessor.ensureMethod(MethodConventionConf.class, ExampleService.class, - "example2", "example4")) - .withMessage("Found 2 static methods instead of exactly one, matching a name in [example2, example4] with return type " + - ExampleService.class.getName() + " on class " + MethodConventionConf.class.getName()); + .isThrownBy(() -> findTestBeanFactoryMethod(clazz, returnType, "example2", "example4")) + .withMessage(""" + Found %d competing static test bean factory methods in %s with return type %s \ + whose name matches one of the supported candidates %s""".formatted( + 2, clazz.getName(), returnType.getName(), List.of("example2", "example4"))); } @Test - void ensureMethodNoNameProvided() { + void findTestBeanFactoryMethodNoNameProvided() { assertThatIllegalArgumentException() - .isThrownBy(() -> TestBeanOverrideProcessor.ensureMethod(MethodConventionConf.class, ExampleService.class)) - .withMessage("At least one expectedMethodName is required"); + .isThrownBy(() -> findTestBeanFactoryMethod(MethodConventionConf.class, ExampleService.class)) + .withMessage("At least one candidate method name is required"); } @Test - void createMetaDataForUnknownExplicitMethod() throws NoSuchFieldException { - Field field = ExplicitMethodNameConf.class.getField("a"); + void createMetaDataForUnknownExplicitMethod() throws Exception { + Class clazz = ExplicitMethodNameConf.class; + Class returnType = ExampleService.class; + Field field = clazz.getField("a"); TestBean overrideAnnotation = Objects.requireNonNull(field.getAnnotation(TestBean.class)); + TestBeanOverrideProcessor processor = new TestBeanOverrideProcessor(); assertThatIllegalStateException() - .isThrownBy(() -> processor.createMetadata(field, overrideAnnotation, ResolvableType.forClass(ExampleService.class))) - .withMessage("Found 0 static methods instead of exactly one, matching a name in [explicit1] with return type " + - ExampleService.class.getName() + " on class " + ExplicitMethodNameConf.class.getName()); + .isThrownBy(() -> processor.createMetadata(field, overrideAnnotation, ResolvableType.forClass(returnType))) + .withMessage(""" + Failed to find a static test bean factory method in %s with return type %s \ + whose name matches one of the supported candidates %s""", + clazz.getName(), returnType.getName(), List.of("explicit1")); } @Test - void createMetaDataForKnownExplicitMethod() throws NoSuchFieldException { + void createMetaDataForKnownExplicitMethod() throws Exception { + Class returnType = ExampleService.class; Field field = ExplicitMethodNameConf.class.getField("b"); TestBean overrideAnnotation = Objects.requireNonNull(field.getAnnotation(TestBean.class)); + TestBeanOverrideProcessor processor = new TestBeanOverrideProcessor(); - assertThat(processor.createMetadata(field, overrideAnnotation, ResolvableType.forClass(ExampleService.class))) + assertThat(processor.createMetadata(field, overrideAnnotation, ResolvableType.forClass(returnType))) .isInstanceOf(MethodConventionOverrideMetadata.class); } @Test - void createMetaDataWithDeferredEnsureMethodCheck() throws NoSuchFieldException { + void createMetaDataWithDeferredCheckForExistenceOfConventionBasedFactoryMethod() throws Exception { + Class returnType = ExampleService.class; Field field = MethodConventionConf.class.getField("field"); TestBean overrideAnnotation = Objects.requireNonNull(field.getAnnotation(TestBean.class)); + TestBeanOverrideProcessor processor = new TestBeanOverrideProcessor(); - assertThat(processor.createMetadata(field, overrideAnnotation, ResolvableType.forClass(ExampleService.class))) + // When in convention-based mode, createMetadata() will not verify that + // the factory method actually exists. So, we don't expect an exception + // for this use case. + assertThat(processor.createMetadata(field, overrideAnnotation, ResolvableType.forClass(returnType))) .isInstanceOf(MethodConventionOverrideMetadata.class); }