From 7f0f04dfe38ead84e502f0d34897d1c415565cdd Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 22 Apr 2015 21:32:06 +0200 Subject: [PATCH] Support annotations on interfaces in AnnotatedElementUtils This commit introduces support in AnnotatedElementUtils for finding annotations declared on interfaces at the type level. NB: this commit does not include support for finding annotations declared on interface methods. In order to maintain backward compatibility with @Transactional annotation attribute processing, a new getAnnotationAttributes() method has been added to AnnotatedElementUtils that provides a flag to control whether interfaces should be searched. SpringTransactionAnnotationParser and JtaTransactionAnnotationParser have been updated accordingly to ensure that interfaces are not unintentionally searched in the @Transactional resolution process. This commit also introduces additional tests and updates TODOs for SPR-12738. Issue: SPR-12944, SPR-12738 --- .../aspectj/TransactionAspectTests.java | 12 +- .../annotation/AnnotatedElementUtils.java | 124 ++++++++++++++---- .../AnnotatedElementUtilsTests.java | 109 +++++++++++++-- .../JtaTransactionAnnotationParser.java | 6 +- .../SpringTransactionAnnotationParser.java | 6 +- 5 files changed, 210 insertions(+), 47 deletions(-) diff --git a/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java b/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java index d1de2db427..10d8991642 100644 --- a/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java +++ b/spring-aspects/src/test/java/org/springframework/transaction/aspectj/TransactionAspectTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 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. @@ -18,7 +18,6 @@ package org.springframework.transaction.aspectj; import java.lang.reflect.Method; -import org.springframework.test.AbstractDependencyInjectionSpringContextTests; import org.springframework.tests.transaction.CallCountingTransactionManager; import org.springframework.transaction.annotation.AnnotationTransactionAttributeSource; import org.springframework.transaction.interceptor.TransactionAspectSupport; @@ -28,8 +27,10 @@ import org.springframework.transaction.interceptor.TransactionAttribute; * @author Rod Johnson * @author Ramnivas Laddad * @author Juergen Hoeller + * @author Sam Brannen */ -public class TransactionAspectTests extends AbstractDependencyInjectionSpringContextTests { +@SuppressWarnings("deprecation") +public class TransactionAspectTests extends org.springframework.test.AbstractDependencyInjectionSpringContextTests { private TransactionAspectSupport transactionAspect; @@ -206,17 +207,14 @@ public class TransactionAspectTests extends AbstractDependencyInjectionSpringCon * Note: resolution does not occur. Thus we can't make a class transactional if * it implements a transactionally annotated interface. This behaviour could only * be changed in AbstractFallbackTransactionAttributeSource in Spring proper. - * @throws SecurityException - * @throws NoSuchMethodException */ - public void testDoesNotResolveTxAnnotationOnMethodFromClassImplementingAnnotatedInterface() throws SecurityException, NoSuchMethodException { + public void testDoesNotResolveTxAnnotationOnMethodFromClassImplementingAnnotatedInterface() throws Exception { AnnotationTransactionAttributeSource atas = new AnnotationTransactionAttributeSource(); Method m = ImplementsAnnotatedInterface.class.getMethod("echo", Throwable.class); TransactionAttribute ta = atas.getTransactionAttribute(m, ImplementsAnnotatedInterface.class); assertNull(ta); } - public void testDefaultRollbackOnImplementationOfAnnotatedInterface() throws Throwable { // testRollback(new TransactionOperationCallback() { // public Object performTransactionalOperation() throws Throwable { diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java index b7a528609d..2cc0564c7b 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java @@ -27,8 +27,8 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; /** - * Utility class used to collect all annotation values including those declared on - * meta-annotations. + * Utility class used to collect all annotation attributes, including those + * declared on meta-annotations. * * @author Phillip Webb * @author Juergen Hoeller @@ -39,7 +39,7 @@ public class AnnotatedElementUtils { public static Set getMetaAnnotationTypes(AnnotatedElement element, String annotationType) { final Set types = new LinkedHashSet(); - process(element, annotationType, false, new Processor() { + process(element, annotationType, true, false, new Processor() { @Override public Object process(Annotation annotation, int metaDepth) { if (metaDepth > 0) { @@ -55,7 +55,7 @@ public class AnnotatedElementUtils { } public static boolean hasMetaAnnotationTypes(AnnotatedElement element, String annotationType) { - return Boolean.TRUE.equals(process(element, annotationType, false, new Processor() { + return Boolean.TRUE.equals(process(element, annotationType, true, false, new Processor() { @Override public Boolean process(Annotation annotation, int metaDepth) { if (metaDepth > 0) { @@ -70,7 +70,7 @@ public class AnnotatedElementUtils { } public static boolean isAnnotated(AnnotatedElement element, String annotationType) { - return Boolean.TRUE.equals(process(element, annotationType, false, new Processor() { + return Boolean.TRUE.equals(process(element, annotationType, true, false, new Processor() { @Override public Boolean process(Annotation annotation, int metaDepth) { return Boolean.TRUE; @@ -81,14 +81,59 @@ public class AnnotatedElementUtils { })); } + /** + * Delegates to {@link #getAnnotationAttributes(AnnotatedElement, String, boolean, boolean)}, + * supplying {@code false} for {@code classValuesAsString} and {@code nestedAnnotationsAsMap}. + * + * @param element the annotated element + * @param annotationType the annotation type to find + * @see #getAnnotationAttributes(AnnotatedElement, String, boolean, boolean) + */ public static AnnotationAttributes getAnnotationAttributes(AnnotatedElement element, String annotationType) { return getAnnotationAttributes(element, annotationType, false, false); } + /** + * Delegates to {@link #getAnnotationAttributes(AnnotatedElement, String, boolean, boolean, boolean, boolean)}, + * supplying {@code true} for {@code searchInterfaces} and {@code false} for {@code searchClassHierarchy}. + * + * @param element the annotated element + * @param annotationType the annotation type to find + * @param classValuesAsString whether to convert Class references into + * Strings or to preserve them as Class references + * @param nestedAnnotationsAsMap whether to turn nested Annotation instances + * into {@link AnnotationAttributes} maps or to preserve them as Annotation + * instances + * @see #getAnnotationAttributes(AnnotatedElement, String, boolean, boolean, boolean, boolean) + */ public static AnnotationAttributes getAnnotationAttributes(AnnotatedElement element, String annotationType, - final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) { + boolean classValuesAsString, boolean nestedAnnotationsAsMap) { + return getAnnotationAttributes(element, annotationType, true, false, classValuesAsString, + nestedAnnotationsAsMap); + } - return process(element, annotationType, false, new Processor() { + /** + * Find annotation attributes of the specified {@code annotationType} in + * the annotation hierarchy of the supplied {@link AnnotatedElement}, + * and merge the results into an {@link AnnotationAttributes} map. + * + * @param element the annotated element + * @param annotationType the annotation type to find + * @param searchInterfaces whether or not to search on interfaces, if the + * annotated element is a class + * @param searchClassHierarchy whether or not to search the class hierarchy + * recursively, if the annotated element is a class + * @param classValuesAsString whether to convert Class references into + * Strings or to preserve them as Class references + * @param nestedAnnotationsAsMap whether to turn nested Annotation instances + * into {@link AnnotationAttributes} maps or to preserve them as Annotation + * instances + */ + public static AnnotationAttributes getAnnotationAttributes(AnnotatedElement element, String annotationType, + boolean searchInterfaces, boolean searchClassHierarchy, final boolean classValuesAsString, + final boolean nestedAnnotationsAsMap) { + + return process(element, annotationType, searchInterfaces, searchClassHierarchy, new Processor() { @Override public AnnotationAttributes process(Annotation annotation, int metaDepth) { return AnnotationUtils.getAnnotationAttributes(annotation, classValuesAsString, nestedAnnotationsAsMap); @@ -115,7 +160,7 @@ public class AnnotatedElementUtils { final String annotationType, final boolean classValuesAsString, final boolean nestedAnnotationsAsMap) { final MultiValueMap attributes = new LinkedMultiValueMap(); - process(element, annotationType, false, new Processor() { + process(element, annotationType, true, false, new Processor() { @Override public Void process(Annotation annotation, int metaDepth) { if (annotation.annotationType().getName().equals(annotationType)) { @@ -144,22 +189,26 @@ public class AnnotatedElementUtils { /** * Process all annotations of the specified {@code annotationType} and * recursively all meta-annotations on the specified {@code element}. - *

If the {@code traverseClassHierarchy} flag is {@code true} and the sought + * + *

If the {@code searchClassHierarchy} flag is {@code true} and the sought * annotation is neither directly present 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 - * @param traverseClassHierarchy whether or not to traverse up the class - * hierarchy recursively + * @param searchInterfaces whether or not to search on interfaces, if the + * annotated element is a class + * @param searchClassHierarchy whether or not to search the class hierarchy + * recursively, if the annotated element is a class * @param processor the processor to delegate to * @return the result of the processor */ - private static T process(AnnotatedElement element, String annotationType, boolean traverseClassHierarchy, - Processor processor) { + private static T process(AnnotatedElement element, String annotationType, boolean searchInterfaces, + boolean searchClassHierarchy, Processor processor) { try { - return doProcess(element, annotationType, traverseClassHierarchy, processor, + return doProcess(element, annotationType, searchInterfaces, searchClassHierarchy, processor, new HashSet(), 0); } catch (Throwable ex) { @@ -171,54 +220,79 @@ public class AnnotatedElementUtils { * Perform the search algorithm for the {@link #process} method, avoiding * endless recursion by tracking which annotated elements have already been * visited. + * *

The {@code metaDepth} parameter represents the depth of the annotation * relative to the initial element. For example, an annotation that is * present 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 searchInterfaces whether or not to search on interfaces, if the + * annotated element is a class + * @param searchClassHierarchy whether or not to search the class hierarchy + * recursively, if the annotated element is a class * @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 doProcess(AnnotatedElement element, String annotationType, boolean traverseClassHierarchy, - Processor processor, Set visited, int metaDepth) { + private static T doProcess(AnnotatedElement element, String annotationType, boolean searchInterfaces, + boolean searchClassHierarchy, Processor processor, Set visited, int metaDepth) { if (visited.add(element)) { try { + + // Local annotations: either directly declared or inherited. Annotation[] annotations = - (traverseClassHierarchy ? element.getDeclaredAnnotations() : element.getAnnotations()); + (searchClassHierarchy ? element.getDeclaredAnnotations() : element.getAnnotations()); + + // Search in local annotations 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, traverseClassHierarchy, - processor, visited, metaDepth + 1); + result = doProcess(annotation.annotationType(), annotationType, searchInterfaces, + searchClassHierarchy, processor, visited, metaDepth + 1); if (result != null) { processor.postProcess(annotation, result); return result; } } } + + // Search in meta annotations on location annotations for (Annotation annotation : annotations) { if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation)) { - T result = doProcess(annotation.annotationType(), annotationType, traverseClassHierarchy, - processor, visited, metaDepth); + T result = doProcess(annotation.annotationType(), annotationType, searchInterfaces, + searchClassHierarchy, processor, visited, metaDepth); if (result != null) { processor.postProcess(annotation, result); return result; } } } - if (traverseClassHierarchy && element instanceof Class) { + + // Search on interfaces + if (searchInterfaces && element instanceof Class) { + Class clazz = (Class) element; + for (Class ifc : clazz.getInterfaces()) { + T result = doProcess(ifc, annotationType, searchInterfaces, searchClassHierarchy, processor, + visited, metaDepth); + if (result != null) { + return result; + } + } + } + + // Search on superclass + if (searchClassHierarchy && element instanceof Class) { Class superclass = ((Class) element).getSuperclass(); if (superclass != null && !superclass.equals(Object.class)) { - T result = doProcess(superclass, annotationType, true, processor, visited, metaDepth); + T result = doProcess(superclass, annotationType, searchInterfaces, searchClassHierarchy, + processor, visited, metaDepth); if (result != null) { return result; } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java index 4d5e3fa329..4aa8d4f55e 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java @@ -37,6 +37,7 @@ import static org.springframework.core.annotation.AnnotatedElementUtils.*; * Unit tests for {@link AnnotatedElementUtils}. * * @author Sam Brannen + * @author Rossen Stoyanchev * @since 4.0.3 */ public class AnnotatedElementUtilsTests { @@ -125,33 +126,92 @@ public class AnnotatedElementUtilsTests { attributes.getBoolean("readOnly")); } - // SPR-12738 - + /** @since 4.2 */ @Test - public void getAnnotationAttributesInheritedFromInterface() { + public void getAnnotationAttributesOnInheritedAnnotationInterface() { String name = Transactional.class.getName(); - AnnotationAttributes attributes = getAnnotationAttributes(ConcreteClassWithInheritedAnnotation.class, name); -// assertNotNull(attributes); + AnnotationAttributes attributes = getAnnotationAttributes(InheritedAnnotationInterface.class, name); + assertNotNull("Should find @Transactional on InheritedAnnotationInterface", attributes); } - // SPR-12738 + /** @since 4.2 */ + @Test + public void getAnnotationAttributesOnSubInheritedAnnotationInterface() { + String name = Transactional.class.getName(); + AnnotationAttributes attributes = getAnnotationAttributes(SubInheritedAnnotationInterface.class, name); + assertNotNull("Should find @Transactional on SubInheritedAnnotationInterface", attributes); + } + /** @since 4.2 */ + @Test + public void getAnnotationAttributesOnSubSubInheritedAnnotationInterface() { + String name = Transactional.class.getName(); + AnnotationAttributes attributes = getAnnotationAttributes(SubSubInheritedAnnotationInterface.class, name); + assertNotNull("Should find @Transactional on SubSubInheritedAnnotationInterface", attributes); + } + + /** @since 4.2 */ + @Test + public void getAnnotationAttributesOnNonInheritedAnnotationInterface() { + String name = Order.class.getName(); + AnnotationAttributes attributes = getAnnotationAttributes(NonInheritedAnnotationInterface.class, name); + assertNotNull("Should find @Order on NonInheritedAnnotationInterface", attributes); + } + + /** @since 4.2 */ + @Test + public void getAnnotationAttributesOnSubNonInheritedAnnotationInterface() { + String name = Order.class.getName(); + AnnotationAttributes attributes = getAnnotationAttributes(SubNonInheritedAnnotationInterface.class, name); + assertNotNull("Should find @Order on SubNonInheritedAnnotationInterface", attributes); + } + + /** @since 4.2 */ + @Test + public void getAnnotationAttributesOnSubSubNonInheritedAnnotationInterface() { + String name = Order.class.getName(); + AnnotationAttributes attributes = getAnnotationAttributes(SubSubNonInheritedAnnotationInterface.class, name); + assertNotNull("Should find @Order on SubSubNonInheritedAnnotationInterface", attributes); + } + + // TODO [SPR-11598] Enable test. + @Ignore("Disabled until SPR-11598 is resolved") + @Test + public void getAnnotationAttributesFromInterfaceImplementedBySuperclass() { + String name = Transactional.class.getName(); + AnnotationAttributes attributes = getAnnotationAttributes(ConcreteClassWithInheritedAnnotation.class, name); + assertNotNull("Should find @Transactional on ConcreteClassWithInheritedAnnotation", attributes); + } + + // TODO [SPR-12738] Enable test. + @Ignore("Disabled until SPR-12738 is resolved") + @Test + public void getAnnotationAttributesInheritedFromInterfaceMethod() throws NoSuchMethodException { + String name = Order.class.getName(); + Method method = ConcreteClassWithInheritedAnnotation.class.getMethod("handleFromInterface"); + AnnotationAttributes attributes = getAnnotationAttributes(method, name); + assertNotNull("Should find @Order on ConcreteClassWithInheritedAnnotation.handleFromInterface() method", + attributes); + } + + // TODO [SPR-12738] Enable test. + @Ignore("Disabled until SPR-12738 is resolved") @Test public void getAnnotationAttributesInheritedFromAbstractMethod() throws NoSuchMethodException { String name = Transactional.class.getName(); Method method = ConcreteClassWithInheritedAnnotation.class.getMethod("handle"); AnnotationAttributes attributes = getAnnotationAttributes(method, name); -// assertNotNull(attributes); + assertNotNull("Should find @Transactional on ConcreteClassWithInheritedAnnotation.handle() method", attributes); } - // SPR-12738 - + // TODO [SPR-12738] Enable test. + @Ignore("Disabled until SPR-12738 is resolved") @Test public void getAnnotationAttributesInheritedFromParameterizedMethod() throws NoSuchMethodException { String name = Transactional.class.getName(); Method method = ConcreteClassWithInheritedAnnotation.class.getMethod("handleParameterized", String.class); - AnnotationAttributes attributes = getAnnotationAttributes(ConcreteClassWithInheritedAnnotation.class, name); -// assertNotNull(attributes); + AnnotationAttributes attributes = getAnnotationAttributes(method, name); + assertNotNull("Should find @Transactional on ConcreteClassWithInheritedAnnotation.handleParameterized() method", attributes); } @@ -259,6 +319,9 @@ public class AnnotatedElementUtilsTests { @Transactional static interface InterfaceWithInheritedAnnotation { + + @Order + void handleFromInterface(); } static abstract class AbstractClassWithInheritedAnnotation implements InterfaceWithInheritedAnnotation { @@ -280,6 +343,30 @@ public class AnnotatedElementUtilsTests { @Override public void handleParameterized(String s) { } + + @Override + public void handleFromInterface() { + } + } + + @Transactional + public static interface InheritedAnnotationInterface { + } + + public static interface SubInheritedAnnotationInterface extends InheritedAnnotationInterface { + } + + public static interface SubSubInheritedAnnotationInterface extends SubInheritedAnnotationInterface { + } + + @Order + public static interface NonInheritedAnnotationInterface { + } + + public static interface SubNonInheritedAnnotationInterface extends NonInheritedAnnotationInterface { + } + + public static interface SubSubNonInheritedAnnotationInterface extends SubNonInheritedAnnotationInterface { } } diff --git a/spring-tx/src/main/java/org/springframework/transaction/annotation/JtaTransactionAnnotationParser.java b/spring-tx/src/main/java/org/springframework/transaction/annotation/JtaTransactionAnnotationParser.java index 8b05331450..a175bf97db 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/annotation/JtaTransactionAnnotationParser.java +++ b/spring-tx/src/main/java/org/springframework/transaction/annotation/JtaTransactionAnnotationParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 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. @@ -32,6 +32,7 @@ import org.springframework.transaction.interceptor.TransactionAttribute; * Strategy implementation for parsing JTA 1.2's {@link javax.transaction.Transactional} annotation. * * @author Juergen Hoeller + * @author Sam Brannen * @since 4.0 */ @SuppressWarnings("serial") @@ -39,7 +40,8 @@ public class JtaTransactionAnnotationParser implements TransactionAnnotationPars @Override public TransactionAttribute parseTransactionAnnotation(AnnotatedElement ae) { - AnnotationAttributes ann = AnnotatedElementUtils.getAnnotationAttributes(ae, javax.transaction.Transactional.class.getName()); + AnnotationAttributes ann = AnnotatedElementUtils.getAnnotationAttributes(ae, + javax.transaction.Transactional.class.getName(), false, false, false, false); if (ann != null) { return parseTransactionAnnotation(ann); } diff --git a/spring-tx/src/main/java/org/springframework/transaction/annotation/SpringTransactionAnnotationParser.java b/spring-tx/src/main/java/org/springframework/transaction/annotation/SpringTransactionAnnotationParser.java index 60ae9b8fda..4cc2aa2ded 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/annotation/SpringTransactionAnnotationParser.java +++ b/spring-tx/src/main/java/org/springframework/transaction/annotation/SpringTransactionAnnotationParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 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. @@ -32,6 +32,7 @@ import org.springframework.transaction.interceptor.TransactionAttribute; * Strategy implementation for parsing Spring's {@link Transactional} annotation. * * @author Juergen Hoeller + * @author Sam Brannen * @since 2.5 */ @SuppressWarnings("serial") @@ -39,7 +40,8 @@ public class SpringTransactionAnnotationParser implements TransactionAnnotationP @Override public TransactionAttribute parseTransactionAnnotation(AnnotatedElement ae) { - AnnotationAttributes ann = AnnotatedElementUtils.getAnnotationAttributes(ae, Transactional.class.getName()); + AnnotationAttributes ann = AnnotatedElementUtils.getAnnotationAttributes(ae, Transactional.class.getName(), + false, false, false, false); if (ann != null) { return parseTransactionAnnotation(ann); }