Don't mutate annotation metadata when merging attrs

Prior to this commit, invoking the getMergedAnnotationAttributes()
method in AnnotationReadingVisitorUtils resulted in mutation of the
internal state of the ASM-based annotation metadata supplied to the
method.

This commit fixes this issue by making a copy of the original
AnnotationAttributes for the target annotation before merging attribute
values from the meta-annotation hierarchy.

This commit also introduces a slight performance improvement by
avoiding duplicate processing of the attributes of the target
annotation.

Issue: SPR-11710

Author: sbrannen

spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java

@@ -27,7 +27,7 @@
 import org.springframework.core.annotation.AnnotationAttributes;
 import org.springframework.util.LinkedMultiValueMap;
 
-import static org.springframework.core.annotation.AnnotationUtils.VALUE;
+import org.springframework.core.annotation.AnnotationUtils;
 
 /**
  * Internal utility class used when reading annotations.
@@ -119,19 +119,24 @@ public static AnnotationAttributes getMergedAnnotationAttributes(
 			return null;
 		}
 
-		// To start with, we populate the results with all attribute values from the
-		// target annotation.
-		AnnotationAttributes results = attributesList.get(0);
+		// To start with, we populate the results with a copy of all attribute
+		// values from the target annotation. A copy is necessary so that we do
+		// not inadvertently mutate the state of the metadata passed to this
+		// method.
+		AnnotationAttributes results = new AnnotationAttributes(attributesList.get(0));
 
 		Set<String> overridableAttributeNames = new HashSet<String>(results.keySet());
-		overridableAttributeNames.remove(VALUE);
+		overridableAttributeNames.remove(AnnotationUtils.VALUE);
 
 		// Since the map is a LinkedMultiValueMap, we depend on the ordering of
 		// elements in the map and reverse the order of the keys in order to traverse
 		// "down" the annotation hierarchy.
 		List<String> annotationTypes = new ArrayList<String>(attributesMap.keySet());
 		Collections.reverse(annotationTypes);
 
+		// No need to revisit the target annotation type:
+		annotationTypes.remove(annotationType);
+
 		for (String currentAnnotationType : annotationTypes) {
 			List<AnnotationAttributes> currentAttributesList = attributesMap.get(currentAnnotationType);
 			if (currentAttributesList != null && !currentAttributesList.isEmpty()) {

spring-core/src/test/java/org/springframework/core/type/AnnotationMetadataTests.java

@@ -33,6 +33,7 @@
 import org.springframework.core.type.classreading.MetadataReaderFactory;
 import org.springframework.core.type.classreading.SimpleMetadataReaderFactory;
 import org.springframework.stereotype.Component;
+import org.springframework.util.MultiValueMap;
 
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
@@ -111,7 +112,23 @@ public void metaAnnotationOverridesUsingAnnotationMetadataReadingVisitor() throw
 		assertMetaAnnotationOverrides(metadata);
 	}
 
+	/**
+	 * @param metadata the metadata for {@link ComposedConfigurationWithAttributeOverridesClass}
+	 */
 	private void assertMetaAnnotationOverrides(AnnotationMetadata metadata) {
+		assertAllAttributesForMetaAnnotationOverrides(metadata);
+		assertAttributesForMetaAnnotationOverrides(metadata);
+
+		// SPR-11710: Invoke a 2nd time after invoking getAnnotationAttributes() in order
+		// to ensure that getMergedAnnotationAttributes() in AnnotationReadingVisitorUtils
+		// does not mutate the state of the metadata.
+		assertAllAttributesForMetaAnnotationOverrides(metadata);
+	}
+
+	/**
+	 * @param metadata the metadata for {@link ComposedConfigurationWithAttributeOverridesClass}
+	 */
+	private void assertAttributesForMetaAnnotationOverrides(AnnotationMetadata metadata) {
 		AnnotationAttributes attributes = (AnnotationAttributes) metadata.getAnnotationAttributes(
 			TestComponentScan.class.getName(), false);
 		String[] basePackages = attributes.getStringArray("basePackages");
@@ -123,6 +140,30 @@ private void assertMetaAnnotationOverrides(AnnotationMetadata metadata) {
 		assertThat("length of basePackageClasses[]", basePackageClasses.length, is(0));
 	}
 
+	/**
+	 * @param metadata the metadata for {@link ComposedConfigurationWithAttributeOverridesClass}
+	 */
+	private void assertAllAttributesForMetaAnnotationOverrides(AnnotationMetadata metadata) {
+		MultiValueMap<String, Object> map = metadata.getAllAnnotationAttributes(TestComponentScan.class.getName());
+		List<Object> basePackages = map.get("basePackages");
+		assertThat("length of basePackages list", basePackages.size(), is(1));
+
+		// Ideally, the expected base package should be "org.example.componentscan", but
+		// since Spring's annotation processing currently does not support meta-annotation
+		// attribute overrides when searching for "all attributes", the actual value found
+		// is "bogus".
+		String expectedBasePackage = "bogus";
+		assertThat("basePackages[0]", ((String[]) basePackages.get(0))[0], is(expectedBasePackage));
+
+		List<Object> value = map.get("value");
+		assertThat("length of value list", value.size(), is(1));
+		assertThat("length of 0th value array", ((String[]) value.get(0)).length, is(0));
+
+		List<Object> basePackageClasses = map.get("basePackageClasses");
+		assertThat("length of basePackageClasses list", basePackageClasses.size(), is(1));
+		assertThat("length of 0th basePackageClasses array", ((Class<?>[]) basePackageClasses.get(0)).length, is(0));
+	}
+
 	private void doTestAnnotationInfo(AnnotationMetadata metadata) {
 		assertThat(metadata.getClassName(), is(AnnotatedComponent.class.getName()));
 		assertThat(metadata.isInterface(), is(false));
@@ -318,8 +359,10 @@ public static enum SomeEnum {
 	// SPR-10914
 	public static enum SubclassEnum {
 		FOO {
+		/* Do not delete! This subclassing is intentional. */
 		},
 		BAR {
+		/* Do not delete! This subclassing is intentional. */
 		};
 	}