From 4521a79b2dfd4802bcbe2fba4c841510b5f0da54 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 8 Aug 2018 23:52:47 +0200 Subject: [PATCH] Find annotations on implemented generic superclass methods as well Includes Java 8 getDeclaredAnnotation shortcut for lookup on Class. Issue: SPR-17146 --- .../annotation/AnnotatedElementUtils.java | 70 ++++++++----------- .../core/annotation/AnnotationUtils.java | 19 +++-- .../AnnotatedElementUtilsTests.java | 28 +++++--- .../core/annotation/AnnotationUtilsTests.java | 53 +++++++++----- 4 files changed, 92 insertions(+), 78 deletions(-) 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 a68ade2dba..be6ec8b1ad 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 @@ -386,13 +386,14 @@ public abstract class AnnotatedElementUtils { @Nullable public static A getMergedAnnotation(AnnotatedElement element, Class 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 abstract class AnnotatedElementUtils { @Nullable public static A findMergedAnnotation(AnnotatedElement element, Class 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 @@ public abstract class AnnotatedElementUtils { Set 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 @@ public abstract class AnnotatedElementUtils { Set 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 @@ public abstract class AnnotatedElementUtils { /** * 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 @@ public abstract class AnnotatedElementUtils { /** * Callback interface that is used to process annotations during a search. - *

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). - *

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} + *

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). + *

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. - *

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. + *

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 the type of result returned by the processor */ private interface Processor { 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 c0981e1bc4..94e9d648b9 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 @@ -520,7 +520,7 @@ public abstract class AnnotationUtils { /** * 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 directly present on the given * method itself. *

Correctly handles bridge {@link Method Methods} generated by the compiler. @@ -560,8 +560,7 @@ public abstract class AnnotationUtils { Set 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 @@ public abstract class AnnotationUtils { 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 abstract class AnnotationUtils { /** * Determine whether an annotation of the specified {@code annotationType} - * is declared locally (i.e., directly present) on the supplied + * is declared locally (i.e. directly present) on the supplied * {@code clazz}. *

The supplied {@link Class} may represent any type. *

Meta-annotations will not be searched. @@ -872,8 +871,8 @@ public abstract class AnnotationUtils { /** * Determine whether an annotation of the specified {@code annotationType} * is present on the supplied {@code clazz} and is - * {@linkplain java.lang.annotation.Inherited inherited} (i.e., not - * directly present). + * {@linkplain java.lang.annotation.Inherited inherited} + * (i.e. not directly present). *

Meta-annotations will not be searched. *

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 @@ public abstract class AnnotationUtils { * in the supplied annotation type. *

The map is keyed by attribute name with each value representing * a list of names of aliased attributes. - *

For explicit alias pairs such as x and y (i.e., where x + *

For explicit 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)}. - *

For implicit aliases (i.e., attributes that are declared + *

For implicit 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 @@ public abstract class AnnotationUtils { /** * Determine if annotations of the supplied {@code annotationType} are - * synthesizable (i.e., in need of being wrapped in a dynamic + * synthesizable (i.e. in need of being wrapped in a dynamic * proxy that provides functionality above that of a standard JDK * annotation). *

Specifically, an annotation is synthesizable if it declares 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 7fa7760adf..80d7201f30 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 @@ -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 class AnnotatedElementUtilsTests { assertNotNull("Should find @Transactional on ConcreteClassWithInheritedAnnotation.handle() method", attributes); } - /** - *

{@code AbstractClassWithInheritedAnnotation} declares {@code handleParameterized(T)}; whereas, - * {@code ConcreteClassWithInheritedAnnotation} declares {@code handleParameterized(String)}. - *

As of Spring 4.2, {@code AnnotatedElementUtils.processWithFindSemantics()} does not resolve an - * equivalent method in {@code AbstractClassWithInheritedAnnotation} for the bridged - * {@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 class AnnotatedElementUtilsTests { * @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 class AnnotatedElementUtilsTests { 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); + } + // ------------------------------------------------------------------------- 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 51fc3ea0d3..2fb351dee0 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 @@ -167,9 +167,7 @@ public class AnnotationUtilsTests { assertNull(bridgedMethod.getAnnotation(Order.class)); assertNull(getAnnotation(bridgedMethod, Order.class)); - // AnnotationUtils.findAnnotation(Method, Class) 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 class AnnotationUtilsTests { 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,7 +209,7 @@ public class AnnotationUtilsTests { assertNotNull(order); } - /** @since 4.1.2 */ + // @since 4.1.2 @Test public void findClassAnnotationFavorsMoreLocallyDeclaredComposedAnnotationsOverAnnotationsOnInterfaces() { Component component = findAnnotation(ClassWithLocalMetaAnnotationAndMetaAnnotatedInterface.class, Component.class); @@ -212,7 +217,7 @@ public class AnnotationUtilsTests { assertEquals("meta2", component.value()); } - /** @since 4.0.3 */ + // @since 4.0.3 @Test public void findClassAnnotationFavorsMoreLocallyDeclaredComposedAnnotationsOverInheritedAnnotations() { Transactional transactional = findAnnotation(SubSubClassWithInheritedAnnotation.class, Transactional.class); @@ -220,7 +225,7 @@ public class AnnotationUtilsTests { 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 class AnnotationUtilsTests { public static class SubTransactionalAndOrderedClass extends TransactionalAndOrderedClass { } - public interface InterfaceWithGenericAnnotatedMethod { - - @Order - void foo(T t); - } - - public static class ImplementsInterfaceWithGenericAnnotatedMethod implements InterfaceWithGenericAnnotatedMethod { - - public void foo(String t) { - } - } - public interface InterfaceWithAnnotatedMethod { @Order @@ -1806,6 +1799,30 @@ public class AnnotationUtilsTests { } } + public interface InterfaceWithGenericAnnotatedMethod { + + @Order + void foo(T t); + } + + public static class ImplementsInterfaceWithGenericAnnotatedMethod implements InterfaceWithGenericAnnotatedMethod { + + public void foo(String t) { + } + } + + public static abstract class BaseClassWithGenericAnnotatedMethod { + + @Order + abstract void foo(T t); + } + + public static class ExtendsBaseClassWithGenericAnnotatedMethod extends BaseClassWithGenericAnnotatedMethod { + + public void foo(String t) { + } + } + @Retention(RetentionPolicy.RUNTIME) @Inherited @interface MyRepeatableContainer {