Skip to content

Commit 4da27c2

Browse files
committed
Avoid unnecessary introspection on methods and meta-annotations
Issue: SPR-16667
1 parent b104897 commit 4da27c2

File tree

5 files changed

+80
-33
lines changed

5 files changed

+80
-33
lines changed

spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassEnhancer.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -431,7 +431,8 @@ private Object resolveBeanReference(Method beanMethod, Object[] beanMethodArgs,
431431

432432
@Override
433433
public boolean isMatch(Method candidateMethod) {
434-
return BeanAnnotationHelper.isBeanAnnotated(candidateMethod);
434+
return (candidateMethod.getDeclaringClass() != Object.class &&
435+
BeanAnnotationHelper.isBeanAnnotated(candidateMethod));
435436
}
436437

437438
private ConfigurableBeanFactory getBeanFactory(Object enhancedConfigInstance) {

spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import org.springframework.core.BridgeMethodResolver;
3232
import org.springframework.lang.Nullable;
33+
import org.springframework.util.CollectionUtils;
3334
import org.springframework.util.LinkedMultiValueMap;
3435
import org.springframework.util.MultiValueMap;
3536

@@ -232,7 +233,6 @@ private static boolean hasMetaAnnotationTypes(AnnotatedElement element, @Nullabl
232233

233234
return Boolean.TRUE.equals(
234235
searchWithGetSemantics(element, annotationType, annotationName, new SimpleAnnotationProcessor<Boolean>() {
235-
236236
@Override
237237
@Nullable
238238
public Boolean process(@Nullable AnnotatedElement annotatedElement, Annotation annotation, int metaDepth) {
@@ -950,7 +950,7 @@ else if (currentAnnotationType == containerType) {
950950
// Recursively search in meta-annotations
951951
for (Annotation annotation : annotations) {
952952
Class<? extends Annotation> currentAnnotationType = annotation.annotationType();
953-
if (!AnnotationUtils.isInJavaLangAnnotationPackage(currentAnnotationType)) {
953+
if (hasSearchableMetaAnnotations(currentAnnotationType, annotationType, annotationName)) {
954954
T result = searchWithGetSemantics(currentAnnotationType, annotationType,
955955
annotationName, containerType, processor, visited, metaDepth + 1);
956956
if (result != null) {
@@ -1083,10 +1083,10 @@ else if (currentAnnotationType == containerType) {
10831083
}
10841084
}
10851085

1086-
// Search in meta annotations on local annotations
1086+
// Recursively search in meta-annotations
10871087
for (Annotation annotation : annotations) {
10881088
Class<? extends Annotation> currentAnnotationType = annotation.annotationType();
1089-
if (!AnnotationUtils.isInJavaLangAnnotationPackage(currentAnnotationType)) {
1089+
if (hasSearchableMetaAnnotations(currentAnnotationType, annotationType, annotationName)) {
10901090
T result = searchWithFindSemantics(currentAnnotationType, annotationType, annotationName,
10911091
containerType, processor, visited, metaDepth + 1);
10921092
if (result != null) {
@@ -1101,28 +1101,33 @@ else if (currentAnnotationType == containerType) {
11011101
}
11021102
}
11031103

1104-
if (aggregatedResults != null) {
1104+
if (!CollectionUtils.isEmpty(aggregatedResults)) {
11051105
// Prepend to support top-down ordering within class hierarchies
11061106
processor.getAggregatedResults().addAll(0, aggregatedResults);
11071107
}
11081108

11091109
if (element instanceof Method) {
11101110
Method method = (Method) element;
1111+
T result;
11111112

11121113
// Search on possibly bridged method
11131114
Method resolvedMethod = BridgeMethodResolver.findBridgedMethod(method);
1114-
T result = searchWithFindSemantics(resolvedMethod, annotationType, annotationName, containerType,
1115-
processor, visited, metaDepth);
1116-
if (result != null) {
1117-
return result;
1115+
if (resolvedMethod != method) {
1116+
result = searchWithFindSemantics(resolvedMethod, annotationType, annotationName,
1117+
containerType, processor, visited, metaDepth);
1118+
if (result != null) {
1119+
return result;
1120+
}
11181121
}
11191122

11201123
// Search on methods in interfaces declared locally
11211124
Class<?>[] ifcs = method.getDeclaringClass().getInterfaces();
1122-
result = searchOnInterfaces(method, annotationType, annotationName, containerType, processor,
1123-
visited, metaDepth, ifcs);
1124-
if (result != null) {
1125-
return result;
1125+
if (ifcs.length > 0) {
1126+
result = searchOnInterfaces(method, annotationType, annotationName, containerType,
1127+
processor, visited, metaDepth, ifcs);
1128+
if (result != null) {
1129+
return result;
1130+
}
11261131
}
11271132

11281133
// Search on methods in class hierarchy and interface hierarchy
@@ -1189,10 +1194,10 @@ private static <T> T searchOnInterfaces(Method method, @Nullable Class<? extends
11891194
@Nullable String annotationName, @Nullable Class<? extends Annotation> containerType,
11901195
Processor<T> processor, Set<AnnotatedElement> visited, int metaDepth, Class<?>[] ifcs) {
11911196

1192-
for (Class<?> iface : ifcs) {
1193-
if (AnnotationUtils.isInterfaceWithAnnotatedMethods(iface)) {
1197+
for (Class<?> ifc : ifcs) {
1198+
if (AnnotationUtils.isInterfaceWithAnnotatedMethods(ifc)) {
11941199
try {
1195-
Method equivalentMethod = iface.getMethod(method.getName(), method.getParameterTypes());
1200+
Method equivalentMethod = ifc.getMethod(method.getName(), method.getParameterTypes());
11961201
T result = searchWithFindSemantics(equivalentMethod, annotationType, annotationName, containerType,
11971202
processor, visited, metaDepth);
11981203
if (result != null) {
@@ -1208,6 +1213,26 @@ private static <T> T searchOnInterfaces(Method method, @Nullable Class<? extends
12081213
return null;
12091214
}
12101215

1216+
/**
1217+
* Determine whether the current annotation type is generally expected to have
1218+
* meta-annotations of the specified annotation type that we're searching for,
1219+
* explicitly excluding some common cases that would never deliver any results.
1220+
*/
1221+
private static boolean hasSearchableMetaAnnotations(Class<? extends Annotation> currentAnnotationType,
1222+
@Nullable Class<?> annotationType, @Nullable String annotationName) {
1223+
1224+
if (AnnotationUtils.isInJavaLangAnnotationPackage(currentAnnotationType)) {
1225+
return false;
1226+
}
1227+
if (currentAnnotationType == Nullable.class || currentAnnotationType.getName().startsWith("java")) {
1228+
// @Nullable and standard Java annotations are only meant to have standard Java meta-annotations
1229+
// -> not worth searching otherwise.
1230+
return ((annotationType != null && annotationType.getName().startsWith("java")) ||
1231+
(annotationName != null && annotationName.startsWith("java")));
1232+
}
1233+
return true;
1234+
}
1235+
12111236
/**
12121237
* Get the array of raw (unsynthesized) annotations from the {@code value}
12131238
* attribute of the supplied repeatable annotation {@code container}.

spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.springframework.core.BridgeMethodResolver;
4141
import org.springframework.lang.Nullable;
4242
import org.springframework.util.Assert;
43+
import org.springframework.util.ClassUtils;
4344
import org.springframework.util.ConcurrentReferenceHashMap;
4445
import org.springframework.util.ObjectUtils;
4546
import org.springframework.util.ReflectionUtils;
@@ -147,18 +148,18 @@ public abstract class AnnotationUtils {
147148
* <p>Note that this method supports only a single level of meta-annotations.
148149
* For support for arbitrary levels of meta-annotations, use one of the
149150
* {@code find*()} methods instead.
150-
* @param ann the Annotation to check
151+
* @param annotation the Annotation to check
151152
* @param annotationType the annotation type to look for, both locally and as a meta-annotation
152153
* @return the first matching annotation, or {@code null} if not found
153154
* @since 4.0
154155
*/
155156
@SuppressWarnings("unchecked")
156157
@Nullable
157-
public static <A extends Annotation> A getAnnotation(Annotation ann, Class<A> annotationType) {
158-
if (annotationType.isInstance(ann)) {
159-
return synthesizeAnnotation((A) ann);
158+
public static <A extends Annotation> A getAnnotation(Annotation annotation, Class<A> annotationType) {
159+
if (annotationType.isInstance(annotation)) {
160+
return synthesizeAnnotation((A) annotation);
160161
}
161-
Class<? extends Annotation> annotatedElement = ann.annotationType();
162+
Class<? extends Annotation> annotatedElement = annotation.annotationType();
162163
try {
163164
return synthesizeAnnotation(annotatedElement.getAnnotation(annotationType), annotatedElement);
164165
}
@@ -574,10 +575,10 @@ public static <A extends Annotation> A findAnnotation(Method method, @Nullable C
574575
@Nullable
575576
private static <A extends Annotation> A searchOnInterfaces(Method method, Class<A> annotationType, Class<?>... ifcs) {
576577
A annotation = null;
577-
for (Class<?> iface : ifcs) {
578-
if (isInterfaceWithAnnotatedMethods(iface)) {
578+
for (Class<?> ifc : ifcs) {
579+
if (isInterfaceWithAnnotatedMethods(ifc)) {
579580
try {
580-
Method equivalentMethod = iface.getMethod(method.getName(), method.getParameterTypes());
581+
Method equivalentMethod = ifc.getMethod(method.getName(), method.getParameterTypes());
581582
annotation = getAnnotation(equivalentMethod, annotationType);
582583
}
583584
catch (NoSuchMethodException ex) {
@@ -591,15 +592,20 @@ private static <A extends Annotation> A searchOnInterfaces(Method method, Class<
591592
return annotation;
592593
}
593594

594-
static boolean isInterfaceWithAnnotatedMethods(Class<?> iface) {
595-
Boolean found = annotatedInterfaceCache.get(iface);
595+
static boolean isInterfaceWithAnnotatedMethods(Class<?> ifc) {
596+
if (ClassUtils.isJavaLanguageInterface(ifc)) {
597+
return false;
598+
}
599+
600+
Boolean found = annotatedInterfaceCache.get(ifc);
596601
if (found != null) {
597602
return found;
598603
}
599604
found = Boolean.FALSE;
600-
for (Method ifcMethod : iface.getMethods()) {
605+
for (Method ifcMethod : ifc.getMethods()) {
601606
try {
602-
if (ifcMethod.getAnnotations().length > 0) {
607+
Annotation[] anns = ifcMethod.getAnnotations();
608+
if (anns.length > 1 || (anns.length == 1 && anns[0].annotationType() != Nullable.class)) {
603609
found = Boolean.TRUE;
604610
break;
605611
}
@@ -608,7 +614,7 @@ static boolean isInterfaceWithAnnotatedMethods(Class<?> iface) {
608614
handleIntrospectionFailure(ifcMethod, ex);
609615
}
610616
}
611-
annotatedInterfaceCache.put(iface, found);
617+
annotatedInterfaceCache.put(ifc, found);
612618
return found;
613619
}
614620

spring-core/src/main/java/org/springframework/core/type/filter/AnnotationTypeFilter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ protected Boolean hasAnnotation(String typeName) {
114114
}
115115
else if (typeName.startsWith("java")) {
116116
if (!this.annotationType.getName().startsWith("java")) {
117-
// Standard Java classes don't have non-standard annotations on them.
117+
// Standard Java types do not have non-standard annotations on them ->
118+
// skip any load attempt, in particular for Java language interfaces.
118119
return false;
119120
}
120121
try {

spring-core/src/test/java/org/springframework/core/annotation/AnnotationUtilsTests.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@
3636

3737
import org.springframework.core.Ordered;
3838
import org.springframework.core.annotation.subpackage.NonPublicAnnotatedClass;
39+
import org.springframework.lang.Nullable;
3940
import org.springframework.stereotype.Component;
4041
import org.springframework.util.ClassUtils;
4142
import org.springframework.util.ReflectionUtils;
@@ -1541,6 +1542,13 @@ public void synthesizeAnnotationWithArrayOfChars() throws Exception {
15411542
assertArrayEquals(new char[] { 'x', 'y', 'z' }, chars);
15421543
}
15431544

1545+
@Test
1546+
public void interfaceWithAnnotatedMethods() {
1547+
assertFalse(AnnotationUtils.isInterfaceWithAnnotatedMethods(NonAnnotatedInterface.class));
1548+
assertTrue(AnnotationUtils.isInterfaceWithAnnotatedMethods(AnnotatedInterface.class));
1549+
assertFalse(AnnotationUtils.isInterfaceWithAnnotatedMethods(NullableAnnotatedInterface.class));
1550+
}
1551+
15441552

15451553
@SafeVarargs
15461554
static <T> T[] asArray(T... arr) {
@@ -1634,6 +1642,12 @@ public interface AnnotatedInterface {
16341642
void fromInterfaceImplementedByRoot();
16351643
}
16361644

1645+
public interface NullableAnnotatedInterface {
1646+
1647+
@Nullable
1648+
void fromInterfaceImplementedByRoot();
1649+
}
1650+
16371651
public static class Root implements AnnotatedInterface {
16381652

16391653
@Order(27)

0 commit comments

Comments
 (0)