Skip to content

Commit c9479ff

Browse files
philwebbsbrannen
philwebb
authored and
sbrannen
committed
Refine @TestPropertySource merged annotation calls
See gh-23320
1 parent 1f8abef commit c9479ff

File tree

2 files changed

+102
-166
lines changed

2 files changed

+102
-166
lines changed

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

Lines changed: 92 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,22 @@
1616

1717
package org.springframework.test.context.support;
1818

19+
import java.util.ArrayList;
20+
import java.util.Arrays;
1921
import java.util.List;
2022

23+
import org.apache.commons.logging.Log;
24+
import org.apache.commons.logging.LogFactory;
25+
26+
import org.springframework.core.annotation.MergedAnnotation;
27+
import org.springframework.core.io.ClassPathResource;
28+
import org.springframework.core.log.LogMessage;
2129
import org.springframework.core.style.ToStringCreator;
2230
import org.springframework.test.context.TestPropertySource;
2331
import org.springframework.util.Assert;
32+
import org.springframework.util.ClassUtils;
2433
import org.springframework.util.ObjectUtils;
34+
import org.springframework.util.ResourceUtils;
2535

2636
/**
2737
* {@code TestPropertySourceAttributes} encapsulates attributes declared
@@ -37,48 +47,96 @@
3747
*/
3848
class TestPropertySourceAttributes {
3949

50+
private static final Log logger = LogFactory.getLog(TestPropertySourceAttributes.class);
51+
52+
53+
private final int aggregateIndex;
54+
4055
private final Class<?> declaringClass;
4156

42-
private final String[] locations;
57+
private final List<String> locations;
4358

4459
private final boolean inheritLocations;
4560

46-
private final String[] properties;
61+
private final List<String> properties;
4762

4863
private final boolean inheritProperties;
4964

5065

51-
/**
52-
* Create a new {@code TestPropertySourceAttributes} instance for the supplied
53-
* values and enforce configuration rules.
54-
* @param declaringClass the class that declared {@code @TestPropertySource}
55-
* @param locations the merged {@link TestPropertySource#locations()}
56-
* @param inheritLocations the {@link TestPropertySource#inheritLocations()} flag
57-
* @param properties the merged {@link TestPropertySource#properties()}
58-
* @param inheritProperties the {@link TestPropertySource#inheritProperties()} flag
59-
* @since 5.2
60-
*/
61-
TestPropertySourceAttributes(Class<?> declaringClass, List<String> locations, boolean inheritLocations,
62-
List<String> properties, boolean inheritProperties) {
66+
TestPropertySourceAttributes(MergedAnnotation<TestPropertySource> annotation) {
67+
this.aggregateIndex = annotation.getAggregateIndex();
68+
this.declaringClass = (Class<?>) annotation.getSource();
69+
this.inheritLocations = annotation.getBoolean("inheritLocations");
70+
this.inheritProperties = annotation.getBoolean("inheritProperties");
71+
this.properties = new ArrayList<>();
72+
this.locations = new ArrayList<>();
73+
mergePropertiesAndLocations(annotation);
74+
}
6375

64-
this(declaringClass, locations.toArray(new String[0]), inheritLocations, properties.toArray(new String[0]),
65-
inheritProperties);
76+
77+
boolean canMerge(MergedAnnotation<TestPropertySource> annotation) {
78+
return annotation.getAggregateIndex() == this.aggregateIndex;
79+
}
80+
81+
void merge(MergedAnnotation<TestPropertySource> annotation) {
82+
Assert.state((Class<?>) annotation.getSource() == this.declaringClass,
83+
() -> "Detected @TestPropertySource declarations within an aggregate index "
84+
+ "with different source: " + this.declaringClass + " and "
85+
+ annotation.getSource());
86+
logger.trace(LogMessage.format("Retrieved %s for declaring class [%s].",
87+
annotation, this.declaringClass.getName()));
88+
assertSameBooleanAttribute(this.inheritLocations, annotation, "inheritLocations");
89+
assertSameBooleanAttribute(this.inheritProperties, annotation, "inheritProperties");
90+
mergePropertiesAndLocations(annotation);
6691
}
6792

68-
private TestPropertySourceAttributes(Class<?> declaringClass, String[] locations, boolean inheritLocations,
69-
String[] properties, boolean inheritProperties) {
93+
private void assertSameBooleanAttribute(boolean expected,
94+
MergedAnnotation<TestPropertySource> annotation, String attribute) {
95+
Assert.isTrue(expected == annotation.getBoolean(attribute), () -> String.format(
96+
"Classes %s and %s must declare the same value for '%s' as other directly " +
97+
"present or meta-present @TestPropertySource annotations", this.declaringClass.getName(),
98+
((Class<?>) annotation.getSource()).getName(), attribute));
99+
}
70100

71-
Assert.notNull(declaringClass, "'declaringClass' must not be null");
72-
Assert.isTrue(!ObjectUtils.isEmpty(locations) || !ObjectUtils.isEmpty(properties),
73-
"Either 'locations' or 'properties' are required");
101+
private void mergePropertiesAndLocations(
102+
MergedAnnotation<TestPropertySource> annotation) {
103+
String[] locations = annotation.getStringArray("locations");
104+
String[] properties = annotation.getStringArray("properties");
105+
boolean prepend = annotation.getDistance() > 0;
106+
if (ObjectUtils.isEmpty(locations) && ObjectUtils.isEmpty(properties)) {
107+
addAll(prepend, this.locations, detectDefaultPropertiesFile(annotation));
108+
}
109+
else {
110+
addAll(prepend, this.locations, locations);
111+
addAll(prepend, this.properties, properties);
112+
}
113+
}
74114

75-
this.declaringClass = declaringClass;
76-
this.locations = locations;
77-
this.inheritLocations = inheritLocations;
78-
this.properties = properties;
79-
this.inheritProperties = inheritProperties;
115+
private void addAll(boolean prepend, List<String> list, String... additions) {
116+
list.addAll(prepend ? 0 : list.size(), Arrays.asList(additions));
80117
}
81118

119+
private String detectDefaultPropertiesFile(
120+
MergedAnnotation<TestPropertySource> annotation) {
121+
Class<?> testClass = (Class<?>) annotation.getSource();
122+
String resourcePath = ClassUtils.convertClassNameToResourcePath(testClass.getName()) + ".properties";
123+
ClassPathResource classPathResource = new ClassPathResource(resourcePath);
124+
if (!classPathResource.exists()) {
125+
String msg = String.format(
126+
"Could not detect default properties file for test class [%s]: "
127+
+ "%s does not exist. Either declare the 'locations' or 'properties' attributes "
128+
+ "of @TestPropertySource or make the default properties file available.",
129+
testClass.getName(), classPathResource);
130+
logger.error(msg);
131+
throw new IllegalStateException(msg);
132+
}
133+
String prefixedResourcePath = ResourceUtils.CLASSPATH_URL_PREFIX + resourcePath;
134+
if (logger.isInfoEnabled()) {
135+
logger.info(String.format("Detected default properties file \"%s\" for test class [%s]",
136+
prefixedResourcePath, testClass.getName()));
137+
}
138+
return prefixedResourcePath;
139+
}
82140

83141
/**
84142
* Get the {@linkplain Class class} that declared {@code @TestPropertySource}.
@@ -98,7 +156,7 @@ Class<?> getDeclaringClass() {
98156
* @see TestPropertySource#locations
99157
*/
100158
String[] getLocations() {
101-
return this.locations;
159+
return this.locations.toArray(new String[0]);
102160
}
103161

104162
/**
@@ -119,7 +177,7 @@ boolean isInheritLocations() {
119177
* @see TestPropertySource#properties
120178
*/
121179
String[] getProperties() {
122-
return this.properties;
180+
return this.properties.toArray(new String[0]);
123181
}
124182

125183
/**
@@ -138,12 +196,12 @@ boolean isInheritProperties() {
138196
@Override
139197
public String toString() {
140198
return new ToStringCreator(this)//
141-
.append("declaringClass", this.declaringClass.getName())//
142-
.append("locations", ObjectUtils.nullSafeToString(this.locations))//
143-
.append("inheritLocations", this.inheritLocations)//
144-
.append("properties", ObjectUtils.nullSafeToString(this.properties))//
145-
.append("inheritProperties", this.inheritProperties)//
146-
.toString();
199+
.append("declaringClass", this.declaringClass.getName())
200+
.append("locations", this.locations)
201+
.append("inheritLocations", this.inheritLocations)
202+
.append("properties", this.properties)
203+
.append("inheritProperties", this.inheritProperties)
204+
.toString();
147205
}
148206

149207
}

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

Lines changed: 10 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,10 @@
2020
import java.io.StringReader;
2121
import java.util.ArrayList;
2222
import java.util.Arrays;
23-
import java.util.Collections;
24-
import java.util.Comparator;
2523
import java.util.LinkedHashMap;
2624
import java.util.List;
2725
import java.util.Map;
2826
import java.util.Properties;
29-
import java.util.TreeMap;
30-
import java.util.stream.Collectors;
3127

3228
import org.apache.commons.logging.Log;
3329
import org.apache.commons.logging.LogFactory;
@@ -41,16 +37,13 @@
4137
import org.springframework.core.env.MapPropertySource;
4238
import org.springframework.core.env.PropertySource;
4339
import org.springframework.core.env.PropertySources;
44-
import org.springframework.core.io.ClassPathResource;
4540
import org.springframework.core.io.Resource;
4641
import org.springframework.core.io.ResourceLoader;
4742
import org.springframework.core.io.support.ResourcePropertySource;
4843
import org.springframework.test.context.TestPropertySource;
4944
import org.springframework.test.context.util.TestContextResourceUtils;
5045
import org.springframework.util.Assert;
51-
import org.springframework.util.ClassUtils;
5246
import org.springframework.util.ObjectUtils;
53-
import org.springframework.util.ResourceUtils;
5447
import org.springframework.util.StringUtils;
5548

5649
/**
@@ -75,19 +68,6 @@ public abstract class TestPropertySourceUtils {
7568

7669
private static final Log logger = LogFactory.getLog(TestPropertySourceUtils.class);
7770

78-
/**
79-
* Compares {@link MergedAnnotation} instances (presumably within the same
80-
* aggregate index) by their meta-distance, in reverse order.
81-
* <p>Using this {@link Comparator} to sort according to reverse meta-distance
82-
* ensures that directly present annotations take precedence over meta-present
83-
* annotations (within a given aggregate index). In other words, this follows
84-
* the last-one-wins principle of overriding properties.
85-
* @see MergedAnnotation#getAggregateIndex()
86-
* @see MergedAnnotation#getDistance()
87-
*/
88-
private static final Comparator<? super MergedAnnotation<?>> reversedMetaDistanceComparator =
89-
Comparator.<MergedAnnotation<?>> comparingInt(MergedAnnotation::getDistance).reversed();
90-
9171

9272
static MergedTestPropertySources buildMergedTestPropertySources(Class<?> testClass) {
9373
MergedAnnotations mergedAnnotations = MergedAnnotations.from(testClass, SearchStrategy.EXHAUSTIVE);
@@ -103,123 +83,21 @@ private static MergedTestPropertySources mergeTestPropertySources(MergedAnnotati
10383
private static List<TestPropertySourceAttributes> resolveTestPropertySourceAttributes(
10484
MergedAnnotations mergedAnnotations) {
10585

106-
// Group by aggregate index to ensure proper separation of inherited and local annotations.
107-
Map<Integer, List<MergedAnnotation<TestPropertySource>>> aggregateIndexMap = mergedAnnotations
108-
.stream(TestPropertySource.class)
109-
.collect(Collectors.groupingBy(MergedAnnotation::getAggregateIndex, TreeMap::new,
110-
Collectors.mapping(x -> x, Collectors.toList())));
111-
112-
// Stream the lists of annotations per aggregate index, merge each list into a
113-
// single TestPropertySourceAttributes instance, and collect the results.
114-
return aggregateIndexMap.values().stream()
115-
.map(TestPropertySourceUtils::createTestPropertySourceAttributes)
116-
.collect(Collectors.toList());
86+
List<TestPropertySourceAttributes> result = new ArrayList<>();
87+
mergedAnnotations.stream(TestPropertySource.class)
88+
.forEach(annotation -> addOrMergeTestPropertySourceAttributes(result, annotation));
89+
return result;
11790
}
11891

119-
/**
120-
* Create a merged {@link TestPropertySourceAttributes} instance from all
121-
* annotations in the supplied list for a given aggregate index as if there
122-
* were only one such annotation.
123-
* <p>Within the supplied list, sort according to reversed meta-distance of
124-
* the annotations from the declaring class. This ensures that directly present
125-
* annotations take precedence over meta-present annotations within the current
126-
* aggregate index.
127-
* <p>If a given {@link TestPropertySource @TestPropertySource} does not
128-
* declare properties or locations, an attempt will be made to detect a default
129-
* properties file.
130-
*/
131-
private static TestPropertySourceAttributes createTestPropertySourceAttributes(
132-
List<MergedAnnotation<TestPropertySource>> list) {
133-
134-
list.sort(reversedMetaDistanceComparator);
135-
136-
List<String> locations = new ArrayList<>();
137-
List<String> properties = new ArrayList<>();
138-
Class<?> declaringClass = null;
139-
Boolean inheritLocations = null;
140-
Boolean inheritProperties = null;
141-
142-
// Merge all @TestPropertySource annotations within the current
143-
// aggregate index into a single TestPropertySourceAttributes instance,
144-
// simultaneously ensuring that all such annotations have the same
145-
// declaringClass, inheritLocations, and inheritProperties values.
146-
for (MergedAnnotation<TestPropertySource> mergedAnnotation : list) {
147-
Class<?> currentDeclaringClass = (Class<?>) mergedAnnotation.getSource();
148-
if (declaringClass != null && !declaringClass.equals(currentDeclaringClass)) {
149-
throw new IllegalStateException("Detected @TestPropertySource declarations within an aggregate index " +
150-
"with different declaring classes: " + declaringClass.getName() + " and " +
151-
currentDeclaringClass.getName());
152-
}
153-
declaringClass = currentDeclaringClass;
154-
155-
TestPropertySource testPropertySource = mergedAnnotation.synthesize();
156-
if (logger.isTraceEnabled()) {
157-
logger.trace(String.format("Retrieved %s for declaring class [%s].", testPropertySource,
158-
declaringClass.getName()));
159-
}
160-
161-
Boolean currentInheritLocations = testPropertySource.inheritLocations();
162-
assertConsistentValues(testPropertySource, declaringClass, "inheritLocations", inheritLocations,
163-
currentInheritLocations);
164-
inheritLocations = currentInheritLocations;
92+
private static void addOrMergeTestPropertySourceAttributes(
93+
List<TestPropertySourceAttributes> result,
94+
MergedAnnotation<TestPropertySource> annotation) {
16595

166-
Boolean currentInheritProperties = testPropertySource.inheritProperties();
167-
assertConsistentValues(testPropertySource, declaringClass, "inheritProperties", inheritProperties,
168-
currentInheritProperties);
169-
inheritProperties = currentInheritProperties;
170-
171-
String[] currentLocations = testPropertySource.locations();
172-
String[] currentProperties = testPropertySource.properties();
173-
if (ObjectUtils.isEmpty(currentLocations) && ObjectUtils.isEmpty(currentProperties)) {
174-
locations.add(detectDefaultPropertiesFile(declaringClass));
175-
}
176-
else {
177-
Collections.addAll(locations, currentLocations);
178-
Collections.addAll(properties, currentProperties);
179-
}
180-
}
181-
182-
TestPropertySourceAttributes attributes = new TestPropertySourceAttributes(declaringClass, locations,
183-
inheritLocations, properties, inheritProperties);
184-
if (logger.isTraceEnabled()) {
185-
logger.trace(String.format("Resolved @TestPropertySource attributes %s for declaring class [%s].",
186-
attributes, declaringClass.getName()));
187-
}
188-
return attributes;
189-
}
190-
191-
private static void assertConsistentValues(TestPropertySource testPropertySource, Class<?> declaringClass,
192-
String attributeName, Object trackedValue, Object currentValue) {
193-
194-
Assert.isTrue((trackedValue == null || trackedValue.equals(currentValue)),
195-
() -> String.format("%s on class [%s] must declare the same value for '%s' " +
196-
"as other directly present or meta-present @TestPropertySource annotations on [%2$s].",
197-
testPropertySource, declaringClass.getName(), attributeName));
198-
}
199-
200-
/**
201-
* Detect a default properties file for the supplied class, as specified
202-
* in the class-level Javadoc for {@link TestPropertySource}.
203-
*/
204-
private static String detectDefaultPropertiesFile(Class<?> testClass) {
205-
String resourcePath = ClassUtils.convertClassNameToResourcePath(testClass.getName()) + ".properties";
206-
ClassPathResource classPathResource = new ClassPathResource(resourcePath);
207-
208-
if (classPathResource.exists()) {
209-
String prefixedResourcePath = ResourceUtils.CLASSPATH_URL_PREFIX + resourcePath;
210-
if (logger.isInfoEnabled()) {
211-
logger.info(String.format("Detected default properties file \"%s\" for test class [%s]",
212-
prefixedResourcePath, testClass.getName()));
213-
}
214-
return prefixedResourcePath;
96+
if (result.isEmpty() || !result.get(result.size()-1).canMerge(annotation)) {
97+
result.add(new TestPropertySourceAttributes(annotation));
21598
}
21699
else {
217-
String msg = String.format("Could not detect default properties file for test class [%s]: " +
218-
"%s does not exist. Either declare the 'locations' or 'properties' attributes " +
219-
"of @TestPropertySource or make the default properties file available.", testClass.getName(),
220-
classPathResource);
221-
logger.error(msg);
222-
throw new IllegalStateException(msg);
100+
result.get(result.size() - 1).merge(annotation);
223101
}
224102
}
225103

0 commit comments

Comments
 (0)