From 433b1c480cdc0f4dbe2a4d4a4686461344d79b09 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. Closes gh-29685 --- .../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 52adbc034bb0..e163f7d92928 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 @@ private static Method getRepeatedAnnotationsMethod(Class a 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 c70c576559b5..3eba643295e1 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.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 @@ * @see AnnotationUtilsTests * @see MultipleComposedAnnotationsOnSingleAnnotatedElementTests * @see ComposedRepeatableAnnotationsTests + * @see NestedRepeatableAnnotationsTests */ class AnnotatedElementUtilsTests { @@ -908,6 +910,31 @@ void getMergedAnnotationOnThreeDeepMetaWithValue() { 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"); + } // ------------------------------------------------------------------------- @@ -1567,4 +1594,24 @@ class ForAnnotationsClass { 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 7946b1dc4fc4..fd2b89da9256 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 @@ void standardRepeatablesWhenContainerReturnsRepeats() { 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 @@ static class SingleStandardRepeatableTestCase { 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 { }