Revise @⁠TestBean support

See gh-29917

Author: sbrannen

Diff for: spring-test/src/main/java/org/springframework/test/context/bean/override/convention/TestBean.java

@@ -22,23 +22,24 @@
 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}.
  *
- * <p>The instance is created from a no-arg static method in the declaring
+ * <p>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.
  * <ul>
- * <li>if the {@link #methodName()} is specified, look for a static method with
+ * <li>If the {@link #methodName()} is specified, look for a static method with
  * that name.</li>
- * <li>if not, look for exactly one static method named with a suffix equal to
- * {@value #CONVENTION_SUFFIX} and either starting with the annotated field
- * name, or starting with the bean name.</li>
+ * <li>If a method name is not specified, look for exactly one static method named
+ * with a suffix equal to {@value #CONVENTION_SUFFIX} and starting with either the
+ * name of the annotated field or the name of the bean.</li>
  * </ul>
  *
- * <p>Consider the following example:
+ * <p>Consider the following example.
  *
  * <pre><code>
  * class CustomerServiceTests {
@@ -54,13 +55,13 @@
  * }</code></pre>
  *
  * <p>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.
  *
  * <p>To make things more explicit, the method name can be set, as shown in the
- * following example:
+ * following example.
  *
  * <pre><code>
  * class CustomerServiceTests {
@@ -75,11 +76,13 @@
  *     }
  * }</code></pre>
  *
- * <p>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.
+ * <p>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 @@
 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.
-	 * <p>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.
-	 * <p>Default to {@code ""} (the empty String) to use the name of the
-	 * annotated field.
+	 * <p>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.
+	 * <p>Defaults to {@code ""} (the empty String) to signal that the factory
+	 * method should be detected based on convention.
+	 */
+	String methodName() default "";
+
 }

Diff for: 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.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.
+	 * <ul>
+	 * <li>The method is static.
+	 * <li>The method does not accept any arguments.
+	 * <li>The method's return type matches the supplied {@code methodReturnType}.
+	 * <li>The method's name is one of the supplied {@code methodNames}.
+	 * </ul>
+	 * @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<String> expectedNames = new LinkedHashSet<>(Arrays.asList(expectedMethodNames));
-		List<Method> 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<String> supportedNames = new LinkedHashSet<>(Arrays.asList(methodNames));
+		List<Method> 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 OverrideMetadata createMetadata(Field field, Annotation overrideAnnotatio
 			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 OverrideMetadata createMetadata(Field field, Annotation overrideAnnotatio
 		return new MethodConventionOverrideMetadata(field, null, null, overrideAnnotation, typeToOverride);
 	}
 
+
 	static final class MethodConventionOverrideMetadata extends OverrideMetadata {
 
 		@Nullable
@@ -124,22 +143,19 @@ protected Object createOverride(String beanName, @Nullable BeanDefinition existi
 
 			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 for: spring-test/src/test/java/org/springframework/test/context/bean/override/convention/TestBeanOverrideProcessorTests.java

@@ -18,6 +18,7 @@
 
 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 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);
 	}