From 716916b281d25cc843b4b98530f4550f7f6c0ca8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 20 Oct 2014 21:47:47 +0200 Subject: [PATCH] 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 --- .../core/annotation/AnnotationUtils.java | 103 +++++++++--------- .../core/annotation/AnnotationUtilsTests.java | 21 ++-- 2 files changed, 61 insertions(+), 63 deletions(-) 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 7b37574081..80a6d52cd6 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 @@ -66,7 +66,6 @@ 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 findAnnotationCache = new ConcurrentReferenceHashMap(256); @@ -74,6 +73,8 @@ public abstract class AnnotationUtils { private static final Map, Boolean> annotatedInterfaceCache = new ConcurrentReferenceHashMap, Boolean>(256); + private static transient Log logger; + /** * Get a single {@link Annotation} of {@code annotationType} from the supplied @@ -93,10 +94,7 @@ public abstract class AnnotationUtils { } 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 abstract class AnnotationUtils { } 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 abstract class AnnotationUtils { } 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 abstract class AnnotationUtils { } 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 @@ public abstract class AnnotationUtils { } 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 abstract class AnnotationUtils { * @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 findAnnotation(Class clazz, Class annotationType, Set visited) { Assert.notNull(clazz, "Class must not be null"); - if (isAnnotationDeclaredLocally(annotationType, clazz)) { - try { - return clazz.getAnnotation(annotationType); - } - 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); + + try { + Annotation[] anns = clazz.getDeclaredAnnotations(); + for (Annotation ann : anns) { + if (ann.annotationType().equals(annotationType)) { + return (A) ann; + } + } + 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,8 +463,8 @@ public abstract class AnnotationUtils { 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; } @@ -482,10 +472,7 @@ public abstract class AnnotationUtils { } 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 abstract class AnnotationUtils { } + 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 abstract class AnnotationUtils { @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()); } } } 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 a03d4085e8..e192d6666a 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 @@ -99,16 +99,15 @@ public class AnnotationUtilsTests { // 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()); } /** @@ -116,8 +115,8 @@ public class AnnotationUtilsTests { */ @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 class AnnotationUtilsTests { */ @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()); }