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

AbstractHandlerMethodMapping should allow for customized bean retrieval [SPR-15535] #20094

Closed
spring-projects-issues opened this issue May 11, 2017 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: task A general task
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented May 11, 2017

Petar Tahchiev opened SPR-15535 and commented

Hello,

looking at: AbstractHandlerMethodMapping#initHandlerMethods here's what we see:

		String[] beanNames = (this.detectHandlerMethodsInAncestorContexts ?
				BeanFactoryUtils.beanNamesForTypeIncludingAncestors(getApplicationContext(), Object.class) :
				getApplicationContext().getBeanNamesForType(Object.class));

		for (String beanName : beanNames) {

All the beans of type java.lang.Object are taken, then we iterate over their names and for each one you call:

beanType = getApplicationContext().getType(beanName);

and then:

if (beanType != null && isHandler(beanType)) {
         detectHandlerMethods(beanName);
}

and this last call inspects the bean if it has certain annotation or fulfills certain criteria (which might be an expensive operation when you call it for each bean and the user has 5000 beans for instance).

In my application I have several handler mappings that extend from AbstractHandlerMethodMapping - one comes from Spring Webmvc(RequestMappingInfoHandlerMapping), the other one from spring-data(BasePathAwareHandlerMapping), one I wrote for myself, and several others from spring-boot(EndpointHandlerMapping, CloudFoundryEndpointHandlerMapping).

I would like to propose instead of iterating over the all the java.lang.Object beans, then getting the class for each bean, then using reflection to find out if this bean fits the criteria to be a handler method (has annotation or whatsoever), why not delegate the this task to the HandlerMapping itself. This way the AbstractHandlerMethodMapping can have an abstract method like this:

List<String> getBeanNamesEligibleForHandling(ApplicationContext applicationContext);

and the SpringWebMVC RequestMappingInfoHandlerMapping will provide the following implementation:

:
 List<String> getBeanNamesEligibleForHandling(ApplicationContext applicationContext) {

      List<String> result = new ArrayList<>();
      result.addAll(applicationContext.getBeansAnnotatedWith(Controller.class)));
      result.addAll(applicationContext.getBeansAnnotatedWith(RequestMapping.class)));
      return result;
}

and the others too can find only the beans they are interested in.
This would of course require a new method called getBeansAnnotatedWith, but I think it is really worth it as this would increase the performance.

One more thing we can do is give the responsibility of finding the correct beanType to the delegating handler mapping. Right now they all extend the AbstractMethodHandlerMapping which does the following:

beanType = getApplicationContext().getType(beanName);

However, I want it to do the following:

beanType = AopProxyUtils.ultimateTargetClass(getApplicationContext().getBean(beanName))

so now I have to override the whole initHandlerMethods to achieve it.

Thank you and keep up the good work :)


Affects: 4.3.8

Issue Links:

Referenced from: commits fc16b2d

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Oct 31, 2017

Petar Tahchiev commented

Any reason why this was not included in the 5.0 or 5.0.1 release? It seems quite straight-forward

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 1, 2018

Petar Tahchiev commented

Juergen Hoeller Is there anything I can do to have this implemented faster :) I really think it would be a big performance improvement :)

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 9, 2018

Juergen Hoeller commented

The main problem here is that the bean factory itself wouldn't necessarily be faster in finding all beans with the specified annotations. It would iterate over all the non-null beans itself (effectively using getBeanNamesForType(Object.class) as well) and check for the presence of those annotations. Unless we performed some upfront annotation scanning with pre-resolved annotation types in the factory, I don't see much benefit here.

That said, we can certainly open up AbstractHandlerMethodMapping to allow for customized retrieval...

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 10, 2018

Juergen Hoeller commented

I've refactored AbstractHandlerMethodMapping to provide getCandidateBeanNames and processCandidateBean template methods as delegates from initHandlerMethods. This should allow for easier customization at least, filtering the initial bean names and/or determining the type in a different way. Note that you can pass a fully resolved bean instance to detectHandlerMethods already, so if an overridden processCandidateBean checks the type against a getBean result anyway, you may pass that resulting instance straight on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: task A general task
Projects
None yet
Development

No branches or pull requests

2 participants