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

@ControllerAdvice not detected from parent contexts (even though @Controller is) [SPR-11570] #16194

Closed
spring-projects-issues opened this issue Mar 18, 2014 · 2 comments
Assignees
Labels
in: web type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 18, 2014

stripybadger opened SPR-11570 and commented

I use ContextLoaderListener to load most of my spring stuff (lets call this context P), then have a DispatcherServlet. Each DispatcherServlet creates its own application context (lets call this D) which gets parented by P.

If I have beans in P which are controllers (annotated with @Controller), they still get exposed by the DispatcherServlet. However any ControllerAdvice beans in P do not get picked up.

I think this inconsistency is a bug. Particularly as many users will want to be able to share controller advice beans between DispatcherServlet (e.g. it may be useful to share exception handling across all servlets).

See AbstractDetectingUrlHandlerMapping:74 - calls BeanFactoryUtils.beanNamesForTypeIncludingAncestor
ControllerAdviceBean:92 - calls applicationContext.getBeanDefinitionNames() (which only appears to get beans from the current context, not parents)


Affects: 3.2.3

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 18, 2014

Juergen Hoeller commented

Actually, AbstractDetectingUrlHandlerMapping as well as AbstractHandlerMethodMapping do not detect controllers in parent contexts by default; both have a "detect in ancestor contexts" flag that needs to be flipped to "true" first. So the non-detection of ControllerAdvice beans in a parent context by isn't an inconsistency in default behavior; it's rather a limitation that can't be overcome through a flag at this point.

I does make sense to share ControllerAdvice beans in ancestor contexts, and it seems like we can do this in a backwards-compatible fashion since ControllerAdvice beans in parent contexts would be 'dead' objects currently, so starting to discover them by default seems like a plus in all cases. This is different from controllers which would lead to duplicate mappings in the case of multiple DispatcherServlets picking them up from the root context. With ControllerAdvice beans, they'd simply be shared across all DispatcherServlets, which seems like a clear and non-conflicting intention that doesn't even have to be explicitly activated through a flag.

Rossen Stoyanchev, what's your take on this, in particular on the default detection of ControllerAdvice beans in ancestor contexts? Do we even need a flag there, or should we simply always detect them in ancestors as I suggested above?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 18, 2014

Rossen Stoyanchev commented

Yes it makes sense to share ControllerAdvice beans that way. There is no need for a special setting and it would be difficult in any case to control things that way. Also keep in mind that in 4.0 we provided fine-grained control over which controllers a ControllerAdvice bean applies to (via annotation attributes), which should provide sufficient control whatever the scenario may be.

@spring-projects-issues spring-projects-issues added type: enhancement in: web labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 4.0.3 milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants