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

Add ConditionalOnBean support for generic bean argument types #29500

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

VladislavSevruk
Copy link

@VladislavSevruk VladislavSevruk commented Jan 20, 2022

Add generic bean support using @ConditionalOnBean and @ConditionalOnMissingBean annotations as described in #28845. Added two method to annotations: typeArguments and typeArgumentNames for classes and class names respectively.

Usage example:

// loaded via spring factories
public class SomeAutoConfiguration {

    @ConditionalOnMissingBean(typeArguments = { String.class, Integer.class })
    @Bean
    public Converter<String, Integer> converter() {
         // ...
     }
}

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 20, 2022
@philwebb philwebb added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 21, 2022
@philwebb philwebb self-assigned this Feb 2, 2022
@philwebb
Copy link
Member

philwebb commented Feb 3, 2022

Thanks very much for the pull-request @VladislavSevruk. We discussed this today on our team call and we'd like to try and remove the typeArguments and typeArgumentNames attributes and instead detect the generics from the return type.

If you're interested in updating your pull-request in that direction, that would be great but please be aware that we're currently focusing on some Spring Boot 3.0 issues and this one is in the "general backlog" so we might not be able to review and merge things all that quickly.

No worries if you don't want to spend any additional time on this. We'll leave this issue open and mark it for merge-with-amendments.

@philwebb philwebb added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Feb 3, 2022
@philwebb philwebb added this to the General Backlog milestone Feb 3, 2022
@philwebb philwebb added the for: merge-with-amendments Needs some changes when we merge label Feb 3, 2022
@VladislavSevruk
Copy link
Author

Thank you for feedback @philwebb. I've updated code to auto-detect type arguments for generics from method return type.
But I also left typeArguments and typeArgumentNames attributes at annotations for cases when user uses type or value attributes.
Please let me know if I need to remove annotation attributes anyway.

@snicoll
Copy link
Member

snicoll commented Feb 3, 2022

@VladislavSevruk yes please. We really want to support that mode only for the return type.

@VladislavSevruk
Copy link
Author

@philwebb @snicoll the typeArguments and typeArgumentNames attributes were removed

@snicoll
Copy link
Member

snicoll commented Feb 4, 2022

Thanks a lot for the follow-up @VladislavSevruk. When merging this, we probably want to restore the existing tests using ExampleBean and create a separate test class with a generic as moving everything to generic doesn't seem right IMO.

@VladislavSevruk
Copy link
Author

@snicoll tests with ExampleBean are reverted to initial state, tests for generic return type are extracted to separate classes

@mitasov-ra
Copy link

Any updates on when this PR will be merged?

Will it affect older versions of Spring Boot (pre 3.x) or is it just for 3.x?

@wilkinsona
Copy link
Member

@mitasov-ra The issue remains in the general backlog so things haven't changed since this comment. We consider the change to be an enhancement so it will only be made in a new minor (or major) release and won't affect older versions of Boot.

@Bennett-Lynch
Copy link

Is this possible at all today? E.g., taking the example:

@ConditionalOnMissingBean(typeArguments = { String.class, Integer.class })
@Bean
public Converter<String, Integer> converter() {
    // ...
}

Is it possible to achieve this with the current parameterizedContainer behavior? E.g.,

@ConditionalOnMissingBean(value = { String.class, Integer.class }, parameterizedContainer = { Converter.class })
@Bean
public Converter<String, Integer> converter() {
    // ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants