diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java index 9f6516a12f4..a001ff2e78b 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java @@ -1041,15 +1041,36 @@ public class AnnotatedElementUtils { annotation = AnnotationUtils.synthesizeAnnotation(annotation, element); Class targetAnnotationType = attributes.annotationType(); + // Track which attribute values have already been replaced so that we can short + // circuit the search algorithms. + Set valuesAlreadyReplaced = new HashSet(); + 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 targetAttributeNames = new ArrayList(); + targetAttributeNames.add(attributeOverrideName); + valuesAlreadyReplaced.add(attributeOverrideName); + + // Ensure all aliased attributes in the target annotation are also overridden. (SPR-14069) + List 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 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); } + } } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java index a183a8fb4fd..a0470aaff96 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java +++ b/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 shadows 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")