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

Allow usage of BeanNameGenerator in ImportBeanDefinitionRegistrars #22591

Closed
odrotbohm opened this issue Mar 14, 2019 · 8 comments
Closed

Allow usage of BeanNameGenerator in ImportBeanDefinitionRegistrars #22591

odrotbohm opened this issue Mar 14, 2019 · 8 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@odrotbohm
Copy link
Member

odrotbohm commented Mar 14, 2019

A BeanNameGenerator can be configured for usage in component scanning configuration classes. That support allows custom annotation based activation of functionality via annotations importing an ImportBeanDefinitionRegistrar. Its primary callback method unfortunately currently doesn't provide access to the BeanNameGenerator potentially provided so that a customization of that generator cannot be considered for components registered via the registrar.

It would be nice, if there was a way to consume the BeanNameGenerator to allow components being registered to follow the configured naming strategy.

@odrotbohm odrotbohm assigned odrotbohm and jhoeller and unassigned odrotbohm Mar 14, 2019
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 14, 2019
@jhoeller jhoeller changed the title Allow usage of BeanNamGenerator in ImportBeanDefinitionRegistars Allow usage of BeanNameGenerator in ImportBeanDefinitionRegistrars Mar 14, 2019
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 14, 2019
@jhoeller jhoeller added this to the 5.2 M1 milestone Mar 14, 2019
odrotbohm added a commit to spring-projects/spring-data-commons that referenced this issue Mar 14, 2019
…eanNameGenerator.

We now implement the newly introduced ImportBeanDefinitionRegistrar overload additional provided with a BeanNameGenerator configured for the import scanning. We forward this into our bean name generation infrastructure as fallback.

Related tickets: spring-projects/spring-framework#22591
odrotbohm added a commit to spring-projects/spring-data-commons that referenced this issue Mar 14, 2019
…eanNameGenerator.

We now implement the newly introduced ImportBeanDefinitionRegistrar overload additional provided with a BeanNameGenerator configured for the import scanning. We forward this into our bean name generation infrastructure as fallback.

Related tickets: spring-projects/spring-framework#22591
@odrotbohm
Copy link
Member Author

It looks like we're getting the generator that's used for the configuration class scanning, not the one used to name scanned components. I've put in check for the enclosing class of the provided instance to only opt in using it if it's not ConfigurationClassPostProcessor. Is there maybe a chance we can expose the default one used in a slightly more convenient way (a constant in ConfigurationClassPostProcessor maybe)?

@sbrannen
Copy link
Member

Reopening to consider comments from @odrotbohm.

@sbrannen sbrannen reopened this Mar 17, 2019
@jhoeller
Copy link
Contributor

jhoeller commented Apr 2, 2019

@odrotbohm So you'd simply like to be able to ignore the default import bean name generator more easily, e.g. by comparing it to a constant on ConfigurationClassPostProcessor, proceeding with an externally configured BeanNameGenerator only?

@odrotbohm
Copy link
Member Author

Yes. Fundamentally, we're scanning for components ourselves. I.e. for us, the bean name generator used to name scanned components would be the more appropriate one. It looks like the user override is trumping both default variants Spring Framework uses internally.

If that stays the same, I'm fine with just using the one handed down, if it's not the default one. To detect that, something to easily compare the handed down instance to would allow us to remove the rather brittle check of inspecting the surrounding class.

@jhoeller
Copy link
Contributor

jhoeller commented Apr 2, 2019

Alright, I've added an IMPORT_BEAN_NAME_GENERATOR constant on ConfigurationClassPostProcessor, along with a common AnnotationBeanNameGenerator.INSTANCE that serves as the default for component scanning purposes.

odrotbohm added a commit to spring-projects/spring-data-commons that referenced this issue Apr 3, 2019
…eanNameGenerator.

We now implement the newly introduced ImportBeanDefinitionRegistrar overload additional provided with a BeanNameGenerator configured for the import scanning. We forward this into our bean name generation infrastructure as fallback.

Related tickets: spring-projects/spring-framework#22591
@odrotbohm
Copy link
Member Author

I hate to bring this up again but we face the same issue with the XML flavor as the BeanDefinitionReader exposes an instance of DefaultBeanNameGenerator. If that in use, we have to fall back to an AnnotationBeanNameGenerator. I fall back to our default value if the handed in instance is a exactly DefaultBeanNameGenerator assuming that all customizations would have to either provide a subtype or a completely different implementation of BeanNameGenerator.

However, ultimately we're facing the same slightly brittle situation of detecting the default.

odrotbohm added a commit to spring-projects/spring-data-commons that referenced this issue Apr 3, 2019
RepositoryBeanDefinitionRegistrarSupport now also overrides registerBeanDefinitions(AnnotationMetadata, BeanDefinitionRegistry) so that unit test in downstream modules still work.

Added defaulting of BeanNameGenerator in XmlRepositoryConfigurationSource so that we use an AnnotationBeanNameGenerator if a DefaultBeanNameGenerator is handed in.

Related tickets: spring-projects/spring-framework#22591
@jhoeller
Copy link
Contributor

jhoeller commented Apr 3, 2019

Hmm, in line with the common instance constants, DefaultBeanNameGenerator should have one too... So if we introduce an INSTANCE constant there as well, used for defaulting like we do for AnnotationBeanNameGenerator already, would that be sufficient for your purposes?

@jhoeller jhoeller reopened this Apr 3, 2019
@odrotbohm
Copy link
Member Author

Yes, I think so.

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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants