From 796af90ba7a87198e01b8b15246db9ccfe30c20f Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 21 Mar 2014 15:26:13 +0100 Subject: [PATCH] Polish ASM-based annotation processing - AnnotationAttributesReadingVisitor no longer processes annotations from the java.lang.annotation package. - Simplified logic in AnnotationReadingVisitorUtils getMergedAnnotationAttributes(). Issue: SPR-11574 --- .../AnnotationAttributesReadingVisitor.java | 53 +++++++++++-------- .../AnnotationMetadataReadingVisitor.java | 14 ++--- .../AnnotationReadingVisitorUtils.java | 48 ++++++++--------- 3 files changed, 61 insertions(+), 54 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java index 48d88752a0..76403e1490 100644 --- a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java +++ b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java @@ -130,8 +130,8 @@ final class RecursiveAnnotationArrayVisitor extends AbstractRecursiveAnnotationV } else { Class arrayClass = newValue.getClass(); - if(Enum.class.isAssignableFrom(arrayClass)) { - while(arrayClass.getSuperclass() != null && !arrayClass.isEnum()) { + if (Enum.class.isAssignableFrom(arrayClass)) { + while (arrayClass.getSuperclass() != null && !arrayClass.isEnum()) { arrayClass = arrayClass.getSuperclass(); } } @@ -153,8 +153,8 @@ final class RecursiveAnnotationArrayVisitor extends AbstractRecursiveAnnotationV @Override public void visitEnd() { if (!this.allNestedAttributes.isEmpty()) { - this.attributes.put(this.attributeName, this.allNestedAttributes.toArray( - new AnnotationAttributes[this.allNestedAttributes.size()])); + this.attributes.put(this.attributeName, + this.allNestedAttributes.toArray(new AnnotationAttributes[this.allNestedAttributes.size()])); } } } @@ -169,8 +169,9 @@ class RecursiveAnnotationAttributesVisitor extends AbstractRecursiveAnnotationVi private final String annotationType; - public RecursiveAnnotationAttributesVisitor( - String annotationType, AnnotationAttributes attributes, ClassLoader classLoader) { + + public RecursiveAnnotationAttributesVisitor(String annotationType, AnnotationAttributes attributes, + ClassLoader classLoader) { super(classLoader, attributes); this.annotationType = annotationType; } @@ -182,8 +183,8 @@ class RecursiveAnnotationAttributesVisitor extends AbstractRecursiveAnnotationVi doVisitEnd(annotationClass); } catch (ClassNotFoundException ex) { - this.logger.debug("Failed to class-load type while reading annotation metadata. " + - "This is a non-fatal error, but certain annotation metadata may be unavailable.", ex); + this.logger.debug("Failed to class-load type while reading annotation metadata. " + + "This is a non-fatal error, but certain annotation metadata may be unavailable.", ex); } } @@ -192,8 +193,9 @@ class RecursiveAnnotationAttributesVisitor extends AbstractRecursiveAnnotationVi } private void registerDefaultValues(Class annotationClass) { - // Only do further scanning for public annotations; we'd run into IllegalAccessExceptions - // otherwise, and don't want to mess with accessibility in a SecurityManager environment. + // Only do further scanning for public annotations; we'd run into + // IllegalAccessExceptions otherwise, and we don't want to mess with + // accessibility in a SecurityManager environment. if (Modifier.isPublic(annotationClass.getModifiers())) { // Check declared default values of attributes in the annotation type. Method[] annotationAttributes = annotationClass.getMethods(); @@ -202,15 +204,15 @@ class RecursiveAnnotationAttributesVisitor extends AbstractRecursiveAnnotationVi Object defaultValue = annotationAttribute.getDefaultValue(); if (defaultValue != null && !this.attributes.containsKey(attributeName)) { if (defaultValue instanceof Annotation) { - defaultValue = AnnotationAttributes.fromMap( - AnnotationUtils.getAnnotationAttributes((Annotation) defaultValue, false, true)); + defaultValue = AnnotationAttributes.fromMap(AnnotationUtils.getAnnotationAttributes( + (Annotation) defaultValue, false, true)); } else if (defaultValue instanceof Annotation[]) { Annotation[] realAnnotations = (Annotation[]) defaultValue; AnnotationAttributes[] mappedAnnotations = new AnnotationAttributes[realAnnotations.length]; for (int i = 0; i < realAnnotations.length; i++) { - mappedAnnotations[i] = AnnotationAttributes.fromMap( - AnnotationUtils.getAnnotationAttributes(realAnnotations[i], false, true)); + mappedAnnotations[i] = AnnotationAttributes.fromMap(AnnotationUtils.getAnnotationAttributes( + realAnnotations[i], false, true)); } defaultValue = mappedAnnotations; } @@ -233,6 +235,7 @@ class RecursiveAnnotationAttributesVisitor extends AbstractRecursiveAnnotationVi * @author Juergen Hoeller * @author Chris Beams * @author Phillip Webb + * @author Sam Brannen * @since 3.0 */ final class AnnotationAttributesReadingVisitor extends RecursiveAnnotationAttributesVisitor { @@ -244,9 +247,9 @@ final class AnnotationAttributesReadingVisitor extends RecursiveAnnotationAttrib private final Map> metaAnnotationMap; - public AnnotationAttributesReadingVisitor( - String annotationType, MultiValueMap attributesMap, - Map> metaAnnotationMap, ClassLoader classLoader) { + public AnnotationAttributesReadingVisitor(String annotationType, + MultiValueMap attributesMap, Map> metaAnnotationMap, + ClassLoader classLoader) { super(annotationType, new AnnotationAttributes(), classLoader); this.annotationType = annotationType; @@ -267,7 +270,9 @@ final class AnnotationAttributesReadingVisitor extends RecursiveAnnotationAttrib } Set metaAnnotationTypeNames = new LinkedHashSet(); for (Annotation metaAnnotation : annotationClass.getAnnotations()) { - recursivelyCollectMetaAnnotations(metaAnnotationTypeNames, metaAnnotation); + if (!AnnotationUtils.isInJavaLangAnnotationPackage(metaAnnotation)) { + recursivelyCollectMetaAnnotations(metaAnnotationTypeNames, metaAnnotation); + } } if (this.metaAnnotationMap != null) { this.metaAnnotationMap.put(annotationClass.getName(), metaAnnotationTypeNames); @@ -275,16 +280,18 @@ final class AnnotationAttributesReadingVisitor extends RecursiveAnnotationAttrib } private void recursivelyCollectMetaAnnotations(Set visited, Annotation annotation) { - if (visited.add(annotation.annotationType().getName())) { - // Only do further scanning for public annotations; we'd run into IllegalAccessExceptions - // otherwise, and don't want to mess with accessibility in a SecurityManager environment. + String annotationName = annotation.annotationType().getName(); + if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation) && visited.add(annotationName)) { + // Only do further scanning for public annotations; we'd run into + // IllegalAccessExceptions otherwise, and we don't want to mess with + // accessibility in a SecurityManager environment. if (Modifier.isPublic(annotation.annotationType().getModifiers())) { - this.attributesMap.add(annotation.annotationType().getName(), - AnnotationUtils.getAnnotationAttributes(annotation, false, true)); + this.attributesMap.add(annotationName, AnnotationUtils.getAnnotationAttributes(annotation, false, true)); for (Annotation metaMetaAnnotation : annotation.annotationType().getAnnotations()) { recursivelyCollectMetaAnnotations(visited, metaMetaAnnotation); } } } } + } diff --git a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationMetadataReadingVisitor.java b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationMetadataReadingVisitor.java index 4abd7d869b..46946f5543 100644 --- a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationMetadataReadingVisitor.java +++ b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationMetadataReadingVisitor.java @@ -53,11 +53,11 @@ public class AnnotationMetadataReadingVisitor extends ClassMetadataReadingVisito protected final Map> metaAnnotationMap = new LinkedHashMap>(4); /** - * Declared as a {@link LinkedMultiValueMap} instead of {@link MultiValueMap} - * in order to ensure that ordering of entries is enforced. + * Declared as a {@link LinkedMultiValueMap} instead of a {@link MultiValueMap} + * to ensure that the hierarchical ordering of the entries is preserved. * @see AnnotationReadingVisitorUtils#getMergedAnnotationAttributes(LinkedMultiValueMap, String) */ - protected final LinkedMultiValueMap attributeMap = new LinkedMultiValueMap( + protected final LinkedMultiValueMap attributesMap = new LinkedMultiValueMap( 4); protected final Set methodMetadataSet = new LinkedHashSet(4); @@ -77,7 +77,7 @@ public class AnnotationMetadataReadingVisitor extends ClassMetadataReadingVisito public AnnotationVisitor visitAnnotation(final String desc, boolean visible) { String className = Type.getType(desc).getClassName(); this.annotationSet.add(className); - return new AnnotationAttributesReadingVisitor(className, this.attributeMap, this.metaAnnotationMap, this.classLoader); + return new AnnotationAttributesReadingVisitor(className, this.attributesMap, this.metaAnnotationMap, this.classLoader); } @@ -109,7 +109,7 @@ public class AnnotationMetadataReadingVisitor extends ClassMetadataReadingVisito @Override public boolean isAnnotated(String annotationType) { - return this.attributeMap.containsKey(annotationType); + return this.attributesMap.containsKey(annotationType); } @Override @@ -119,7 +119,7 @@ public class AnnotationMetadataReadingVisitor extends ClassMetadataReadingVisito @Override public AnnotationAttributes getAnnotationAttributes(String annotationType, boolean classValuesAsString) { - AnnotationAttributes raw = AnnotationReadingVisitorUtils.getMergedAnnotationAttributes(this.attributeMap, + AnnotationAttributes raw = AnnotationReadingVisitorUtils.getMergedAnnotationAttributes(this.attributesMap, annotationType); return AnnotationReadingVisitorUtils.convertClassValues(this.classLoader, raw, classValuesAsString); } @@ -132,7 +132,7 @@ public class AnnotationMetadataReadingVisitor extends ClassMetadataReadingVisito @Override public MultiValueMap getAllAnnotationAttributes(String annotationType, boolean classValuesAsString) { MultiValueMap allAttributes = new LinkedMultiValueMap(); - List attributes = this.attributeMap.get(annotationType); + List attributes = this.attributesMap.get(annotationType); if (attributes == null) { return null; } diff --git a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java index 88d47e7433..736d608826 100644 --- a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java +++ b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java @@ -18,6 +18,7 @@ package org.springframework.core.type.classreading; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -26,7 +27,7 @@ import org.springframework.asm.Type; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.util.LinkedMultiValueMap; -import static org.springframework.core.annotation.AnnotationUtils.*; +import static org.springframework.core.annotation.AnnotationUtils.VALUE; /** * Internal utility class used when reading annotations. @@ -98,22 +99,22 @@ abstract class AnnotationReadingVisitorUtils { /** * Retrieve the merged attributes of the annotation of the given type, if any, - * from the supplied {@code attributeMap}. + * from the supplied {@code attributesMap}. *

Annotation attribute values appearing lower in the annotation * hierarchy (i.e., closer to the declaring class) will override those * defined higher in the annotation hierarchy. - * @param attributeMap the map of annotation attribute lists, keyed by - * annotation type - * @param annotationType the annotation type to look for + * @param attributesMap the map of annotation attribute lists, keyed by + * annotation type name + * @param annotationType the name of the annotation type to look for * @return the merged annotation attributes; or {@code null} if no matching - * annotation is present in the {@code attributeMap} + * annotation is present in the {@code attributesMap} * @since 4.0.3 */ public static AnnotationAttributes getMergedAnnotationAttributes( - LinkedMultiValueMap attributeMap, String annotationType) { + LinkedMultiValueMap attributesMap, String annotationType) { // Get the unmerged list of attributes for the target annotation. - List attributesList = attributeMap.get(annotationType); + List attributesList = attributesMap.get(annotationType); if (attributesList == null || attributesList.isEmpty()) { return null; } @@ -121,28 +122,27 @@ abstract class AnnotationReadingVisitorUtils { // To start with, we populate the results with all attribute values from the // target annotation. AnnotationAttributes results = attributesList.get(0); - Set supportedAttributeNames = results.keySet(); + + Set overridableAttributeNames = new HashSet(results.keySet()); + overridableAttributeNames.remove(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 meta-annotation hierarchy. - List annotationTypes = new ArrayList(attributeMap.keySet()); + // "down" the annotation hierarchy. + List annotationTypes = new ArrayList(attributesMap.keySet()); Collections.reverse(annotationTypes); for (String currentAnnotationType : annotationTypes) { - if (!currentAnnotationType.startsWith("java.lang.annotation")) { - for (String attributeName : supportedAttributeNames) { - if (!VALUE.equals(attributeName)) { - List currentAttributes = attributeMap.get(currentAnnotationType); - if (currentAttributes != null && !currentAttributes.isEmpty()) { - Object value = currentAttributes.get(0).get(attributeName); - if (value != null) { - // Overwrite value from target annotation with the value - // from an attribute of the same name found lower in the - // meta-annotation hierarchy. - results.put(attributeName, value); - } - } + List currentAttributesList = attributesMap.get(currentAnnotationType); + if (currentAttributesList != null && !currentAttributesList.isEmpty()) { + AnnotationAttributes currentAttributes = currentAttributesList.get(0); + for (String overridableAttributeName : overridableAttributeNames) { + Object value = currentAttributes.get(overridableAttributeName); + if (value != null) { + // Store the value, potentially overriding a value from an + // attribute of the same name found higher in the annotation + // hierarchy. + results.put(overridableAttributeName, value); } } }