-
Notifications
You must be signed in to change notification settings - Fork 38.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AnnotatedElementUtils does not find merged repeatable annotations on other repeatable annotations #20279
Comments
Thanks for raising the issue, and I apologize that we took so long to triage it. I was able to reproduce this against import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.assertEquals;
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.List;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.springframework.core.annotation.AliasFor;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.core.annotation.AnnotationUtils;
@SuppressWarnings("unused")
class AnnotationRetrievalTest {
@Test
void findAnnotationsSingle() throws Exception {
Method singleAnnotatedMethod = getClass().getDeclaredMethod("singleAnnotatedMethod");
// Passes.
performTest(singleAnnotatedMethod, 1);
}
@Test
void findAnnotationsMulti() throws Exception {
Method multiAnnotatedMethod = getClass().getDeclaredMethod("multiAnnotatedMethod");
// Fails (for all 3 sub-assertions).
performTest(multiAnnotatedMethod, 2);
}
private void performTest(Method method, int expectedAnnotationCount) {
Set<A> fromFindMergedRepeatable = AnnotatedElementUtils.findMergedRepeatableAnnotations(method, A.class);
Set<A> fromFindMergedRepeatableWithContainer = AnnotatedElementUtils.findMergedRepeatableAnnotations(method,
A.class, A.Container.class);
Set<A> fromGetRepeatable = AnnotationUtils.getRepeatableAnnotations(method, A.class);
List<A> fromJUnitFindRepeatable = org.junit.platform.commons.util.AnnotationUtils
.findRepeatableAnnotations(method, A.class);
assertAll(() -> assertEquals(expectedAnnotationCount, fromFindMergedRepeatable.size()),
() -> assertEquals(expectedAnnotationCount, fromFindMergedRepeatableWithContainer.size()),
() -> assertEquals(expectedAnnotationCount, fromGetRepeatable.size()),
() -> assertEquals(expectedAnnotationCount, fromJUnitFindRepeatable.size()));
}
@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
public @interface B {
@AliasFor(annotation = A.class)
int value();
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.METHOD, ElementType.ANNOTATION_TYPE })
@interface Container {
B[] value();
}
}
@B(5)
void singleAnnotatedMethod() {
}
@B(5)
@B(10)
void multiAnnotatedMethod() {
}
} Interestingly, the supplied @philwebb and @jhoeller, do you think we should try to add support for this scenario? |
Actually, after having put further thought into it, the expectation for
|
Reopening to address the same issue in |
It turns out that we already had support for finding repeatable annotations used as meta-annotations on other repeatable annotations. This is possible via the However, the way that I fixed this in 828f74f and 9876701. See the commit messages as well as the associated tests for details. |
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
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 spring-projectsgh-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 spring-projectsgh-29685
Historically, the Spring Framework first had support for repeatable annotations based on convention and later added explicit support for Java 8's @Repeatable facility. Consequently, the support for both types of repeatable annotations has grown a bit intertwined over the years. However, modern Java applications typically make use of @Repeatable, and convention-based repeatable annotations have become more of a niche. The RepeatableContainers API supports both types of repeatable annotations with @Repeatable support being the default. However, RepeatableContainers.of() makes it very easy to enable support for convention-based repeatable annotations while accidentally disabling support for @Repeatable, which can lead to subtle bugs – for example, if convention-based annotations are combined with @Repeatable annotations. In addition, it is not readily clear how to combine @Repeatable support with convention-based repeatable annotations. In light of the above, this commit revises the RepeatableContainers API to better guide developers to use @Repeatable support for almost all use cases while still supporting convention-based repeatable annotations for special use cases. Specifically: - RepeatableContainers.of() is now deprecated in favor of the new RepeatableContainers.explicitRepeatable() method. - RepeatableContainers.and() is now deprecated in favor of the new RepeatableContainers.plus() method which declares the repeatable and container arguments in the same order as the rest of Spring Framework's repeated annotation APIs. For example, instead of the following confusing mixture of repeatable/container and container/repeatable: RepeatableContainers.of(A.class, A.Container.class) .and(B.Container.class, B.class) Developers are now be able to use: RepeatableContainers.explicitRepeatable(A.class, A.Container.class) .plus(B.class, B.Container.class) This commit also overhauls the Javadoc for RepeatableContainers and explicitly points out that the following is the recommended approach to support convention-based repeatable annotations while retaining support for @Repeatable. RepeatableContainers.standardRepeatables() .plus(MyRepeatable1.class, MyContainer1.class) .plus(MyRepeatable2.class, MyContainer2.class) See spring-projectsgh-20279 Closes spring-projectsgh-34637
Historically, the Spring Framework first had support for repeatable annotations based on convention and later added explicit support for Java 8's @Repeatable facility. Consequently, the support for both types of repeatable annotations has grown a bit intertwined over the years. However, modern Java applications typically make use of @Repeatable, and convention-based repeatable annotations have become more of a niche. The RepeatableContainers API supports both types of repeatable annotations with @Repeatable support being the default. However, RepeatableContainers.of() makes it very easy to enable support for convention-based repeatable annotations while accidentally disabling support for @Repeatable, which can lead to subtle bugs – for example, if convention-based annotations are combined with @Repeatable annotations. In addition, it is not readily clear how to combine @Repeatable support with convention-based repeatable annotations. In light of the above, this commit revises the RepeatableContainers API to better guide developers to use @Repeatable support for almost all use cases while still supporting convention-based repeatable annotations for special use cases. Specifically: - RepeatableContainers.of() is now deprecated in favor of the new RepeatableContainers.explicitRepeatable() method. - RepeatableContainers.and() is now deprecated in favor of the new RepeatableContainers.plus() method which declares the repeatable and container arguments in the same order as the rest of Spring Framework's repeated annotation APIs. For example, instead of the following confusing mixture of repeatable/container and container/repeatable: RepeatableContainers.of(A.class, A.Container.class) .and(B.Container.class, B.class) Developers are now be able to use: RepeatableContainers.explicitRepeatable(A.class, A.Container.class) .plus(B.class, B.Container.class) This commit also overhauls the Javadoc for RepeatableContainers and explicitly points out that the following is the recommended approach to support convention-based repeatable annotations while retaining support for @Repeatable. RepeatableContainers.standardRepeatables() .plus(MyRepeatable1.class, MyContainer1.class) .plus(MyRepeatable2.class, MyContainer2.class) See spring-projectsgh-20279 Closes spring-projectsgh-34637
Historically, the Spring Framework first had support for repeatable annotations based on convention and later added explicit support for Java 8's @Repeatable facility. Consequently, the support for both types of repeatable annotations has grown a bit intertwined over the years. However, modern Java applications typically make use of @Repeatable, and convention-based repeatable annotations have become more of a niche. The RepeatableContainers API supports both types of repeatable annotations with @Repeatable support being the default. However, RepeatableContainers.of() makes it very easy to enable support for convention-based repeatable annotations while accidentally disabling support for @Repeatable, which can lead to subtle bugs – for example, if convention-based annotations are combined with @Repeatable annotations. In addition, it is not readily clear how to combine @Repeatable support with convention-based repeatable annotations. In light of the above, this commit revises the RepeatableContainers API to better guide developers to use @Repeatable support for almost all use cases while still supporting convention-based repeatable annotations for special use cases. Specifically: - RepeatableContainers.of() is now deprecated in favor of the new RepeatableContainers.explicitRepeatable() method. - RepeatableContainers.and() is now deprecated in favor of the new RepeatableContainers.plus() method which declares the repeatable and container arguments in the same order as the rest of Spring Framework's repeated annotation APIs. For example, instead of the following confusing mixture of repeatable/container and container/repeatable: RepeatableContainers.of(A.class, A.Container.class) .and(B.Container.class, B.class) Developers are now be able to use: RepeatableContainers.explicitRepeatable(A.class, A.Container.class) .plus(B.class, B.Container.class) This commit also overhauls the Javadoc for RepeatableContainers and explicitly points out that the following is the recommended approach to support convention-based repeatable annotations while retaining support for @Repeatable. RepeatableContainers.standardRepeatables() .plus(MyRepeatable1.class, MyContainer1.class) .plus(MyRepeatable2.class, MyContainer2.class) See gh-20279 Closes gh-34637
Liam Bryan opened SPR-15723 and commented
Meta annotations are not detected when present on
Repeatable
annotations.Provided zip contains a quick example highlighting this, but for a contained example here (
@Retention
and@Target
annotations omitted):None of the provided methods in
AnnotationUtils
orAnnotatedElementUtils
can locate the meta-@A
annotations for an element with repeated@B
annotations.Affects: 4.3.8
Attachments:
The text was updated successfully, but these errors were encountered: