Merge branch '5.3.x'

spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java

@@ -725,6 +725,16 @@ public abstract class AnnotatedElementUtils {
 	 * 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 @@ public abstract class AnnotatedElementUtils {
 	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/main/java/org/springframework/core/annotation/RepeatableContainers.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.
@@ -39,6 +39,7 @@ import org.springframework.util.ReflectionUtils;
  * <p>To completely disable repeatable support use {@link #none()}.
  *
  * @author Phillip Webb
+ * @author Sam Brannen
  * @since 5.2
  */
 public abstract class RepeatableContainers {
@@ -101,15 +102,19 @@ public abstract class RepeatableContainers {
 	}
 
 	/**
-	 * Create a {@link RepeatableContainers} instance that uses a defined
-	 * container and repeatable type.
-	 * @param repeatable the contained repeatable annotation
-	 * @param container the container annotation or {@code null}. If specified,
+	 * Create a {@link RepeatableContainers} instance that uses predefined
+	 * repeatable and container types.
+	 * @param repeatable the contained repeatable annotation type
+	 * @param container the container annotation type or {@code null}. If specified,
 	 * this annotation must declare a {@code value} attribute returning an array
 	 * of repeatable annotations. If not specified, the container will be
 	 * deduced by inspecting the {@code @Repeatable} annotation on
 	 * {@code repeatable}.
 	 * @return a {@link RepeatableContainers} instance
+	 * @throws IllegalArgumentException if the supplied container type is
+	 * {@code null} and the annotation type is not a repeatable annotation
+	 * @throws AnnotationConfigurationException if the supplied container type
+	 * is not a properly configured container for a repeatable annotation
 	 */
 	public static RepeatableContainers of(
 			Class<? extends Annotation> repeatable, @Nullable Class<? extends Annotation> container) {

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