Favor 'local' annotations over inherited ones

Prior to this commit, the implementations of findAnnotation() in
AnnotationUtils and getAnnotationAttributes() in AnnotatedElementUtils
favored inherited annotations and inherited composed annotations over
composed annotations that are declared closer to the starting class
passed to these methods.

This commit addresses this issue as follows:

- Refactored AnnotationUtils to use getDeclaredAnnotation() and
  getDeclaredAnnotations() instead of getAnnotation() and
  getAnnotations() where appropriate.

- AnnotatedElementUtils.doProcess() supports a traverseClassHierarchy
  flag to control whether the class hierarchy should be traversed,
  using getDeclaredAnnotations() instead of getAnnotations() if the
  flag is true.

- Overhauled Javadoc in AnnotatedElementUtils.

Issue: SPR-11475

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<String> getMetaAnnotationTypes(AnnotatedElement element, String annotationType) {
 		final Set<String> types = new LinkedHashSet<String>();
-		process(element, annotationType, new Processor<Object>() {
+		process(element, annotationType, true, new Processor<Object>() {
+
 			@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<Boolean>() {
+		return Boolean.TRUE.equals(process(element, annotationType, true, new Processor<Boolean>() {
+
 			@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<Boolean>() {
+		return Boolean.TRUE.equals(process(element, annotationType, true, new Processor<Boolean>() {
+
 			@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<AnnotationAttributes>() {
+		return process(element, annotationType, true, new Processor<AnnotationAttributes>() {
+
 			@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<String, Object> getAllAnnotationAttributes(
-			AnnotatedElement element, final String annotationType,
-			final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) {
+	public static MultiValueMap<String, Object> getAllAnnotationAttributes(AnnotatedElement element,
+			final String annotationType) {
+		return getAllAnnotationAttributes(element, annotationType, false, false);
+	}
+
+	public static MultiValueMap<String, Object> getAllAnnotationAttributes(AnnotatedElement element,
+			final String annotationType, final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) {
 
 		final MultiValueMap<String, Object> attributes = new LinkedMultiValueMap<String, Object>();
-		process(element, annotationType, new Processor<Void>() {
+		process(element, annotationType, false, new Processor<Void>() {
+
 			@Override
-			public Void process(Annotation annotation, int depth) {
+			public Void process(Annotation annotation, int metaDepth) {
 				if (annotation.annotationType().getName().equals(annotationType)) {
-					for (Map.Entry<String, Object> entry :
-							AnnotationUtils.getAnnotationAttributes(
-									annotation, classValuesAsString, nestedAnnotationsAsMap).entrySet()) {
+					for (Map.Entry<String, Object> 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}.
+	 *
+	 * <p>If the {@code traverseClassHierarchy} flag is {@code true} and the sought
+	 * annotation is neither <em>directly present</em> 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> T process(AnnotatedElement element, String annotationType, Processor<T> processor) {
-		return doProcess(element, annotationType, processor, new HashSet<AnnotatedElement>(), 0);
+	private static <T> T process(AnnotatedElement element, String annotationType, boolean traverseClassHierarchy,
+			Processor<T> processor) {
+		return doProcess(element, annotationType, traverseClassHierarchy, processor, new HashSet<AnnotatedElement>(), 0);
 	}
 
-	private static <T> T doProcess(AnnotatedElement element, String annotationType,
-			Processor<T> processor, Set<AnnotatedElement> visited, int depth) {
+	/**
+	 * Perform the search algorithm for the {@link #process} method, avoiding
+	 * endless recursion by tracking which annotated elements have already been
+	 * <em>visited</em>.
+	 * 
+	 * <p>The {@code metaDepth} parameter represents the depth of the annotation
+	 * relative to the initial element. For example, an annotation that is
+	 * <em>present</em> 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> T doProcess(AnnotatedElement element, String annotationType, boolean traverseClassHierarchy,
+			Processor<T> processor, Set<AnnotatedElement> 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.
+		 * 
+		 * <p>The {@code metaDepth} parameter represents the depth of the
+		 * annotation relative to the initial element. For example, an annotation
+		 * that is <em>present</em> 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);
 	}

spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java

@@ -278,7 +278,7 @@ public abstract class AnnotationUtils {
 			Set<Annotation> 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) {

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<String, Object> 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<String, Object> 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<String, Object> 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 {
+	}
+
 }

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