Skip to content
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.findMergedRepeatableAnnotations does not fetch results when other attributes exist in container annotation #29685

Closed
ForteScarlet opened this issue Dec 13, 2022 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Milestone

Comments

@ForteScarlet
Copy link

Affects: v5.3.24, v6.0.2


I have the following code:

public class Main2 {

    @Retention(RetentionPolicy.RUNTIME)
    @Repeatable(Anno1Container.class)
    public @interface Anno1 {
        String value();
    }

    @Retention(RetentionPolicy.RUNTIME)
    public @interface Anno1Container {
        Anno1[] value();
    }

    @Retention(RetentionPolicy.RUNTIME)
    @Repeatable(Anno2Container.class)
    public @interface Anno2 {
        String value();
    }

    @Retention(RetentionPolicy.RUNTIME)
    public @interface Anno2Container {
        Anno2[] value();
        int number() default 2; // other attributes
    }

    public static void main(String[] args) throws NoSuchMethodException {
        Method method = Main2.class.getMethod("a");
        System.out.println(AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Anno1.class));
        // [@example.Main2$Anno1("A1_1"), @example.Main2$Anno1("A1_2")]
        System.out.println(AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Anno2.class));
        // []
    }


    @Anno1("A1_1")
    @Anno1("A1_2")
    @Anno2("A2_1")
    @Anno2("A2_2")
    public void a() {
    }
}

AnnotatedElementUtils.findMergedRepeatableAnnotations cannot get results when other attributes ( Anno2Container.number ) exist for the container annotation ( Anno2Container )

In v5.3.23, AnnotatedElementUtils.findMergedRepeatableAnnotations can successfully obtain the above two annotations

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 13, 2022
@sbrannen sbrannen self-assigned this Dec 13, 2022
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Dec 13, 2022
@sbrannen
Copy link
Member

This is potentially a regression caused by #20279.

I'll investigate it.

@sbrannen
Copy link
Member

There is a bug in the logic for RepeatableContainers.StandardRepeatableContainers.computeRepeatedAnnotationsMethod(...) which has existed since Spring Framework 5.2 (when StandardRepeatableContainers was introduced); however, the changes made in conjunction with #20279 made this bug apparent when using AnnotatedElementUtils.findMergedRepeatableAnnotations.

Thus, this is simultaneously a bug and a regression.

@sbrannen sbrannen added type: bug A general bug type: regression A bug that is also a regression for: backport-to-5.3.x and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 13, 2022
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.3.x labels Dec 13, 2022
@sbrannen sbrannen added this to the 6.0.3 milestone Dec 13, 2022
@sbrannen
Copy link
Member

Anyone encountering this issue in Spring Framework 5.3.24/6.0.2/6.0.3 can work around the bug by using the findMergedRepeatableAnnotations() variant that accepts the container type.

For example, the following allows the provided sample to properly find the repeatable annotations.

AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Anno2.class, Anno2Container.class)

@ForteScarlet
Copy link
Author

Anyone encountering this issue in Spring Framework 5.3.24/6.0.2/6.0.3 can work around the bug by using the findMergedRepeatableAnnotations() variant that accepts the container type.

For example, the following allows the provided sample to properly find the repeatable annotations.

AnnotatedElementUtils.findMergedRepeatableAnnotations(method, Anno2.class, Anno2Container.class)

Thanks, i will try.

sbrannen added a commit to sbrannen/spring-framework that referenced this issue Dec 13, 2022
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.

See spring-projectsgh-29685
Closes spring-projectsgh-29686
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Dec 13, 2022
@sbrannen
Copy link
Member

The fix for this will be available in Spring Framework 5.3.25 and 6.0.3 (as well as the upcoming snapshots for those releases).

@sbrannen sbrannen changed the title AnnotatedElementUtils.findMergedRepeatableAnnotations does not fetch results when other attributes exist for container annotation AnnotatedElementUtils.findMergedRepeatableAnnotations does not fetch results when other attributes exist in container annotation Dec 14, 2022
@ghillert
Copy link

I think this broke a little "hack" I was employing for Coherence Spring to work around the lack of @NonBinding annotation support in Spring 🙂. Over two years I added a related ticket for that: #26302

sbrannen added a commit that referenced this issue May 3, 2024
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
sbrannen added a commit that referenced this issue May 3, 2024
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 4baad16)
sbrannen added a commit that referenced this issue May 3, 2024
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 4baad16)
@snicoll snicoll removed the type: bug A general bug label May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants