Find annotations on implemented generic superclass methods as well

Includes Java 8 getDeclaredAnnotation shortcut for lookup on Class.

Issue: SPR-17146

Author: jhoeller

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

@@ -386,13 +386,14 @@ public static AnnotationAttributes getMergedAnnotationAttributes(AnnotatedElemen
 	@Nullable
 	public static <A extends Annotation> A getMergedAnnotation(AnnotatedElement element, Class<A> annotationType) {
 		// Shortcut: directly present on the element, with no merging needed?
-		if (!(element instanceof Class)) {
-			// Do not use this shortcut against a Class: Inherited annotations
-			// would get preferred over locally declared composed annotations.
-			A annotation = element.getAnnotation(annotationType);
-			if (annotation != null) {
-				return AnnotationUtils.synthesizeAnnotation(annotation, element);
-			}
+		A annotation = element.getDeclaredAnnotation(annotationType);
+		if (annotation != null) {
+			return AnnotationUtils.synthesizeAnnotation(annotation, element);
+		}
+
+		// Shortcut: no non-java annotations to be found on plain Java classes and org.springframework.lang types...
+		if (AnnotationUtils.hasPlainJavaAnnotationsOnly(element) && !annotationType.getName().startsWith("java")) {
+			return null;
 		}
 
 		// Exhaustive retrieval of merged annotation attributes...
@@ -671,13 +672,9 @@ public static AnnotationAttributes findMergedAnnotationAttributes(AnnotatedEleme
 	@Nullable
 	public static <A extends Annotation> A findMergedAnnotation(AnnotatedElement element, Class<A> annotationType) {
 		// Shortcut: directly present on the element, with no merging needed?
-		if (!(element instanceof Class)) {
-			// Do not use this shortcut against a Class: Inherited annotations
-			// would get preferred over locally declared composed annotations.
-			A annotation = element.getAnnotation(annotationType);
-			if (annotation != null) {
-				return AnnotationUtils.synthesizeAnnotation(annotation, element);
-			}
+		A annotation = element.getDeclaredAnnotation(annotationType);
+		if (annotation != null) {
+			return AnnotationUtils.synthesizeAnnotation(annotation, element);
 		}
 
 		// Shortcut: no non-java annotations to be found on plain Java classes and org.springframework.lang types...
@@ -1145,8 +1142,7 @@ else if (currentAnnotationType == containerType) {
 						Set<Method> annotatedMethods = AnnotationUtils.getAnnotatedMethodsInBaseType(clazz);
 						if (!annotatedMethods.isEmpty()) {
 							for (Method annotatedMethod : annotatedMethods) {
-								if (annotatedMethod.getName().equals(method.getName()) &&
-										Arrays.equals(annotatedMethod.getParameterTypes(), method.getParameterTypes())) {
+								if (AnnotationUtils.isOverride(method, annotatedMethod)) {
 									Method resolvedSuperMethod = BridgeMethodResolver.findBridgedMethod(annotatedMethod);
 									result = searchWithFindSemantics(resolvedSuperMethod, annotationType, annotationName,
 											containerType, processor, visited, metaDepth);
@@ -1203,8 +1199,7 @@ private static <T> T searchOnInterfaces(Method method, @Nullable Class<? extends
 			Set<Method> annotatedMethods = AnnotationUtils.getAnnotatedMethodsInBaseType(ifc);
 			if (!annotatedMethods.isEmpty()) {
 				for (Method annotatedMethod : annotatedMethods) {
-					if (annotatedMethod.getName().equals(method.getName()) &&
-							Arrays.equals(annotatedMethod.getParameterTypes(), method.getParameterTypes())) {
+					if (AnnotationUtils.isOverride(method, annotatedMethod)) {
 						T result = searchWithFindSemantics(annotatedMethod, annotationType, annotationName,
 								containerType, processor, visited, metaDepth);
 						if (result != null) {
@@ -1280,7 +1275,7 @@ private static Class<? extends Annotation> resolveContainerType(Class<? extends
 
 	/**
 	 * Validate that the supplied {@code containerType} is a proper container
-	 * annotation for the supplied repeatable {@code annotationType} (i.e.,
+	 * annotation for the supplied repeatable {@code annotationType} (i.e.
 	 * that it declares a {@code value} attribute that holds an array of the
 	 * {@code annotationType}).
 	 * @throws AnnotationConfigurationException if the supplied {@code containerType}
@@ -1322,27 +1317,24 @@ private static <A extends Annotation> Set<A> postProcessAndSynthesizeAggregatedR
 
 	/**
 	 * Callback interface that is used to process annotations during a search.
-	 * <p>Depending on the use case, a processor may choose to
-	 * {@linkplain #process} a single target annotation, multiple target
-	 * annotations, or all annotations discovered by the currently executing
-	 * search. The term "target" in this context refers to a matching
-	 * annotation (i.e., a specific annotation type that was found during
-	 * the search).
-	 * <p>Returning a non-null value from the {@link #process}
-	 * method instructs the search algorithm to stop searching further;
-	 * whereas, returning {@code null} from the {@link #process} method
-	 * instructs the search algorithm to continue searching for additional
-	 * annotations. One exception to this rule applies to processors
-	 * that {@linkplain #aggregates aggregate} results. If an aggregating
-	 * processor returns a non-null value, that value will be added to the
-	 * list of {@linkplain #getAggregatedResults aggregated results}
+	 * <p>Depending on the use case, a processor may choose to {@linkplain #process}
+	 * a single target annotation, multiple target annotations, or all annotations
+	 * discovered by the currently executing search. The term "target" in this
+	 * context refers to a matching annotation (i.e. a specific annotation type
+	 * that was found during the search).
+	 * <p>Returning a non-null value from the {@link #process} method instructs
+	 * the search algorithm to stop searching further; whereas, returning
+	 * {@code null} from the {@link #process} method instructs the search
+	 * algorithm to continue searching for additional annotations. One exception
+	 * to this rule applies to processors that {@linkplain #aggregates aggregate}
+	 * results. If an aggregating processor returns a non-null value, that value
+	 * will be added to the {@linkplain #getAggregatedResults aggregated results}
 	 * and the search algorithm will continue.
-	 * <p>Processors can optionally {@linkplain #postProcess post-process}
-	 * the result of the {@link #process} method as the search algorithm
-	 * goes back down the annotation hierarchy from an invocation of
-	 * {@link #process} that returned a non-null value down to the
-	 * {@link AnnotatedElement} that was supplied as the starting point to
-	 * the search algorithm.
+	 * <p>Processors can optionally {@linkplain #postProcess post-process} the
+	 * result of the {@link #process} method as the search algorithm goes back
+	 * down the annotation hierarchy from an invocation of {@link #process} that
+	 * returned a non-null value down to the {@link AnnotatedElement} that was
+	 * supplied as the starting point to the search algorithm.
 	 * @param <T> the type of result returned by the processor
 	 */
 	private interface Processor<T> {

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

@@ -520,7 +520,7 @@ private static <A extends Annotation> A findAnnotation(
 
 	/**
 	 * Find a single {@link Annotation} of {@code annotationType} on the supplied
-	 * {@link Method}, traversing its super methods (i.e., from superclasses and
+	 * {@link Method}, traversing its super methods (i.e. from superclasses and
 	 * interfaces) if the annotation is not <em>directly present</em> on the given
 	 * method itself.
 	 * <p>Correctly handles bridge {@link Method Methods} generated by the compiler.
@@ -560,8 +560,7 @@ public static <A extends Annotation> A findAnnotation(Method method, @Nullable C
 				Set<Method> annotatedMethods = getAnnotatedMethodsInBaseType(clazz);
 				if (!annotatedMethods.isEmpty()) {
 					for (Method annotatedMethod : annotatedMethods) {
-						if (annotatedMethod.getName().equals(method.getName()) &&
-								Arrays.equals(annotatedMethod.getParameterTypes(), method.getParameterTypes())) {
+						if (isOverride(method, annotatedMethod)) {
 							Method resolvedSuperMethod = BridgeMethodResolver.findBridgedMethod(annotatedMethod);
 							result = findAnnotation((AnnotatedElement) resolvedSuperMethod, annotationType);
 							if (result != null) {
@@ -650,7 +649,7 @@ private static boolean hasSearchableAnnotations(Method ifcMethod) {
 		return false;
 	}
 
-	private static boolean isOverride(Method method, Method candidate) {
+	static boolean isOverride(Method method, Method candidate) {
 		if (!candidate.getName().equals(method.getName()) ||
 				candidate.getParameterCount() != method.getParameterCount()) {
 			return false;
@@ -843,7 +842,7 @@ public static Class<?> findAnnotationDeclaringClassForTypes(List<Class<? extends
 
 	/**
 	 * Determine whether an annotation of the specified {@code annotationType}
-	 * is declared locally (i.e., <em>directly present</em>) on the supplied
+	 * is declared locally (i.e. <em>directly present</em>) on the supplied
 	 * {@code clazz}.
 	 * <p>The supplied {@link Class} may represent any type.
 	 * <p>Meta-annotations will <em>not</em> be searched.
@@ -872,8 +871,8 @@ public static boolean isAnnotationDeclaredLocally(Class<? extends Annotation> an
 	/**
 	 * Determine whether an annotation of the specified {@code annotationType}
 	 * is <em>present</em> on the supplied {@code clazz} and is
-	 * {@linkplain java.lang.annotation.Inherited inherited} (i.e., not
-	 * <em>directly present</em>).
+	 * {@linkplain java.lang.annotation.Inherited inherited}
+	 * (i.e. not <em>directly present</em>).
 	 * <p>Meta-annotations will <em>not</em> be searched.
 	 * <p>If the supplied {@code clazz} is an interface, only the interface
 	 * itself will be checked. In accordance with standard meta-annotation
@@ -1681,10 +1680,10 @@ static <A extends Annotation> A[] synthesizeAnnotationArray(
 	 * in the supplied annotation type.
 	 * <p>The map is keyed by attribute name with each value representing
 	 * a list of names of aliased attributes.
-	 * <p>For <em>explicit</em> alias pairs such as x and y (i.e., where x
+	 * <p>For <em>explicit</em> alias pairs such as x and y (i.e. where x
 	 * is an {@code @AliasFor("y")} and y is an {@code @AliasFor("x")}, there
 	 * will be two entries in the map: {@code x -> (y)} and {@code y -> (x)}.
-	 * <p>For <em>implicit</em> aliases (i.e., attributes that are declared
+	 * <p>For <em>implicit</em> aliases (i.e. attributes that are declared
 	 * as attribute overrides for the same attribute in the same meta-annotation),
 	 * there will be n entries in the map. For example, if x, y, and z are
 	 * implicit aliases, the map will contain the following entries:
@@ -1733,7 +1732,7 @@ private static boolean canExposeSynthesizedMarker(Class<? extends Annotation> an
 
 	/**
 	 * Determine if annotations of the supplied {@code annotationType} are
-	 * <em>synthesizable</em> (i.e., in need of being wrapped in a dynamic
+	 * <em>synthesizable</em> (i.e. in need of being wrapped in a dynamic
 	 * proxy that provides functionality above that of a standard JDK
 	 * annotation).
 	 * <p>Specifically, an annotation is <em>synthesizable</em> if it declares

spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -525,19 +525,11 @@ public void findMergedAnnotationAttributesInheritedFromAbstractMethod() throws N
 		assertNotNull("Should find @Transactional on ConcreteClassWithInheritedAnnotation.handle() method", attributes);
 	}
 
-	/**
-	 * <p>{@code AbstractClassWithInheritedAnnotation} declares {@code handleParameterized(T)}; whereas,
-	 * {@code ConcreteClassWithInheritedAnnotation} declares {@code handleParameterized(String)}.
-	 * <p>As of Spring 4.2, {@code AnnotatedElementUtils.processWithFindSemantics()} does not resolve an
-	 * <em>equivalent</em> method in {@code AbstractClassWithInheritedAnnotation} for the <em>bridged</em>
-	 * {@code handleParameterized(String)} method.
-	 * @since 4.2
-	 */
 	@Test
 	public void findMergedAnnotationAttributesInheritedFromBridgedMethod() throws NoSuchMethodException {
 		Method method = ConcreteClassWithInheritedAnnotation.class.getMethod("handleParameterized", String.class);
 		AnnotationAttributes attributes = findMergedAnnotationAttributes(method, Transactional.class);
-		assertNull("Should not find @Transactional on bridged ConcreteClassWithInheritedAnnotation.handleParameterized()", attributes);
+		assertNotNull("Should find @Transactional on bridged ConcreteClassWithInheritedAnnotation.handleParameterized()", attributes);
 	}
 
 	/**
@@ -546,7 +538,7 @@ public void findMergedAnnotationAttributesInheritedFromBridgedMethod() throws No
 	 * @since 4.2
 	 */
 	@Test
-	public void findMergedAnnotationAttributesFromBridgeMethod() throws NoSuchMethodException {
+	public void findMergedAnnotationAttributesFromBridgeMethod() {
 		Method[] methods = StringGenericParameter.class.getMethods();
 		Method bridgeMethod = null;
 		Method bridgedMethod = null;
@@ -733,6 +725,20 @@ public void findAllMergedAnnotationsOnClassWithInterface() throws Exception {
 		assertEquals(1, allMergedAnnotations.size());
 	}
 
+	@Test  // SPR-16060
+	public void findMethodAnnotationFromGenericInterface() throws Exception {
+		Method method = ImplementsInterfaceWithGenericAnnotatedMethod.class.getMethod("foo", String.class);
+		Order order = findMergedAnnotation(method, Order.class);
+		assertNotNull(order);
+	}
+
+	@Test  // SPR-17146
+	public void findMethodAnnotationFromGenericSuperclass() throws Exception {
+		Method method = ExtendsBaseClassWithGenericAnnotatedMethod.class.getMethod("foo", String.class);
+		Order order = findMergedAnnotation(method, Order.class);
+		assertNotNull(order);
+	}
+
 
 	// -------------------------------------------------------------------------
 

spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java

@@ -167,9 +167,7 @@ public void findMethodAnnotationOnBridgedMethod() throws Exception {
 
 		assertNull(bridgedMethod.getAnnotation(Order.class));
 		assertNull(getAnnotation(bridgedMethod, Order.class));
-		// AnnotationUtils.findAnnotation(Method, Class<A>) will not find an annotation on
-		// the bridge method for a bridged method.
-		assertNull(findAnnotation(bridgedMethod, Order.class));
+		assertNotNull(findAnnotation(bridgedMethod, Order.class));
 
 		assertNotNull(bridgedMethod.getAnnotation(Transactional.class));
 		assertNotNull(getAnnotation(bridgedMethod, Transactional.class));
@@ -190,6 +188,13 @@ public void findMethodAnnotationFromGenericInterface() throws Exception {
 		assertNotNull(order);
 	}
 
+	@Test  // SPR-17146
+	public void findMethodAnnotationFromGenericSuperclass() throws Exception {
+		Method method = ExtendsBaseClassWithGenericAnnotatedMethod.class.getMethod("foo", String.class);
+		Order order = findAnnotation(method, Order.class);
+		assertNotNull(order);
+	}
+
 	@Test
 	public void findMethodAnnotationFromInterfaceOnSuper() throws Exception {
 		Method method = SubOfImplementsInterfaceWithAnnotatedMethod.class.getMethod("foo");
@@ -204,23 +209,23 @@ public void findMethodAnnotationFromInterfaceWhenSuperDoesNotImplementMethod() t
 		assertNotNull(order);
 	}
 
-	/** @since 4.1.2 */
+	// @since 4.1.2
 	@Test
 	public void findClassAnnotationFavorsMoreLocallyDeclaredComposedAnnotationsOverAnnotationsOnInterfaces() {
 		Component component = findAnnotation(ClassWithLocalMetaAnnotationAndMetaAnnotatedInterface.class, Component.class);
 		assertNotNull(component);
 		assertEquals("meta2", component.value());
 	}
 
-	/** @since 4.0.3 */
+	// @since 4.0.3
 	@Test
 	public void findClassAnnotationFavorsMoreLocallyDeclaredComposedAnnotationsOverInheritedAnnotations() {
 		Transactional transactional = findAnnotation(SubSubClassWithInheritedAnnotation.class, Transactional.class);
 		assertNotNull(transactional);
 		assertTrue("readOnly flag for SubSubClassWithInheritedAnnotation", transactional.readOnly());
 	}
 
-	/** @since 4.0.3 */
+	// @since 4.0.3
 	@Test
 	public void findClassAnnotationFavorsMoreLocallyDeclaredComposedAnnotationsOverInheritedComposedAnnotations() {
 		Component component = findAnnotation(SubSubClassWithInheritedMetaAnnotation.class, Component.class);
@@ -1762,18 +1767,6 @@ public static class TransactionalAndOrderedClass extends TransactionalClass {
 	public static class SubTransactionalAndOrderedClass extends TransactionalAndOrderedClass {
 	}
 
-	public interface InterfaceWithGenericAnnotatedMethod<T> {
-
-		@Order
-		void foo(T t);
-	}
-
-	public static class ImplementsInterfaceWithGenericAnnotatedMethod implements InterfaceWithGenericAnnotatedMethod<String> {
-
-		public void foo(String t) {
-		}
-	}
-
 	public interface InterfaceWithAnnotatedMethod {
 
 		@Order
@@ -1806,6 +1799,30 @@ public void foo() {
 		}
 	}
 
+	public interface InterfaceWithGenericAnnotatedMethod<T> {
+
+		@Order
+		void foo(T t);
+	}
+
+	public static class ImplementsInterfaceWithGenericAnnotatedMethod implements InterfaceWithGenericAnnotatedMethod<String> {
+
+		public void foo(String t) {
+		}
+	}
+
+	public static abstract class BaseClassWithGenericAnnotatedMethod<T> {
+
+		@Order
+		abstract void foo(T t);
+	}
+
+	public static class ExtendsBaseClassWithGenericAnnotatedMethod extends BaseClassWithGenericAnnotatedMethod<String> {
+
+		public void foo(String t) {
+		}
+	}
+
 	@Retention(RetentionPolicy.RUNTIME)
 	@Inherited
 	@interface MyRepeatableContainer {