Skip to content

Commit

Permalink
Remove unused class filtering support in AnnotationScanner
Browse files Browse the repository at this point in the history 
PR spring-projectsgh-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 spring-projectsgh-25477
  • Loading branch information
@sbrannen
sbrannen committed Jul 28, 2020
1 parent 5442d87 commit 2b3fdfa
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 139 deletions.
134 changes: 40 additions & 94 deletions spring-core/src/main/java/org/springframework/core/annotation/AnnotationsScanner.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -75,70 +74,49 @@ private AnnotationsScanner() {
static <C, R> R scan(C context, AnnotatedElement source, SearchStrategy searchStrategy,
AnnotationsProcessor<C, R> 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 <C, R> R scan(C context, AnnotatedElement source, SearchStrategy searchStrategy,
AnnotationsProcessor<C, R> processor, @Nullable BiPredicate<C, Class<?>> classFilter) {

R result = process(context, source, searchStrategy, processor, classFilter);
R result = process(context, source, searchStrategy, processor);
return processor.finish(result);
}

@Nullable
private static <C, R> R process(C context, AnnotatedElement source,
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> processor,
@Nullable BiPredicate<C, Class<?>> classFilter) {
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> 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 <C, R> R processClass(C context, Class<?> source,
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> processor,
@Nullable BiPredicate<C, Class<?>> classFilter) {
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> 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 <C, R> R processClassInheritedAnnotations(C context, Class<?> source,
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> processor, @Nullable BiPredicate<C, Class<?>> classFilter) {
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> 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;
Expand All @@ -150,14 +128,7 @@ private static <C, R> 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;
Expand Down Expand Up @@ -195,17 +166,15 @@ private static <C, R> R processClassInheritedAnnotations(C context, Class<?> sou

@Nullable
private static <C, R> R processClassHierarchy(C context, Class<?> source,
AnnotationsProcessor<C, R> processor, @Nullable BiPredicate<C, Class<?>> classFilter,
boolean includeInterfaces, boolean includeEnclosing) {
AnnotationsProcessor<C, R> processor, boolean includeInterfaces, boolean includeEnclosing) {

return processClassHierarchy(context, new int[] {0}, source, processor,
classFilter, includeInterfaces, includeEnclosing);
includeInterfaces, includeEnclosing);
}

@Nullable
private static <C, R> R processClassHierarchy(C context, int[] aggregateIndex, Class<?> source,
AnnotationsProcessor<C, R> processor, @Nullable BiPredicate<C, Class<?>> classFilter,
boolean includeInterfaces, boolean includeEnclosing) {
AnnotationsProcessor<C, R> processor, boolean includeInterfaces, boolean includeEnclosing) {

try {
R result = processor.doWithAggregate(context, aggregateIndex[0]);
Expand All @@ -215,7 +184,7 @@ private static <C, R> 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;
Expand All @@ -224,7 +193,7 @@ private static <C, R> 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;
}
Expand All @@ -233,7 +202,7 @@ private static <C, R> 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;
}
Expand All @@ -248,7 +217,7 @@ private static <C, R> 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;
}
Expand All @@ -267,32 +236,31 @@ private static <C, R> R processClassHierarchy(C context, int[] aggregateIndex, C

@Nullable
private static <C, R> R processMethod(C context, Method source,
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> processor,
@Nullable BiPredicate<C, Class<?>> classFilter) {
SearchStrategy searchStrategy, AnnotationsProcessor<C, R> 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 <C, R> R processMethodInheritedAnnotations(C context, Method source,
AnnotationsProcessor<C, R> processor, @Nullable BiPredicate<C, Class<?>> classFilter) {
AnnotationsProcessor<C, R> 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);
Expand All @@ -302,8 +270,7 @@ private static <C, R> R processMethodInheritedAnnotations(C context, Method sour

@Nullable
private static <C, R> R processMethodHierarchy(C context, int[] aggregateIndex,
Class<?> sourceClass, AnnotationsProcessor<C, R> processor,
@Nullable BiPredicate<C, Class<?>> classFilter, Method rootMethod,
Class<?> sourceClass, AnnotationsProcessor<C, R> processor, Method rootMethod,
boolean includeInterfaces) {

try {
Expand All @@ -317,17 +284,17 @@ private static <C, R> 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;
Expand All @@ -344,7 +311,7 @@ private static <C, R> 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;
}
Expand All @@ -353,7 +320,7 @@ private static <C, R> 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;
}
Expand All @@ -365,11 +332,8 @@ private static <C, R> R processMethodHierarchy(C context, int[] aggregateIndex,
return null;
}

private static <C> Method[] getBaseTypeMethods(
C context, Class<?> baseType, @Nullable BiPredicate<C, Class<?>> classFilter) {

if (baseType == Object.class || hasPlainJavaAnnotationsOnly(baseType) ||
isFiltered(baseType, context, classFilter)) {
private static <C> Method[] getBaseTypeMethods(C context, Class<?> baseType) {
if (baseType == Object.class || hasPlainJavaAnnotationsOnly(baseType)) {
return NO_METHODS;
}

Expand Down Expand Up @@ -433,16 +397,16 @@ private static boolean hasSameGenericTypeParameters(

@Nullable
private static <C, R> R processMethodAnnotations(C context, int aggregateIndex, Method source,
AnnotationsProcessor<C, R> processor, @Nullable BiPredicate<C, Class<?>> classFilter) {
AnnotationsProcessor<C, R> 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;
Expand All @@ -455,31 +419,19 @@ private static <C, R> R processMethodAnnotations(C context, int aggregateIndex,

@Nullable
private static <C, R> R processElement(C context, AnnotatedElement source,
AnnotationsProcessor<C, R> processor, @Nullable BiPredicate<C, Class<?>> classFilter) {
AnnotationsProcessor<C, R> 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);
}
return null;
}

private static <C, R> Annotation[] getDeclaredAnnotations(C context,
AnnotatedElement source, @Nullable BiPredicate<C, Class<?>> 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 extends Annotation> A getDeclaredAnnotation(AnnotatedElement source, Class<A> annotationType) {
Expand Down Expand Up @@ -525,12 +477,6 @@ static Annotation[] getDeclaredAnnotations(AnnotatedElement source, boolean defe
return annotations.clone();
}

private static <C> boolean isFiltered(
Class<?> sourceClass, C context, @Nullable BiPredicate<C, Class<?>> classFilter) {

return (classFilter != null && classFilter.test(context, sourceClass));
}

private static boolean isIgnorable(Class<?> annotationType) {
return AnnotationFilter.PLAIN.matches(annotationType);
}
Expand Down
45 changes: 0 additions & 45 deletions spring-core/src/test/java/org/springframework/core/annotation/AnnotationsScannerTests.java
View file
Expand Up @@ -128,51 +128,6 @@ void inheritedAnnotationsStrategyOnClassWhenHasAnnotationOnBothClassesIncudesOnl
assertThat(scan(source, SearchStrategy.INHERITED_ANNOTATIONS)).containsExactly("0:TestInheritedAnnotation2");
}

@Test
void inheritedAnnotationsStrategyOnClassWithAllClassesFilteredOut() {
List<String> 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<String> 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<String> 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;
Expand Down

0 comments on commit 2b3fdfa

Please sign in to comment.