From 828f74f71a068f30b9c158f2a182f7fb9dc50b5e Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 11 Oct 2022 17:19:02 +0200 Subject: [PATCH] Support nesting in AnnotatedElementUtils.findMergedRepeatableAnnotations() Prior to this commit, the findMergedRepeatableAnnotations() methods in AnnotatedElementUtils failed to find repeatable annotations declared on other repeatable annotations (i.e., when one repeatable annotation type was used as a meta-annotation on a different repeatable annotation type). The reason is that findMergedRepeatableAnnotations(element, annotationType, containerType) always used RepeatableContainers.of(annotationType, containerType) to create a RepeatableContainers instance, even if the supplied containerType was null. Doing so restricts the search to supporting only repeatable annotations whose container is the supplied containerType and prevents the search from finding repeatable annotations declared as meta-annotations on other types of repeatable annotations. Note, however, that direct use of the MergedAnnotations API already supported finding nested repeatable annotations when using RepeatableContainers.standardRepeatables() or RepeatableContainers.of(...).and(...).and(...). The latter composes support for multiple repeatable annotation types and their containers. This commit addresses the issue for findMergedRepeatableAnnotations() when the containerType is null or not provided. However, findMergedRepeatableAnnotations(element, annotationType, containerType) still suffers from the aforementioned limitation, and the Javadoc has been updated to make that clear. Closes gh-20279 --- .../annotation/AnnotatedElementUtils.java | 30 ++- .../NestedRepeatableAnnotationsTests.java | 177 ++++++++++++++++++ 2 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 spring-core/src/test/java/org/springframework/core/annotation/NestedRepeatableAnnotationsTests.java 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 850cee07d216..e5a107c574c2 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -725,6 +725,16 @@ public static Set findMergedRepeatableAnnotations(Anno * single annotation and within annotation hierarchies. *

This method follows find semantics as described in the * {@linkplain AnnotatedElementUtils class-level javadoc}. + *

WARNING: if the supplied {@code containerType} is not + * {@code null}, the search will be restricted to supporting only repeatable + * annotations whose container is the supplied {@code containerType}. This + * prevents the search from finding repeatable annotations declared as + * meta-annotations on other types of repeatable annotations. If you need to + * support such a use case, favor {@link #findMergedRepeatableAnnotations(AnnotatedElement, Class)} + * over this method or alternatively use the {@link MergedAnnotations} API + * directly in conjunction with {@link RepeatableContainers} that are + * {@linkplain RepeatableContainers#and(Class, Class) composed} to support + * multiple repeatable annotation types. * @param element the annotated element (never {@code null}) * @param annotationType the annotation type to find (never {@code null}) * @param containerType the type of the container that holds the annotations; @@ -767,7 +777,23 @@ private static MergedAnnotations findAnnotations(AnnotatedElement element) { private static MergedAnnotations findRepeatableAnnotations(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 + // findMergedRepeatableAnnotations() 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.TYPE_HIERARCHY, 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 new file mode 100644 index 000000000000..97df12dee47d --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/annotation/NestedRepeatableAnnotationsTests.java @@ -0,0 +1,177 @@ +/* + * Copyright 2002-2022 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.annotation; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Repeatable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.lang.reflect.Method; +import java.util.Set; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; +import org.springframework.util.ReflectionUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for various ways to search for repeatable annotations that are + * nested (i.e., repeatable annotations used as meta-annotations on other + * repeatable annotations). + * + * @author Sam Brannen + * @since 5.3.24 + * @see https://github.com/spring-projects/spring-framework/issues/20279 + */ +@SuppressWarnings("unused") +class NestedRepeatableAnnotationsTests { + + @Nested + class SingleRepeatableAnnotationTests { + + private final Method method = ReflectionUtils.findMethod(getClass(), "annotatedMethod"); + + @Test + void streamRepeatableAnnotations_MergedAnnotationsApi() { + Set annotations = MergedAnnotations.from(method, SearchStrategy.TYPE_HIERARCHY) + .stream(A.class).collect(MergedAnnotationCollectors.toAnnotationSet()); + // Merged, so we expect to find @A once with its value coming from @B(5). + assertThat(annotations).extracting(A::value).containsExactly(5); + } + + @Test + void findMergedRepeatableAnnotations_AnnotatedElementUtils() { + Set annotations = AnnotatedElementUtils.findMergedRepeatableAnnotations(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 + @SuppressWarnings("deprecation") + void getRepeatableAnnotations_AnnotationUtils() { + Set annotations = AnnotationUtils.getRepeatableAnnotations(method, A.class); + // Not merged, so we expect to find @A once with the default value of 0. + // @A will actually be found twice, but we have Set semantics here. + assertThat(annotations).extracting(A::value).containsExactly(0); + } + + @B(5) + void annotatedMethod() { + } + + } + + @Nested + class MultipleRepeatableAnnotationsTests { + + private final Method method = ReflectionUtils.findMethod(getClass(), "annotatedMethod"); + + @Test + void streamRepeatableAnnotationsWithStandardRepeatables_MergedAnnotationsApi() { + RepeatableContainers repeatableContainers = RepeatableContainers.standardRepeatables(); + Set annotations = MergedAnnotations.from(method, SearchStrategy.TYPE_HIERARCHY, repeatableContainers) + .stream(A.class).collect(MergedAnnotationCollectors.toAnnotationSet()); + // 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 streamRepeatableAnnotationsWithExplicitRepeatables_MergedAnnotationsApi() { + RepeatableContainers repeatableContainers = + RepeatableContainers.of(A.class, A.Container.class).and(B.Container.class, B.class); + Set annotations = MergedAnnotations.from(method, SearchStrategy.TYPE_HIERARCHY, repeatableContainers) + .stream(A.class).collect(MergedAnnotationCollectors.toAnnotationSet()); + // 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 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); + } + + @Test + void findMergedRepeatableAnnotationsWithExplicitContainer_AnnotatedElementUtils() { + Set annotations = AnnotatedElementUtils.findMergedRepeatableAnnotations(method, A.class, A.Container.class); + // When findMergedRepeatableAnnotations(...) 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() { + Set annotations = AnnotationUtils.getRepeatableAnnotations(method, A.class); + // Not merged, so we expect to find a single @A with default value of 0. + // @A will actually be found twice, but we have Set semantics here. + assertThat(annotations).extracting(A::value).containsExactly(0); + } + + @B(5) + @B(10) + void annotatedMethod() { + } + + } + + + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE }) + @Repeatable(A.Container.class) + public @interface A { + + int value() default 0; + + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE }) + @interface Container { + A[] value(); + } + } + + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE }) + @Repeatable(B.Container.class) + @A + @A + public @interface B { + + @AliasFor(annotation = A.class) + int value(); + + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE }) + @interface Container { + B[] value(); + } + } + +}