Skip to content

Enhance conditional bean creation when using method overloading #28019

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

Closed
wants to merge 1 commit into from
Closed

Enhance conditional bean creation when using method overloading #28019

wants to merge 1 commit into from

Conversation

Hakky54
Copy link

@Hakky54 Hakky54 commented Feb 8, 2022

This pull request closes the issue mentioned here: #22609

Short summary:
A bean won't be created when it is being skipped by a condition for example when using @ConditionalOnProperty or @ConditionalOnExpression because the condition is evaluated to false. However a second bean with exactly the same name with different parameters may evaluate to true because the condition has met, so this bean should be created but that does not happen.

An example code snippet:

class Config {

    @Bean
    @ConditionalOnExpression("${some-property-enabled} == true and ${some-different-property} == false")
    public ExampleBean baz() {
        return new ExampleBean();
    }

    @Bean
    @ConditionalOnExpression("${some-property-enabled} == false and ${some-different-property} == true")
    public ExampleBean baz(Object foo) {
        return new ExampleBean();
    }

}

When some-property-enabled is false and some-different-property is true the second method should be called and the bean should be created, however this does not happen. The bean name is already in the to be skipped beans list.

The pull request contains the changes to reconsider a skipped bean when it encounters a new bean definition having a different condition. If it should not be skipped it will be created. The PR also takes into inheritance into consideration (for backwards compatibility, see

)

In that specific use case the parent class defines a bean which should be created, however it is getting overridden by a subclass which defines a condition on the bean, so it will be created conditionally and may not even be created when it evaluates to false.

This PR uses a wrapper for the MethodMetadata as a helper class for resolving the inheritance with the condition on a subclass when it still needs to be skipped.

My use case

In my use case I wanted to refactor my code base. I have the bean configuration below:

github/mutual-tls-ssl/SSLConfig.java

@Component
public class SSLConfig {

    @Bean
    @Scope("prototype")
    public SSLFactory sslFactory(
            @Value("${client.ssl.one-way-authentication-enabled:false}") boolean oneWayAuthenticationEnabled,
            @Value("${client.ssl.two-way-authentication-enabled:false}") boolean twoWayAuthenticationEnabled,
            @Value("${client.ssl.key-store:}") String keyStorePath,
            @Value("${client.ssl.key-store-password:}") char[] keyStorePassword,
            @Value("${client.ssl.trust-store:}") String trustStorePath,
            @Value("${client.ssl.trust-store-password:}") char[] trustStorePassword) {
        SSLFactory sslFactory = null;

        if (oneWayAuthenticationEnabled) {
            sslFactory = SSLFactory.builder()
                    .withTrustMaterial(trustStorePath, trustStorePassword)
                    .withProtocols("TLSv1.3")
                    .build();
        }

        if (twoWayAuthenticationEnabled) {
            sslFactory = SSLFactory.builder()
                    .withIdentityMaterial(keyStorePath, keyStorePassword)
                    .withTrustMaterial(trustStorePath, trustStorePassword)
                    .withProtocols("TLSv1.3")
                    .build();
        }

        return sslFactory;
    }

}

And I wanted to refactor it to the following snippet:

@Component
public class SSLConfig {

    @Bean
    @Scope("prototype")
    @ConditionalOnExpression("${client.ssl.one-way-authentication-enabled} == true and ${client.ssl.two-way-authentication-enabled} == false")
    public SSLFactory sslFactory(@Value("${client.ssl.trust-store:}") String trustStorePath,
                                 @Value("${client.ssl.trust-store-password:}") char[] trustStorePassword) {

        return SSLFactory.builder()
                .withTrustMaterial(trustStorePath, trustStorePassword)
                .withProtocols("TLSv1.3")
                .build();
    }


    @Bean
    @Scope("prototype")
    @ConditionalOnExpression("${client.ssl.two-way-authentication-enabled} == true and ${client.ssl.one-way-authentication-enabled} == false")
    public SSLFactory sslFactory(@Value("${client.ssl.key-store:}") String keyStorePath,
                                 @Value("${client.ssl.key-store-password:}") char[] keyStorePassword,
                                 @Value("${client.ssl.trust-store:}") String trustStorePath,
                                 @Value("${client.ssl.trust-store-password:}") char[] trustStorePassword) {

        return SSLFactory.builder()
                .withIdentityMaterial(keyStorePath, keyStorePassword)
                .withTrustMaterial(trustStorePath, trustStorePassword)
                .withProtocols("TLSv1.3")
                .build();
    }

}

However it fails. But with the code changes of the Pull request it will create one of the beans when the conditions is true.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 8, 2022
@rstoyanchev rstoyanchev added this to the Triage Queue milestone Feb 15, 2022
@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 15, 2022
@jhoeller
Copy link
Contributor

Thanks for the PR and the use cases raised, however, we intend to proceed in a different direction and disallow @Bean method overloading completely. See #22609 (comment) for an extensive explanation.

@jhoeller jhoeller closed this Feb 15, 2022
@jhoeller jhoeller removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 15, 2022
@jhoeller jhoeller removed this from the Triage Queue milestone Feb 15, 2022
@Hakky54 Hakky54 deleted the feature/enhance-conditional-bean-creation-for-method-overloading branch February 15, 2022 14:50
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants