From de3c115614d58782f96953c548f56e650fc97ab2 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 31 Jul 2019 22:05:46 +0200 Subject: [PATCH] Fix bug in TYPE_HIERARCHY_AND_ENCLOSING_CLASSES strategy Prior to this commit, the new `TYPE_HIERARCHY_AND_ENCLOSING_CLASSES` annotation search strategy failed to find annotations on enclosing classes if the source class was a nested class that itself had no annotations present. This commit fixes this by adding special logic to AnnotationsScanner's isWithoutHierarchy() method to properly support nested classes. Closes gh-23378 --- .../core/annotation/AnnotationsScanner.java | 17 +++++++------ .../annotation/MergedAnnotationsTests.java | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java index 05737a5a9a..28669b8426 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java @@ -39,6 +39,7 @@ import org.springframework.util.ReflectionUtils; * {@link AnnotatedElement}. * * @author Phillip Webb + * @author Sam Brannen * @since 5.2 * @see AnnotationsProcessor */ @@ -121,7 +122,7 @@ abstract class AnnotationsScanner { case DIRECT: return processElement(context, source, processor, classFilter); case INHERITED_ANNOTATIONS: - return processClassInheritedAnnotations(context, source, processor, classFilter); + return processClassInheritedAnnotations(context, source, searchStrategy, processor, classFilter); case SUPERCLASS: return processClassHierarchy(context, source, processor, classFilter, false, false); case EXHAUSTIVE: @@ -135,9 +136,9 @@ abstract class AnnotationsScanner { @Nullable private static R processClassInheritedAnnotations(C context, Class source, - AnnotationsProcessor processor, @Nullable BiPredicate> classFilter) { + SearchStrategy searchStrategy, AnnotationsProcessor processor, @Nullable BiPredicate> classFilter) { - if (isWithoutHierarchy(source)) { + if (isWithoutHierarchy(source, searchStrategy)) { return processElement(context, source, processor, classFilter); } Annotation[] relevant = null; @@ -505,7 +506,7 @@ abstract class AnnotationsScanner { if (hasPlainJavaAnnotationsOnly(source)) { return true; } - if (searchStrategy == SearchStrategy.DIRECT || isWithoutHierarchy(source)) { + if (searchStrategy == SearchStrategy.DIRECT || isWithoutHierarchy(source, searchStrategy)) { if (source instanceof Method && ((Method) source).isBridge()) { return false; } @@ -530,19 +531,21 @@ abstract class AnnotationsScanner { return (type.getName().startsWith("java.") || type == Ordered.class); } - private static boolean isWithoutHierarchy(AnnotatedElement source) { + private static boolean isWithoutHierarchy(AnnotatedElement source, SearchStrategy searchStrategy) { if (source == Object.class) { return true; } if (source instanceof Class) { Class sourceClass = (Class) source; - return (sourceClass.getSuperclass() == Object.class && + boolean noSuperTypes = (sourceClass.getSuperclass() == Object.class && sourceClass.getInterfaces().length == 0); + return (searchStrategy == SearchStrategy.TYPE_HIERARCHY_AND_ENCLOSING_CLASSES ? noSuperTypes && + sourceClass.getEnclosingClass() == null : noSuperTypes); } if (source instanceof Method) { Method sourceMethod = (Method) source; return (Modifier.isPrivate(sourceMethod.getModifiers()) || - isWithoutHierarchy(sourceMethod.getDeclaringClass())); + isWithoutHierarchy(sourceMethod.getDeclaringClass(), searchStrategy)); } return true; } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java index 7469349bf9..2680d7094f 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsTests.java @@ -692,6 +692,19 @@ public class MergedAnnotationsTests { Transactional.class)).hasSize(1); } + @Test + public void streamTypeHierarchyAndEnclosingClassesFromNonAnnotatedInnerClassWithAnnotatedEnclosingClass() { + Stream> classes = MergedAnnotations.from(AnnotatedClass.NonAnnotatedInnerClass.class, + SearchStrategy.TYPE_HIERARCHY_AND_ENCLOSING_CLASSES).stream().map(MergedAnnotation::getType); + assertThat(classes).containsExactly(Component.class, Indexed.class); + } + + @Test + public void streamTypeHierarchyAndEnclosingClassesFromNonAnnotatedStaticNestedClassWithAnnotatedEnclosingClass() { + Stream> classes = MergedAnnotations.from(AnnotatedClass.NonAnnotatedStaticNestedClass.class, + SearchStrategy.TYPE_HIERARCHY_AND_ENCLOSING_CLASSES).stream().map(MergedAnnotation::getType); + assertThat(classes).containsExactly(Component.class, Indexed.class); + } @Test public void getFromMethodWithMethodAnnotationOnLeaf() throws Exception { Method method = Leaf.class.getMethod("annotatedOnLeaf"); @@ -2112,6 +2125,16 @@ public class MergedAnnotationsTests { static class NonAnnotatedClass { } + @Component + static class AnnotatedClass { + + class NonAnnotatedInnerClass { + } + + static class NonAnnotatedStaticNestedClass { + } + } + static interface NonAnnotatedInterface { } @@ -2839,6 +2862,7 @@ public class MergedAnnotationsTests { public static class ImplementsInterfaceWithGenericAnnotatedMethod implements InterfaceWithGenericAnnotatedMethod { + @Override public void foo(String t) { } } @@ -2852,6 +2876,7 @@ public class MergedAnnotationsTests { public static class ExtendsBaseClassWithGenericAnnotatedMethod extends BaseClassWithGenericAnnotatedMethod { + @Override public void foo(String t) { } }