From 49fccfb158135d72b586bdf16e79f01c40ba63a3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 28 Mar 2019 14:38:55 +0100 Subject: [PATCH] Consistent retrieval of javax annotation types (e.g. JSR-305) Closes gh-22696 --- .../annotation/AnnotationTypeMappings.java | 5 ++-- .../core/annotation/AnnotationUtils.java | 9 +++++- .../core/annotation/AnnotationsScanner.java | 28 ++++++++----------- .../core/annotation/OrderUtils.java | 5 ++-- .../annotation/TypeMappedAnnotations.java | 13 ++++----- .../AnnotatedElementUtilsTests.java | 25 ++++++++++++++--- 6 files changed, 49 insertions(+), 36 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java index 966293f45d..5ac18b457d 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMappings.java @@ -156,6 +156,7 @@ final class AnnotationTypeMappings { return this.mappings.get(index); } + /** * Return {@link AnnotationTypeMappings} for the specified annotation type. * @param annotationType the source annotation type @@ -202,8 +203,7 @@ final class AnnotationTypeMappings { } /** - * Return or create {@link AnnotationTypeMappings} for the specified - * annotation type. + * Return or create {@link AnnotationTypeMappings} for the specified annotation type. * @param annotationType the annotation type * @return a new or existing {@link AnnotationTypeMapping} instance */ @@ -214,7 +214,6 @@ final class AnnotationTypeMappings { AnnotationTypeMappings createMappings(Class annotationType) { return new AnnotationTypeMappings(this.filter, annotationType); } - } } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java index 8a59547cb4..0351c6d01b 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java @@ -163,7 +163,14 @@ public abstract class AnnotationUtils { * @since 5.2 */ public static boolean isCandidateClass(Class clazz, String annotationName) { - return !clazz.getName().startsWith("java"); + if (annotationName.startsWith("java.")) { + return true; + } + if (AnnotationsScanner.hasPlainJavaAnnotationsOnly(clazz)) { + return false; + } + // TODO: annotation presence registry to be integrated here + return true; } /** 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 2196847620..4e859d268d 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 @@ -462,18 +462,18 @@ abstract class AnnotationsScanner { return annotations.clone(); } - private static boolean isIgnorable(Class annotationType) { - return AnnotationFilter.PLAIN.matches(annotationType); - } - private static boolean isFiltered( Class sourceClass, C context, @Nullable BiPredicate> classFilter) { return (classFilter != null && classFilter.test(context, sourceClass)); } - static boolean isKnownEmpty(AnnotatedElement source,SearchStrategy searchStrategy, AnnotationFilter annotationFilter) { - if (annotationFilter == AnnotationFilter.PLAIN && hasPlainJavaAnnotationsOnly(source)) { + private static boolean isIgnorable(Class annotationType) { + return AnnotationFilter.PLAIN.matches(annotationType); + } + + static boolean isKnownEmpty(AnnotatedElement source, SearchStrategy searchStrategy) { + if (hasPlainJavaAnnotationsOnly(source)) { return true; } if (searchStrategy == SearchStrategy.DIRECT || isWithoutHierarchy(source)) { @@ -486,25 +486,19 @@ abstract class AnnotationsScanner { } static boolean hasPlainJavaAnnotationsOnly(@Nullable Object annotatedElement) { - Class type; if (annotatedElement instanceof Class) { - type = (Class) annotatedElement; + return hasPlainJavaAnnotationsOnly((Class) annotatedElement); } else if (annotatedElement instanceof Member) { - type = ((Member) annotatedElement).getDeclaringClass(); + return hasPlainJavaAnnotationsOnly(((Member) annotatedElement).getDeclaringClass()); } else { return false; } + } - if (type == Ordered.class) { - return true; - } - String name = type.getName(); - return (name.startsWith("java") || - name.startsWith("org.springframework.lang.") || - name.startsWith("org.springframework.util.") || - (name.startsWith("com.sun") && !name.contains("Proxy"))); + static boolean hasPlainJavaAnnotationsOnly(Class type) { + return (type.getName().startsWith("java.") || type == Ordered.class); } private static boolean isWithoutHierarchy(AnnotatedElement source) { diff --git a/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java index 1b7f43f4b5..2e8c216b96 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/OrderUtils.java @@ -127,9 +127,8 @@ public abstract class OrderUtils { */ @Nullable public static Integer getPriority(Class type) { - return MergedAnnotations.from(type, SearchStrategy.EXHAUSTIVE).get( - JAVAX_PRIORITY_ANNOTATION).getValue(MergedAnnotation.VALUE, - Integer.class).orElse(null); + return MergedAnnotations.from(type, SearchStrategy.EXHAUSTIVE).get(JAVAX_PRIORITY_ANNOTATION) + .getValue(MergedAnnotation.VALUE, Integer.class).orElse(null); } } diff --git a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java index 1b091c3772..ddeded4e70 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java @@ -244,10 +244,10 @@ final class TypeMappedAnnotations implements MergedAnnotations { } - static MergedAnnotations from(@Nullable AnnotatedElement element, SearchStrategy searchStrategy, + static MergedAnnotations from(AnnotatedElement element, SearchStrategy searchStrategy, RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) { - if (element == null || AnnotationsScanner.isKnownEmpty(element, searchStrategy, annotationFilter)) { + if (AnnotationsScanner.isKnownEmpty(element, searchStrategy)) { return NONE; } return new TypeMappedAnnotations(element, searchStrategy, repeatableContainers, annotationFilter); @@ -262,12 +262,9 @@ final class TypeMappedAnnotations implements MergedAnnotations { return new TypeMappedAnnotations(source, annotations, repeatableContainers, annotationFilter); } - private static boolean isMappingForType(@Nullable AnnotationTypeMapping mapping, + private static boolean isMappingForType(AnnotationTypeMapping mapping, AnnotationFilter annotationFilter, @Nullable Object requiredType) { - if (mapping == null) { - return false; - } Class actualType = mapping.getAnnotationType(); return (!annotationFilter.matches(actualType) && (requiredType == null || actualType == requiredType || actualType.getName().equals(requiredType))); @@ -523,7 +520,7 @@ final class TypeMappedAnnotations implements MergedAnnotations { @Nullable AnnotationTypeMapping getMapping(int annotationIndex, int mappingIndex) { AnnotationTypeMappings mappings = getMappings(annotationIndex); - return mappingIndex < mappings.size() ? mappings.get(mappingIndex) : null; + return (mappingIndex < mappings.size() ? mappings.get(mappingIndex) : null); } AnnotationTypeMappings getMappings(int annotationIndex) { @@ -612,7 +609,7 @@ final class TypeMappedAnnotations implements MergedAnnotations { AnnotationTypeMapping mapping; do { mapping = aggregate.getMapping(annotationIndex, cursors[annotationIndex]); - if (isMappingForType(mapping, annotationFilter, this.requiredType)) { + if (mapping != null && isMappingForType(mapping, annotationFilter, this.requiredType)) { return mapping; } cursors[annotationIndex]++; diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java index ddbcb6d2f9..e6273e4d21 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java @@ -32,6 +32,7 @@ import java.util.Set; import javax.annotation.Nonnull; import javax.annotation.ParametersAreNonnullByDefault; import javax.annotation.Resource; +import javax.annotation.meta.When; import org.junit.Ignore; import org.junit.Rule; @@ -198,7 +199,7 @@ public class AnnotatedElementUtilsTests { public void getAllAnnotationAttributesOnClassWithLocalAnnotation() { MultiValueMap attributes = getAllAnnotationAttributes(TxConfig.class, TX_NAME); assertNotNull("Annotation attributes map for @Transactional on TxConfig", attributes); - assertEquals("value for TxConfig.", asList("TxConfig"), attributes.get("value")); + assertEquals("value for TxConfig", asList("TxConfig"), attributes.get("value")); } @Test @@ -234,7 +235,7 @@ public class AnnotatedElementUtilsTests { public void getAllAnnotationAttributesOnClassWithLocalAnnotationThatShadowsAnnotationFromSuperclass() { MultiValueMap attributes = getAllAnnotationAttributes(DerivedTxConfig.class, TX_NAME); assertNotNull("Annotation attributes map for @Transactional on DerivedTxConfig", attributes); - assertEquals("value for DerivedTxConfig.", asList("DerivedTxConfig"), attributes.get("value")); + assertEquals("value for DerivedTxConfig", asList("DerivedTxConfig"), attributes.get("value")); } /** @@ -249,13 +250,29 @@ public class AnnotatedElementUtilsTests { attributes.get("value")); } + @Test + public void getAllAnnotationAttributesOnLangType() { + MultiValueMap attributes = getAllAnnotationAttributes( + NonNullApi.class, Nonnull.class.getName()); + assertNotNull("Annotation attributes map for @Nonnull on NonNullApi", attributes); + assertEquals("value for NonNullApi", asList(When.ALWAYS), attributes.get("when")); + } + + @Test + public void getAllAnnotationAttributesOnJavaxType() { + MultiValueMap attributes = getAllAnnotationAttributes( + ParametersAreNonnullByDefault.class, Nonnull.class.getName()); + assertNotNull("Annotation attributes map for @Nonnull on NonNullApi", attributes); + assertEquals("value for NonNullApi", asList(When.ALWAYS), attributes.get("when")); + } + @Test public void getMergedAnnotationAttributesOnClassWithLocalAnnotation() { Class element = TxConfig.class; String name = TX_NAME; AnnotationAttributes attributes = getMergedAnnotationAttributes(element, name); assertNotNull("Annotation attributes for @Transactional on TxConfig", attributes); - assertEquals("value for TxConfig.", "TxConfig", attributes.getString("value")); + assertEquals("value for TxConfig", "TxConfig", attributes.getString("value")); // Verify contracts between utility methods: assertTrue(isAnnotated(element, name)); } @@ -266,7 +283,7 @@ public class AnnotatedElementUtilsTests { String name = TX_NAME; AnnotationAttributes attributes = getMergedAnnotationAttributes(element, name); assertNotNull("Annotation attributes for @Transactional on DerivedTxConfig", attributes); - assertEquals("value for DerivedTxConfig.", "DerivedTxConfig", attributes.getString("value")); + assertEquals("value for DerivedTxConfig", "DerivedTxConfig", attributes.getString("value")); // Verify contracts between utility methods: assertTrue(isAnnotated(element, name)); }