diff --git a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java index 6687722f9a..bc86279830 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceAttributes.java @@ -41,6 +41,7 @@ import org.springframework.util.ResourceUtils; * {@code TestPropertySourceAttributes} also enforces configuration rules. * * @author Sam Brannen + * @author Phillip Webb * @since 4.1 * @see TestPropertySource * @see MergedTestPropertySources @@ -54,35 +55,59 @@ class TestPropertySourceAttributes { private final Class declaringClass; - private final List locations; + private final MergedAnnotation rootAnnotation; + + private final List locations = new ArrayList<>(); private final boolean inheritLocations; - private final List properties; + private final List properties = new ArrayList<>(); private final boolean inheritProperties; TestPropertySourceAttributes(MergedAnnotation annotation) { this.aggregateIndex = annotation.getAggregateIndex(); - this.declaringClass = (Class) annotation.getSource(); + this.declaringClass = declaringClass(annotation); + this.rootAnnotation = annotation.getRoot(); this.inheritLocations = annotation.getBoolean("inheritLocations"); this.inheritProperties = annotation.getBoolean("inheritProperties"); - this.properties = new ArrayList<>(); - this.locations = new ArrayList<>(); mergePropertiesAndLocations(annotation); } - boolean canMerge(MergedAnnotation annotation) { + /** + * Determine if the annotation represented by this + * {@code TestPropertySourceAttributes} instance can be merged with the + * supplied {@code annotation}. + *

This method effectively checks that two annotations are declared at + * the same level in the type hierarchy (i.e., have the same + * {@linkplain MergedAnnotation#getAggregateIndex() aggregate index}). + * @since 5.2 + * @see #mergeWith(MergedAnnotation) + */ + boolean canMergeWith(MergedAnnotation annotation) { return annotation.getAggregateIndex() == this.aggregateIndex; } - void merge(MergedAnnotation annotation) { - Assert.state((Class) annotation.getSource() == this.declaringClass, + /** + * Merge this {@code TestPropertySourceAttributes} instance with the + * supplied {@code annotation}, asserting that the two sets of test property + * source attributes have identical values for the + * {@link TestPropertySource#inheritLocations} and + * {@link TestPropertySource#inheritProperties} flags and that the two + * underlying annotations were declared on the same class. + *

This method should only be invoked if {@link #canMergeWith(MergedAnnotation)} + * returns {@code true}. + * @since 5.2 + * @see #canMergeWith(MergedAnnotation) + */ + void mergeWith(MergedAnnotation annotation) { + Class source = declaringClass(annotation); + Assert.state(source == this.declaringClass, () -> "Detected @TestPropertySource declarations within an aggregate index " - + "with different source: " + this.declaringClass + " and " - + annotation.getSource()); + + "with different sources: " + this.declaringClass.getName() + " and " + + source.getName()); logger.trace(LogMessage.format("Retrieved %s for declaring class [%s].", annotation, this.declaringClass.getName())); assertSameBooleanAttribute(this.inheritLocations, annotation, "inheritLocations"); @@ -90,18 +115,25 @@ class TestPropertySourceAttributes { mergePropertiesAndLocations(annotation); } - private void assertSameBooleanAttribute(boolean expected, - MergedAnnotation annotation, String attribute) { + private void assertSameBooleanAttribute(boolean expected, MergedAnnotation annotation, + String attribute) { + Assert.isTrue(expected == annotation.getBoolean(attribute), () -> String.format( - "Classes %s and %s must declare the same value for '%s' as other directly " + - "present or meta-present @TestPropertySource annotations", this.declaringClass.getName(), - ((Class) annotation.getSource()).getName(), attribute)); + "@%s on %s and @%s on %s must declare the same value for '%s' as other " + + "directly present or meta-present @TestPropertySource annotations", + this.rootAnnotation.getType().getSimpleName(), this.declaringClass.getSimpleName(), + annotation.getRoot().getType().getSimpleName(), declaringClass(annotation).getSimpleName(), + attribute)); } - private void mergePropertiesAndLocations( - MergedAnnotation annotation) { + private void mergePropertiesAndLocations(MergedAnnotation annotation) { String[] locations = annotation.getStringArray("locations"); String[] properties = annotation.getStringArray("properties"); + // If the meta-distance is positive, that means the annotation is + // meta-present and should therefore have lower priority than directly + // present annotations (i.e., it should be prepended to the list instead + // of appended). This follows the rule of last-one-wins for overriding + // properties. boolean prepend = annotation.getDistance() > 0; if (ObjectUtils.isEmpty(locations) && ObjectUtils.isEmpty(properties)) { addAll(prepend, this.locations, detectDefaultPropertiesFile(annotation)); @@ -112,20 +144,28 @@ class TestPropertySourceAttributes { } } - private void addAll(boolean prepend, List list, String... additions) { - list.addAll(prepend ? 0 : list.size(), Arrays.asList(additions)); + /** + * Add all of the supplied elements to the provided list, honoring the + * {@code prepend} flag. + *

If the {@code prepend} flag is {@code false}, the elements will appended + * to the list. + * @param prepend whether the elements should be prepended to the list + * @param list the list to which to add the elements + * @param elements the elements to add to the list + */ + private void addAll(boolean prepend, List list, String... elements) { + list.addAll((prepend ? 0 : list.size()), Arrays.asList(elements)); } - private String detectDefaultPropertiesFile( - MergedAnnotation annotation) { - Class testClass = (Class) annotation.getSource(); + private String detectDefaultPropertiesFile(MergedAnnotation annotation) { + Class testClass = declaringClass(annotation); String resourcePath = ClassUtils.convertClassNameToResourcePath(testClass.getName()) + ".properties"; ClassPathResource classPathResource = new ClassPathResource(resourcePath); if (!classPathResource.exists()) { 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.", + "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); @@ -195,7 +235,7 @@ class TestPropertySourceAttributes { */ @Override public String toString() { - return new ToStringCreator(this)// + return new ToStringCreator(this) .append("declaringClass", this.declaringClass.getName()) .append("locations", this.locations) .append("inheritLocations", this.inheritLocations) @@ -204,4 +244,8 @@ class TestPropertySourceAttributes { .toString(); } + private static Class declaringClass(MergedAnnotation mergedAnnotation) { + return (Class) mergedAnnotation.getSource(); + } + } diff --git a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java index df3658e036..96b61349a3 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/TestPropertySourceUtils.java @@ -54,6 +54,7 @@ import org.springframework.util.StringUtils; * * @author Sam Brannen * @author Anatoliy Korovin + * @author Phillip Webb * @since 4.1 * @see TestPropertySource */ @@ -83,21 +84,26 @@ public abstract class TestPropertySourceUtils { private static List resolveTestPropertySourceAttributes( MergedAnnotations mergedAnnotations) { - List result = new ArrayList<>(); + List attributesList = new ArrayList<>(); mergedAnnotations.stream(TestPropertySource.class) - .forEach(annotation -> addOrMergeTestPropertySourceAttributes(result, annotation)); - return result; + .forEach(annotation -> addOrMergeTestPropertySourceAttributes(attributesList, annotation)); + return attributesList; } - private static void addOrMergeTestPropertySourceAttributes( - List result, - MergedAnnotation annotation) { + private static void addOrMergeTestPropertySourceAttributes(List attributesList, + MergedAnnotation current) { - if (result.isEmpty() || !result.get(result.size()-1).canMerge(annotation)) { - result.add(new TestPropertySourceAttributes(annotation)); + if (attributesList.isEmpty()) { + attributesList.add(new TestPropertySourceAttributes(current)); } else { - result.get(result.size() - 1).merge(annotation); + TestPropertySourceAttributes previous = attributesList.get(attributesList.size() - 1); + if (previous.canMergeWith(current)) { + previous.mergeWith(current); + } + else { + attributesList.add(new TestPropertySourceAttributes(current)); + } } } diff --git a/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java b/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java index d000e7b253..420081c3b4 100644 --- a/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/support/TestPropertySourceUtilsTests.java @@ -16,6 +16,8 @@ package org.springframework.test.context.support; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.Map; import org.junit.Test; @@ -81,14 +83,18 @@ public class TestPropertySourceUtilsTests { public void repeatedTestPropertySourcesWithConflictingInheritLocationsFlags() { assertThatIllegalArgumentException() .isThrownBy(() -> buildMergedTestPropertySources(RepeatedPropertySourcesWithConflictingInheritLocationsFlags.class)) - .withMessageContaining("must declare the same value for 'inheritLocations' as other directly present or meta-present @TestPropertySource annotations"); + .withMessage("@TestPropertySource on RepeatedPropertySourcesWithConflictingInheritLocationsFlags and " + + "@InheritLocationsFalseTestProperty on RepeatedPropertySourcesWithConflictingInheritLocationsFlags " + + "must declare the same value for 'inheritLocations' as other directly present or meta-present @TestPropertySource annotations"); } @Test public void repeatedTestPropertySourcesWithConflictingInheritPropertiesFlags() { assertThatIllegalArgumentException() .isThrownBy(() -> buildMergedTestPropertySources(RepeatedPropertySourcesWithConflictingInheritPropertiesFlags.class)) - .withMessageContaining("must declare the same value for 'inheritProperties' as other directly present or meta-present @TestPropertySource annotations"); + .withMessage("@TestPropertySource on RepeatedPropertySourcesWithConflictingInheritPropertiesFlags and " + + "@InheritPropertiesFalseTestProperty on RepeatedPropertySourcesWithConflictingInheritPropertiesFlags " + + "must declare the same value for 'inheritProperties' as other directly present or meta-present @TestPropertySource annotations"); } @Test @@ -271,6 +277,15 @@ public class TestPropertySourceUtilsTests { return arr; } + @Retention(RetentionPolicy.RUNTIME) + @TestPropertySource(locations = "foo.properties", inheritLocations = false) + @interface InheritLocationsFalseTestProperty { + } + + @Retention(RetentionPolicy.RUNTIME) + @TestPropertySource(properties = "a = b", inheritProperties = false) + @interface InheritPropertiesFalseTestProperty { + } @TestPropertySource static class EmptyPropertySources { @@ -280,13 +295,13 @@ public class TestPropertySourceUtilsTests { static class ExtendedEmptyPropertySources extends EmptyPropertySources { } - @TestPropertySource(locations = "foo.properties", inheritLocations = false) + @InheritLocationsFalseTestProperty @TestPropertySource(locations = "bar.properties", inheritLocations = true) static class RepeatedPropertySourcesWithConflictingInheritLocationsFlags { } - @TestPropertySource(properties = "a = b", inheritProperties = false) @TestPropertySource(properties = "x = y", inheritProperties = true) + @InheritPropertiesFalseTestProperty static class RepeatedPropertySourcesWithConflictingInheritPropertiesFlags { }