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

Restore ability to override bean definition names in DefaultListableBeanFactory subclass [SPR-12667] #17266

Closed
spring-projects-issues opened this issue Jan 27, 2015 · 2 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 27, 2015

Martin Lippert opened SPR-12667 and commented

There are changes made to DefaultListableBeanFactory that can cause subclasses to break:

965bea7

The problem is that inside method "doGetBeanNames..." the access of the bean definition names is changed from calling "getBeanDefinitionNames" to the direct field access. This breaks subclasses that have overridden "getBeanDefinitonNames" in order to do special things.

This is the case in Spring IDE, where there is a subclass of DefaultListableBeanFactory, which doesn't track the bean definition names itself, but delegates that to a BeanDefinitionRegistry object. This delegation is implemented for the BeanDefinitionRegistry methods in DefaultListableBeanFactory. I am not sure whether this is the optimal way to do this or if there is a more elegant way, but I tried to avoid to implement the methods of ListableBeanFactory by re-using them from the DefaultListableBeanFactory. That works if the implementation of DefaultListableBeanFactory is consequently calling the methods of BeanDefinitionRegistry instead of using fields directly. But I am open to other ideas, of course.


Affects: 4.1.2, 4.1.3, 4.1.4

Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

There is actually a good reason for direct field access there: We iterate over the bean definition names collection directly, instead of converting it to a String array for every single iteration attempt. Since these are quite critical paths, such an optimization is very much worthwhile. This was initially driven by Boot performance tests but applies quite generally.

Which leaves the question of how to deal with the BeanDefinitionRegistry delegation scenario. We could introduce dedicated protected hooks for such purposes, based on collections instead of arrays... Any proposal for a collection-based solution which does the job for Spring IDE? Is it just about retrieving bean definition names or is there further delegation?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Martin Lippert commented

I can see the reason to optimize this using the collection of the bean definition names directly. Hmmm...

Putting in dedicated hooks (based on collections) sounds like a possible solution for the delegation scenario, but it also sounds like a lot of extra code just to make this extremely special use case possible. Therefore I don't like this very much either.

My scenario is: I have a BeanDefinitionRegistry object that contains all the necessary bean definition names, etc. This is not implementing ListableBeanFactory. In a slightly different scenario, I need to put in a ListableBeanFactory object that is backed by the BeanDefinitionRegistry that I have. Therefore I chose to inherit from DefalutListableBeanFactory and override all methods from BeanDefinitionRegistry by delegating to the separate BeanDefinitionRegistry object. This works only if DefaultListableBeanFactory accesses information from the BeanDefinitionRegistry by calling those methods on its own instead of accessing any field directly. This general approach would work, but it is not a clean and good design, IMHO. Therefore I am hesitant to put in extra hooks to allow this ugly design to continue to work (and make it even uglier).

The other solution that I have in mind is to re-implement ListableBeanFactory myself, basically copying the code from DefaultListableBeanFactory, and use the BeanDefinitonRegistry object directly in there, but that would result in a lot of duplicated code that I would have to maintain with every new release of the Spring core framework.

Any other idea?

@spring-projects-issues spring-projects-issues added type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 11, 2019
@jhoeller jhoeller added the status: declined A suggestion or change that we don't feel we should currently apply label Jul 13, 2023
@jhoeller jhoeller removed this from the General Backlog milestone Jul 13, 2023
@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Jul 13, 2023
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: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants