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

MergedAnnotations search does not find container for repeatable annotation #32731

Closed
heihei180 opened this issue Apr 30, 2024 · 10 comments
Closed
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

@heihei180
Copy link

when i use applicationContext.getBeansWithAnnotation(Extensions.class);
in spring framework 5.3.20 , i can got a bean,
in spring framework 5.3.28, i can't got a bean.


definition of Extensions:

@Inherited
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE})
@Component
public @interface Extensions {

    String[] bizId() default BizScenario.DEFAULT_BIZ_ID;

    String[] useCase() default BizScenario.DEFAULT_USE_CASE;

    String[] scenario() default BizScenario.DEFAULT_SCENARIO;

    Extension[] value() default {};

}

Definition of variable "applicationContext":

@Component
public class ExtensionBootstrap implements ApplicationContextAware {
    private ApplicationContext applicationContext;

    @Override
    public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
        this.applicationContext = applicationContext;
    }
}

Can someone tell me what changes have occurred here?
I think this should be a problem, but I couldn't find where the problem occurred.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 30, 2024
@snicoll snicoll changed the title [BUG] getBeanNamesForAnnotation obtained a different result getBeanNamesForAnnotation no longer return match for composed annotation Apr 30, 2024
@snicoll
Copy link
Member

snicoll commented Apr 30, 2024

@heihei180 I am not sure what could have changed this. The code in text you've shared isn't actionable unfortunately, please share a small sample we can run ourselves that reproduces this behavior. You can attach a zip here or push the code to a GitHub repository.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Apr 30, 2024
@heihei180
Copy link
Author

@snicoll Thank you for your reply~

I will upload a zip file that contains two folders, demo1 and demo2, both of which are springboot programs, but the versions are different.

  • the spring boot version for demo1 is 2.6.8, the spring framework version used is 5.3.20,
  • the spring boot version for demo2 is 2.7.13, and the spring framework version used is 5.3.28
    Additionally, you can run unit tests for two projects, and demo1 (5.3.20) will run successfully,
    Demo2 (5.3.28) will fail to run!
    testSpring.zip

Looking forward to your reply! Thank you again.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 30, 2024
@snicoll snicoll assigned snicoll and unassigned snicoll May 1, 2024
@snicoll
Copy link
Member

snicoll commented May 1, 2024

Thanks for the sample. I can reproduce, including with the latest release and I don't know if that's intended.

Relying on the container annotation for this looks a bit odd to me though. If you use Extension it works obviously and I would expect you to use that, rather than the @Repeatable container. I've asked @sbrannen if that rings a bell.

@snicoll snicoll added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label May 1, 2024
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label May 2, 2024
@sbrannen
Copy link
Member

sbrannen commented May 2, 2024

Thanks for the demo projects, @heihei180. 👍

I've reduced this to the following test.

@Test
void test() {
	Extensions extensions = FooService.class.getAnnotation(Extensions.class);
	assertThat(extensions).as("@Extensions").isNotNull();

	extensions = MergedAnnotations.from(FooService.class, SearchStrategy.TYPE_HIERARCHY)
			.get(Extensions.class)
			.synthesize(MergedAnnotation::isPresent)
			.orElse(null);
	assertThat(extensions).as("@Extensions").isNotNull();
}

java.lang.Class.getAnnotation(Class<Extensions>) of course finds @Extensions on FooService, but Spring's MergedAnnotations support no longer does.

I consider that a regression, and I'll investigate further.

@sbrannen sbrannen added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels May 2, 2024
@sbrannen sbrannen self-assigned this May 2, 2024
@sbrannen sbrannen added this to the 6.1.7 milestone May 2, 2024
@sbrannen
Copy link
Member

sbrannen commented May 2, 2024

The above test actually fails on 5.3.20 as well if you remove all attributes from Extensions.class except the value attribute.

This appears to be related to #29685, and I assume this bug may have existed since Spring Framework 5.2 when the MergedAnnotations infrastructure was introduced.

@sbrannen sbrannen changed the title getBeanNamesForAnnotation no longer return match for composed annotation MergedAnnotations search does not find container for repeatable annotation May 2, 2024
@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 May 2, 2024
@sbrannen sbrannen removed the type: bug A general bug label May 2, 2024
@sbrannen
Copy link
Member

sbrannen commented May 2, 2024

This issue turned out be an interesting one... and a combination of a few things.

The above test actually fails on 5.3.20 as well if you remove all attributes from Extensions.class except the value attribute.

That's due to a bug in the MergedAnnotations support that has existed since it was introduced in 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 the repeated annotations but not for the container itself.

The reason is that MergedAnnotationFinder.process(Object, int, Object, Annotation) does not process the container annotation and instead only processes the "contained" annotations.

This appears to be related to #29685, and I assume this bug may have existed since Spring Framework 5.2 when the MergedAnnotations infrastructure was introduced.

That's indeed the case: #29685 allowed the MergedAnnotations infrastructure to recognize an annotation as a container even when the annotation declares attributes other than value.

In other words, since Spring Framework 5.3.25, MergedAnnotations considers the @Extensions annotation a container, and due to the aforementioned bug the container is no longer processed, which results in a regression in behavior for use cases like the one in the supplied demo projects.

@heihei180

This comment was marked as resolved.

@snicoll

This comment was marked as resolved.

@heihei180

This comment was marked as resolved.

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)
@sbrannen
Copy link
Member

sbrannen commented May 3, 2024

This has been addressed in 4baad16 which will be included in Spring Framework 6.1.7, 6.0.20, and 5.3.35.

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

4 participants