From a2f1169e820c76df7f06f2b2b921b184b8775b45 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 25 Mar 2014 17:48:06 +0100 Subject: [PATCH] Don't detect annotations on superclass in StAnMeta Changes introduced in conjunction with issue SPR-11475 altered the behavior of StandardAnnotationMetadata such that annotations could be detected on superclasses, specifically in the case where the AnnotatedElementUtils.getAllAnnotationAttributes() method is invoked to obtain multiple annotations of the same type (on the lowest level in the class hierarchy), as is the case for @Profile and @Conditional. This commit partially reverts these changes as follows: - All methods in AnnotatedElementUtils now set the traverseClassHierarchy to false, thereby effectively reverting the changes made in commit 1d30bf83a0d6f303ce38526286b90172c3c596c6. Note, however, that the changes made to AnnotationUtils remain in place. - Introduced tests in AnnotationMetadataTests that verify behavior present in Spring Framework 4.0.2 and earlier. - Updated tests in AnnotatedElementUtilsTests so that they pass against the reverted changes (i.e., align with the behavior present in Spring Framework 4.0.2 and earlier). - Refined Javadoc in AnnotationMetadata with regard to annotations being "present" vs. "defined". - Refined Javadoc in AnnotatedTypeMetadata. Issue: SPR-11475, SPR-11595 --- .../annotation/AnnotatedElementUtils.java | 40 ++++++++++++------- .../core/type/AnnotatedTypeMetadata.java | 10 +++-- .../core/type/AnnotationMetadata.java | 24 ++++++----- .../AnnotatedElementUtilsTests.java | 10 ++++- .../core/type/AnnotationMetadataTests.java | 33 +++++++++++++++ 5 files changed, 86 insertions(+), 31 deletions(-) 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 78fc584adb..e67cb62fd6 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 @@ -26,8 +26,6 @@ import java.util.Set; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import static org.springframework.core.annotation.AnnotationUtils.*; - /** * Utility class used to collect all annotation values including those declared on * meta-annotations. @@ -41,7 +39,8 @@ public class AnnotatedElementUtils { public static Set getMetaAnnotationTypes(AnnotatedElement element, String annotationType) { final Set types = new LinkedHashSet(); - process(element, annotationType, true, new Processor() { + process(element, annotationType, false, new Processor() { + @Override public Object process(Annotation annotation, int metaDepth) { if (metaDepth > 0) { @@ -49,6 +48,7 @@ public class AnnotatedElementUtils { } return null; } + @Override public void postProcess(Annotation annotation, Object result) { } @@ -57,7 +57,8 @@ public class AnnotatedElementUtils { } public static boolean hasMetaAnnotationTypes(AnnotatedElement element, String annotationType) { - return Boolean.TRUE.equals(process(element, annotationType, true, new Processor() { + return Boolean.TRUE.equals(process(element, annotationType, false, new Processor() { + @Override public Boolean process(Annotation annotation, int metaDepth) { if (metaDepth > 0) { @@ -65,6 +66,7 @@ public class AnnotatedElementUtils { } return null; } + @Override public void postProcess(Annotation annotation, Boolean result) { } @@ -72,11 +74,13 @@ public class AnnotatedElementUtils { } public static boolean isAnnotated(AnnotatedElement element, String annotationType) { - return Boolean.TRUE.equals(process(element, annotationType, true, new Processor() { + return Boolean.TRUE.equals(process(element, annotationType, false, new Processor() { + @Override public Boolean process(Annotation annotation, int metaDepth) { return Boolean.TRUE; } + @Override public void postProcess(Annotation annotation, Boolean result) { } @@ -90,16 +94,18 @@ public class AnnotatedElementUtils { public static AnnotationAttributes getAnnotationAttributes(AnnotatedElement element, String annotationType, final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) { - return process(element, annotationType, true, new Processor() { + return process(element, annotationType, false, new Processor() { + @Override public AnnotationAttributes process(Annotation annotation, int metaDepth) { return AnnotationUtils.getAnnotationAttributes(annotation, classValuesAsString, nestedAnnotationsAsMap); } + @Override public void postProcess(Annotation annotation, AnnotationAttributes result) { for (String key : result.keySet()) { - if (!VALUE.equals(key)) { - Object value = getValue(annotation, key); + if (!AnnotationUtils.VALUE.equals(key)) { + Object value = AnnotationUtils.getValue(annotation, key); if (value != null) { result.put(key, value); } @@ -109,7 +115,8 @@ public class AnnotatedElementUtils { }); } - public static MultiValueMap getAllAnnotationAttributes(AnnotatedElement element, String annotationType) { + public static MultiValueMap getAllAnnotationAttributes(AnnotatedElement element, + String annotationType) { return getAllAnnotationAttributes(element, annotationType, false, false); } @@ -118,6 +125,7 @@ public class AnnotatedElementUtils { final MultiValueMap attributes = new LinkedMultiValueMap(); process(element, annotationType, false, new Processor() { + @Override public Void process(Annotation annotation, int metaDepth) { if (annotation.annotationType().getName().equals(annotationType)) { @@ -128,11 +136,12 @@ public class AnnotatedElementUtils { } return null; } + @Override public void postProcess(Annotation annotation, Void result) { for (String key : attributes.keySet()) { - if (!VALUE.equals(key)) { - Object value = getValue(annotation, key); + if (!AnnotationUtils.VALUE.equals(key)) { + Object value = AnnotationUtils.getValue(annotation, key); if (value != null) { attributes.add(key, value); } @@ -161,7 +170,8 @@ public class AnnotatedElementUtils { Processor processor) { try { - return doProcess(element, annotationType, traverseClassHierarchy, processor, new HashSet(), 0); + return doProcess(element, annotationType, traverseClassHierarchy, processor, + new HashSet(), 0); } catch (Throwable ex) { throw new IllegalStateException("Failed to introspect annotations: " + element, ex); @@ -189,8 +199,8 @@ public class AnnotatedElementUtils { Processor processor, Set visited, int metaDepth) { if (visited.add(element)) { - Annotation[] annotations = - (traverseClassHierarchy ? element.getDeclaredAnnotations() : element.getAnnotations()); + Annotation[] annotations = (traverseClassHierarchy ? element.getDeclaredAnnotations() + : element.getAnnotations()); for (Annotation annotation : annotations) { if (annotation.annotationType().getName().equals(annotationType) || metaDepth > 0) { T result = processor.process(annotation, metaDepth); @@ -206,7 +216,7 @@ public class AnnotatedElementUtils { } } for (Annotation annotation : annotations) { - if (!isInJavaLangAnnotationPackage(annotation)) { + if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation)) { T result = doProcess(annotation.annotationType(), annotationType, traverseClassHierarchy, processor, visited, metaDepth); if (result != null) { diff --git a/spring-core/src/main/java/org/springframework/core/type/AnnotatedTypeMetadata.java b/spring-core/src/main/java/org/springframework/core/type/AnnotatedTypeMetadata.java index 60c9cf2647..802896d0b7 100644 --- a/spring-core/src/main/java/org/springframework/core/type/AnnotatedTypeMetadata.java +++ b/spring-core/src/main/java/org/springframework/core/type/AnnotatedTypeMetadata.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,6 +30,7 @@ import org.springframework.util.MultiValueMap; * @author Mark Pollack * @author Chris Beams * @author Phillip Webb + * @author Sam Brannen * @since 4.0 * @see AnnotationMetadata * @see MethodMetadata @@ -73,17 +74,20 @@ public interface AnnotatedTypeMetadata { /** * Retrieve all attributes of all annotations of the given type, if any (i.e. if - * defined on the underlying method, as direct annotation or as meta-annotation). + * defined on the underlying type ({@link AnnotationMetadata class} or + * {@link MethodMetadata method}), as direct annotation or as meta-annotation). * @param annotationType the annotation type to look for * @return a MultiMap of attributes, with the attribute name as key (e.g. "value") * and a list of the defined attribute values as Map value. This return value will * be {@code null} if no matching annotation is defined. + * @see #getAllAnnotationAttributes(String, boolean) */ MultiValueMap getAllAnnotationAttributes(String annotationType); /** * Retrieve all attributes of all annotations of the given type, if any (i.e. if - * defined on the underlying method, as direct annotation or as meta-annotation). + * defined on the underlying type ({@link AnnotationMetadata class} or + * {@link MethodMetadata method}), as direct annotation or as meta-annotation). * @param annotationType the annotation type to look for * @param classValuesAsString whether to convert class references to String * @return a MultiMap of attributes, with the attribute name as key (e.g. "value") diff --git a/spring-core/src/main/java/org/springframework/core/type/AnnotationMetadata.java b/spring-core/src/main/java/org/springframework/core/type/AnnotationMetadata.java index 435e934415..8c2ef4ca10 100644 --- a/spring-core/src/main/java/org/springframework/core/type/AnnotationMetadata.java +++ b/spring-core/src/main/java/org/springframework/core/type/AnnotationMetadata.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ import java.util.Set; * @author Juergen Hoeller * @author Mark Fisher * @author Phillip Webb + * @author Sam Brannen * @since 2.5 * @see StandardAnnotationMetadata * @see org.springframework.core.type.classreading.MetadataReader#getAnnotationMetadata() @@ -33,32 +34,33 @@ import java.util.Set; public interface AnnotationMetadata extends ClassMetadata, AnnotatedTypeMetadata { /** - * Return the names of all annotation types defined on the underlying class. + * Return the names of all annotation types that are present on the + * underlying class. * @return the annotation type names */ Set getAnnotationTypes(); /** - * Return the names of all meta-annotation types defined on the - * given annotation type of the underlying class. + * Return the names of all meta-annotation types present on the + * given annotation type on the underlying class. * @param annotationType the meta-annotation type to look for * @return the meta-annotation type names */ Set getMetaAnnotationTypes(String annotationType); /** - * Determine whether the underlying class has an annotation of the given - * type defined. + * Determine whether an annotation of the given type is present on + * the underlying class. * @param annotationType the annotation type to look for - * @return whether a matching annotation is defined + * @return whether a matching annotation is present */ boolean hasAnnotation(String annotationType); /** - * Determine whether the underlying class has an annotation that - * is itself annotated with the meta-annotation of the given type. + * Determine whether the underlying class has an annotation that is itself + * annotated with the meta-annotation of the given type. * @param metaAnnotationType the meta-annotation type to look for - * @return whether a matching meta-annotation is defined + * @return whether a matching meta-annotation is present */ boolean hasMetaAnnotation(String metaAnnotationType); @@ -74,7 +76,7 @@ public interface AnnotationMetadata extends ClassMetadata, AnnotatedTypeMetadata *

For any returned method, {@link MethodMetadata#isAnnotated} will * return {@code true} for the given annotation type. * @param annotationType the annotation type to look for - * @return a Set of {@link MethodMetadata} for methods that have a matching + * @return a set of {@link MethodMetadata} for methods that have a matching * annotation. The return value will be an empty set if no methods match * the annotation type. */ 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 2aa3a0abd3..7c75a9e9ca 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 @@ -103,7 +103,10 @@ public class AnnotatedElementUtilsTests { AnnotationAttributes attributes = getAnnotationAttributes(SubSubClassWithInheritedAnnotation.class, Transactional.class.getName()); assertNotNull("AnnotationAttributes for @Transactional on SubSubClassWithInheritedAnnotation", attributes); - assertEquals("readOnly flag for SubSubClassWithInheritedAnnotation.", true, attributes.getBoolean("readOnly")); + + // TODO [SPR-11475] Set expected to true. + boolean expected = false; + assertEquals("readOnly flag for SubSubClassWithInheritedAnnotation.", expected, attributes.getBoolean("readOnly")); } @Test @@ -112,7 +115,10 @@ public class AnnotatedElementUtilsTests { Transactional.class.getName()); assertNotNull("AnnotationAttributtes for @Transactional on SubSubClassWithInheritedComposedAnnotation.", attributes); - assertEquals("readOnly flag for SubSubClassWithInheritedComposedAnnotation.", true, + + // TODO [SPR-11475] Set expected to true. + boolean expected = false; + assertEquals("readOnly flag for SubSubClassWithInheritedComposedAnnotation.", expected, attributes.getBoolean("readOnly")); } diff --git a/spring-core/src/test/java/org/springframework/core/type/AnnotationMetadataTests.java b/spring-core/src/test/java/org/springframework/core/type/AnnotationMetadataTests.java index c60594291a..914229c808 100644 --- a/spring-core/src/test/java/org/springframework/core/type/AnnotationMetadataTests.java +++ b/spring-core/src/test/java/org/springframework/core/type/AnnotationMetadataTests.java @@ -64,6 +64,20 @@ public class AnnotationMetadataTests { doTestMethodAnnotationInfo(metadata); } + @Test + public void standardAnnotationMetadataForSubclass() throws Exception { + AnnotationMetadata metadata = new StandardAnnotationMetadata(AnnotatedComponentSubClass.class, true); + doTestSubClassAnnotationInfo(metadata); + } + + @Test + public void asmAnnotationMetadataForSubclass() throws Exception { + MetadataReaderFactory metadataReaderFactory = new SimpleMetadataReaderFactory(); + MetadataReader metadataReader = metadataReaderFactory.getMetadataReader(AnnotatedComponentSubClass.class.getName()); + AnnotationMetadata metadata = metadataReader.getAnnotationMetadata(); + doTestSubClassAnnotationInfo(metadata); + } + /** * In order to preserve backward-compatibility, {@link StandardAnnotationMetadata} * defaults to return nested annotations and annotation arrays as actual @@ -211,6 +225,21 @@ public class AnnotationMetadataTests { } } + private void doTestSubClassAnnotationInfo(AnnotationMetadata metadata) { + assertThat(metadata.getClassName(), is(AnnotatedComponentSubClass.class.getName())); + assertThat(metadata.isAnnotated(Component.class.getName()), is(false)); + assertThat(metadata.isAnnotated(Scope.class.getName()), is(false)); + assertThat(metadata.isAnnotated(SpecialAttr.class.getName()), is(false)); + assertThat(metadata.hasAnnotation(Component.class.getName()), is(false)); + assertThat(metadata.hasAnnotation(Scope.class.getName()), is(false)); + assertThat(metadata.hasAnnotation(SpecialAttr.class.getName()), is(false)); + assertThat(metadata.getAnnotationTypes().size(), is(0)); + assertThat(metadata.getAnnotationAttributes(Component.class.getName()), nullValue()); + assertThat(metadata.getAnnotatedMethods(DirectAnnotation.class.getName()).size(), equalTo(0)); + assertThat(metadata.isAnnotated(IsAnnotatedAnnotation.class.getName()), equalTo(false)); + assertThat(metadata.getAllAnnotationAttributes(DirectAnnotation.class.getName()), nullValue()); + } + private void doTestMethodAnnotationInfo(AnnotationMetadata classMetadata) { Set methods = classMetadata.getAnnotatedMethods(TestAutowired.class.getName()); assertThat(methods.size(), is(1)); @@ -317,6 +346,10 @@ public class AnnotationMetadataTests { } } + private static class AnnotatedComponentSubClass extends AnnotatedComponent { + + } + @Target(ElementType.TYPE) @Retention(RetentionPolicy.RUNTIME) @Component