From 9876701493717704cfe6e8258c6f1d6ce0c016e1 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 11 Oct 2022 18:06:00 +0200 Subject: [PATCH] Support nesting in AnnotatedElementUtils.getMergedRepeatableAnnotations() This commit is a follow up to 828f74f71a068f30b9c158f2a182f7fb9dc50b5e and applies to same fix for getMergedRepeatableAnnotations(). See the previous commit for details. Closes gh-20279 --- .../annotation/AnnotatedElementUtils.java | 18 ++++++++- .../NestedRepeatableAnnotationsTests.java | 37 ++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java index e5a107c574c2..3302aaca947a 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java @@ -766,7 +766,23 @@ private static MergedAnnotations getAnnotations(AnnotatedElement element) { private static MergedAnnotations getRepeatableAnnotations(AnnotatedElement element, @Nullable Class containerType, Class annotationType) { - RepeatableContainers repeatableContainers = RepeatableContainers.of(annotationType, containerType); + RepeatableContainers repeatableContainers; + if (containerType == null) { + // Invoke RepeatableContainers.of() in order to adhere to the contract of + // getMergedRepeatableAnnotations() which states that an IllegalArgumentException + // will be thrown if the the container cannot be resolved. + // + // In any case, we use standardRepeatables() in order to support repeatable + // annotations on other types of repeatable annotations (i.e., nested repeatable + // annotation types). + // + // See https://github.com/spring-projects/spring-framework/issues/20279 + RepeatableContainers.of(annotationType, null); + repeatableContainers = RepeatableContainers.standardRepeatables(); + } + else { + repeatableContainers = RepeatableContainers.of(annotationType, containerType); + } return MergedAnnotations.from(element, SearchStrategy.INHERITED_ANNOTATIONS, repeatableContainers); } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/NestedRepeatableAnnotationsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/NestedRepeatableAnnotationsTests.java index 97df12dee47d..1fe1a6fe8b09 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/NestedRepeatableAnnotationsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/NestedRepeatableAnnotationsTests.java @@ -64,6 +64,20 @@ void findMergedRepeatableAnnotations_AnnotatedElementUtils() { assertThat(annotations).extracting(A::value).containsExactly(5); } + @Test + void getMergedRepeatableAnnotationsWithStandardRepeatables_AnnotatedElementUtils() { + Set annotations = AnnotatedElementUtils.getMergedRepeatableAnnotations(method, A.class); + // Merged, so we expect to find @A once with its value coming from @B(5). + assertThat(annotations).extracting(A::value).containsExactly(5); + } + + @Test + void getMergedRepeatableAnnotationsWithExplicitContainer_AnnotatedElementUtils() { + Set annotations = AnnotatedElementUtils.getMergedRepeatableAnnotations(method, A.class, A.Container.class); + // Merged, so we expect to find @A once with its value coming from @B(5). + assertThat(annotations).extracting(A::value).containsExactly(5); + } + @Test @SuppressWarnings("deprecation") void getRepeatableAnnotations_AnnotationUtils() { @@ -107,7 +121,6 @@ void streamRepeatableAnnotationsWithExplicitRepeatables_MergedAnnotationsApi() { void findMergedRepeatableAnnotationsWithStandardRepeatables_AnnotatedElementUtils() { Set annotations = AnnotatedElementUtils.findMergedRepeatableAnnotations(method, A.class); // Merged, so we expect to find @A twice with values coming from @B(5) and @B(10). - // However, findMergedRepeatableAnnotations() currently finds ZERO annotations. assertThat(annotations).extracting(A::value).containsExactly(5, 10); } @@ -126,6 +139,28 @@ void findMergedRepeatableAnnotationsWithExplicitContainer_AnnotatedElementUtils( assertThat(annotations).isEmpty(); } + @Test + void getMergedRepeatableAnnotationsWithStandardRepeatables_AnnotatedElementUtils() { + Set annotations = AnnotatedElementUtils.getMergedRepeatableAnnotations(method, A.class); + // Merged, so we expect to find @A twice with values coming from @B(5) and @B(10). + assertThat(annotations).extracting(A::value).containsExactly(5, 10); + } + + @Test + void getMergedRepeatableAnnotationsWithExplicitContainer_AnnotatedElementUtils() { + Set annotations = AnnotatedElementUtils.getMergedRepeatableAnnotations(method, A.class, A.Container.class); + // When getMergedRepeatableAnnotations(...) is invoked with an explicit container + // type, it uses RepeatableContainers.of(...) which limits the repeatable annotation + // support to a single container type. + // + // In this test case, we are therefore limiting the support to @A.Container, which + // means that @B.Container is unsupported and effectively ignored as a repeatable + // container type. + // + // Long story, short: the search doesn't find anything. + assertThat(annotations).isEmpty(); + } + @Test @SuppressWarnings("deprecation") void getRepeatableAnnotations_AnnotationUtils() {