From 5ddc984192b6150fee98419aedd63c2fac11cb54 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 13 Dec 2022 15:35:30 +0100 Subject: [PATCH] Support repeatable annotation containers with multiple attributes Prior to this commit, there was a bug in the implementation of StandardRepeatableContainers.computeRepeatedAnnotationsMethod() which has existed since Spring Framework 5.2 (when StandardRepeatableContainers was introduced). Specifically, StandardRepeatableContainers ignored any repeatable container annotation if it declared attributes other than `value()`. However, Java permits any number of attributes in a repeatable container annotation. In addition, the changes made in conjunction with gh-20279 made the bug in StandardRepeatableContainers apparent when using the getMergedRepeatableAnnotations() or findMergedRepeatableAnnotations() method in AnnotatedElementUtils, resulting in regressions for the behavior of those two methods. This commit fixes the regressions and bug by altering the logic in StandardRepeatableContainers.computeRepeatedAnnotationsMethod() so that it explicitly looks for the `value()` method and ignores any other methods declared in a repeatable container annotation candidate. See gh-29685 Closes gh-29686 --- .../core/annotation/RepeatableContainers.java | 4 +- .../AnnotatedElementUtilsTests.java | 47 +++++++++++++++++++ .../annotation/RepeatableContainersTests.java | 29 ++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java b/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java index 52adbc034bb..e163f7d9292 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/RepeatableContainers.java @@ -166,8 +166,8 @@ public abstract class RepeatableContainers { private static Object computeRepeatedAnnotationsMethod(Class annotationType) { AttributeMethods methods = AttributeMethods.forAnnotationType(annotationType); - if (methods.hasOnlyValueAttribute()) { - Method method = methods.get(0); + Method method = methods.get(MergedAnnotation.VALUE); + if (method != null) { Class returnType = method.getReturnType(); if (returnType.isArray()) { Class componentType = returnType.getComponentType(); diff --git a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java index d90cfec05b3..336ca99b75c 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/AnnotatedElementUtilsTests.java @@ -20,6 +20,7 @@ import java.lang.annotation.Annotation; import java.lang.annotation.Documented; import java.lang.annotation.ElementType; import java.lang.annotation.Inherited; +import java.lang.annotation.Repeatable; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @@ -77,6 +78,7 @@ import static org.springframework.core.annotation.AnnotationUtilsTests.asArray; * @see AnnotationUtilsTests * @see MultipleComposedAnnotationsOnSingleAnnotatedElementTests * @see ComposedRepeatableAnnotationsTests + * @see NestedRepeatableAnnotationsTests */ class AnnotatedElementUtilsTests { @@ -908,6 +910,31 @@ class AnnotatedElementUtilsTests { assertThat(annotation.value()).containsExactly("FromValueAttributeMeta"); } + /** + * @since 5.3.25 + */ + @Test // gh-29685 + void getMergedRepeatableAnnotationsWithContainerWithMultipleAttributes() { + Set repeatableAnnotations = + AnnotatedElementUtils.getMergedRepeatableAnnotations( + StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class, + StandardRepeatableWithContainerWithMultipleAttributes.class); + assertThat(repeatableAnnotations).map(StandardRepeatableWithContainerWithMultipleAttributes::value) + .containsExactly("a", "b"); + } + + /** + * @since 5.3.25 + */ + @Test // gh-29685 + void findMergedRepeatableAnnotationsWithContainerWithMultipleAttributes() { + Set repeatableAnnotations = + AnnotatedElementUtils.findMergedRepeatableAnnotations( + StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class, + StandardRepeatableWithContainerWithMultipleAttributes.class); + assertThat(repeatableAnnotations).map(StandardRepeatableWithContainerWithMultipleAttributes::value) + .containsExactly("a", "b"); + } // ------------------------------------------------------------------------- @@ -1557,4 +1584,24 @@ class AnnotatedElementUtilsTests { static class ValueAttributeMetaMetaClass { } + @Retention(RetentionPolicy.RUNTIME) + @interface StandardContainerWithMultipleAttributes { + + StandardRepeatableWithContainerWithMultipleAttributes[] value(); + + String name() default ""; + } + + @Retention(RetentionPolicy.RUNTIME) + @Repeatable(StandardContainerWithMultipleAttributes.class) + @interface StandardRepeatableWithContainerWithMultipleAttributes { + + String value() default ""; + } + + @StandardRepeatableWithContainerWithMultipleAttributes("a") + @StandardRepeatableWithContainerWithMultipleAttributes("b") + static class StandardRepeatablesWithContainerWithMultipleAttributesTestCase { + } + } diff --git a/spring-core/src/test/java/org/springframework/core/annotation/RepeatableContainersTests.java b/spring-core/src/test/java/org/springframework/core/annotation/RepeatableContainersTests.java index 7946b1dc4fc..fd2b89da925 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/RepeatableContainersTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/RepeatableContainersTests.java @@ -67,6 +67,15 @@ class RepeatableContainersTests { StandardRepeatablesTestCase.class, StandardContainer.class); assertThat(values).containsExactly("a", "b"); } + + @Test + void standardRepeatablesWithContainerWithMultipleAttributes() { + Object[] values = findRepeatedAnnotationValues(RepeatableContainers.standardRepeatables(), + StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class, + StandardContainerWithMultipleAttributes.class); + assertThat(values).containsExactly("a", "b"); + } + } @Nested @@ -247,6 +256,26 @@ class RepeatableContainersTests { static class StandardRepeatablesTestCase { } + @Retention(RetentionPolicy.RUNTIME) + @interface StandardContainerWithMultipleAttributes { + + StandardRepeatableWithContainerWithMultipleAttributes[] value(); + + String name() default ""; + } + + @Retention(RetentionPolicy.RUNTIME) + @Repeatable(StandardContainerWithMultipleAttributes.class) + @interface StandardRepeatableWithContainerWithMultipleAttributes { + + String value() default ""; + } + + @StandardRepeatableWithContainerWithMultipleAttributes("a") + @StandardRepeatableWithContainerWithMultipleAttributes("b") + static class StandardRepeatablesWithContainerWithMultipleAttributesTestCase { + } + @ExplicitContainer({ @ExplicitRepeatable("a"), @ExplicitRepeatable("b") }) static class ExplicitRepeatablesTestCase { }