spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
55.27k stars 37.62k forks source link

`MergedAnnotations` search does not find container for repeatable annotation #32731

Closed heihei180 closed 1 week ago

heihei180 commented 2 weeks ago

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.

snicoll commented 2 weeks ago

@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.

heihei180 commented 2 weeks ago

@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.

Looking forward to your reply! Thank you again.

snicoll commented 2 weeks ago

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.

sbrannen commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

Thank you for your reply, @sbrannen , I understand the cause of the problem and will try to add a new solutions to temporarily solve the problem. But I would like to confirm, will the Spring team support in the future?

snicoll commented 1 week ago

I would like to confirm, will the Spring team support in the future?

Perhaps you're looking at this issue on your phone? On a desktop you can see on the right hand side that it's flagged as a regression, scheduled for 6.1.7 and for backports. If you follow the link above you can see backports for 6.0.x and 5.3.x are scheduled. So, yes, the plan is to address this issue.

heihei180 commented 1 week ago

Yes,i read this issue on my phone and thank you for your reply!

------------------ Original ------------------ From: Stéphane Nicoll @.> Date: Fri,May 3,2024 1:47 PM To: spring-projects/spring-framework @.> Cc: heihei180 @.>, Mention @.> Subject: Re: [spring-projects/spring-framework] MergedAnnotations searchdoes not find container for repeatable annotation (Issue #32731)

sbrannen commented 1 week ago

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