Ensure all aliased attributes in target annotation are overridden

Prior to this commit, it was possible that implicit aliases and
transitive implicit aliases (configured via @AliasFor) might not be
honored in certain circumstances, in particular if implicit aliases
were declared to override different attributes within an alias pair in
the target meta-annotation.

This commit addresses this issue by ensuring that all aliased
attributes in the target meta-annotation are overridden during the
merge process in AnnotatedElementUtils.

In addition, concrete default values for attributes in a
meta-annotation declaration can now be effectively shadowed by
transitive implicit aliases in composed annotations.

Issue: SPR-14069

spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java

@@ -1041,15 +1041,36 @@ public class AnnotatedElementUtils {
 			annotation = AnnotationUtils.synthesizeAnnotation(annotation, element);
 			Class<? extends Annotation> targetAnnotationType = attributes.annotationType();
 
+			// Track which attribute values have already been replaced so that we can short
+			// circuit the search algorithms.
+			Set<String> valuesAlreadyReplaced = new HashSet<String>();
+
 			for (Method attributeMethod : AnnotationUtils.getAttributeMethods(annotation.annotationType())) {
 				String attributeName = attributeMethod.getName();
 				String attributeOverrideName = AnnotationUtils.getAttributeOverrideName(attributeMethod, targetAnnotationType);
 
 				// Explicit annotation attribute override declared via @AliasFor
 				if (attributeOverrideName != null) {
-					if (attributes.containsKey(attributeOverrideName)) {
-						overrideAttribute(element, annotation, attributes, attributeName, attributeOverrideName);
+					if (valuesAlreadyReplaced.contains(attributeOverrideName)) {
+						continue;
 					}
+
+					List<String> targetAttributeNames = new ArrayList<String>();
+					targetAttributeNames.add(attributeOverrideName);
+					valuesAlreadyReplaced.add(attributeOverrideName);
+
+					// Ensure all aliased attributes in the target annotation are also overridden. (SPR-14069)
+					List<String> aliases = AnnotationUtils.getAttributeAliasMap(targetAnnotationType).get(attributeOverrideName);
+					if (aliases != null) {
+						for (String alias : aliases) {
+							if (!valuesAlreadyReplaced.contains(alias)) {
+								targetAttributeNames.add(alias);
+								valuesAlreadyReplaced.add(alias);
+							}
+						}
+					}
+
+					overrideAttributes(element, annotation, attributes, attributeName, targetAttributeNames);
 				}
 				// Implicit annotation attribute override based on convention
 				else if (!AnnotationUtils.VALUE.equals(attributeName) && attributes.containsKey(attributeName)) {
@@ -1058,14 +1079,27 @@ public class AnnotatedElementUtils {
 			}
 		}
 
-		private void overrideAttribute(AnnotatedElement element, Annotation annotation,
-				AnnotationAttributes attributes, String sourceAttributeName, String targetAttributeName) {
+		private void overrideAttributes(AnnotatedElement element, Annotation annotation, AnnotationAttributes attributes,
+				String sourceAttributeName, List<String> targetAttributeNames) {
 
 			Object value = AnnotationUtils.getValue(annotation, sourceAttributeName);
-			Object adaptedValue = AnnotationUtils.adaptValue(
-					element, value, this.classValuesAsString, this.nestedAnnotationsAsMap);
+			Object adaptedValue = AnnotationUtils.adaptValue(element, value, this.classValuesAsString,
+				this.nestedAnnotationsAsMap);
+
+			for (String targetAttributeName : targetAttributeNames) {
+				attributes.put(targetAttributeName, adaptedValue);
+			}
+		}
+
+		private void overrideAttribute(AnnotatedElement element, Annotation annotation, AnnotationAttributes attributes,
+				String sourceAttributeName, String targetAttributeName) {
+
+			Object value = AnnotationUtils.getValue(annotation, sourceAttributeName);
+			Object adaptedValue = AnnotationUtils.adaptValue(element, value, this.classValuesAsString,
+				this.nestedAnnotationsAsMap);
 			attributes.put(targetAttributeName, adaptedValue);
 		}
+
 	}
 
 }

spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java

@@ -442,15 +442,15 @@ public class AnnotatedElementUtilsTests {
 	}
 
 	@Test
-	public void getMergedAnnotationAttributesWithInvalidAliasedComposedAnnotation() {
-		Class<?> element = InvalidAliasedComposedContextConfigClass.class;
-		exception.expect(AnnotationConfigurationException.class);
-		exception.expectMessage(either(containsString("attribute 'value' and its alias 'locations'")).or(
-				containsString("attribute 'locations' and its alias 'value'")));
-		exception.expectMessage(either(containsString("values of [{duplicateDeclaration}] and [{test.xml}]")).or(
-				containsString("values of [{test.xml}] and [{duplicateDeclaration}]")));
-		exception.expectMessage(containsString("but only one is permitted"));
-		getMergedAnnotationAttributes(element, ContextConfig.class);
+	public void getMergedAnnotationAttributesWithShadowedAliasComposedAnnotation() {
+		Class<?> element = ShadowedAliasComposedContextConfigClass.class;
+		AnnotationAttributes attributes = getMergedAnnotationAttributes(element, ContextConfig.class);
+
+		String[] expected = asArray("test.xml");
+
+		assertNotNull("Should find @ContextConfig on " + element.getSimpleName(), attributes);
+		assertArrayEquals("locations", expected, attributes.getStringArray("locations"));
+		assertArrayEquals("value", expected, attributes.getStringArray("value"));
 	}
 
 	@Test
@@ -891,9 +891,8 @@ public class AnnotatedElementUtilsTests {
 		@AliasFor(annotation = ContextConfig.class)
 		String[] locations() default {};
 
-		// TODO [SPR-14069] Determine why transitive implicit alias does not work
-		// hoping to be able to omit: attribute = "locations"
-		@AliasFor(annotation = ContextConfig.class, attribute = "locations")
+		// intentionally omitted: attribute = "locations" (SPR-14069)
+		@AliasFor(annotation = ContextConfig.class)
 		String[] value() default {};
 	}
 
@@ -947,14 +946,16 @@ public class AnnotatedElementUtilsTests {
 	}
 
 	/**
-	 * Invalid because the configuration declares a value for 'value' and
-	 * requires a value for the aliased 'locations'. So we likely end up with
-	 * both 'value' and 'locations' being present in {@link AnnotationAttributes}
-	 * but with different values, which violates the contract of {@code @AliasFor}.
+	 * Although the configuration declares an explicit value for 'value' and
+	 * requires a value for the aliased 'locations', this does not result in
+	 * an error since 'locations' effectively <em>shadows</em> the 'value'
+	 * attribute (which cannot be set via the composed annotation anyway).
+	 *
+	 * If 'value' were not shadowed, such a declaration would not make sense.
 	 */
 	@ContextConfig(value = "duplicateDeclaration")
 	@Retention(RetentionPolicy.RUNTIME)
-	@interface InvalidAliasedComposedContextConfig {
+	@interface ShadowedAliasComposedContextConfig {
 
 		@AliasFor(annotation = ContextConfig.class, attribute = "locations")
 		String[] xmlConfigFiles();
@@ -1217,8 +1218,8 @@ public class AnnotatedElementUtilsTests {
 	static class ComposedImplicitAliasesContextConfigClass {
 	}
 
-	@InvalidAliasedComposedContextConfig(xmlConfigFiles = "test.xml")
-	static class InvalidAliasedComposedContextConfigClass {
+	@ShadowedAliasComposedContextConfig(xmlConfigFiles = "test.xml")
+	static class ShadowedAliasComposedContextConfigClass {
 	}
 
 	@AliasedComposedContextConfigAndTestPropSource(xmlConfigFiles = "test.xml")