From b9b0b78fa1f80efa5f5549a862d44d11b0bc3ba1 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 22 Apr 2015 02:26:24 +0200 Subject: [PATCH] Support n meta-annotation levels on methods in AnnotationUtils Prior to this commit, the search algorithm used by the findAnnotation(Method, Class) method in AnnotationUtils only found direct annotations or direct meta-annotations (i.e., one level of meta-annotations). This commit reworks the search algorithm so that it supports arbitrary levels of meta-annotations on methods. To make this possible, a new findAnnotation(AnnotatedElement, Class) method has been introduced in AnnotationUtils. This fix also allows for the @Ignore'd tests in TransactionalEventListenerTests to be re-enabled. Issue: SPR-12941 --- .../core/annotation/AnnotationUtils.java | 113 ++++++++++++++---- .../core/annotation/AnnotationUtilsTests.java | 46 ++++++- .../TransactionalEventListenerTests.java | 10 +- 3 files changed, 140 insertions(+), 29 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 5ec94f43dd..9db91d5ff1 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 @@ -91,8 +91,9 @@ public abstract class AnnotationUtils { * Get a single {@link Annotation} of {@code annotationType} from the supplied * annotation: either the given annotation itself or a direct meta-annotation * thereof. - *

Note that this method does not support arbitrary levels of - * meta-annotations. + *

Note that this method supports only a single level of meta-annotations. + * For support for arbitrary levels of meta-annotations, use one of the + * {@code find*()} methods instead. * @param ann the Annotation to check * @param annotationType the annotation type to look for, both locally and as a meta-annotation * @return the matching annotation, or {@code null} if not found @@ -115,9 +116,11 @@ public abstract class AnnotationUtils { /** * Get a single {@link Annotation} of {@code annotationType} from the supplied - * {@link AnnotatedElement}. - *

Meta-annotations will be searched if the annotation is not - * directly present on the supplied element. + * {@link AnnotatedElement}, where the {@code AnnotatedElement} is either + * directly annotated or meta-annotated with the {@code annotationType}. + *

Note that this method supports only a single level of meta-annotations. + * For support for arbitrary levels of meta-annotations, use + * {@link #findAnnotation(AnnotatedElement, Class)} instead. * @param annotatedElement the {@code AnnotatedElement} from which to get the annotation * @param annotationType the annotation type to look for, both locally and as a meta-annotation * @return the matching annotation, or {@code null} if not found @@ -144,10 +147,13 @@ public abstract class AnnotationUtils { } /** - * Get a single {@link Annotation} of {@code annotationType} from the supplied {@link Method}. + * Get a single {@link Annotation} of {@code annotationType} from the + * supplied {@link Method}, where the method is either directly annotated + * or meta-annotated with the {@code annotationType}. *

Correctly handles bridge {@link Method Methods} generated by the compiler. - *

Meta-annotations will be searched if the annotation is not - * directly present on the supplied method. + *

Note that this method supports only a single level of meta-annotations. + * For support for arbitrary levels of meta-annotations, use + * {@link #findAnnotation(Method, Class)} instead. * @param method the method to look for annotations on * @param annotationType the annotation type to look for * @return the matching annotation, or {@code null} if not found @@ -256,26 +262,91 @@ public abstract class AnnotationUtils { } /** - * Find a single {@link Annotation} of {@code annotationType} from the supplied + * Find a single {@link Annotation} of {@code annotationType} on the + * supplied {@link AnnotatedElement}. + *

Meta-annotations will be searched if the annotation is not + * directly present on the supplied element. + *

Warning: this method operates generically on + * annotated elements. In other words, this method does not execute + * specialized search algorithms for classes or methods. If you require + * the more specific semantics of {@link #findAnnotation(Class, Class)} + * or {@link #findAnnotation(Method, Class)}, invoke one of those methods + * instead. + * @param annotatedElement the {@code AnnotatedElement} on which to find the annotation + * @param annotationType the annotation type to look for, both locally and as a meta-annotation + * @return the matching annotation, or {@code null} if not found + * @since 4.2 + */ + public static A findAnnotation(AnnotatedElement annotatedElement, Class annotationType) { + // Do NOT store result in the findAnnotationCache since doing so could break + // findAnnotation(Class, Class) and findAnnotation(Method, Class). + return findAnnotation(annotatedElement, annotationType, new HashSet()); + } + + /** + * Perform the search algorithm for {@link #findAnnotation(AnnotatedElement, Class)} + * avoiding endless recursion by tracking which annotations have already + * been visited. + * @param annotatedElement the {@code AnnotatedElement} on which to find the annotation + * @param annotationType the annotation type to look for, both locally and as a meta-annotation + * @param visited the set of annotations that have already been visited + * @return the matching annotation, or {@code null} if not found + * @since 4.2 + */ + @SuppressWarnings("unchecked") + private static T findAnnotation(AnnotatedElement annotatedElement, Class annotationType, Set visited) { + Assert.notNull(annotatedElement, "AnnotatedElement must not be null"); + try { + Annotation[] anns = annotatedElement.getDeclaredAnnotations(); + for (Annotation ann : anns) { + if (ann.annotationType().equals(annotationType)) { + return (T) ann; + } + } + for (Annotation ann : anns) { + if (!isInJavaLangAnnotationPackage(ann) && visited.add(ann)) { + T annotation = findAnnotation((AnnotatedElement) ann.annotationType(), annotationType, visited); + if (annotation != null) { + return annotation; + } + } + } + } + catch (Exception ex) { + // Assuming nested Class values not resolvable within annotation attributes... + logIntrospectionFailure(annotatedElement, ex); + } + return null; + } + + /** + * Find a single {@link Annotation} of {@code annotationType} on the supplied * {@link Method}, traversing its super methods (i.e., from superclasses and * interfaces) if no annotation can be found on the given method itself. + *

Correctly handles bridge {@link Method Methods} generated by the compiler. + *

Meta-annotations will be searched if the annotation is not + * directly present on the method. *

Annotations on methods are not inherited by default, so we need to handle * this explicitly. - *

Meta-annotations will not be searched. * @param method the method to look for annotations on * @param annotationType the annotation type to look for * @return the matching annotation, or {@code null} if not found + * @see #getAnnotation(Method, Class) */ @SuppressWarnings("unchecked") public static A findAnnotation(Method method, Class annotationType) { AnnotationCacheKey cacheKey = new AnnotationCacheKey(method, annotationType); A result = (A) findAnnotationCache.get(cacheKey); + if (result == null) { - result = getAnnotation(method, annotationType); - Class clazz = method.getDeclaringClass(); + Method resolvedMethod = BridgeMethodResolver.findBridgedMethod(method); + result = findAnnotation((AnnotatedElement) resolvedMethod, annotationType); + if (result == null) { - result = searchOnInterfaces(method, annotationType, clazz.getInterfaces()); + result = searchOnInterfaces(method, annotationType, method.getDeclaringClass().getInterfaces()); } + + Class clazz = method.getDeclaringClass(); while (result == null) { clazz = clazz.getSuperclass(); if (clazz == null || clazz.equals(Object.class)) { @@ -283,7 +354,8 @@ public abstract class AnnotationUtils { } try { Method equivalentMethod = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes()); - result = getAnnotation(equivalentMethod, annotationType); + Method resolvedEquivalentMethod = BridgeMethodResolver.findBridgedMethod(equivalentMethod); + result = findAnnotation((AnnotatedElement) resolvedEquivalentMethod, annotationType); } catch (NoSuchMethodException ex) { // No equivalent method found @@ -292,9 +364,10 @@ public abstract class AnnotationUtils { result = searchOnInterfaces(method, annotationType, clazz.getInterfaces()); } } - if (result != null) { - findAnnotationCache.put(cacheKey, result); - } + } + + if (result != null) { + findAnnotationCache.put(cacheKey, result); } return result; } @@ -680,7 +753,7 @@ public abstract class AnnotationUtils { } /** - * Retrieve the value of the {@code "value"} attribute of a + * Retrieve the value of the {@code value} attribute of a * single-element Annotation, given an annotation instance. * @param annotation the annotation instance from which to retrieve the value * @return the attribute value, or {@code null} if not found @@ -712,7 +785,7 @@ public abstract class AnnotationUtils { } /** - * Retrieve the default value of the {@code "value"} attribute + * Retrieve the default value of the {@code value} attribute * of a single-element Annotation, given an annotation instance. * @param annotation the annotation instance from which to retrieve the default value * @return the default value, or {@code null} if not found @@ -737,7 +810,7 @@ public abstract class AnnotationUtils { } /** - * Retrieve the default value of the {@code "value"} attribute + * Retrieve the default value of the {@code value} attribute * of a single-element Annotation, given the {@link Class annotation type}. * @param annotationType the annotation type for which the default value should be retrieved * @return the default value, or {@code null} if not found 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 8905a8b631..07d16032b8 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 @@ -50,23 +50,47 @@ public class AnnotationUtilsTests { @Test public void findMethodAnnotationOnLeaf() throws Exception { - Method m = Leaf.class.getMethod("annotatedOnLeaf", (Class[]) null); + Method m = Leaf.class.getMethod("annotatedOnLeaf"); assertNotNull(m.getAnnotation(Order.class)); assertNotNull(getAnnotation(m, Order.class)); assertNotNull(findAnnotation(m, Order.class)); } + @Test + public void findMethodAnnotationWithMetaAnnotationOnLeaf() throws Exception { + Method m = Leaf.class.getMethod("metaAnnotatedOnLeaf"); + assertNull(m.getAnnotation(Order.class)); + assertNotNull(getAnnotation(m, Order.class)); + assertNotNull(findAnnotation(m, Order.class)); + } + + @Test + public void findMethodAnnotationWithMetaMetaAnnotationOnLeaf() throws Exception { + Method m = Leaf.class.getMethod("metaMetaAnnotatedOnLeaf"); + assertNull(m.getAnnotation(Component.class)); + assertNull(getAnnotation(m, Component.class)); + assertNotNull(findAnnotation(m, Component.class)); + } + @Test public void findMethodAnnotationOnRoot() throws Exception { - Method m = Leaf.class.getMethod("annotatedOnRoot", (Class[]) null); + Method m = Leaf.class.getMethod("annotatedOnRoot"); assertNotNull(m.getAnnotation(Order.class)); assertNotNull(getAnnotation(m, Order.class)); assertNotNull(findAnnotation(m, Order.class)); } + @Test + public void findMethodAnnotationWithMetaAnnotationOnRoot() throws Exception { + Method m = Leaf.class.getMethod("metaAnnotatedOnRoot"); + assertNull(m.getAnnotation(Order.class)); + assertNotNull(getAnnotation(m, Order.class)); + assertNotNull(findAnnotation(m, Order.class)); + } + @Test public void findMethodAnnotationOnRootButOverridden() throws Exception { - Method m = Leaf.class.getMethod("overrideWithoutNewAnnotation", (Class[]) null); + Method m = Leaf.class.getMethod("overrideWithoutNewAnnotation"); assertNull(m.getAnnotation(Order.class)); assertNull(getAnnotation(m, Order.class)); assertNotNull(findAnnotation(m, Order.class)); @@ -74,7 +98,7 @@ public class AnnotationUtilsTests { @Test public void findMethodAnnotationNotAnnotated() throws Exception { - Method m = Leaf.class.getMethod("notAnnotated", (Class[]) null); + Method m = Leaf.class.getMethod("notAnnotated"); assertNull(findAnnotation(m, Order.class)); } @@ -85,7 +109,7 @@ public class AnnotationUtilsTests { assertNull(m.getAnnotation(Order.class)); assertNull(getAnnotation(m, Order.class)); assertNotNull(findAnnotation(m, Order.class)); - // TODO: actually found on OpenJDK 8 b99 and higher! + // TODO: getAnnotation() on bridge method actually found on OpenJDK 8 b99 and higher! // assertNull(m.getAnnotation(Transactional.class)); assertNotNull(getAnnotation(m, Transactional.class)); assertNotNull(findAnnotation(m, Transactional.class)); @@ -462,6 +486,10 @@ public class AnnotationUtilsTests { public void annotatedOnRoot() { } + @Meta1 + public void metaAnnotatedOnRoot() { + } + public void overrideToAnnotate() { } @@ -483,6 +511,14 @@ public class AnnotationUtilsTests { public void annotatedOnLeaf() { } + @Meta1 + public void metaAnnotatedOnLeaf() { + } + + @MetaMeta + public void metaMetaAnnotatedOnLeaf() { + } + @Override @Order(1) public void overrideToAnnotate() { diff --git a/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java b/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java index c3728c8387..8ef57bbf60 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.Map; import org.junit.After; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -51,7 +50,12 @@ import static org.junit.Assert.*; import static org.springframework.transaction.event.TransactionPhase.*; /** + * Integration tests for {@link TransactionalEventListener @TransactionalEventListener} + * support + * * @author Stephane Nicoll + * @author Sam Brannen + * @since 4.2 */ public class TransactionalEventListenerTests { @@ -265,8 +269,7 @@ public class TransactionalEventListenerTests { } @Test - @Ignore("not an event listener if not tagged") - public void afterCommitMetaAnnotation() { + public void afterCommitMetaAnnotation() throws Exception { load(AfterCommitMetaAnnotationTestListener.class); this.transactionTemplate.execute(status -> { getContext().publishEvent("test"); @@ -279,7 +282,6 @@ public class TransactionalEventListenerTests { } @Test - @Ignore("not an event listener if not tagged + condition found on wrong annotation") public void conditionFoundOnMetaAnnotation() { load(AfterCommitMetaAnnotationTestListener.class); this.transactionTemplate.execute(status -> {