Skip to content

Commit c985770

Browse files
jhoellerjhoeller
committed
Refined retrieval of plain annotations through direct presence checks
Shortcut checks apply for hasPlainJavaAnnotationsOnly types as well now. Closes gh-22685
1 parent cb84c56 commit c985770

File tree

8 files changed

+97
-79
lines changed

8 files changed

+97
-79
lines changed

‎spring-core/spring-core.gradle

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ dependencies {
8585
optional("io.netty:netty-buffer")
8686
testCompile("io.projectreactor:reactor-test")
8787
testCompile("org.apache.tomcat.embed:tomcat-embed-core:${tomcatVersion}")
88+
testCompile("com.google.code.findbugs:jsr305:3.0.2")
8889
testCompile("org.xmlunit:xmlunit-matchers:2.6.2")
8990
testCompile("javax.xml.bind:jaxb-api:2.3.1")
9091
testCompile("com.fasterxml.woodstox:woodstox-core:5.2.0") {

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

+11-23
Original file line numberDiff line numberDiff line change
@@ -200,15 +200,12 @@ public static boolean hasMetaAnnotationTypes(AnnotatedElement element, String an
200200
* @since 4.2.3
201201
* @see #hasAnnotation(AnnotatedElement, Class)
202202
*/
203-
public static boolean isAnnotated(AnnotatedElement element,Class<? extends Annotation> annotationType) {
204-
// Shortcut: directly present on the element, with no processing needed?
205-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
203+
public static boolean isAnnotated(AnnotatedElement element, Class<? extends Annotation> annotationType) {
204+
// Shortcut: directly present on the element, with no merging needed?
205+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
206+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
206207
return element.isAnnotationPresent(annotationType);
207208
}
208-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
209-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
210-
return false;
211-
}
212209
// Exhaustive retrieval of merged annotations...
213210
return getAnnotations(element).isPresent(annotationType);
214211
}
@@ -332,13 +329,10 @@ public static AnnotationAttributes getMergedAnnotationAttributes(AnnotatedElemen
332329
@Nullable
333330
public static <A extends Annotation> A getMergedAnnotation(AnnotatedElement element, Class<A> annotationType) {
334331
// Shortcut: directly present on the element, with no merging needed?
335-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
332+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
333+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
336334
return element.getDeclaredAnnotation(annotationType);
337335
}
338-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
339-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
340-
return null;
341-
}
342336
// Exhaustive retrieval of merged annotations...
343337
return getAnnotations(element)
344338
.get(annotationType, null, MergedAnnotationSelectors.firstDirectlyDeclared())
@@ -528,14 +522,11 @@ public static MultiValueMap<String, Object> getAllAnnotationAttributes(Annotated
528522
* @see #isAnnotated(AnnotatedElement, Class)
529523
*/
530524
public static boolean hasAnnotation(AnnotatedElement element, Class<? extends Annotation> annotationType) {
531-
// Shortcut: directly present on the element, with no processing needed?
532-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
525+
// Shortcut: directly present on the element, with no merging needed?
526+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
527+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
533528
return element.isAnnotationPresent(annotationType);
534529
}
535-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
536-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
537-
return false;
538-
}
539530
// Exhaustive retrieval of merged annotations...
540531
return findAnnotations(element).isPresent(annotationType);
541532
}
@@ -633,13 +624,10 @@ public static AnnotationAttributes findMergedAnnotationAttributes(AnnotatedEleme
633624
@Nullable
634625
public static <A extends Annotation> A findMergedAnnotation(AnnotatedElement element, Class<A> annotationType) {
635626
// Shortcut: directly present on the element, with no merging needed?
636-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
627+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
628+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
637629
return element.getDeclaredAnnotation(annotationType);
638630
}
639-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
640-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(element)) {
641-
return null;
642-
}
643631
// Exhaustive retrieval of merged annotations...
644632
return findAnnotations(element)
645633
.get(annotationType, null, MergedAnnotationSelectors.firstDirectlyDeclared())

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

+4-5
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,16 @@
2828
public interface AnnotationFilter {
2929

3030
/**
31-
* {@link AnnotationFilter} that matches annotations is in the
32-
* {@code java.lang.*} or in the
33-
* {@code org.springframework.lang.*} package.
31+
* {@link AnnotationFilter} that matches annotations in the
32+
* {@code java.lang.*} and {@code org.springframework.lang.*} packages.
3433
*/
3534
AnnotationFilter PLAIN = packages("java.lang", "org.springframework.lang");
3635

3736
/**
3837
* {@link AnnotationFilter} that matches annotations in the
39-
* {@code java.lang.*} package.
38+
* {@code java.*}/{@code javax.*} namespaces.
4039
*/
41-
AnnotationFilter JAVA = packages("java.lang");
40+
AnnotationFilter JAVA = packages("java", "javax");
4241

4342
/**
4443
* {@link AnnotationFilter} that never matches and can be used when no

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

+12-28
Original file line numberDiff line numberDiff line change
@@ -210,13 +210,10 @@ public static <A extends Annotation> A getAnnotation(Annotation annotation, Clas
210210
@Nullable
211211
public static <A extends Annotation> A getAnnotation(AnnotatedElement annotatedElement, Class<A> annotationType) {
212212
// Shortcut: directly present on the element, with no merging needed?
213-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
213+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
214+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(annotatedElement)) {
214215
return annotatedElement.getAnnotation(annotationType);
215216
}
216-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
217-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(annotatedElement)) {
218-
return null;
219-
}
220217
// Exhaustive retrieval of merged annotations...
221218
return MergedAnnotations.from(annotatedElement, SearchStrategy.INHERITED_ANNOTATIONS,
222219
RepeatableContainers.none(), AnnotationFilter.PLAIN)
@@ -483,13 +480,10 @@ public static <A extends Annotation> A findAnnotation(
483480
return null;
484481
}
485482
// Shortcut: directly present on the element, with no merging needed?
486-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
483+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
484+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(annotatedElement)) {
487485
return annotatedElement.getDeclaredAnnotation(annotationType);
488486
}
489-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
490-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(annotatedElement)) {
491-
return null;
492-
}
493487
// Exhaustive retrieval of merged annotations...
494488
return MergedAnnotations.from(annotatedElement, SearchStrategy.INHERITED_ANNOTATIONS)
495489
.get(annotationType).withNonMergedAttributes()
@@ -517,13 +511,10 @@ public static <A extends Annotation> A findAnnotation(Method method, @Nullable C
517511
return null;
518512
}
519513
// Shortcut: directly present on the element, with no merging needed?
520-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
514+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
515+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(method)) {
521516
return method.getDeclaredAnnotation(annotationType);
522517
}
523-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
524-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(method)) {
525-
return null;
526-
}
527518
// Exhaustive retrieval of merged annotations...
528519
return MergedAnnotations.from(method, SearchStrategy.EXHAUSTIVE)
529520
.get(annotationType).withNonMergedAttributes()
@@ -558,13 +549,10 @@ public static <A extends Annotation> A findAnnotation(Class<?> clazz, @Nullable
558549
return null;
559550
}
560551
// Shortcut: directly present on the element, with no merging needed?
561-
if (AnnotationFilter.PLAIN.matches(annotationType)) {
552+
if (AnnotationFilter.PLAIN.matches(annotationType) ||
553+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(clazz)) {
562554
return clazz.getDeclaredAnnotation(annotationType);
563555
}
564-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
565-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(clazz)) {
566-
return null;
567-
}
568556
// Exhaustive retrieval of merged annotations...
569557
return MergedAnnotations.from(clazz, SearchStrategy.EXHAUSTIVE)
570558
.get(annotationType).withNonMergedAttributes()
@@ -710,17 +698,13 @@ public static boolean isAnnotationMetaPresent(Class<? extends Annotation> annota
710698
return false;
711699
}
712700
// Shortcut: directly present on the element, with no merging needed?
713-
if (AnnotationFilter.PLAIN.matches(annotationType) ||
714-
AnnotationFilter.PLAIN.matches(metaAnnotationType)) {
701+
if (AnnotationFilter.PLAIN.matches(metaAnnotationType) ||
702+
AnnotationsScanner.hasPlainJavaAnnotationsOnly(annotationType)) {
715703
return annotationType.isAnnotationPresent(metaAnnotationType);
716704
}
717-
// Shortcut: no searchable annotations to be found on plain Java classes and core Spring types...
718-
if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(annotationType)) {
719-
return false;
720-
}
721705
// Exhaustive retrieval of merged annotations...
722-
return (MergedAnnotations.from(
723-
annotationType, SearchStrategy.INHERITED_ANNOTATIONS).isPresent(metaAnnotationType));
706+
return MergedAnnotations.from(annotationType, SearchStrategy.INHERITED_ANNOTATIONS,
707+
RepeatableContainers.none(), AnnotationFilter.PLAIN).isPresent(metaAnnotationType);
724708
}
725709

726710
/**

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

+7-11
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
*/
4141
final class TypeMappedAnnotations implements MergedAnnotations {
4242

43-
private static final AnnotationFilter FILTER_ALL = annotationType -> true;
43+
private static final AnnotationFilter FILTER_ALL = (annotationType -> true);
4444

4545
private static final MergedAnnotations NONE = new TypeMappedAnnotations(
4646
null, new Annotation[0], RepeatableContainers.none(), FILTER_ALL);
@@ -108,8 +108,8 @@ public boolean isPresent(String annotationType) {
108108
}
109109

110110
@Override
111-
public <A extends Annotation> boolean isDirectlyPresent(@Nullable Class<A> annotationType) {
112-
if (annotationType == null || this.annotationFilter.matches(annotationType)) {
111+
public <A extends Annotation> boolean isDirectlyPresent(Class<A> annotationType) {
112+
if (this.annotationFilter.matches(annotationType)) {
113113
return false;
114114
}
115115
return Boolean.TRUE.equals(scan(annotationType,
@@ -176,19 +176,15 @@ public <A extends Annotation> MergedAnnotation<A> get(String annotationType,
176176
}
177177

178178
@Override
179-
public <A extends Annotation> Stream<MergedAnnotation<A>> stream(
180-
@Nullable Class<A> annotationType) {
181-
179+
public <A extends Annotation> Stream<MergedAnnotation<A>> stream(Class<A> annotationType) {
182180
if (this.annotationFilter == FILTER_ALL) {
183181
return Stream.empty();
184182
}
185183
return StreamSupport.stream(spliterator(annotationType), false);
186184
}
187185

188186
@Override
189-
public <A extends Annotation> Stream<MergedAnnotation<A>> stream(
190-
@Nullable String annotationType) {
191-
187+
public <A extends Annotation> Stream<MergedAnnotation<A>> stream(String annotationType) {
192188
if (this.annotationFilter == FILTER_ALL) {
193189
return Stream.empty();
194190
}
@@ -219,8 +215,7 @@ public Spliterator<MergedAnnotation<Annotation>> spliterator() {
219215
return spliterator(null);
220216
}
221217

222-
private <A extends Annotation> Spliterator<MergedAnnotation<A>> spliterator(
223-
@Nullable Object annotationType) {
218+
private <A extends Annotation> Spliterator<MergedAnnotation<A>> spliterator(@Nullable Object annotationType) {
224219
return new AggregatesSpliterator<>(annotationType, getAggregates());
225220
}
226221

@@ -248,6 +243,7 @@ private <C, R> R scan(C criteria, AnnotationsProcessor<C, R> processor) {
248243
return null;
249244
}
250245

246+
251247
static MergedAnnotations from(@Nullable AnnotatedElement element, SearchStrategy searchStrategy,
252248
RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) {
253249

‎spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java

+51-10
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.core.annotation;
1818

1919
import java.lang.annotation.Annotation;
20+
import java.lang.annotation.Documented;
2021
import java.lang.annotation.ElementType;
2122
import java.lang.annotation.Inherited;
2223
import java.lang.annotation.Retention;
@@ -28,6 +29,8 @@
2829
import java.util.Date;
2930
import java.util.List;
3031
import java.util.Set;
32+
import javax.annotation.Nonnull;
33+
import javax.annotation.ParametersAreNonnullByDefault;
3134
import javax.annotation.Resource;
3235

3336
import org.junit.Ignore;
@@ -36,6 +39,7 @@
3639
import org.junit.internal.ArrayComparisonFailure;
3740
import org.junit.rules.ExpectedException;
3841

42+
import org.springframework.lang.NonNullApi;
3943
import org.springframework.lang.Nullable;
4044
import org.springframework.stereotype.Component;
4145
import org.springframework.stereotype.Indexed;
@@ -121,33 +125,70 @@ public void hasMetaAnnotationTypesOnClassWithMetaDepth2() {
121125

122126
@Test
123127
public void isAnnotatedOnNonAnnotatedClass() {
124-
assertFalse(isAnnotated(NonAnnotatedClass.class, TX_NAME));
128+
assertFalse(isAnnotated(NonAnnotatedClass.class, Transactional.class));
125129
}
126130

127131
@Test
128-
public void isAnnotatedOnClassWithMetaDepth0() {
129-
assertTrue(isAnnotated(TransactionalComponentClass.class, TransactionalComponent.class.getName()));
132+
public void isAnnotatedOnClassWithMetaDepth() {
133+
assertTrue(isAnnotated(TransactionalComponentClass.class, TransactionalComponent.class));
134+
assertFalse("isAnnotated() does not search the class hierarchy.",
135+
isAnnotated(SubTransactionalComponentClass.class, TransactionalComponent.class));
136+
assertTrue(isAnnotated(TransactionalComponentClass.class, Transactional.class));
137+
assertTrue(isAnnotated(TransactionalComponentClass.class, Component.class));
138+
assertTrue(isAnnotated(ComposedTransactionalComponentClass.class, Transactional.class));
139+
assertTrue(isAnnotated(ComposedTransactionalComponentClass.class, Component.class));
140+
assertTrue(isAnnotated(ComposedTransactionalComponentClass.class, ComposedTransactionalComponent.class));
130141
}
131142

132143
@Test
133-
public void isAnnotatedOnSubclassWithMetaDepth0() {
134-
assertFalse("isAnnotated() does not search the class hierarchy.",
135-
isAnnotated(SubTransactionalComponentClass.class, TransactionalComponent.class.getName()));
144+
public void isAnnotatedForPlainTypes() {
145+
assertTrue(isAnnotated(Order.class, Documented.class));
146+
assertTrue(isAnnotated(NonNullApi.class, Documented.class));
147+
assertTrue(isAnnotated(NonNullApi.class, Nonnull.class));
148+
assertTrue(isAnnotated(ParametersAreNonnullByDefault.class, Nonnull.class));
136149
}
137150

138151
@Test
139-
public void isAnnotatedOnClassWithMetaDepth1() {
140-
assertTrue(isAnnotated(TransactionalComponentClass.class, TX_NAME));
141-
assertTrue(isAnnotated(TransactionalComponentClass.class, Component.class.getName()));
152+
public void isAnnotatedWithNameOnNonAnnotatedClass() {
153+
assertFalse(isAnnotated(NonAnnotatedClass.class, TX_NAME));
142154
}
143155

144156
@Test
145-
public void isAnnotatedOnClassWithMetaDepth2() {
157+
public void isAnnotatedWithNameOnClassWithMetaDepth() {
158+
assertTrue(isAnnotated(TransactionalComponentClass.class, TransactionalComponent.class.getName()));
159+
assertFalse("isAnnotated() does not search the class hierarchy.",
160+
isAnnotated(SubTransactionalComponentClass.class, TransactionalComponent.class.getName()));
161+
assertTrue(isAnnotated(TransactionalComponentClass.class, TX_NAME));
162+
assertTrue(isAnnotated(TransactionalComponentClass.class, Component.class.getName()));
146163
assertTrue(isAnnotated(ComposedTransactionalComponentClass.class, TX_NAME));
147164
assertTrue(isAnnotated(ComposedTransactionalComponentClass.class, Component.class.getName()));
148165
assertTrue(isAnnotated(ComposedTransactionalComponentClass.class, ComposedTransactionalComponent.class.getName()));
149166
}
150167

168+
@Test
169+
public void hasAnnotationOnNonAnnotatedClass() {
170+
assertFalse(hasAnnotation(NonAnnotatedClass.class, Transactional.class));
171+
}
172+
173+
@Test
174+
public void hasAnnotationOnClassWithMetaDepth() {
175+
assertTrue(hasAnnotation(TransactionalComponentClass.class, TransactionalComponent.class));
176+
assertTrue(hasAnnotation(SubTransactionalComponentClass.class, TransactionalComponent.class));
177+
assertTrue(hasAnnotation(TransactionalComponentClass.class, Transactional.class));
178+
assertTrue(hasAnnotation(TransactionalComponentClass.class, Component.class));
179+
assertTrue(hasAnnotation(ComposedTransactionalComponentClass.class, Transactional.class));
180+
assertTrue(hasAnnotation(ComposedTransactionalComponentClass.class, Component.class));
181+
assertTrue(hasAnnotation(ComposedTransactionalComponentClass.class, ComposedTransactionalComponent.class));
182+
}
183+
184+
@Test
185+
public void hasAnnotationForPlainTypes() {
186+
assertTrue(hasAnnotation(Order.class, Documented.class));
187+
assertTrue(hasAnnotation(NonNullApi.class, Documented.class));
188+
assertTrue(hasAnnotation(NonNullApi.class, Nonnull.class));
189+
assertTrue(hasAnnotation(ParametersAreNonnullByDefault.class, Nonnull.class));
190+
}
191+
151192
@Test
152193
public void getAllAnnotationAttributesOnNonAnnotatedClass() {
153194
assertNull(getAllAnnotationAttributes(NonAnnotatedClass.class, TX_NAME));

‎spring-core/src/test/java/org/springframework/core/annotation/AnnotationFilterTests.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
import java.lang.annotation.Retention;
2020
import java.lang.annotation.RetentionPolicy;
21-
import java.util.Arrays;
21+
import javax.annotation.Nonnull;
2222

2323
import org.junit.Test;
2424

@@ -82,6 +82,11 @@ public void javaWhenJavaLangAnnotationReturnsTrue() {
8282
assertThat(AnnotationFilter.JAVA.matches(Retention.class)).isTrue();
8383
}
8484

85+
@Test
86+
public void javaWhenJavaxAnnotationReturnsTrue() {
87+
assertThat(AnnotationFilter.JAVA.matches(Nonnull.class)).isTrue();
88+
}
89+
8590
@Test
8691
public void javaWhenSpringLangAnnotationReturnsFalse() {
8792
assertThat(AnnotationFilter.JAVA.matches(Nullable.class)).isFalse();

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.util.List;
3030
import java.util.Map;
3131
import java.util.Set;
32+
import javax.annotation.Nonnull;
33+
import javax.annotation.ParametersAreNonnullByDefault;
3234

3335
import org.junit.Before;
3436
import org.junit.Rule;
@@ -438,9 +440,11 @@ public void isAnnotationInheritedForAllScenarios() {
438440
}
439441

440442
@Test
441-
public void isAnnotationMetaPresentForJavaLangType() {
443+
public void isAnnotationMetaPresentForPlainType() {
442444
assertTrue(isAnnotationMetaPresent(Order.class, Documented.class));
443445
assertTrue(isAnnotationMetaPresent(NonNullApi.class, Documented.class));
446+
assertTrue(isAnnotationMetaPresent(NonNullApi.class, Nonnull.class));
447+
assertTrue(isAnnotationMetaPresent(ParametersAreNonnullByDefault.class, Nonnull.class));
444448
}
445449

446450
@Test

0 commit comments

Comments
 (0)