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

Condition evaluation report entry for a @ConditionalOnSingleCandidate that does not match due to multiple primary beans isn't as clear as it could be #30073

Closed
tombyong opened this issue Mar 5, 2022 · 8 comments
Assignees
Labels
type: bug
Milestone

Comments

@tombyong
Copy link

@tombyong tombyong commented Mar 5, 2022

In the event there is multiple @primary DataSource but one with proper 'dataSource' naming, no EntityManagerFactory bean will be created at all.

This can only be seen when the logging info is set at TRACE level, to find out how many dataSource beans created, and causing this AutoConfiguration not "properly" configured.

Consider to either allow the ConditionalOnSingleCandidate annotation to add attribute whether to throw error or warn upon there is multiple @primary DataSource defined.

Or to allow this AutoConfiguration to log warning message when detect multiple @primary DataSource beans

@tombyong
Copy link
Author

@tombyong tombyong commented Mar 5, 2022

of course, the server can't be started at all due to missing EntityManagerFactory bean

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage label Mar 5, 2022
@wilkinsona
Copy link
Member

@wilkinsona wilkinsona commented Mar 5, 2022

Thanks for the suggestion.

The condition evaluation report (available by starting the app with --debug or the Actuator’s conditions endpoint) should show that the single candidate condition did not match as there were multiple candidates. Can you please check that’s the case in your situation? If it is not, we should correct the report.

Beyond that, I am not keen on treating this situation specially as it is not really any different to any other situation where auto-configuration backs off.

@wilkinsona wilkinsona added the status: waiting-for-feedback label Mar 5, 2022
@snicoll
Copy link
Member

@snicoll snicoll commented Mar 5, 2022

If you have multiple @Primary bean for the same type, the ApplicationContext should throw an exception. Can you share a sample where you have multiple primary datasource beans and that does not occur?

@tombyong
Copy link
Author

@tombyong tombyong commented Mar 7, 2022

Hi @snicoll, that's what I thought so, but drilling into evaluation report (or using TRACE) kind of counter-intiutive.

We defined in this way.

2 @configuration classes having 2 DataSource defined, but one is @primary.
And both get @ConfigurationProperties flavored, with different prefixed, properties are defined.

(so these DataSource has the actual database connections)

and another "hidden" DataSource defined in @configuration class (which the team not aware of), having @primary but prefix for @ConfigurationProperties not defined in anywhere.

(this DataSource hence not used anywhere)

@spring-projects-issues spring-projects-issues added status: feedback-provided and removed status: waiting-for-feedback labels Mar 7, 2022
@snicoll
Copy link
Member

@snicoll snicoll commented Mar 7, 2022

but drilling into evaluation report (or using TRACE) kind of counter-intiutive.

The evaluation report is exactly meant for that kind of thing. If you expect a bean to be created, and it isn't, then that is the place to look at.

Back to my original ask, thanks for the details but that's not the sample I was expecting and I am even more confused now. Is the "hidden" datasource defined as a bean ultimately? If it isn't, then the whole report needs to be revisited. For us to make progress, we'd need a sample that we can run ourselves that demonstrates what you've described.

@snicoll snicoll added status: waiting-for-feedback and removed status: feedback-provided labels Mar 7, 2022
@tombyong
Copy link
Author

@tombyong tombyong commented Mar 8, 2022

Total 3 @Configuration class that having DataSource @Bean defined for each

they get defined in below manner,

only properties for prefix app1.datasource and spring.datasource are defined.
there is no properties defined for app3.datasource

App2 and App3 are the one that having @Primary datasource defined.

// App1
@Configuration
public class App1DatasourceConfig {

    @Bean("app1DataSource")
    @ConfigurationProperties(prefix="app1.datasource")
    public DataSource app1DataSource() {
        return DataSourceBuilder.create().build();
    }

    @Bean("app1JdbcTemplate")
    public JdbcTemplate app1JdbcTemplate(@Autowired @Qualifier("app1DataSource") DataSource app1DataSource){
        return new JdbcTemplate(app1DataSource);
    }
}

//App2
@Configuration
public class App2DatasourceConfig {

    @Primary
    @Bean("app2DataSource")
    @ConfigurationProperties(prefix="spring.datasource")
    public DataSource app2DataSource() {
        return DataSourceBuilder.create().build();
    }

    @Bean("app2JdbcTemplate")
    public JdbcTemplate app2JdbcTemplate(@Autowired @Qualifier("app2DataSource") DataSource app2DataSource){
        return new JdbcTemplate(app2DataSource);
    }
}

//App3
@Configuration
public class App3DatasourceConfig {

    @Primary
    @Bean("app3DataSource")
    @ConfigurationProperties(prefix="app3.datasource")
    public DataSource app3DataSource() {
        return DataSourceBuilder.create().build();
    }

    @Bean("app3JdbcTemplate")
    public JdbcTemplate app3JdbcTemplate(@Autowired @Qualifier("app3DataSource") DataSource app3DataSource){
        return new JdbcTemplate(app3DataSource);
    }
}

@spring-projects-issues spring-projects-issues added status: feedback-provided and removed status: waiting-for-feedback labels Mar 8, 2022
@wilkinsona
Copy link
Member

@wilkinsona wilkinsona commented Mar 8, 2022

If you have multiple @primary bean for the same type, the ApplicationContext should throw an exception

This will only happen if something injects a DataSource bean. In the example provided, every DataSource injection point backs off due to the conditions so no failure occurs. If there is an unconditional injection point for a DataSource bean, it will fail:

Caused by: org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type 'javax.sql.DataSource' available: more than one 'primary' bean found among candidates: [app1DataSource, app2DataSource, app3DataSource]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.determinePrimaryCandidate(DefaultListableBeanFactory.java:1673) ~[spring-beans-5.3.16.jar:5.3.16]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.determineAutowireCandidate(DefaultListableBeanFactory.java:1633) ~[spring-beans-5.3.16.jar:5.3.16]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1364) ~[spring-beans-5.3.16.jar:5.3.16]
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1309) ~[spring-beans-5.3.16.jar:5.3.16]
	at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:887) ~[spring-beans-5.3.16.jar:5.3.16]
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:791) ~[spring-beans-5.3.16.jar:5.3.16]
	... 18 common frames omitted

With the example configuration, the condition evaluation report shows that HibernateJpaConfiguration has backed off as there is no single candidate:

HibernateJpaConfiguration:
   Did not match:
      - @ConditionalOnSingleCandidate (types: javax.sql.DataSource; SearchStrategy: all) did not find a primary bean from beans 'app1DataSource', 'app3DataSource', 'app2DataSource' (OnBeanCondition)

The message in the condition evaluation report isn't quite as good as the message of the NoUniqueBeanDefinitionException. I think it could be improved by mentioning that there's more than one primary bean. I don't think it should log a warning or throw an exception.

@wilkinsona wilkinsona changed the title Consider to allow @ConditionalOnSingleCandidate to report warning or error if there is multiple @Primary candidates Condition evaluation report entry for a @ConditionalOnSingleCandidate that does not match due to multiple primary beans isn't as clear as it could be Mar 8, 2022
@wilkinsona wilkinsona added type: bug and removed status: waiting-for-triage status: feedback-provided labels Mar 8, 2022
@wilkinsona wilkinsona added this to the 2.5.x milestone Mar 8, 2022
@wilkinsona wilkinsona self-assigned this Mar 8, 2022
@wilkinsona wilkinsona removed this from the 2.5.x milestone Mar 8, 2022
@wilkinsona wilkinsona added this to the 2.5.11 milestone Mar 8, 2022
@tombyong
Copy link
Author

@tombyong tombyong commented Mar 8, 2022

hi @wilkinsona that's good enough. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug
Projects
None yet
Development

No branches or pull requests

4 participants