Skip to content

Commit 6b3dd07

Browse files
jhoellerjhoeller
committed
Consistently skip unnecessary search on superclasses and empty elements
Issue: SPR-16933
1 parent bf7fa39 commit 6b3dd07

File tree

5 files changed

+96
-70
lines changed

5 files changed

+96
-70
lines changed

‎spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public class AutowiredAnnotationBeanPostProcessor extends InstantiationAwareBean
120120

121121
protected final Log logger = LogFactory.getLog(getClass());
122122

123-
private final Set<Class<? extends Annotation>> autowiredAnnotationTypes = new LinkedHashSet<>();
123+
private final Set<Class<? extends Annotation>> autowiredAnnotationTypes = new LinkedHashSet<>(4);
124124

125125
private String requiredParameterName = "required";
126126

@@ -346,8 +346,8 @@ else if (candidates.size() == 1 && logger.isWarnEnabled()) {
346346
else if (rawCandidates.length == 1 && rawCandidates[0].getParameterCount() > 0) {
347347
candidateConstructors = new Constructor<?>[] {rawCandidates[0]};
348348
}
349-
else if (nonSyntheticConstructors == 2 && primaryConstructor != null
350-
&& defaultConstructor != null && !primaryConstructor.equals(defaultConstructor)) {
349+
else if (nonSyntheticConstructors == 2 && primaryConstructor != null &&
350+
defaultConstructor != null && !primaryConstructor.equals(defaultConstructor)) {
351351
candidateConstructors = new Constructor<?>[] {primaryConstructor, defaultConstructor};
352352
}
353353
else if (nonSyntheticConstructors == 1 && primaryConstructor != null) {
@@ -478,7 +478,7 @@ private InjectionMetadata buildAutowiringMetadata(final Class<?> clazz) {
478478

479479
@Nullable
480480
private AnnotationAttributes findAutowiredAnnotation(AccessibleObject ao) {
481-
if (ao.getAnnotations().length > 0) {
481+
if (ao.getAnnotations().length > 0) { // autowiring annotations have to be local
482482
for (Class<? extends Annotation> type : this.autowiredAnnotationTypes) {
483483
AnnotationAttributes attributes = AnnotatedElementUtils.getMergedAnnotationAttributes(ao, type);
484484
if (attributes != null) {

‎spring-beans/src/main/java/org/springframework/beans/factory/annotation/QualifierAnnotationAutowireCandidateResolver.java

Lines changed: 7 additions & 5 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.
@@ -347,10 +347,12 @@ public Object getSuggestedValue(DependencyDescriptor descriptor) {
347347
*/
348348
@Nullable
349349
protected Object findValue(Annotation[] annotationsToSearch) {
350-
AnnotationAttributes attr = AnnotatedElementUtils.getMergedAnnotationAttributes(
351-
AnnotatedElementUtils.forAnnotations(annotationsToSearch), this.valueAnnotationType);
352-
if (attr != null) {
353-
return extractValue(attr);
350+
if (annotationsToSearch.length > 0) { // qualifier annotations have to be local
351+
AnnotationAttributes attr = AnnotatedElementUtils.getMergedAnnotationAttributes(
352+
AnnotatedElementUtils.forAnnotations(annotationsToSearch), this.valueAnnotationType);
353+
if (attr != null) {
354+
return extractValue(attr);
355+
}
354356
}
355357
return null;
356358
}

‎spring-context/src/test/java/org/springframework/cache/annotation/AnnotationCacheOperationSourceTests.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 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.
@@ -52,40 +52,40 @@ public class AnnotationCacheOperationSourceTests {
5252

5353

5454
@Test
55-
public void singularAnnotation() throws Exception {
55+
public void singularAnnotation() {
5656
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "singular", 1);
5757
assertTrue(ops.iterator().next() instanceof CacheableOperation);
5858
}
5959

6060
@Test
61-
public void multipleAnnotation() throws Exception {
61+
public void multipleAnnotation() {
6262
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "multiple", 2);
6363
Iterator<CacheOperation> it = ops.iterator();
6464
assertTrue(it.next() instanceof CacheableOperation);
6565
assertTrue(it.next() instanceof CacheEvictOperation);
6666
}
6767

6868
@Test
69-
public void caching() throws Exception {
69+
public void caching() {
7070
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "caching", 2);
7171
Iterator<CacheOperation> it = ops.iterator();
7272
assertTrue(it.next() instanceof CacheableOperation);
7373
assertTrue(it.next() instanceof CacheEvictOperation);
7474
}
7575

7676
@Test
77-
public void emptyCaching() throws Exception {
77+
public void emptyCaching() {
7878
getOps(AnnotatedClass.class, "emptyCaching", 0);
7979
}
8080

8181
@Test
82-
public void singularStereotype() throws Exception {
82+
public void singularStereotype() {
8383
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "singleStereotype", 1);
8484
assertTrue(ops.iterator().next() instanceof CacheEvictOperation);
8585
}
8686

8787
@Test
88-
public void multipleStereotypes() throws Exception {
88+
public void multipleStereotypes() {
8989
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "multipleStereotype", 3);
9090
Iterator<CacheOperation> it = ops.iterator();
9191
assertTrue(it.next() instanceof CacheableOperation);
@@ -98,7 +98,7 @@ public void multipleStereotypes() throws Exception {
9898
}
9999

100100
@Test
101-
public void singleComposedAnnotation() throws Exception {
101+
public void singleComposedAnnotation() {
102102
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "singleComposed", 2);
103103
Iterator<CacheOperation> it = ops.iterator();
104104

@@ -114,7 +114,7 @@ public void singleComposedAnnotation() throws Exception {
114114
}
115115

116116
@Test
117-
public void multipleComposedAnnotations() throws Exception {
117+
public void multipleComposedAnnotations() {
118118
Collection<CacheOperation> ops = getOps(AnnotatedClass.class, "multipleComposed", 4);
119119
Iterator<CacheOperation> it = ops.iterator();
120120

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

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Collections;
2525
import java.util.HashSet;
2626
import java.util.LinkedHashSet;
27+
import java.util.LinkedList;
2728
import java.util.List;
2829
import java.util.Map;
2930
import java.util.Set;
@@ -854,19 +855,21 @@ private static <T> T searchWithGetSemantics(AnnotatedElement element,
854855
return result;
855856
}
856857

857-
if (element instanceof Class) { // otherwise getAnnotations doesn't return anything new
858-
List<Annotation> inheritedAnnotations = new ArrayList<>();
859-
for (Annotation annotation : element.getAnnotations()) {
860-
if (!declaredAnnotations.contains(annotation)) {
861-
inheritedAnnotations.add(annotation);
858+
if (element instanceof Class) { // otherwise getAnnotations doesn't return anything new
859+
Class<?> superclass = ((Class) element).getSuperclass();
860+
if (superclass != null && superclass != Object.class) {
861+
List<Annotation> inheritedAnnotations = new LinkedList<>();
862+
for (Annotation annotation : element.getAnnotations()) {
863+
if (!declaredAnnotations.contains(annotation)) {
864+
inheritedAnnotations.add(annotation);
865+
}
866+
}
867+
// Continue searching within inherited annotations
868+
result = searchWithGetSemanticsInAnnotations(element, inheritedAnnotations,
869+
annotationType, annotationName, containerType, processor, visited, metaDepth);
870+
if (result != null) {
871+
return result;
862872
}
863-
}
864-
865-
// Continue searching within inherited annotations
866-
result = searchWithGetSemanticsInAnnotations(element, inheritedAnnotations,
867-
annotationType, annotationName, containerType, processor, visited, metaDepth);
868-
if (result != null) {
869-
return result;
870873
}
871874
}
872875
}
@@ -1126,7 +1129,7 @@ else if (currentAnnotationType == containerType) {
11261129
Class<?> clazz = method.getDeclaringClass();
11271130
while (true) {
11281131
clazz = clazz.getSuperclass();
1129-
if (clazz == null || Object.class == clazz) {
1132+
if (clazz == null || clazz == Object.class) {
11301133
break;
11311134
}
11321135
Set<Method> annotatedMethods = AnnotationUtils.getAnnotatedMethodsInBaseType(clazz);
@@ -1152,23 +1155,23 @@ else if (currentAnnotationType == containerType) {
11521155
}
11531156
else if (element instanceof Class) {
11541157
Class<?> clazz = (Class<?>) element;
1155-
1156-
// Search on interfaces
1157-
for (Class<?> ifc : clazz.getInterfaces()) {
1158-
T result = searchWithFindSemantics(ifc, annotationType, annotationName,
1159-
containerType, processor, visited, metaDepth);
1160-
if (result != null) {
1161-
return result;
1158+
if (!Annotation.class.isAssignableFrom(clazz)) {
1159+
// Search on interfaces
1160+
for (Class<?> ifc : clazz.getInterfaces()) {
1161+
T result = searchWithFindSemantics(ifc, annotationType, annotationName,
1162+
containerType, processor, visited, metaDepth);
1163+
if (result != null) {
1164+
return result;
1165+
}
11621166
}
1163-
}
1164-
1165-
// Search on superclass
1166-
Class<?> superclass = clazz.getSuperclass();
1167-
if (superclass != null && Object.class != superclass) {
1168-
T result = searchWithFindSemantics(superclass, annotationType, annotationName,
1169-
containerType, processor, visited, metaDepth);
1170-
if (result != null) {
1171-
return result;
1167+
// Search on superclass
1168+
Class<?> superclass = clazz.getSuperclass();
1169+
if (superclass != null && superclass != Object.class) {
1170+
T result = searchWithFindSemantics(superclass, annotationType, annotationName,
1171+
containerType, processor, visited, metaDepth);
1172+
if (result != null) {
1173+
return result;
1174+
}
11721175
}
11731176
}
11741177
}

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

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ public static <A extends Annotation> Set<A> getRepeatableAnnotations(AnnotatedEl
346346

347347
if (annotatedElement instanceof Class) {
348348
Class<?> superclass = ((Class<?>) annotatedElement).getSuperclass();
349-
if (superclass != null && Object.class != superclass) {
349+
if (superclass != null && superclass != Object.class) {
350350
return getRepeatableAnnotations(superclass, annotationType, containerAnnotationType);
351351
}
352352
}
@@ -553,7 +553,7 @@ public static <A extends Annotation> A findAnnotation(Method method, @Nullable C
553553
Class<?> clazz = method.getDeclaringClass();
554554
while (result == null) {
555555
clazz = clazz.getSuperclass();
556-
if (clazz == null || Object.class == clazz) {
556+
if (clazz == null || clazz == Object.class) {
557557
break;
558558
}
559559
Set<Method> annotatedMethods = getAnnotatedMethodsInBaseType(clazz);
@@ -600,6 +600,35 @@ private static <A extends Annotation> A searchOnInterfaces(Method method, Class<
600600
return null;
601601
}
602602

603+
/**
604+
* Does the given method override the given candidate method?
605+
* @param method the overriding method
606+
* @param candidate the potentially overridden method
607+
* @since 5.0.8
608+
*/
609+
static boolean isOverride(Method method, Method candidate) {
610+
if (!candidate.getName().equals(method.getName()) ||
611+
candidate.getParameterCount() != method.getParameterCount()) {
612+
return false;
613+
}
614+
Class<?>[] paramTypes = method.getParameterTypes();
615+
if (Arrays.equals(candidate.getParameterTypes(), paramTypes)) {
616+
return true;
617+
}
618+
for (int i = 0; i < paramTypes.length; i++) {
619+
if (paramTypes[i] != ResolvableType.forMethodParameter(candidate, i, method.getDeclaringClass()).resolve()) {
620+
return false;
621+
}
622+
}
623+
return true;
624+
}
625+
626+
/**
627+
* Determine the methods on the given type with searchable annotations on them.
628+
* @param baseType the superclass or interface to search
629+
* @return the cached set of annotated methods
630+
* @since 5.0.5
631+
*/
603632
static Set<Method> getAnnotatedMethodsInBaseType(Class<?> baseType) {
604633
boolean ifcCheck = baseType.isInterface();
605634
if (ifcCheck && ClassUtils.isJavaLanguageInterface(baseType)) {
@@ -634,6 +663,13 @@ static Set<Method> getAnnotatedMethodsInBaseType(Class<?> baseType) {
634663
return annotatedMethods;
635664
}
636665

666+
/**
667+
* Determine whether the specified method has searchable annotations,
668+
* i.e. not just {@code java.lang} or {@code org.springframework.lang}
669+
* annotations such as {@link Deprecated} and {@link Nullable}.
670+
* @param ifcMethod the interface method to check
671+
* @@since 5.0.5
672+
*/
637673
private static boolean hasSearchableAnnotations(Method ifcMethod) {
638674
Annotation[] anns = ifcMethod.getAnnotations();
639675
if (anns.length == 0) {
@@ -648,23 +684,6 @@ private static boolean hasSearchableAnnotations(Method ifcMethod) {
648684
return false;
649685
}
650686

651-
static boolean isOverride(Method method, Method candidate) {
652-
if (!candidate.getName().equals(method.getName()) ||
653-
candidate.getParameterCount() != method.getParameterCount()) {
654-
return false;
655-
}
656-
Class<?>[] paramTypes = method.getParameterTypes();
657-
if (Arrays.equals(candidate.getParameterTypes(), paramTypes)) {
658-
return true;
659-
}
660-
for (int i = 0; i < paramTypes.length; i++) {
661-
if (paramTypes[i] != ResolvableType.forMethodParameter(candidate, i, method.getDeclaringClass()).resolve()) {
662-
return false;
663-
}
664-
}
665-
return true;
666-
}
667-
668687
/**
669688
* Find a single {@link Annotation} of {@code annotationType} on the
670689
* supplied {@link Class}, traversing its interfaces, annotations, and
@@ -763,7 +782,7 @@ private static <A extends Annotation> A findAnnotation(Class<?> clazz, Class<A>
763782
}
764783

765784
Class<?> superclass = clazz.getSuperclass();
766-
if (superclass == null || Object.class == superclass) {
785+
if (superclass == null || superclass == Object.class) {
767786
return null;
768787
}
769788
return findAnnotation(superclass, annotationType, visited);
@@ -793,7 +812,7 @@ private static <A extends Annotation> A findAnnotation(Class<?> clazz, Class<A>
793812
*/
794813
@Nullable
795814
public static Class<?> findAnnotationDeclaringClass(Class<? extends Annotation> annotationType, @Nullable Class<?> clazz) {
796-
if (clazz == null || Object.class == clazz) {
815+
if (clazz == null || clazz == Object.class) {
797816
return null;
798817
}
799818
if (isAnnotationDeclaredLocally(annotationType, clazz)) {
@@ -827,8 +846,10 @@ public static Class<?> findAnnotationDeclaringClass(Class<? extends Annotation>
827846
* @see #isAnnotationDeclaredLocally(Class, Class)
828847
*/
829848
@Nullable
830-
public static Class<?> findAnnotationDeclaringClassForTypes(List<Class<? extends Annotation>> annotationTypes, @Nullable Class<?> clazz) {
831-
if (clazz == null || Object.class == clazz) {
849+
public static Class<?> findAnnotationDeclaringClassForTypes(
850+
List<Class<? extends Annotation>> annotationTypes, @Nullable Class<?> clazz) {
851+
852+
if (clazz == null || clazz == Object.class) {
832853
return null;
833854
}
834855
for (Class<? extends Annotation> annotationType : annotationTypes) {

0 commit comments

Comments
 (0)