From b841e8560c60bd3dae2127b1008177aad06176fd Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 29 Jul 2020 14:01:29 +0200 Subject: [PATCH 1/3] Polish MergedAnnotation API internals --- .../annotation/AnnotationTypeMappings.java | 26 +++++++------------ .../core/annotation/MergedAnnotations.java | 6 ++--- .../AnnotationTypeMappingsTests.java | 8 +++--- 3 files changed, 16 insertions(+), 24 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 e60231ed6da..7032c736050 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 @@ -30,7 +30,7 @@ import org.springframework.util.ConcurrentReferenceHashMap; * Provides {@link AnnotationTypeMapping} information for a single source * annotation type. Performs a recursive breadth first crawl of all * meta-annotations to ultimately provide a quick way to map the attributes of - * root {@link Annotation}. + * a root {@link Annotation}. * *

Supports convention based merging of meta-annotations as well as implicit * and explicit {@link AliasFor @AliasFor} aliases. Also provides information @@ -81,14 +81,12 @@ final class AnnotationTypeMappings { } private void addMetaAnnotationsToQueue(Deque queue, AnnotationTypeMapping source) { - Annotation[] metaAnnotations = - AnnotationsScanner.getDeclaredAnnotations(source.getAnnotationType(), false); + Annotation[] metaAnnotations = AnnotationsScanner.getDeclaredAnnotations(source.getAnnotationType(), false); for (Annotation metaAnnotation : metaAnnotations) { if (!isMappable(source, metaAnnotation)) { continue; } - Annotation[] repeatedAnnotations = this.repeatableContainers - .findRepeatedAnnotations(metaAnnotation); + Annotation[] repeatedAnnotations = this.repeatableContainers.findRepeatedAnnotations(metaAnnotation); if (repeatedAnnotations != null) { for (Annotation repeatedAnnotation : repeatedAnnotations) { if (!isMappable(source, metaAnnotation)) { @@ -103,9 +101,7 @@ final class AnnotationTypeMappings { } } - private void addIfPossible(Deque queue, - AnnotationTypeMapping source, Annotation ann) { - + private void addIfPossible(Deque queue, AnnotationTypeMapping source, Annotation ann) { addIfPossible(queue, source, ann.annotationType(), ann); } @@ -183,21 +179,20 @@ final class AnnotationTypeMappings { static AnnotationTypeMappings forAnnotationType( Class annotationType, AnnotationFilter annotationFilter) { - return forAnnotationType(annotationType, - RepeatableContainers.standardRepeatables(), annotationFilter); + return forAnnotationType(annotationType, RepeatableContainers.standardRepeatables(), annotationFilter); } /** * Create {@link AnnotationTypeMappings} for the specified annotation type. * @param annotationType the source annotation type + * @param repeatableContainers the repeatable containers that may be used by + * the meta-annotations * @param annotationFilter the annotation filter used to limit which * annotations are considered * @return type mappings for the annotation type */ - static AnnotationTypeMappings forAnnotationType( - Class annotationType, - RepeatableContainers repeatableContainers, - AnnotationFilter annotationFilter) { + static AnnotationTypeMappings forAnnotationType(Class annotationType, + RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) { if (repeatableContainers == RepeatableContainers.standardRepeatables()) { return standardRepeatablesCache.computeIfAbsent(annotationFilter, @@ -207,8 +202,7 @@ final class AnnotationTypeMappings { return noRepeatablesCache.computeIfAbsent(annotationFilter, key -> new Cache(repeatableContainers, key)).get(annotationType); } - return new AnnotationTypeMappings(repeatableContainers, annotationFilter, - annotationType); + return new AnnotationTypeMappings(repeatableContainers, annotationFilter, annotationType); } static void clearCache() { diff --git a/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotations.java b/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotations.java index 32055b2a815..97ce25598a4 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotations.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/MergedAnnotations.java @@ -326,7 +326,7 @@ public interface MergedAnnotations extends Iterable static MergedAnnotations from(AnnotatedElement element, SearchStrategy searchStrategy, RepeatableContainers repeatableContainers) { - return TypeMappedAnnotations.from(element, searchStrategy, repeatableContainers, AnnotationFilter.PLAIN); + return from(element, searchStrategy, repeatableContainers, AnnotationFilter.PLAIN); } /** @@ -340,7 +340,7 @@ public interface MergedAnnotations extends Iterable * @param annotationFilter an annotation filter used to restrict the * annotations considered * @return a {@link MergedAnnotations} instance containing the merged - * element annotations + * annotations for the supplied element */ static MergedAnnotations from(AnnotatedElement element, SearchStrategy searchStrategy, RepeatableContainers repeatableContainers, AnnotationFilter annotationFilter) { @@ -386,7 +386,7 @@ public interface MergedAnnotations extends Iterable * @return a {@link MergedAnnotations} instance containing the annotations */ static MergedAnnotations from(Object source, Annotation[] annotations, RepeatableContainers repeatableContainers) { - return TypeMappedAnnotations.from(source, annotations, repeatableContainers, AnnotationFilter.PLAIN); + return from(source, annotations, repeatableContainers, AnnotationFilter.PLAIN); } /** diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java index 950054ef4d5..571373234bc 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.stream.IntStream; import javax.annotation.Nullable; @@ -38,6 +39,7 @@ import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets.Mirr import org.springframework.lang.UsesSunMisc; import org.springframework.util.ReflectionUtils; +import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -500,11 +502,7 @@ class AnnotationTypeMappingsTests { private List getAll(AnnotationTypeMappings mappings) { // AnnotationTypeMappings does not implement Iterable so we don't create // too many garbage Iterators - List result = new ArrayList<>(mappings.size()); - for (int i = 0; i < mappings.size(); i++) { - result.add(mappings.get(i)); - } - return result; + return IntStream.range(0, mappings.size()).mapToObj(mappings::get).collect(toList()); } private List getNames(MirrorSet mirrorSet) { From 83a95832e6a1c721a509e513dc6e911c06cc6e7b Mon Sep 17 00:00:00 2001 From: yilianhuaixiao <85142297@qq.com> Date: Tue, 28 Jul 2020 15:37:22 +0800 Subject: [PATCH 2/3] Filter repeatable annotations in AnnotationTypeMappings Prior to this commit, AnnotationTypeMappings did not filter repeatable annotations with the supplied annotation filter. Closes gh-25483 --- .../core/annotation/AnnotationTypeMappings.java | 2 +- .../core/annotation/AnnotationTypeMappingsTests.java | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) 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 7032c736050..7bd535086ee 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 @@ -89,7 +89,7 @@ final class AnnotationTypeMappings { Annotation[] repeatedAnnotations = this.repeatableContainers.findRepeatedAnnotations(metaAnnotation); if (repeatedAnnotations != null) { for (Annotation repeatedAnnotation : repeatedAnnotations) { - if (!isMappable(source, metaAnnotation)) { + if (!isMappable(source, repeatedAnnotation)) { continue; } addIfPossible(queue, source, repeatedAnnotation); diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java index 571373234bc..220e2938240 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java @@ -37,6 +37,7 @@ import org.junit.jupiter.api.Test; import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets; import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets.MirrorSet; import org.springframework.lang.UsesSunMisc; +import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import static java.util.stream.Collectors.toList; @@ -476,6 +477,16 @@ class AnnotationTypeMappingsTests { return result; } + @Test + void forAnnotationTypeWhenRepeatableMetaAnnotationFilterd() { + AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(WithRepeatedMetaAnnotations.class, + annotationType -> + ObjectUtils.nullSafeEquals(annotationType, Repeating.class.getName())); + assertThat(getAll(mappings)).flatExtracting( + AnnotationTypeMapping::getAnnotationType).containsExactly( + WithRepeatedMetaAnnotations.class); + } + @Nullable private Method getAliasMapping(AnnotationTypeMapping mapping, int attributeIndex) { int mapped = mapping.getAliasMapping(attributeIndex); From 7dbf42e858b466b0665b0352b3c19c6390c518e7 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 29 Jul 2020 17:08:24 +0200 Subject: [PATCH 3/3] Polish contribution See gh-25483 --- .../AnnotationTypeMappingsTests.java | 19 +++---- ...dAnnotationsRepeatableAnnotationTests.java | 51 ++++++++++++++++--- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java index 220e2938240..a6baf3a8e71 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotationTypeMappingsTests.java @@ -37,7 +37,6 @@ import org.junit.jupiter.api.Test; import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets; import org.springframework.core.annotation.AnnotationTypeMapping.MirrorSets.MirrorSet; import org.springframework.lang.UsesSunMisc; -import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; import static java.util.stream.Collectors.toList; @@ -86,6 +85,14 @@ class AnnotationTypeMappingsTests { WithRepeatedMetaAnnotations.class, Repeating.class, Repeating.class); } + @Test + void forAnnotationTypeWhenRepeatableMetaAnnotationIsFiltered() { + AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(WithRepeatedMetaAnnotations.class, + Repeating.class.getName()::equals); + assertThat(getAll(mappings)).flatExtracting(AnnotationTypeMapping::getAnnotationType) + .containsExactly(WithRepeatedMetaAnnotations.class); + } + @Test void forAnnotationTypeWhenSelfAnnotatedReturnsMapping() { AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(SelfAnnotated.class); @@ -477,16 +484,6 @@ class AnnotationTypeMappingsTests { return result; } - @Test - void forAnnotationTypeWhenRepeatableMetaAnnotationFilterd() { - AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType(WithRepeatedMetaAnnotations.class, - annotationType -> - ObjectUtils.nullSafeEquals(annotationType, Repeating.class.getName())); - assertThat(getAll(mappings)).flatExtracting( - AnnotationTypeMapping::getAnnotationType).containsExactly( - WithRepeatedMetaAnnotations.class); - } - @Nullable private Method getAliasMapping(AnnotationTypeMapping mapping, int attributeIndex) { int mapped = mapping.getAliasMapping(attributeIndex); diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java index 0b9fe0bbdb5..40b1fdf5b8f 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.AnnotatedElement; import java.util.Set; +import java.util.stream.Stream; import org.assertj.core.api.ThrowableTypeAssert; import org.junit.jupiter.api.Test; @@ -222,14 +223,35 @@ class MergedAnnotationsRepeatableAnnotationTests { "B", "C"); } - private Set getAnnotations( - Class container, Class repeatable, - SearchStrategy searchStrategy, AnnotatedElement element) { + @Test + void typeHierarchyAnnotationsWithLocalComposedAnnotationWhoseRepeatableMetaAnnotationsAreFiltered() { + Class element = WithRepeatedMetaAnnotationsClass.class; + SearchStrategy searchStrategy = SearchStrategy.TYPE_HIERARCHY; + AnnotationFilter annotationFilter = PeteRepeat.class.getName()::equals; + + Set annotations = getAnnotations(null, PeteRepeat.class, searchStrategy, element, annotationFilter); + assertThat(annotations).isEmpty(); + + MergedAnnotations mergedAnnotations = MergedAnnotations.from(element, searchStrategy, + RepeatableContainers.standardRepeatables(), annotationFilter); + Stream> annotationTypes = mergedAnnotations.stream() + .map(MergedAnnotation::synthesize) + .map(Annotation::annotationType); + assertThat(annotationTypes).containsExactly(WithRepeatedMetaAnnotations.class, Noninherited.class, Noninherited.class); + } + + private Set getAnnotations(Class container, + Class repeatable, SearchStrategy searchStrategy, AnnotatedElement element) { + + return getAnnotations(container, repeatable, searchStrategy, element, AnnotationFilter.PLAIN); + } + + private Set getAnnotations(Class container, + Class repeatable, SearchStrategy searchStrategy, AnnotatedElement element, AnnotationFilter annotationFilter) { + RepeatableContainers containers = RepeatableContainers.of(repeatable, container); - MergedAnnotations annotations = MergedAnnotations.from(element, - searchStrategy, containers, AnnotationFilter.PLAIN); - return annotations.stream(repeatable).collect( - MergedAnnotationCollectors.toAnnotationSet()); + MergedAnnotations annotations = MergedAnnotations.from(element, searchStrategy, containers, annotationFilter); + return annotations.stream(repeatable).collect(MergedAnnotationCollectors.toAnnotationSet()); } private void nonRepeatableRequirements(Exception ex) { @@ -414,4 +436,17 @@ class MergedAnnotationsRepeatableAnnotationTests { } + @Retention(RetentionPolicy.RUNTIME) + @PeteRepeat("A") + @PeteRepeat("B") + @interface WithRepeatedMetaAnnotations { + } + + @WithRepeatedMetaAnnotations + @PeteRepeat("C") + @Noninherited("X") + @Noninherited("Y") + static class WithRepeatedMetaAnnotationsClass { + } + }