Support nesting in AnnotatedElementUtils.findMergedRepeatableAnnotati…

…ons()

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

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 <A extends Annotation> Set<A> findMergedRepeatableAnnotations(Anno
 	 * single annotation and within annotation hierarchies.
 	 * <p>This method follows <em>find semantics</em> as described in the
 	 * {@linkplain AnnotatedElementUtils class-level javadoc}.
+	 * <p><strong>WARNING</strong>: 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<? extends Annotation> containerType, Class<? extends Annotation> 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);
 	}
 

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<A> 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<A> 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<A> 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<A> 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<A> 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<A> 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<A> 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<A> 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();
+		}
+	}
+
+}