Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Overhaul repeatable @TestPropertySource support

Browse Source 
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
This commit is contained in:
Sam Brannen
2019-07-25 23:15:33 +02:00
parent 2e476ca14f
commit 136af0b164
36 changed files with 687 additions and 854 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -68,6 +68,8 @@ import org.springframework.core.annotation.AliasFor;
* <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

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

@@ -16,7 +16,6 @@
package org.springframework.test.context;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Inherited;
@@ -24,16 +23,16 @@ 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 @@ import java.lang.annotation.Target;
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();
}

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

@@ -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 @@ import org.springframework.util.ResourceUtils;
*/
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 TestPropertySourceAttributes {
/**
* 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 @@ class TestPropertySourceAttributes {
/**
* 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 @@ class TestPropertySourceAttributes {
.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);
}
}
}

174
spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java
View File

@@ -20,10 +20,13 @@ import java.io.IOException;
import java.io.StringReader;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.TreeMap;
import java.util.stream.Collectors;
import org.apache.commons.logging.Log;
@@ -32,18 +35,22 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.context.ConfigurableApplicationContext;
import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;
import org.springframework.core.annotation.MergedAnnotations.SearchStrategy;
import org.springframework.core.env.ConfigurableEnvironment;
import org.springframework.core.env.Environment;
import org.springframework.core.env.MapPropertySource;
import org.springframework.core.env.PropertySource;
import org.springframework.core.env.PropertySources;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
import org.springframework.core.io.support.ResourcePropertySource;
import org.springframework.test.context.TestPropertySource;
import org.springframework.test.context.util.TestContextResourceUtils;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.ObjectUtils;
import org.springframework.util.ResourceUtils;
import org.springframework.util.StringUtils;
/**
@@ -68,61 +75,154 @@ public abstract class TestPropertySourceUtils {
private static final Log logger = LogFactory.getLog(TestPropertySourceUtils.class);
/**
* Compares {@link MergedAnnotation} instances (presumably within the same
* aggregate index) by their meta-distance, in reverse order.
* <p>Using this {@link Comparator} to sort according to reverse meta-distance
* ensures that directly present annotations take precedence over meta-present
* annotations (within a given aggregate index). In other words, this follows
* the last-one-wins principle of overriding properties.
* @see MergedAnnotation#getAggregateIndex()
* @see MergedAnnotation#getDistance()
*/
private static final Comparator<? super MergedAnnotation<?>> reversedMetaDistanceComparator =
Comparator.<MergedAnnotation<?>> comparingInt(MergedAnnotation::getDistance).reversed();
static MergedTestPropertySources buildMergedTestPropertySources(Class<?> testClass) {
if (!isPresentTestPropertySourceAnnotation(testClass)) {
return new MergedTestPropertySources();
}
else {
return mergeTestPropertySources(testClass);
}
MergedAnnotations mergedAnnotations = MergedAnnotations.from(testClass, SearchStrategy.EXHAUSTIVE);
return (mergedAnnotations.isPresent(TestPropertySource.class) ? mergeTestPropertySources(mergedAnnotations) :
new MergedTestPropertySources());
}
private static boolean isPresentTestPropertySourceAnnotation(Class<?> testClass) {
return MergedAnnotations
.from(testClass, MergedAnnotations.SearchStrategy.EXHAUSTIVE)
.get(TestPropertySource.class).isPresent();
private static MergedTestPropertySources mergeTestPropertySources(MergedAnnotations mergedAnnotations) {
List<TestPropertySourceAttributes> attributesList = resolveTestPropertySourceAttributes(mergedAnnotations);
return new MergedTestPropertySources(mergeLocations(attributesList), mergeProperties(attributesList));
}
private static MergedTestPropertySources mergeTestPropertySources(Class<?> testClass) {
private static List<TestPropertySourceAttributes> resolveTestPropertySourceAttributes(
MergedAnnotations mergedAnnotations) {
List<TestPropertySourceAttributes> attributesList = resolveTestPropertySourceAttributes(
testClass);
String[] locations = mergeLocations(attributesList);
String[] properties = mergeProperties(attributesList);
return new MergedTestPropertySources(locations, properties);
}
private static List<TestPropertySourceAttributes> resolveTestPropertySourceAttributes(Class<?> testClass) {
Assert.notNull(testClass, "Class must not be null");
return MergedAnnotations
.from(testClass, MergedAnnotations.SearchStrategy.EXHAUSTIVE)
// Group by aggregate index to ensure proper separation of inherited and local annotations.
Map<Integer, List<MergedAnnotation<TestPropertySource>>> aggregateIndexMap = mergedAnnotations
.stream(TestPropertySource.class)
.map(TestPropertySourceUtils::makeTestPropertySourceAttribute)
.collect(Collectors.groupingBy(MergedAnnotation::getAggregateIndex, TreeMap::new,
Collectors.mapping(x -> x, Collectors.toList())));
// Stream the lists of annotations per aggregate index, merge each list into a
// single TestPropertySourceAttributes instance, and collect the results.
return aggregateIndexMap.values().stream()
.map(TestPropertySourceUtils::createTestPropertySourceAttributes)
.collect(Collectors.toList());
}
private static TestPropertySourceAttributes makeTestPropertySourceAttribute(
MergedAnnotation<TestPropertySource> annotation) {
/**
* Create a merged {@link TestPropertySourceAttributes} instance from all
* annotations in the supplied list for a given aggregate index as if there
* were only one such annotation.
* <p>Within the supplied list, sort according to reversed meta-distance of
* the annotations from the declaring class. This ensures that directly present
* annotations take precedence over meta-present annotations within the current
* aggregate index.
* <p>If a given {@link TestPropertySource @TestPropertySource} does not
* declare properties or locations, an attempt will be made to detect a default
* properties file.
*/
private static TestPropertySourceAttributes createTestPropertySourceAttributes(
List<MergedAnnotation<TestPropertySource>> list) {
TestPropertySource testPropertySource = annotation.synthesize();
Class<?> rootDeclaringClass = (Class<?>) annotation.getSource();
if (logger.isTraceEnabled()) {
logger.trace(String.format(
"Retrieved @TestPropertySource [%s] for declaring class [%s].",
testPropertySource, rootDeclaringClass.getName()));
list.sort(reversedMetaDistanceComparator);
List<String> locations = new ArrayList<>();
List<String> properties = new ArrayList<>();
Class<?> declaringClass = null;
Boolean inheritLocations = null;
Boolean inheritProperties = null;
// Merge all @TestPropertySource annotations within the current
// aggregate index into a single TestPropertySourceAttributes instance,
// simultaneously ensuring that all such annotations have the same
// declaringClass, inheritLocations, and inheritProperties values.
for (MergedAnnotation<TestPropertySource> mergedAnnotation : list) {
Class<?> currentDeclaringClass = (Class<?>) mergedAnnotation.getSource();
if (declaringClass != null && !declaringClass.equals(currentDeclaringClass)) {
throw new IllegalStateException("Detected @TestPropertySource declarations within an aggregate index " +
"with different declaring classes: " + declaringClass.getName() + " and " +
currentDeclaringClass.getName());
}
declaringClass = currentDeclaringClass;
TestPropertySource testPropertySource = mergedAnnotation.synthesize();
if (logger.isTraceEnabled()) {
logger.trace(String.format("Retrieved %s for declaring class [%s].", testPropertySource,
declaringClass.getName()));
}
Boolean currentInheritLocations = testPropertySource.inheritLocations();
assertConsistentValues(testPropertySource, declaringClass, "inheritLocations", inheritLocations,
currentInheritLocations);
inheritLocations = currentInheritLocations;
Boolean currentInheritProperties = testPropertySource.inheritProperties();
assertConsistentValues(testPropertySource, declaringClass, "inheritProperties", inheritProperties,
currentInheritProperties);
inheritProperties = currentInheritProperties;
String[] currentLocations = testPropertySource.locations();
String[] currentProperties = testPropertySource.properties();
if (ObjectUtils.isEmpty(currentLocations) && ObjectUtils.isEmpty(currentProperties)) {
locations.add(detectDefaultPropertiesFile(declaringClass));
}
else {
Collections.addAll(locations, currentLocations);
Collections.addAll(properties, currentProperties);
}
}
TestPropertySourceAttributes attributes = new TestPropertySourceAttributes(
rootDeclaringClass, testPropertySource);
TestPropertySourceAttributes attributes = new TestPropertySourceAttributes(declaringClass, locations,
inheritLocations, properties, inheritProperties);
if (logger.isTraceEnabled()) {
logger.trace("Resolved TestPropertySource attributes: " + attributes);
logger.trace(String.format("Resolved @TestPropertySource attributes %s for declaring class [%s].",
attributes, declaringClass.getName()));
}
return attributes;
}
private static void assertConsistentValues(TestPropertySource testPropertySource, Class<?> declaringClass,
String attributeName, Object trackedValue, Object currentValue) {
Assert.isTrue((trackedValue == null || trackedValue.equals(currentValue)),
() -> String.format("%s on class [%s] must declare the same value for '%s' " +
"as other directly present or meta-present @TestPropertySource annotations on [%2$s].",
testPropertySource, declaringClass.getName(), attributeName));
}
/**
* 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 class [%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);
}
}
private static String[] mergeLocations(List<TestPropertySourceAttributes> attributesList) {
List<String> locations = new ArrayList<>();
for (TestPropertySourceAttributes attrs : attributesList) {