Skip to content

Commit 027fd78

Browse files
sbrannensbrannen
committed
Further refine @TestPropertySource merged annotation calls
See gh-23320
1 parent c9479ff commit 027fd78

File tree

3 files changed

+104
-39
lines changed

3 files changed

+104
-39
lines changed

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

Lines changed: 70 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
* {@code TestPropertySourceAttributes} also enforces configuration rules.
4242
*
4343
* @author Sam Brannen
44+
* @author Phillip Webb
4445
* @since 4.1
4546
* @see TestPropertySource
4647
* @see MergedTestPropertySources
@@ -54,54 +55,85 @@ class TestPropertySourceAttributes {
5455

5556
private final Class<?> declaringClass;
5657

57-
private final List<String> locations;
58+
private final MergedAnnotation<?> rootAnnotation;
59+
60+
private final List<String> locations = new ArrayList<>();
5861

5962
private final boolean inheritLocations;
6063

61-
private final List<String> properties;
64+
private final List<String> properties = new ArrayList<>();
6265

6366
private final boolean inheritProperties;
6467

6568

6669
TestPropertySourceAttributes(MergedAnnotation<TestPropertySource> annotation) {
6770
this.aggregateIndex = annotation.getAggregateIndex();
68-
this.declaringClass = (Class<?>) annotation.getSource();
71+
this.declaringClass = declaringClass(annotation);
72+
this.rootAnnotation = annotation.getRoot();
6973
this.inheritLocations = annotation.getBoolean("inheritLocations");
7074
this.inheritProperties = annotation.getBoolean("inheritProperties");
71-
this.properties = new ArrayList<>();
72-
this.locations = new ArrayList<>();
7375
mergePropertiesAndLocations(annotation);
7476
}
7577

7678

77-
boolean canMerge(MergedAnnotation<TestPropertySource> annotation) {
79+
/**
80+
* Determine if the annotation represented by this
81+
* {@code TestPropertySourceAttributes} instance can be merged with the
82+
* supplied {@code annotation}.
83+
* <p>This method effectively checks that two annotations are declared at
84+
* the same level in the type hierarchy (i.e., have the same
85+
* {@linkplain MergedAnnotation#getAggregateIndex() aggregate index}).
86+
* @since 5.2
87+
* @see #mergeWith(MergedAnnotation)
88+
*/
89+
boolean canMergeWith(MergedAnnotation<TestPropertySource> annotation) {
7890
return annotation.getAggregateIndex() == this.aggregateIndex;
7991
}
8092

81-
void merge(MergedAnnotation<TestPropertySource> annotation) {
82-
Assert.state((Class<?>) annotation.getSource() == this.declaringClass,
93+
/**
94+
* Merge this {@code TestPropertySourceAttributes} instance with the
95+
* supplied {@code annotation}, asserting that the two sets of test property
96+
* source attributes have identical values for the
97+
* {@link TestPropertySource#inheritLocations} and
98+
* {@link TestPropertySource#inheritProperties} flags and that the two
99+
* underlying annotations were declared on the same class.
100+
* <p>This method should only be invoked if {@link #canMergeWith(MergedAnnotation)}
101+
* returns {@code true}.
102+
* @since 5.2
103+
* @see #canMergeWith(MergedAnnotation)
104+
*/
105+
void mergeWith(MergedAnnotation<TestPropertySource> annotation) {
106+
Class<?> source = declaringClass(annotation);
107+
Assert.state(source == this.declaringClass,
83108
() -> "Detected @TestPropertySource declarations within an aggregate index "
84-
+ "with different source: " + this.declaringClass + " and "
85-
+ annotation.getSource());
109+
+ "with different sources: " + this.declaringClass.getName() + " and "
110+
+ source.getName());
86111
logger.trace(LogMessage.format("Retrieved %s for declaring class [%s].",
87112
annotation, this.declaringClass.getName()));
88113
assertSameBooleanAttribute(this.inheritLocations, annotation, "inheritLocations");
89114
assertSameBooleanAttribute(this.inheritProperties, annotation, "inheritProperties");
90115
mergePropertiesAndLocations(annotation);
91116
}
92117

93-
private void assertSameBooleanAttribute(boolean expected,
94-
MergedAnnotation<TestPropertySource> annotation, String attribute) {
118+
private void assertSameBooleanAttribute(boolean expected, MergedAnnotation<TestPropertySource> annotation,
119+
String attribute) {
120+
95121
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));
122+
"@%s on %s and @%s on %s must declare the same value for '%s' as other " +
123+
"directly present or meta-present @TestPropertySource annotations",
124+
this.rootAnnotation.getType().getSimpleName(), this.declaringClass.getSimpleName(),
125+
annotation.getRoot().getType().getSimpleName(), declaringClass(annotation).getSimpleName(),
126+
attribute));
99127
}
100128

101-
private void mergePropertiesAndLocations(
102-
MergedAnnotation<TestPropertySource> annotation) {
129+
private void mergePropertiesAndLocations(MergedAnnotation<TestPropertySource> annotation) {
103130
String[] locations = annotation.getStringArray("locations");
104131
String[] properties = annotation.getStringArray("properties");
132+
// If the meta-distance is positive, that means the annotation is
133+
// meta-present and should therefore have lower priority than directly
134+
// present annotations (i.e., it should be prepended to the list instead
135+
// of appended). This follows the rule of last-one-wins for overriding
136+
// properties.
105137
boolean prepend = annotation.getDistance() > 0;
106138
if (ObjectUtils.isEmpty(locations) && ObjectUtils.isEmpty(properties)) {
107139
addAll(prepend, this.locations, detectDefaultPropertiesFile(annotation));
@@ -112,20 +144,28 @@ private void mergePropertiesAndLocations(
112144
}
113145
}
114146

115-
private void addAll(boolean prepend, List<String> list, String... additions) {
116-
list.addAll(prepend ? 0 : list.size(), Arrays.asList(additions));
147+
/**
148+
* Add all of the supplied elements to the provided list, honoring the
149+
* {@code prepend} flag.
150+
* <p>If the {@code prepend} flag is {@code false}, the elements will appended
151+
* to the list.
152+
* @param prepend whether the elements should be prepended to the list
153+
* @param list the list to which to add the elements
154+
* @param elements the elements to add to the list
155+
*/
156+
private void addAll(boolean prepend, List<String> list, String... elements) {
157+
list.addAll((prepend ? 0 : list.size()), Arrays.asList(elements));
117158
}
118159

119-
private String detectDefaultPropertiesFile(
120-
MergedAnnotation<TestPropertySource> annotation) {
121-
Class<?> testClass = (Class<?>) annotation.getSource();
160+
private String detectDefaultPropertiesFile(MergedAnnotation<TestPropertySource> annotation) {
161+
Class<?> testClass = declaringClass(annotation);
122162
String resourcePath = ClassUtils.convertClassNameToResourcePath(testClass.getName()) + ".properties";
123163
ClassPathResource classPathResource = new ClassPathResource(resourcePath);
124164
if (!classPathResource.exists()) {
125165
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.",
166+
"Could not detect default properties file for test class [%s]: " +
167+
"%s does not exist. Either declare the 'locations' or 'properties' attributes " +
168+
"of @TestPropertySource or make the default properties file available.",
129169
testClass.getName(), classPathResource);
130170
logger.error(msg);
131171
throw new IllegalStateException(msg);
@@ -195,7 +235,7 @@ boolean isInheritProperties() {
195235
*/
196236
@Override
197237
public String toString() {
198-
return new ToStringCreator(this)//
238+
return new ToStringCreator(this)
199239
.append("declaringClass", this.declaringClass.getName())
200240
.append("locations", this.locations)
201241
.append("inheritLocations", this.inheritLocations)
@@ -204,4 +244,8 @@ public String toString() {
204244
.toString();
205245
}
206246

247+
private static Class<?> declaringClass(MergedAnnotation<?> mergedAnnotation) {
248+
return (Class<?>) mergedAnnotation.getSource();
249+
}
250+
207251
}

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
*
5555
* @author Sam Brannen
5656
* @author Anatoliy Korovin
57+
* @author Phillip Webb
5758
* @since 4.1
5859
* @see TestPropertySource
5960
*/
@@ -83,21 +84,26 @@ private static MergedTestPropertySources mergeTestPropertySources(MergedAnnotati
8384
private static List<TestPropertySourceAttributes> resolveTestPropertySourceAttributes(
8485
MergedAnnotations mergedAnnotations) {
8586

86-
List<TestPropertySourceAttributes> result = new ArrayList<>();
87+
List<TestPropertySourceAttributes> attributesList = new ArrayList<>();
8788
mergedAnnotations.stream(TestPropertySource.class)
88-
.forEach(annotation -> addOrMergeTestPropertySourceAttributes(result, annotation));
89-
return result;
89+
.forEach(annotation -> addOrMergeTestPropertySourceAttributes(attributesList, annotation));
90+
return attributesList;
9091
}
9192

92-
private static void addOrMergeTestPropertySourceAttributes(
93-
List<TestPropertySourceAttributes> result,
94-
MergedAnnotation<TestPropertySource> annotation) {
93+
private static void addOrMergeTestPropertySourceAttributes(List<TestPropertySourceAttributes> attributesList,
94+
MergedAnnotation<TestPropertySource> current) {
9595

96-
if (result.isEmpty() || !result.get(result.size()-1).canMerge(annotation)) {
97-
result.add(new TestPropertySourceAttributes(annotation));
96+
if (attributesList.isEmpty()) {
97+
attributesList.add(new TestPropertySourceAttributes(current));
9898
}
9999
else {
100-
result.get(result.size() - 1).merge(annotation);
100+
TestPropertySourceAttributes previous = attributesList.get(attributesList.size() - 1);
101+
if (previous.canMergeWith(current)) {
102+
previous.mergeWith(current);
103+
}
104+
else {
105+
attributesList.add(new TestPropertySourceAttributes(current));
106+
}
101107
}
102108
}
103109

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

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

19+
import java.lang.annotation.Retention;
20+
import java.lang.annotation.RetentionPolicy;
1921
import java.util.Map;
2022

2123
import org.junit.Test;
@@ -81,14 +83,18 @@ public void extendedEmptyAnnotation() {
8183
public void repeatedTestPropertySourcesWithConflictingInheritLocationsFlags() {
8284
assertThatIllegalArgumentException()
8385
.isThrownBy(() -> buildMergedTestPropertySources(RepeatedPropertySourcesWithConflictingInheritLocationsFlags.class))
84-
.withMessageContaining("must declare the same value for 'inheritLocations' as other directly present or meta-present @TestPropertySource annotations");
86+
.withMessage("@TestPropertySource on RepeatedPropertySourcesWithConflictingInheritLocationsFlags and " +
87+
"@InheritLocationsFalseTestProperty on RepeatedPropertySourcesWithConflictingInheritLocationsFlags " +
88+
"must declare the same value for 'inheritLocations' as other directly present or meta-present @TestPropertySource annotations");
8589
}
8690

8791
@Test
8892
public void repeatedTestPropertySourcesWithConflictingInheritPropertiesFlags() {
8993
assertThatIllegalArgumentException()
9094
.isThrownBy(() -> buildMergedTestPropertySources(RepeatedPropertySourcesWithConflictingInheritPropertiesFlags.class))
91-
.withMessageContaining("must declare the same value for 'inheritProperties' as other directly present or meta-present @TestPropertySource annotations");
95+
.withMessage("@TestPropertySource on RepeatedPropertySourcesWithConflictingInheritPropertiesFlags and " +
96+
"@InheritPropertiesFalseTestProperty on RepeatedPropertySourcesWithConflictingInheritPropertiesFlags " +
97+
"must declare the same value for 'inheritProperties' as other directly present or meta-present @TestPropertySource annotations");
9298
}
9399

94100
@Test
@@ -271,6 +277,15 @@ private static <T> T[] asArray(T... arr) {
271277
return arr;
272278
}
273279

280+
@Retention(RetentionPolicy.RUNTIME)
281+
@TestPropertySource(locations = "foo.properties", inheritLocations = false)
282+
@interface InheritLocationsFalseTestProperty {
283+
}
284+
285+
@Retention(RetentionPolicy.RUNTIME)
286+
@TestPropertySource(properties = "a = b", inheritProperties = false)
287+
@interface InheritPropertiesFalseTestProperty {
288+
}
274289

275290
@TestPropertySource
276291
static class EmptyPropertySources {
@@ -280,13 +295,13 @@ static class EmptyPropertySources {
280295
static class ExtendedEmptyPropertySources extends EmptyPropertySources {
281296
}
282297

283-
@TestPropertySource(locations = "foo.properties", inheritLocations = false)
298+
@InheritLocationsFalseTestProperty
284299
@TestPropertySource(locations = "bar.properties", inheritLocations = true)
285300
static class RepeatedPropertySourcesWithConflictingInheritLocationsFlags {
286301
}
287302

288-
@TestPropertySource(properties = "a = b", inheritProperties = false)
289303
@TestPropertySource(properties = "x = y", inheritProperties = true)
304+
@InheritPropertiesFalseTestProperty
290305
static class RepeatedPropertySourcesWithConflictingInheritPropertiesFlags {
291306
}
292307

0 commit comments

Comments
 (0)