AnnotationUtils favors local composed annotations over interface annotations and consistently logs introspection failures via lazily initialized logger

Issue: SPR-12355
Issue: SPR-12325
Issue: SPR-12329

Author: jhoeller

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

@@ -66,14 +66,15 @@ public abstract class AnnotationUtils {
 	/** The attribute name for annotations with a single element */
 	public static final String VALUE = "value";
 
-	private static final Log logger = LogFactory.getLog(AnnotationUtils.class);
 
 	private static final Map<AnnotationCacheKey, Annotation> findAnnotationCache =
 			new ConcurrentReferenceHashMap<AnnotationCacheKey, Annotation>(256);
 
 	private static final Map<Class<?>, Boolean> annotatedInterfaceCache =
 			new ConcurrentReferenceHashMap<Class<?>, Boolean>(256);
 
+	private static transient Log logger;
+
 
 	/**
 	 * Get a single {@link Annotation} of {@code annotationType} from the supplied
@@ -93,10 +94,7 @@ public static <T extends Annotation> T getAnnotation(Annotation ann, Class<T> an
 		}
 		catch (Exception ex) {
 			// Assuming nested Class values not resolvable within annotation attributes...
-			// We're probably hitting a non-present optional arrangement - let's back out.
-			if (logger.isInfoEnabled()) {
-				logger.info("Failed to introspect annotations on [" + ann.annotationType() + "]: " + ex);
-			}
+			logIntrospectionFailure(ann.annotationType(), ex);
 			return null;
 		}
 	}
@@ -125,10 +123,7 @@ public static <T extends Annotation> T getAnnotation(AnnotatedElement annotatedE
 		}
 		catch (Exception ex) {
 			// Assuming nested Class values not resolvable within annotation attributes...
-			// We're probably hitting a non-present optional arrangement - let's back out.
-			if (logger.isInfoEnabled()) {
-				logger.info("Failed to introspect annotations on [" + annotatedElement + "]: " + ex);
-			}
+			logIntrospectionFailure(annotatedElement, ex);
 			return null;
 		}
 	}
@@ -146,10 +141,7 @@ public static Annotation[] getAnnotations(Method method) {
 		}
 		catch (Exception ex) {
 			// Assuming nested Class values not resolvable within annotation attributes...
-			// We're probably hitting a non-present optional arrangement - let's back out.
-			if (logger.isInfoEnabled()) {
-				logger.info("Failed to introspect annotations on [" + method + "]: " + ex);
-			}
+			logIntrospectionFailure(method, ex);
 			return null;
 		}
 	}
@@ -208,10 +200,7 @@ public static <A extends Annotation> Set<A> getRepeatableAnnotation(AnnotatedEle
 		}
 		catch (Exception ex) {
 			// Assuming nested Class values not resolvable within annotation attributes...
-			// We're probably hitting a non-present optional arrangement - let's back out.
-			if (logger.isInfoEnabled()) {
-				logger.info("Failed to introspect annotations on [" + annotatedElement + "]: " + ex);
-			}
+			logIntrospectionFailure(annotatedElement, ex);
 		}
 		return Collections.emptySet();
 	}
@@ -293,10 +282,7 @@ private static boolean isInterfaceWithAnnotatedMethods(Class<?> iface) {
 			}
 			catch (Exception ex) {
 				// Assuming nested Class values not resolvable within annotation attributes...
-				// We're probably hitting a non-present optional arrangement - let's back out.
-				if (logger.isInfoEnabled()) {
-					logger.info("Failed to introspect annotations on [" + ifcMethod + "]: " + ex);
-				}
+				logIntrospectionFailure(ifcMethod, ex);
 			}
 		}
 		annotatedInterfaceCache.put(iface, found);
@@ -347,35 +333,39 @@ public static <A extends Annotation> A findAnnotation(Class<?> clazz, Class<A> a
 	 * @param visited the set of annotations that have already been visited
 	 * @return the annotation if found, or {@code null} if not found
 	 */
+	@SuppressWarnings("unchecked")
 	private static <A extends Annotation> A findAnnotation(Class<?> clazz, Class<A> annotationType, Set<Annotation> visited) {
 		Assert.notNull(clazz, "Class must not be null");
-		if (isAnnotationDeclaredLocally(annotationType, clazz)) {
-			try {
-				return clazz.getAnnotation(annotationType);
+
+		try {
+			Annotation[] anns = clazz.getDeclaredAnnotations();
+			for (Annotation ann : anns) {
+				if (ann.annotationType().equals(annotationType)) {
+					return (A) ann;
+				}
 			}
-			catch (Exception ex) {
-				// Assuming nested Class values not resolvable within annotation attributes...
-				// We're probably hitting a non-present optional arrangement - let's back out.
-				if (logger.isInfoEnabled()) {
-					logger.info("Failed to introspect annotations on [" + clazz + "]: " + ex);
+			for (Annotation ann : anns) {
+				if (!isInJavaLangAnnotationPackage(ann) && visited.add(ann)) {
+					A annotation = findAnnotation(ann.annotationType(), annotationType, visited);
+					if (annotation != null) {
+						return annotation;
+					}
 				}
-				return null;
 			}
 		}
+		catch (Exception ex) {
+			// Assuming nested Class values not resolvable within annotation attributes...
+			// We're probably hitting a non-present optional arrangement - let's back out.
+			return null;
+		}
+
 		for (Class<?> ifc : clazz.getInterfaces()) {
 			A annotation = findAnnotation(ifc, annotationType, visited);
 			if (annotation != null) {
 				return annotation;
 			}
 		}
-		for (Annotation ann : clazz.getDeclaredAnnotations()) {
-			if (!isInJavaLangAnnotationPackage(ann) && visited.add(ann)) {
-				A annotation = findAnnotation(ann.annotationType(), annotationType, visited);
-				if (annotation != null) {
-					return annotation;
-				}
-			}
-		}
+
 		Class<?> superclass = clazz.getSuperclass();
 		if (superclass == null || superclass.equals(Object.class)) {
 			return null;
@@ -473,19 +463,16 @@ public static boolean isAnnotationDeclaredLocally(Class<? extends Annotation> an
 		Assert.notNull(clazz, "Class must not be null");
 		boolean declaredLocally = false;
 		try {
-			for (Annotation annotation : clazz.getDeclaredAnnotations()) {
-				if (annotation.annotationType().equals(annotationType)) {
+			for (Annotation ann : clazz.getDeclaredAnnotations()) {
+				if (ann.annotationType().equals(annotationType)) {
 					declaredLocally = true;
 					break;
 				}
 			}
 		}
 		catch (Exception ex) {
 			// Assuming nested Class values not resolvable within annotation attributes...
-			// We're probably hitting a non-present optional arrangement - let's back out.
-			if (logger.isInfoEnabled()) {
-				logger.info("Failed to introspect annotations on [" + clazz + "]: " + ex);
-			}
+			logIntrospectionFailure(clazz, ex);
 		}
 		return declaredLocally;
 	}
@@ -710,6 +697,18 @@ public static Object getDefaultValue(Class<? extends Annotation> annotationType,
 	}
 
 
+	private static void logIntrospectionFailure(AnnotatedElement annotatedElement, Exception ex) {
+		Log loggerToUse = logger;
+		if (loggerToUse == null) {
+			loggerToUse = LogFactory.getLog(AnnotationUtils.class);
+			logger = loggerToUse;
+		}
+		if (loggerToUse.isInfoEnabled()) {
+			loggerToUse.info("Failed to introspect annotations on [" + annotatedElement + "]: " + ex);
+		}
+	}
+
+
 	/**
 	 * Cache key for the AnnotatedElement cache.
 	 */
@@ -767,15 +766,15 @@ public Set<A> getResult(AnnotatedElement element) {
 		@SuppressWarnings("unchecked")
 		private void process(AnnotatedElement annotatedElement) {
 			if (this.visited.add(annotatedElement)) {
-				for (Annotation annotation : annotatedElement.getAnnotations()) {
-					if (ObjectUtils.nullSafeEquals(this.annotationType, annotation.annotationType())) {
-						this.result.add((A) annotation);
+				for (Annotation ann : annotatedElement.getAnnotations()) {
+					if (ObjectUtils.nullSafeEquals(this.annotationType, ann.annotationType())) {
+						this.result.add((A) ann);
 					}
-					else if (ObjectUtils.nullSafeEquals(this.containerAnnotationType, annotation.annotationType())) {
-						this.result.addAll(getValue(annotation));
+					else if (ObjectUtils.nullSafeEquals(this.containerAnnotationType, ann.annotationType())) {
+						this.result.addAll(getValue(ann));
 					}
-					else if (!isInJavaLangAnnotationPackage(annotation)) {
-						process(annotation.annotationType());
+					else if (!isInJavaLangAnnotationPackage(ann)) {
+						process(ann.annotationType());
 					}
 				}
 			}

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

@@ -99,25 +99,24 @@ public void findMethodAnnotationOnBridgeMethod() throws Exception {
 	// assertNotNull(o);
 	// }
 
+	/**
+	 * @since 4.1.2
+	 */
 	@Test
-	public void findAnnotationFavorsInterfacesOverLocalMetaAnnotations() {
+	public void findAnnotationFavorsLocalMetaAnnotationsOverInterfaces() {
 		Component component = AnnotationUtils.findAnnotation(
-			ClassWithLocalMetaAnnotationAndMetaAnnotatedInterface.class, Component.class);
+				ClassWithLocalMetaAnnotationAndMetaAnnotatedInterface.class, Component.class);
 		assertNotNull(component);
-
-		// By inspecting ClassWithLocalMetaAnnotationAndMetaAnnotatedInterface, one
-		// might expect that "meta2" should be found; however, with the current
-		// implementation "meta1" will be found.
-		assertEquals("meta1", component.value());
+		assertEquals("meta2", component.value());
 	}
 
 	/**
 	 * @since 4.0.3
 	 */
 	@Test
 	public void findAnnotationFavorsInheritedAnnotationsOverMoreLocallyDeclaredComposedAnnotations() {
-		Transactional transactional = AnnotationUtils.findAnnotation(SubSubClassWithInheritedAnnotation.class,
-			Transactional.class);
+		Transactional transactional = AnnotationUtils.findAnnotation(
+				SubSubClassWithInheritedAnnotation.class, Transactional.class);
 		assertNotNull(transactional);
 		assertTrue("readOnly flag for SubSubClassWithInheritedAnnotation", transactional.readOnly());
 	}
@@ -127,8 +126,8 @@ public void findAnnotationFavorsInheritedAnnotationsOverMoreLocallyDeclaredCompo
 	 */
 	@Test
 	public void findAnnotationFavorsInheritedComposedAnnotationsOverMoreLocallyDeclaredComposedAnnotations() {
-		Component component = AnnotationUtils.findAnnotation(SubSubClassWithInheritedMetaAnnotation.class,
-			Component.class);
+		Component component = AnnotationUtils.findAnnotation(
+				SubSubClassWithInheritedMetaAnnotation.class, Component.class);
 		assertNotNull(component);
 		assertEquals("meta2", component.value());
 	}