From 3b50b6ef943d5fc7d8baf56f3a91308f8fdca424 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 3 May 2024 12:14:44 +0300 Subject: [PATCH] Include repeatable annotation container in MergedAnnotations results A bug has existed in Spring's MergedAnnotations support since it was introduced in Spring Framework 5.2. Specifically, if the MergedAnnotations API is used to search for annotations with "standard repeatable annotation" support enabled (which is the default), it's possible to search for a repeatable annotation but not for the repeatable annotation's container annotation. The reason is that MergedAnnotationFinder.process(Object, int, Object, Annotation) does not process the container annotation and instead only processes the "contained" annotations, which prevents a container annotation from being included in search results. In #29685, we fixed a bug that prevented the MergedAnnotations support from recognizing an annotation as a container if the container annotation declares attributes other than the required `value` attribute. As a consequence of that bug fix, since Spring Framework 5.3.25, the MergedAnnotations infrastructure considers such an annotation a container, and due to the aforementioned bug the container is no longer processed, which results in a regression in behavior for annotation searches for such a container annotation. This commit addresses the original bug as well as the regression by processing container annotations in addition to the contained repeatable annotations. See gh-29685 Closes gh-32731 (cherry picked from commit 4baad16437524f6f16f61fcfb3dc0458f7aaff47) --- .../annotation/TypeMappedAnnotations.java | 7 +- ...dAnnotationsRepeatableAnnotationTests.java | 64 ++++++++++++++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java index 581a74c10bd..7187cceb42a 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/TypeMappedAnnotations.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 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. @@ -418,7 +418,10 @@ final class TypeMappedAnnotations implements MergedAnnotations { Annotation[] repeatedAnnotations = repeatableContainers.findRepeatedAnnotations(annotation); if (repeatedAnnotations != null) { - return doWithAnnotations(type, aggregateIndex, source, repeatedAnnotations); + MergedAnnotation result = doWithAnnotations(type, aggregateIndex, source, repeatedAnnotations); + if (result != null) { + return result; + } } AnnotationTypeMappings mappings = AnnotationTypeMappings.forAnnotationType( annotation.annotationType(), repeatableContainers, annotationFilter); diff --git a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java index 40b1fdf5b8f..f1a80ee56e7 100644 --- a/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java +++ b/spring-core/src/test/java/org/springframework/core/annotation/MergedAnnotationsRepeatableAnnotationTests.java @@ -24,6 +24,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.lang.reflect.AnnotatedElement; +import java.util.Arrays; import java.util.Set; import java.util.stream.Stream; @@ -175,7 +176,7 @@ class MergedAnnotationsRepeatableAnnotationTests { } @Test - void typeHierarchyWhenWhenOnSuperclassReturnsAnnotations() { + void typeHierarchyWhenOnSuperclassReturnsAnnotations() { Set annotations = getAnnotations(null, PeteRepeat.class, SearchStrategy.TYPE_HIERARCHY, SubRepeatableClass.class); assertThat(annotations.stream().map(PeteRepeat::value)).containsExactly("A", "B", @@ -240,6 +241,44 @@ class MergedAnnotationsRepeatableAnnotationTests { assertThat(annotationTypes).containsExactly(WithRepeatedMetaAnnotations.class, Noninherited.class, Noninherited.class); } + @Test // gh-32731 + void searchFindsRepeatableContainerAnnotationAndRepeatedAnnotations() { + Class clazz = StandardRepeatablesWithContainerWithMultipleAttributesTestCase.class; + + // NO RepeatableContainers + MergedAnnotations mergedAnnotations = MergedAnnotations.from(clazz, TYPE_HIERARCHY, RepeatableContainers.none()); + ContainerWithMultipleAttributes container = mergedAnnotations + .get(ContainerWithMultipleAttributes.class) + .synthesize(MergedAnnotation::isPresent).orElse(null); + assertThat(container).as("container").isNotNull(); + assertThat(container.name()).isEqualTo("enigma"); + RepeatableWithContainerWithMultipleAttributes[] repeatedAnnotations = container.value(); + assertThat(Arrays.stream(repeatedAnnotations).map(RepeatableWithContainerWithMultipleAttributes::value)) + .containsExactly("A", "B"); + Set set = + mergedAnnotations.stream(RepeatableWithContainerWithMultipleAttributes.class) + .collect(MergedAnnotationCollectors.toAnnotationSet()); + // Only finds the locally declared repeated annotation. + assertThat(set.stream().map(RepeatableWithContainerWithMultipleAttributes::value)) + .containsExactly("C"); + + // Standard RepeatableContainers + mergedAnnotations = MergedAnnotations.from(clazz, TYPE_HIERARCHY, RepeatableContainers.standardRepeatables()); + container = mergedAnnotations + .get(ContainerWithMultipleAttributes.class) + .synthesize(MergedAnnotation::isPresent).orElse(null); + assertThat(container).as("container").isNotNull(); + assertThat(container.name()).isEqualTo("enigma"); + repeatedAnnotations = container.value(); + assertThat(Arrays.stream(repeatedAnnotations).map(RepeatableWithContainerWithMultipleAttributes::value)) + .containsExactly("A", "B"); + set = mergedAnnotations.stream(RepeatableWithContainerWithMultipleAttributes.class) + .collect(MergedAnnotationCollectors.toAnnotationSet()); + // Finds the locally declared repeated annotation plus the 2 in the container. + assertThat(set.stream().map(RepeatableWithContainerWithMultipleAttributes::value)) + .containsExactly("A", "B", "C"); + } + private Set getAnnotations(Class container, Class repeatable, SearchStrategy searchStrategy, AnnotatedElement element) { @@ -449,4 +488,27 @@ class MergedAnnotationsRepeatableAnnotationTests { static class WithRepeatedMetaAnnotationsClass { } + @Retention(RetentionPolicy.RUNTIME) + @interface ContainerWithMultipleAttributes { + + RepeatableWithContainerWithMultipleAttributes[] value(); + + String name() default ""; + } + + @Retention(RetentionPolicy.RUNTIME) + @Repeatable(ContainerWithMultipleAttributes.class) + @interface RepeatableWithContainerWithMultipleAttributes { + + String value() default ""; + } + + @ContainerWithMultipleAttributes(name = "enigma", value = { + @RepeatableWithContainerWithMultipleAttributes("A"), + @RepeatableWithContainerWithMultipleAttributes("B") + }) + @RepeatableWithContainerWithMultipleAttributes("C") + static class StandardRepeatablesWithContainerWithMultipleAttributesTestCase { + } + }