Overhaul repeatable @TestPropertySource support

Prior to this commit, if multiple, directly present
`@TestPropertySource` annotations declared the same property, the
precedence ordering was top-down instead of bottom-up, in contrast to
the semantics for class hierarchies. In other words, a subsequent
`@TestPropertySource` annotation could not override a property in a
previous `@TestPropertySource` annotation.

This commit overhauls the internals of `TestPropertySourceUtils` in
order to provide proper support for property overrides within local,
directly present `@TestPropertySource` declarations.

Specifically, the `locations` and `properties` attributes from all
`@TestPropertySource` declarations that are directly present or
meta-present on a given class are now merged into a single instance of
`TestPropertySourceAttributes` internally, with assertions in place to
ensure that such "same level" `@TestPropertySource` declarations do not
configure different values for the `inheritLocations` and
`inheritProperties` flags. Effectively, all "same level"
`@TestPropertySource` declarations are treated internally as if there
were only one such annotation declared by the user.

See gh-23320

Author: sbrannen

spring-test/src/main/java/org/springframework/test/context/TestPropertySource.java

@@ -68,6 +68,8 @@
  * <ul>
  * <li>Typically, {@code @TestPropertySource} will be used in conjunction with
  * {@link ContextConfiguration @ContextConfiguration}.</li>
+ * <li>As of Spring Framework 5.2, {@code @TestPropertySource} can be used as a
+ * <em>{@linkplain Repeatable repeatable}</em> annotation.</li>
  * <li>This annotation may be used as a <em>meta-annotation</em> to create
  * custom <em>composed annotations</em>; however, caution should be taken if
  * this annotation and {@code @ContextConfiguration} are combined on a composed

spring-test/src/main/java/org/springframework/test/context/TestPropertySources.java

@@ -16,24 +16,23 @@
 
 package org.springframework.test.context;
 
-
 import java.lang.annotation.Documented;
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Inherited;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
-
 /**
- * {@code @TestPropertySources} is a container for one or more {@link TestPropertySource}
- * declarations.
+ * {@code @TestPropertySources} is a container for one or more
+ * {@link TestPropertySource @TestPropertySource} declarations.
  *
- * <p>Note, however, that use of the {@code @TestPropertySources} container is completely
- * optional since {@code @TestPropertySource} is a {@linkplain java.lang.annotation.Repeatable
- * repeatable} annotation.
+ * <p>Note, however, that use of the {@code @TestPropertySources} container is
+ * completely optional since {@code @TestPropertySource} is a
+ * {@linkplain java.lang.annotation.Repeatable repeatable} annotation.
  *
  * @author Anatoliy Korovin
+ * @author Sam Brannen
  * @since 5.2
  */
 @Target(ElementType.TYPE)
@@ -43,9 +42,9 @@
 public @interface TestPropertySources {
 
 	/**
-	 * An array of one or more {@link TestPropertySource} declarations.
-	 *
-	 * @return array of {@link TestPropertySource} values.
+	 * An array of one or more {@link TestPropertySource @TestPropertySource}
+	 * declarations.
 	 */
 	TestPropertySource[] value();
+
 }

spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java

@@ -16,25 +16,19 @@
 
 package org.springframework.test.context.support;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import java.util.List;
 
-import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.style.ToStringCreator;
-import org.springframework.lang.Nullable;
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.util.Assert;
-import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
-import org.springframework.util.ResourceUtils;
 
 /**
- * {@code TestPropertySourceAttributes} encapsulates the attributes declared
- * via {@link TestPropertySource @TestPropertySource}.
+ * {@code TestPropertySourceAttributes} encapsulates attributes declared
+ * via {@link TestPropertySource @TestPropertySource} annotations.
  *
  * <p>In addition to encapsulating declared attributes,
- * {@code TestPropertySourceAttributes} also enforces configuration rules
- * and detects default properties files.
+ * {@code TestPropertySourceAttributes} also enforces configuration rules.
  *
  * @author Sam Brannen
  * @since 4.1
@@ -43,8 +37,6 @@
  */
 class TestPropertySourceAttributes {
 
-	private static final Log logger = LogFactory.getLog(TestPropertySourceAttributes.class);
-
 	private final Class<?> declaringClass;
 
 	private final String[] locations;
@@ -57,27 +49,29 @@ class TestPropertySourceAttributes {
 
 
 	/**
-	 * Create a new {@code TestPropertySourceAttributes} instance for the
-	 * supplied {@link TestPropertySource @TestPropertySource} annotation and
-	 * the {@linkplain Class test class} that declared it, enforcing
-	 * configuration rules and detecting a default properties file if
-	 * necessary.
+	 * Create a new {@code TestPropertySourceAttributes} instance for the supplied
+	 * values and enforce configuration rules.
 	 * @param declaringClass the class that declared {@code @TestPropertySource}
-	 * @param testPropertySource the annotation from which to retrieve the attributes
-	 * @since 4.2
+	 * @param locations the merged {@link TestPropertySource#locations()}
+	 * @param inheritLocations the {@link TestPropertySource#inheritLocations()} flag
+	 * @param properties the merged {@link TestPropertySource#properties()}
+	 * @param inheritProperties the {@link TestPropertySource#inheritProperties()} flag
+	 * @since 5.2
 	 */
-	TestPropertySourceAttributes(Class<?> declaringClass, TestPropertySource testPropertySource) {
-		this(declaringClass, testPropertySource.locations(), testPropertySource.inheritLocations(),
-			testPropertySource.properties(), testPropertySource.inheritProperties());
+	TestPropertySourceAttributes(Class<?> declaringClass, List<String> locations, boolean inheritLocations,
+			List<String> properties, boolean inheritProperties) {
+
+		this(declaringClass, locations.toArray(new String[0]), inheritLocations, properties.toArray(new String[0]),
+			inheritProperties);
 	}
 
 	private TestPropertySourceAttributes(Class<?> declaringClass, String[] locations, boolean inheritLocations,
 			String[] properties, boolean inheritProperties) {
 
-		Assert.notNull(declaringClass, "declaringClass must not be null");
-		if (ObjectUtils.isEmpty(locations) && ObjectUtils.isEmpty(properties)) {
-			locations = new String[] { detectDefaultPropertiesFile(declaringClass) };
-		}
+		Assert.notNull(declaringClass, "'declaringClass' must not be null");
+		Assert.isTrue(!ObjectUtils.isEmpty(locations) || !ObjectUtils.isEmpty(properties),
+			"Either 'locations' or 'properties' are required");
+
 		this.declaringClass = declaringClass;
 		this.locations = locations;
 		this.inheritLocations = inheritLocations;
@@ -97,7 +91,8 @@ Class<?> getDeclaringClass() {
 	/**
 	 * Get the resource locations that were declared via {@code @TestPropertySource}.
 	 * <p>Note: The returned value may represent a <em>detected default</em>
-	 * that does not match the original value declared via {@code @TestPropertySource}.
+	 * or merged locations that do not match the original value declared via a
+	 * single {@code @TestPropertySource} annotation.
 	 * @return the resource locations; potentially <em>empty</em>
 	 * @see TestPropertySource#value
 	 * @see TestPropertySource#locations
@@ -117,10 +112,12 @@ boolean isInheritLocations() {
 
 	/**
 	 * Get the inlined properties that were declared via {@code @TestPropertySource}.
-	 * @return the inlined properties; potentially {@code null} or <em>empty</em>
+	 * <p>Note: The returned value may represent merged properties that do not
+	 * match the original value declared via a single {@code @TestPropertySource}
+	 * annotation.
+	 * @return the inlined properties; potentially <em>empty</em>
 	 * @see TestPropertySource#properties
 	 */
-	@Nullable
 	String[] getProperties() {
 		return this.properties;
 	}
@@ -149,31 +146,4 @@ public String toString() {
 		.toString();
 	}
 
-
-	/**
-	 * Detect a default properties file for the supplied class, as specified
-	 * in the class-level Javadoc for {@link TestPropertySource}.
-	 */
-	private static String detectDefaultPropertiesFile(Class<?> testClass) {
-		String resourcePath = ClassUtils.convertClassNameToResourcePath(testClass.getName()) + ".properties";
-		ClassPathResource classPathResource = new ClassPathResource(resourcePath);
-
-		if (classPathResource.exists()) {
-			String prefixedResourcePath = ResourceUtils.CLASSPATH_URL_PREFIX + resourcePath;
-			if (logger.isInfoEnabled()) {
-				logger.info(String.format("Detected default properties file \"%s\" for test class [%s]",
-					prefixedResourcePath, testClass.getName()));
-			}
-			return prefixedResourcePath;
-		}
-		else {
-			String msg = String.format("Could not detect default properties file for test [%s]: " +
-					"%s does not exist. Either declare the 'locations' or 'properties' attributes " +
-					"of @TestPropertySource or make the default properties file available.", testClass.getName(),
-					classPathResource);
-			logger.error(msg);
-			throw new IllegalStateException(msg);
-		}
-	}
-
 }