From 49a42f050ce597572041914252b189feb22152aa Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 25 Jul 2020 16:23:08 +0200 Subject: [PATCH] Remove unused class filtering support in AnnotationScanner PR gh-25429 brought it to our attention that there was a bug in AnnotationScanner when using a non-null class filter that filtered out classes; however, it turns out that there is no production code that utilizes the package-private class filtering support. This commit therefore removes all class filtering support from AnnotationScanner since that functionality is effectively unused. Closes gh-25477 --- .../core/annotation/AnnotationsScanner.java | 134 ++++++------------ .../annotation/AnnotationsScannerTests.java | 45 ------ 2 files changed, 40 insertions(+), 139 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 b4841e3cf191..626413838c9b 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 @@ -23,7 +23,6 @@ import java.lang.reflect.Modifier; import java.util.Arrays; import java.util.Map; -import java.util.function.BiPredicate; import org.springframework.core.BridgeMethodResolver; import org.springframework.core.Ordered; @@ -75,70 +74,49 @@ private AnnotationsScanner() { static R scan(C context, AnnotatedElement source, SearchStrategy searchStrategy, AnnotationsProcessor processor) { - return scan(context, source, searchStrategy, processor, null); - } - - /** - * Scan the hierarchy of the specified element for relevant annotations and - * call the processor as required. - * @param context an optional context object that will be passed back to the - * processor - * @param source the source element to scan - * @param searchStrategy the search strategy to use - * @param processor the processor that receives the annotations - * @param classFilter an optional filter that can be used to entirely filter - * out a specific class from the hierarchy - * @return the result of {@link AnnotationsProcessor#finish(Object)} - */ - @Nullable - static R scan(C context, AnnotatedElement source, SearchStrategy searchStrategy, - AnnotationsProcessor processor, @Nullable BiPredicate> classFilter) { - - R result = process(context, source, searchStrategy, processor, classFilter); + R result = process(context, source, searchStrategy, processor); return processor.finish(result); } @Nullable private static R process(C context, AnnotatedElement source, - SearchStrategy searchStrategy, AnnotationsProcessor processor, - @Nullable BiPredicate> classFilter) { + SearchStrategy searchStrategy, AnnotationsProcessor processor) { if (source instanceof Class) { - return processClass(context, (Class) source, searchStrategy, processor, classFilter); + return processClass(context, (Class) source, searchStrategy, processor); } if (source instanceof Method) { - return processMethod(context, (Method) source, searchStrategy, processor, classFilter); + return processMethod(context, (Method) source, searchStrategy, processor); } - return processElement(context, source, processor, classFilter); + return processElement(context, source, processor); } @Nullable private static R processClass(C context, Class source, - SearchStrategy searchStrategy, AnnotationsProcessor processor, - @Nullable BiPredicate> classFilter) { + SearchStrategy searchStrategy, AnnotationsProcessor processor) { switch (searchStrategy) { case DIRECT: - return processElement(context, source, processor, classFilter); + return processElement(context, source, processor); case INHERITED_ANNOTATIONS: - return processClassInheritedAnnotations(context, source, searchStrategy, processor, classFilter); + return processClassInheritedAnnotations(context, source, searchStrategy, processor); case SUPERCLASS: - return processClassHierarchy(context, source, processor, classFilter, false, false); + return processClassHierarchy(context, source, processor, false, false); case TYPE_HIERARCHY: - return processClassHierarchy(context, source, processor, classFilter, true, false); + return processClassHierarchy(context, source, processor, true, false); case TYPE_HIERARCHY_AND_ENCLOSING_CLASSES: - return processClassHierarchy(context, source, processor, classFilter, true, true); + return processClassHierarchy(context, source, processor, true, true); } throw new IllegalStateException("Unsupported search strategy " + searchStrategy); } @Nullable private static R processClassInheritedAnnotations(C context, Class source, - SearchStrategy searchStrategy, AnnotationsProcessor processor, @Nullable BiPredicate> classFilter) { + SearchStrategy searchStrategy, AnnotationsProcessor processor) { try { if (isWithoutHierarchy(source, searchStrategy)) { - return processElement(context, source, processor, classFilter); + return processElement(context, source, processor); } Annotation[] relevant = null; int remaining = Integer.MAX_VALUE; @@ -150,14 +128,7 @@ private static R processClassInheritedAnnotations(C context, Class sou if (result != null) { return result; } - if (isFiltered(source, context, classFilter)) { - // Skip the current level in the class hierarchy. - source = source.getSuperclass(); - aggregateIndex++; - continue; - } - Annotation[] declaredAnnotations = - getDeclaredAnnotations(context, source, classFilter, true); + Annotation[] declaredAnnotations = getDeclaredAnnotations(source, true); if (relevant == null && declaredAnnotations.length > 0) { relevant = root.getAnnotations(); remaining = relevant.length; @@ -195,17 +166,15 @@ private static R processClassInheritedAnnotations(C context, Class sou @Nullable private static R processClassHierarchy(C context, Class source, - AnnotationsProcessor processor, @Nullable BiPredicate> classFilter, - boolean includeInterfaces, boolean includeEnclosing) { + AnnotationsProcessor processor, boolean includeInterfaces, boolean includeEnclosing) { return processClassHierarchy(context, new int[] {0}, source, processor, - classFilter, includeInterfaces, includeEnclosing); + includeInterfaces, includeEnclosing); } @Nullable private static R processClassHierarchy(C context, int[] aggregateIndex, Class source, - AnnotationsProcessor processor, @Nullable BiPredicate> classFilter, - boolean includeInterfaces, boolean includeEnclosing) { + AnnotationsProcessor processor, boolean includeInterfaces, boolean includeEnclosing) { try { R result = processor.doWithAggregate(context, aggregateIndex[0]); @@ -215,7 +184,7 @@ private static R processClassHierarchy(C context, int[] aggregateIndex, C if (hasPlainJavaAnnotationsOnly(source)) { return null; } - Annotation[] annotations = getDeclaredAnnotations(context, source, classFilter, false); + Annotation[] annotations = getDeclaredAnnotations(source, false); result = processor.doWithAnnotations(context, aggregateIndex[0], source, annotations); if (result != null) { return result; @@ -224,7 +193,7 @@ private static R processClassHierarchy(C context, int[] aggregateIndex, C if (includeInterfaces) { for (Class interfaceType : source.getInterfaces()) { R interfacesResult = processClassHierarchy(context, aggregateIndex, - interfaceType, processor, classFilter, true, includeEnclosing); + interfaceType, processor, true, includeEnclosing); if (interfacesResult != null) { return interfacesResult; } @@ -233,7 +202,7 @@ private static R processClassHierarchy(C context, int[] aggregateIndex, C Class superclass = source.getSuperclass(); if (superclass != Object.class && superclass != null) { R superclassResult = processClassHierarchy(context, aggregateIndex, - superclass, processor, classFilter, includeInterfaces, includeEnclosing); + superclass, processor, includeInterfaces, includeEnclosing); if (superclassResult != null) { return superclassResult; } @@ -248,7 +217,7 @@ private static R processClassHierarchy(C context, int[] aggregateIndex, C Class enclosingClass = source.getEnclosingClass(); if (enclosingClass != null) { R enclosingResult = processClassHierarchy(context, aggregateIndex, - enclosingClass, processor, classFilter, includeInterfaces, true); + enclosingClass, processor, includeInterfaces, true); if (enclosingResult != null) { return enclosingResult; } @@ -267,32 +236,31 @@ private static R processClassHierarchy(C context, int[] aggregateIndex, C @Nullable private static R processMethod(C context, Method source, - SearchStrategy searchStrategy, AnnotationsProcessor processor, - @Nullable BiPredicate> classFilter) { + SearchStrategy searchStrategy, AnnotationsProcessor processor) { switch (searchStrategy) { case DIRECT: case INHERITED_ANNOTATIONS: - return processMethodInheritedAnnotations(context, source, processor, classFilter); + return processMethodInheritedAnnotations(context, source, processor); case SUPERCLASS: return processMethodHierarchy(context, new int[] {0}, source.getDeclaringClass(), - processor, classFilter, source, false); + processor, source, false); case TYPE_HIERARCHY: case TYPE_HIERARCHY_AND_ENCLOSING_CLASSES: return processMethodHierarchy(context, new int[] {0}, source.getDeclaringClass(), - processor, classFilter, source, true); + processor, source, true); } throw new IllegalStateException("Unsupported search strategy " + searchStrategy); } @Nullable private static R processMethodInheritedAnnotations(C context, Method source, - AnnotationsProcessor processor, @Nullable BiPredicate> classFilter) { + AnnotationsProcessor processor) { try { R result = processor.doWithAggregate(context, 0); return (result != null ? result : - processMethodAnnotations(context, 0, source, processor, classFilter)); + processMethodAnnotations(context, 0, source, processor)); } catch (Throwable ex) { AnnotationUtils.handleIntrospectionFailure(source, ex); @@ -302,8 +270,7 @@ private static R processMethodInheritedAnnotations(C context, Method sour @Nullable private static R processMethodHierarchy(C context, int[] aggregateIndex, - Class sourceClass, AnnotationsProcessor processor, - @Nullable BiPredicate> classFilter, Method rootMethod, + Class sourceClass, AnnotationsProcessor processor, Method rootMethod, boolean includeInterfaces) { try { @@ -317,17 +284,17 @@ private static R processMethodHierarchy(C context, int[] aggregateIndex, boolean calledProcessor = false; if (sourceClass == rootMethod.getDeclaringClass()) { result = processMethodAnnotations(context, aggregateIndex[0], - rootMethod, processor, classFilter); + rootMethod, processor); calledProcessor = true; if (result != null) { return result; } } else { - for (Method candidateMethod : getBaseTypeMethods(context, sourceClass, classFilter)) { + for (Method candidateMethod : getBaseTypeMethods(context, sourceClass)) { if (candidateMethod != null && isOverride(rootMethod, candidateMethod)) { result = processMethodAnnotations(context, aggregateIndex[0], - candidateMethod, processor, classFilter); + candidateMethod, processor); calledProcessor = true; if (result != null) { return result; @@ -344,7 +311,7 @@ private static R processMethodHierarchy(C context, int[] aggregateIndex, if (includeInterfaces) { for (Class interfaceType : sourceClass.getInterfaces()) { R interfacesResult = processMethodHierarchy(context, aggregateIndex, - interfaceType, processor, classFilter, rootMethod, true); + interfaceType, processor, rootMethod, true); if (interfacesResult != null) { return interfacesResult; } @@ -353,7 +320,7 @@ private static R processMethodHierarchy(C context, int[] aggregateIndex, Class superclass = sourceClass.getSuperclass(); if (superclass != Object.class && superclass != null) { R superclassResult = processMethodHierarchy(context, aggregateIndex, - superclass, processor, classFilter, rootMethod, includeInterfaces); + superclass, processor, rootMethod, includeInterfaces); if (superclassResult != null) { return superclassResult; } @@ -365,11 +332,8 @@ private static R processMethodHierarchy(C context, int[] aggregateIndex, return null; } - private static Method[] getBaseTypeMethods( - C context, Class baseType, @Nullable BiPredicate> classFilter) { - - if (baseType == Object.class || hasPlainJavaAnnotationsOnly(baseType) || - isFiltered(baseType, context, classFilter)) { + private static Method[] getBaseTypeMethods(C context, Class baseType) { + if (baseType == Object.class || hasPlainJavaAnnotationsOnly(baseType)) { return NO_METHODS; } @@ -433,16 +397,16 @@ private static boolean hasSameGenericTypeParameters( @Nullable private static R processMethodAnnotations(C context, int aggregateIndex, Method source, - AnnotationsProcessor processor, @Nullable BiPredicate> classFilter) { + AnnotationsProcessor processor) { - Annotation[] annotations = getDeclaredAnnotations(context, source, classFilter, false); + Annotation[] annotations = getDeclaredAnnotations(source, false); R result = processor.doWithAnnotations(context, aggregateIndex, source, annotations); if (result != null) { return result; } Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(source); if (bridgedMethod != source) { - Annotation[] bridgedAnnotations = getDeclaredAnnotations(context, bridgedMethod, classFilter, true); + Annotation[] bridgedAnnotations = getDeclaredAnnotations(bridgedMethod, true); for (int i = 0; i < bridgedAnnotations.length; i++) { if (ObjectUtils.containsElement(annotations, bridgedAnnotations[i])) { bridgedAnnotations[i] = null; @@ -455,12 +419,12 @@ private static R processMethodAnnotations(C context, int aggregateIndex, @Nullable private static R processElement(C context, AnnotatedElement source, - AnnotationsProcessor processor, @Nullable BiPredicate> classFilter) { + AnnotationsProcessor processor) { try { R result = processor.doWithAggregate(context, 0); return (result != null ? result : processor.doWithAnnotations( - context, 0, source, getDeclaredAnnotations(context, source, classFilter, false))); + context, 0, source, getDeclaredAnnotations(source, false))); } catch (Throwable ex) { AnnotationUtils.handleIntrospectionFailure(source, ex); @@ -468,18 +432,6 @@ private static R processElement(C context, AnnotatedElement source, return null; } - private static Annotation[] getDeclaredAnnotations(C context, - AnnotatedElement source, @Nullable BiPredicate> classFilter, boolean copy) { - - if (source instanceof Class && isFiltered((Class) source, context, classFilter)) { - return NO_ANNOTATIONS; - } - if (source instanceof Method && isFiltered(((Method) source).getDeclaringClass(), context, classFilter)) { - return NO_ANNOTATIONS; - } - return getDeclaredAnnotations(source, copy); - } - @SuppressWarnings("unchecked") @Nullable static A getDeclaredAnnotation(AnnotatedElement source, Class annotationType) { @@ -525,12 +477,6 @@ static Annotation[] getDeclaredAnnotations(AnnotatedElement source, boolean defe return annotations.clone(); } - private static boolean isFiltered( - Class sourceClass, C context, @Nullable BiPredicate> classFilter) { - - return (classFilter != null && classFilter.test(context, sourceClass)); - } - private static boolean isIgnorable(Class annotationType) { return AnnotationFilter.PLAIN.matches(annotationType); } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java index fd929d63bf2c..83518d9b3846 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java @@ -128,51 +128,6 @@ void inheritedAnnotationsStrategyOnClassWhenHasAnnotationOnBothClassesIncudesOnl assertThat(scan(source, SearchStrategy.INHERITED_ANNOTATIONS)).containsExactly("0:TestInheritedAnnotation2"); } - @Test - void inheritedAnnotationsStrategyOnClassWithAllClassesFilteredOut() { - List annotationsFound = new ArrayList<>(); - String scanResult = AnnotationsScanner.scan(this, WithSingleSuperclass.class, - SearchStrategy.INHERITED_ANNOTATIONS, - (context, aggregateIndex, source, annotations) -> { - trackIndexedAnnotations(aggregateIndex, annotations, annotationsFound); - return null; // continue searching - }, - (context, clazz) -> true // filter out all classes - ); - assertThat(annotationsFound).isEmpty(); - assertThat(scanResult).isNull(); - } - - @Test - void inheritedAnnotationsStrategyOnClassWithSourceClassFilteredOut() { - List annotationsFound = new ArrayList<>(); - String scanResult = AnnotationsScanner.scan(this, WithSingleSuperclass.class, - SearchStrategy.INHERITED_ANNOTATIONS, - (context, aggregateIndex, source, annotations) -> { - trackIndexedAnnotations(aggregateIndex, annotations, annotationsFound); - return null; // continue searching - }, - (context, clazz) -> clazz == WithSingleSuperclass.class - ); - assertThat(annotationsFound).containsExactly(/* "0:TestAnnotation1", */ "1:TestInheritedAnnotation2"); - assertThat(scanResult).isNull(); - } - - @Test - void inheritedAnnotationsStrategyInClassHierarchyWithSuperSuperclassFilteredOut() { - List annotationsFound = new ArrayList<>(); - String scanResult = AnnotationsScanner.scan(this, WithHierarchy.class, - SearchStrategy.INHERITED_ANNOTATIONS, - (context, aggregateIndex, source, annotations) -> { - trackIndexedAnnotations(aggregateIndex, annotations, annotationsFound); - return null; // continue searching - }, - (context, clazz) -> clazz == HierarchySuperSuperclass.class - ); - assertThat(annotationsFound).containsExactly("0:TestAnnotation1", "1:TestInheritedAnnotation2"); - assertThat(scanResult).isNull(); - } - @Test void superclassStrategyOnClassWhenNotAnnotatedScansNone() { Class source = WithNoAnnotations.class;