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 75966d98ee2..8494c0f51d2 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,6 +26,8 @@ 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. @@ -39,14 +41,16 @@ public class AnnotatedElementUtils { public static Set getMetaAnnotationTypes(AnnotatedElement element, String annotationType) { final Set types = new LinkedHashSet(); - process(element, annotationType, new Processor() { + process(element, annotationType, true, new Processor() { + @Override - public Object process(Annotation annotation, int depth) { - if (depth > 0) { + public Object process(Annotation annotation, int metaDepth) { + if (metaDepth > 0) { types.add(annotation.annotationType().getName()); } return null; } + @Override public void postProcess(Annotation annotation, Object result) { } @@ -55,14 +59,16 @@ public class AnnotatedElementUtils { } public static boolean hasMetaAnnotationTypes(AnnotatedElement element, String annotationType) { - return Boolean.TRUE.equals(process(element, annotationType, new Processor() { + return Boolean.TRUE.equals(process(element, annotationType, true, new Processor() { + @Override - public Boolean process(Annotation annotation, int depth) { - if (depth > 0) { - return true; + public Boolean process(Annotation annotation, int metaDepth) { + if (metaDepth > 0) { + return Boolean.TRUE; } return null; } + @Override public void postProcess(Annotation annotation, Boolean result) { } @@ -70,11 +76,13 @@ public class AnnotatedElementUtils { } public static boolean isAnnotated(AnnotatedElement element, String annotationType) { - return Boolean.TRUE.equals(process(element, annotationType, new Processor() { + return Boolean.TRUE.equals(process(element, annotationType, true, new Processor() { + @Override - public Boolean process(Annotation annotation, int depth) { - return true; + public Boolean process(Annotation annotation, int metaDepth) { + return Boolean.TRUE; } + @Override public void postProcess(Annotation annotation, Boolean result) { } @@ -85,20 +93,21 @@ public class AnnotatedElementUtils { return getAnnotationAttributes(element, annotationType, false, false); } - public static AnnotationAttributes getAnnotationAttributes( - AnnotatedElement element, String annotationType, + public static AnnotationAttributes getAnnotationAttributes(AnnotatedElement element, String annotationType, final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) { - return process(element, annotationType, new Processor() { + return process(element, annotationType, true, new Processor() { + @Override - public AnnotationAttributes process(Annotation annotation, int depth) { + 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 = AnnotationUtils.getValue(annotation, key); + if (!VALUE.equals(key)) { + Object value = getValue(annotation, key); if (value != null) { result.put(key, value); } @@ -108,28 +117,33 @@ public class AnnotatedElementUtils { }); } - public static MultiValueMap getAllAnnotationAttributes( - AnnotatedElement element, final String annotationType, - final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) { + public static MultiValueMap getAllAnnotationAttributes(AnnotatedElement element, + final String annotationType) { + return getAllAnnotationAttributes(element, annotationType, false, false); + } + + public static MultiValueMap getAllAnnotationAttributes(AnnotatedElement element, + final String annotationType, final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) { final MultiValueMap attributes = new LinkedMultiValueMap(); - process(element, annotationType, new Processor() { + process(element, annotationType, false, new Processor() { + @Override - public Void process(Annotation annotation, int depth) { + public Void process(Annotation annotation, int metaDepth) { if (annotation.annotationType().getName().equals(annotationType)) { - for (Map.Entry entry : - AnnotationUtils.getAnnotationAttributes( - annotation, classValuesAsString, nestedAnnotationsAsMap).entrySet()) { + for (Map.Entry entry : AnnotationUtils.getAnnotationAttributes(annotation, + classValuesAsString, nestedAnnotationsAsMap).entrySet()) { attributes.add(entry.getKey(), entry.getValue()); } } return null; } + @Override public void postProcess(Annotation annotation, Void result) { for (String key : attributes.keySet()) { - if (!"value".equals(key)) { - Object value = AnnotationUtils.getValue(annotation, key); + if (!VALUE.equals(key)) { + Object value = getValue(annotation, key); if (value != null) { attributes.add(key, value); } @@ -141,45 +155,91 @@ public class AnnotatedElementUtils { } /** - * Process all annotations of the specified annotation type and recursively all - * meta-annotations on the specified type. + * Process all annotations of the specified {@code annotationType} and + * recursively all meta-annotations on the specified {@code element}. + * + *

If the {@code traverseClassHierarchy} flag is {@code true} and the sought + * annotation is neither directly present on the given element nor + * present on the given element as a meta-annotation, then the algorithm will + * recursively search through the class hierarchy of the given element. + * * @param element the annotated element - * @param annotationType the annotation type to find. Only items of the specified - * type or meta-annotations of the specified type will be processed. - * @param processor the processor + * @param annotationType the annotation type to find + * @param traverseClassHierarchy whether or not to traverse up the class + * hierarchy recursively + * @param processor the processor to delegate to * @return the result of the processor */ - private static T process(AnnotatedElement element, String annotationType, Processor processor) { - return doProcess(element, annotationType, processor, new HashSet(), 0); + private static T process(AnnotatedElement element, String annotationType, boolean traverseClassHierarchy, + Processor processor) { + return doProcess(element, annotationType, traverseClassHierarchy, processor, new HashSet(), 0); } - private static T doProcess(AnnotatedElement element, String annotationType, - Processor processor, Set visited, int depth) { + /** + * Perform the search algorithm for the {@link #process} method, avoiding + * endless recursion by tracking which annotated elements have already been + * visited. + * + *

The {@code metaDepth} parameter represents the depth of the annotation + * relative to the initial element. For example, an annotation that is + * present on the element will have a depth of 0; a meta-annotation + * will have a depth of 1; and a meta-meta-annotation will have a depth of 2. + * + * @param element the annotated element + * @param annotationType the annotation type to find + * @param traverseClassHierarchy whether or not to traverse up the class + * hierarchy recursively + * @param processor the processor to delegate to + * @param visited the set of annotated elements that have already been visited + * @param metaDepth the depth of the annotation relative to the initial element + * @return the result of the processor + */ + private static T doProcess(AnnotatedElement element, String annotationType, boolean traverseClassHierarchy, + Processor processor, Set visited, int metaDepth) { if (visited.add(element)) { - for (Annotation annotation : element.getAnnotations()) { - if (annotation.annotationType().getName().equals(annotationType) || depth > 0) { - T result = processor.process(annotation, depth); + + 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); if (result != null) { return result; } - result = doProcess(annotation.annotationType(), annotationType, processor, visited, depth + 1); + result = doProcess(annotation.annotationType(), annotationType, traverseClassHierarchy, processor, + visited, metaDepth + 1); if (result != null) { processor.postProcess(annotation, result); return result; } } } - for (Annotation annotation : element.getAnnotations()) { - if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation)) { - T result = doProcess(annotation.annotationType(), annotationType, processor, visited, depth); + + for (Annotation annotation : annotations) { + if (!isInJavaLangAnnotationPackage(annotation)) { + T result = doProcess(annotation.annotationType(), annotationType, traverseClassHierarchy, + processor, visited, metaDepth); if (result != null) { processor.postProcess(annotation, result); return result; } } } + + if (traverseClassHierarchy && element instanceof Class) { + Class superclass = ((Class) element).getSuperclass(); + if (superclass != null && !superclass.equals(Object.class)) { + T result = doProcess(superclass, annotationType, traverseClassHierarchy, processor, visited, + metaDepth); + if (result != null) { + return result; + } + } + } } + return null; } @@ -192,13 +252,18 @@ public class AnnotatedElementUtils { /** * Called to process the annotation. + * + *

The {@code metaDepth} parameter represents the depth of the + * annotation relative to the initial element. For example, an annotation + * that is present on the element will have a depth of 0; a + * meta-annotation will have a depth of 1; and a meta-meta-annotation + * will have a depth of 2. + * * @param annotation the annotation to process - * @param depth the depth of the annotation relative to the initial match. - * For example, a matched annotation will have a depth of 0, a meta-annotation - * 1 and a meta-meta-annotation 2 + * @param metaDepth the depth of the annotation relative to the initial element * @return the result of the processing or {@code null} to continue */ - T process(Annotation annotation, int depth); + T process(Annotation annotation, int metaDepth); void postProcess(Annotation annotation, T result); } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java index d3e51b7c639..3061648a25e 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java @@ -278,7 +278,7 @@ public abstract class AnnotationUtils { Set visited) { Assert.notNull(clazz, "Class must not be null"); - A annotation = clazz.getAnnotation(annotationType); + A annotation = clazz.getDeclaredAnnotation(annotationType); if (annotation != null) { return annotation; } @@ -288,7 +288,7 @@ public abstract class AnnotationUtils { return annotation; } } - for (Annotation ann : clazz.getAnnotations()) { + for (Annotation ann : clazz.getDeclaredAnnotations()) { if (!isInJavaLangAnnotationPackage(ann) && visited.add(ann)) { annotation = findAnnotation(ann.annotationType(), annotationType, visited); if (annotation != null) { 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 4b8df68c962..2aa3a0abd3b 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 @@ -22,10 +22,13 @@ import java.lang.annotation.Inherited; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.Arrays; import org.junit.Test; +import org.springframework.util.MultiValueMap; import static org.junit.Assert.*; +import static org.springframework.core.annotation.AnnotatedElementUtils.*; /** * Unit tests for {@link AnnotatedElementUtils}. @@ -35,37 +38,82 @@ import static org.junit.Assert.*; */ public class AnnotatedElementUtilsTests { + @Test + public void getAllAnnotationAttributesOnClassWithLocalAnnotation() { + MultiValueMap attributes = getAllAnnotationAttributes(TxConfig.class, + Transactional.class.getName()); + assertNotNull("Annotation attributes map for @Transactional on TxConfig", attributes); + assertEquals("value for TxConfig.", Arrays.asList("TxConfig"), attributes.get("value")); + } + + /** + * If the "value" entry contains both "DerivedTxConfig" AND "TxConfig", then + * the algorithm is accidentally picking up shadowed annotations of the same + * type within the class hierarchy. Such undesirable behavior would cause the + * logic in {@link org.springframework.context.annotation.ProfileCondition} + * to fail. + * + * @see org.springframework.core.env.EnvironmentIntegrationTests#mostSpecificDerivedClassDrivesEnvironment_withDevEnvAndDerivedDevConfigClass + */ + @Test + public void getAllAnnotationAttributesOnClassWithLocalAnnotationThatShadowsAnnotationFromSuperclass() { + MultiValueMap attributes = getAllAnnotationAttributes(DerivedTxConfig.class, + Transactional.class.getName()); + assertNotNull("Annotation attributes map for @Transactional on DerivedTxConfig", attributes); + assertEquals("value for DerivedTxConfig.", Arrays.asList("DerivedTxConfig"), attributes.get("value")); + } + + /** + * Note: this functionality is required by {@link org.springframework.context.annotation.ProfileCondition}. + * + * @see org.springframework.core.env.EnvironmentIntegrationTests + */ + @Test + public void getAllAnnotationAttributesOnClassWithMultipleComposedAnnotations() { + MultiValueMap attributes = getAllAnnotationAttributes(TxFromMultipleComposedAnnotations.class, + Transactional.class.getName()); + assertNotNull("Annotation attributes map for @Transactional on TxFromMultipleComposedAnnotations", attributes); + assertEquals("value for TxFromMultipleComposedAnnotations.", Arrays.asList("TxComposed1", "TxComposed2"), + attributes.get("value")); + } + + @Test + public void getAnnotationAttributesOnClassWithLocalAnnotation() { + AnnotationAttributes attributes = getAnnotationAttributes(TxConfig.class, Transactional.class.getName()); + assertNotNull("Annotation attributes for @Transactional on TxConfig", attributes); + assertEquals("value for TxConfig.", "TxConfig", attributes.getString("value")); + } + + @Test + public void getAnnotationAttributesOnClassWithLocalAnnotationThatShadowsAnnotationFromSuperclass() { + AnnotationAttributes attributes = getAnnotationAttributes(DerivedTxConfig.class, Transactional.class.getName()); + assertNotNull("Annotation attributes for @Transactional on DerivedTxConfig", attributes); + assertEquals("value for DerivedTxConfig.", "DerivedTxConfig", attributes.getString("value")); + } + @Test public void getAnnotationAttributesOnMetaCycleAnnotatedClassWithMissingTargetMetaAnnotation() { - AnnotationAttributes attributes = AnnotatedElementUtils.getAnnotationAttributes(MetaCycleAnnotatedClass.class, + AnnotationAttributes attributes = getAnnotationAttributes(MetaCycleAnnotatedClass.class, Transactional.class.getName()); assertNull("Should not find annotation attributes for @Transactional on MetaCycleAnnotatedClass", attributes); } @Test public void getAnnotationAttributesFavorsInheritedAnnotationsOverMoreLocallyDeclaredComposedAnnotations() { - AnnotationAttributes attributes = AnnotatedElementUtils.getAnnotationAttributes( - SubSubClassWithInheritedAnnotation.class, Transactional.class.getName()); - assertNotNull(attributes); - - // By inspecting SubSubClassWithInheritedAnnotation, one might expect that the - // readOnly flag should be true, since the immediate superclass is annotated with - // @Composed2; however, with the current implementation the readOnly flag will be - // false since @Transactional is declared as @Inherited. - assertFalse("readOnly flag for SubSubClassWithInheritedAnnotation", attributes.getBoolean("readOnly")); + AnnotationAttributes attributes = getAnnotationAttributes(SubSubClassWithInheritedAnnotation.class, + Transactional.class.getName()); + assertNotNull("AnnotationAttributes for @Transactional on SubSubClassWithInheritedAnnotation", attributes); + assertEquals("readOnly flag for SubSubClassWithInheritedAnnotation.", true, attributes.getBoolean("readOnly")); } @Test public void getAnnotationAttributesFavorsInheritedComposedAnnotationsOverMoreLocallyDeclaredComposedAnnotations() { - AnnotationAttributes attributes = AnnotatedElementUtils.getAnnotationAttributes( - SubSubClassWithInheritedComposedAnnotation.class, Transactional.class.getName()); - assertNotNull(attributes); - - // By inspecting SubSubClassWithInheritedComposedAnnotation, one might expect that - // the readOnly flag should be true, since the immediate superclass is annotated - // with @Composed2; however, with the current implementation the readOnly flag - // will be false since @Composed1 is declared as @Inherited. - assertFalse("readOnly flag", attributes.getBoolean("readOnly")); + AnnotationAttributes attributes = getAnnotationAttributes(SubSubClassWithInheritedComposedAnnotation.class, + Transactional.class.getName()); + assertNotNull("AnnotationAttributtes for @Transactional on SubSubClassWithInheritedComposedAnnotation.", + attributes); + assertEquals("readOnly flag for SubSubClassWithInheritedComposedAnnotation.", true, + attributes.getBoolean("readOnly")); } @@ -96,12 +144,16 @@ public class AnnotatedElementUtilsTests { static class MetaCycleAnnotatedClass { } + // ------------------------------------------------------------------------- + @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) @Documented @Inherited @interface Transactional { + String value() default ""; + boolean readOnly() default false; } @@ -120,6 +172,18 @@ public class AnnotatedElementUtilsTests { @interface Composed2 { } + @Transactional("TxComposed1") + @Retention(RetentionPolicy.RUNTIME) + @interface TxComposed1 { + } + + @Transactional("TxComposed2") + @Retention(RetentionPolicy.RUNTIME) + @interface TxComposed2 { + } + + // ------------------------------------------------------------------------- + @Transactional static class ClassWithInheritedAnnotation { } @@ -142,4 +206,17 @@ public class AnnotatedElementUtilsTests { static class SubSubClassWithInheritedComposedAnnotation extends SubClassWithInheritedComposedAnnotation { } + @Transactional("TxConfig") + static class TxConfig { + } + + @Transactional("DerivedTxConfig") + static class DerivedTxConfig extends TxConfig { + } + + @TxComposed1 + @TxComposed2 + static class TxFromMultipleComposedAnnotations { + } + } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java index e613a619314..a03d4085e80 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java @@ -111,30 +111,26 @@ public class AnnotationUtilsTests { assertEquals("meta1", component.value()); } + /** + * @since 4.0.3 + */ @Test public void findAnnotationFavorsInheritedAnnotationsOverMoreLocallyDeclaredComposedAnnotations() { Transactional transactional = AnnotationUtils.findAnnotation(SubSubClassWithInheritedAnnotation.class, Transactional.class); assertNotNull(transactional); - - // By inspecting SubSubClassWithInheritedAnnotation, one might expect that the - // readOnly flag should be true, since the immediate superclass is annotated with - // @Composed2; however, with the current implementation the readOnly flag will be - // false since @Transactional is declared as @Inherited. - assertFalse("readOnly flag for SubSubClassWithInheritedAnnotation", transactional.readOnly()); + assertTrue("readOnly flag for SubSubClassWithInheritedAnnotation", transactional.readOnly()); } + /** + * @since 4.0.3 + */ @Test public void findAnnotationFavorsInheritedComposedAnnotationsOverMoreLocallyDeclaredComposedAnnotations() { Component component = AnnotationUtils.findAnnotation(SubSubClassWithInheritedMetaAnnotation.class, Component.class); assertNotNull(component); - - // By inspecting SubSubClassWithInheritedMetaAnnotation, one might expect that - // "meta2" should be found, since the immediate superclass is annotated with - // @Meta2; however, with the current implementation "meta1" will be found since - // @Meta1 is declared as @Inherited. - assertEquals("meta1", component.value()); + assertEquals("meta2", component.value()); } @Test