Regression: ControllerAdviceBean#getOrder() causes BeanCreationException for request scoped advice beans #23985
Conversation
As of Spring Framework 5.2, the way of define order value is changed. 9239ab1#diff-e6efe5c07f3b7003d9229a7140948cebL120 9239ab1#diff-e6efe5c07f3b7003d9229a7140948cebL243-L249 But This change breaking backward compatibility, 9239ab1#diff-e6efe5c07f3b7003d9229a7140948cebR136-R147 In this method, order will be decided by Spring component instance primary, But before Spring Framework 5.2.0, order will be decided from bean type if beanType is not null. In Spring Framework 5.2, 1. org.springframework.web.method.ControllerAdviceBean#getOrder is called in spring initialize phase (Thread of initialize phase is request or session scope) 2. thread of initialize get order of component with trying initiate component via BeanFactory 3. BeanFactory throws Exception because thread of (2) is different from @scope annotated component(ex. scope is like request, session...etc)
yokotaso
changed the title
|
@yokotaso Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
spring-projects-issues
added
the
status: waiting-for-triage
rstoyanchev
added
in: web
|
@sbrannen I've scheduled this as it does look like a regression. As for the solution, maybe we should avoid resolving the bean unless we know it is |
rstoyanchev
changed the title
Thanks
Perhaps. I'll have to investigate. |
|
At a quick glance, it would appear that our assumption in #23163 (comment) was incorrect. |
sbrannen
changed the title
|
I am able to reproduce this exception with the following simplified test case. class ControllerAdviceBeanIntegrationTests {
@Test
void loadContextWithRequestScopedControllerAdviceBean() {
AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
context.setServletContext(new MockServletContext());
context.register(Config.class);
context.refresh();
context.close();
}
@ControllerAdvice
static class RequestScopedControllerAdvice implements Ordered {
@Override
public int getOrder() {
return 45;
}
}
@Configuration
@EnableWebMvc
static class Config {
@Bean
@RequestScope
RequestScopedControllerAdvice requestScopedControllerAdvice() {
return new RequestScopedControllerAdvice();
}
}
} |
sbrannen
changed the title
|
@yokotaso Thank you for signing the Contributor License Agreement! |
That's not a valid option, since In light of that, I am going to introduce a check that avoids eager |
|
@sbrannen I realized your thought. Thank you for fix problem 👍 |
The test introduced in this commit is intended to serve as a regression test for the status quo. See gh-23985
As of Spring Framework 5.2, the way of define order value is changed.
9239ab1#diff-e6efe5c07f3b7003d9229a7140948cebL120
9239ab1#diff-e6efe5c07f3b7003d9229a7140948cebL243-L249
But This change breaks backward compatibility,
9239ab1#diff-e6efe5c07f3b7003d9229a7140948cebR136-R147
In this method, order will be decided by Spring component instance primary,
But before Spring Framework 5.2.0, order will be decided from bean type if beanType is not null.
In Spring Framework 5.2,
(Thread of initialize phase is request or session scope)
@Scopeannotated component(ex. scope is like request, session...etc)If I integrate my applicaiton with Spring Boot 2.2.1, Application won't be failed to start.
So It is good, try to decide order by beanType primarily, I think.