Skip to content

DataSourceProperties should be created with conditional beans in Auto Configuration. #14071

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
kingbbode opened this issue Aug 15, 2018 · 7 comments
Labels
status: superseded An issue that has been superseded by another type: bug A general bug

Comments

@kingbbode
Copy link

kingbbode commented Aug 15, 2018

The DataSourceProperties are public, and there are many customization examples in the documentation.

The example states the following:

@Bean
@Primary
@ConfigurationProperties("app.datasource")
public DataSourceProperties dataSourceProperties() {
	return new DataSourceProperties();
}

I think it's strange about the @Primary setting. Without this setting, throws NoUniqueBeanDefinitionException.

This is because of @EnableConfigurationProperties(DataSourceProperties.class).

@EnableConfigurationProperties (DataSourceProperties.class) is used in several places of spring boot auto-configuration.

The following code is representative.

@Configuration
@ConditionalOnClass({ DataSource.class, EmbeddedDatabaseType.class })
@EnableConfigurationProperties(DataSourceProperties.class)
@Import({ DataSourcePoolMetadataProvidersConfiguration.class,
		DataSourceInitializationConfiguration.class })
public class DataSourceAutoConfiguration {

Why do I need to make a useless DataSourceProperties Bean from "spring.datasource"?

I think, remove EnableConfigurationProperties, and DataSourceProperties should be created with conditional beans.

@Bean
@ConditionalOnMissingBean(DataSourceProperties.class)
public DataSourceProperties dataSourceProperties() {
    return new DataSourceProperties();
}

In addition, all DatasourceProperties inserted into the Spring Boot Auto Configuration must be wrapped in an ObjectProvider.

The following code is representative.

DataSourceInitializerInvoker(ObjectProvider<DataSource> dataSource,
		DataSourceProperties properties, ApplicationContext applicationContext) {
	this.dataSource = dataSource;
	this.properties = properties;
	this.applicationContext = applicationContext;
}

Although the DataSourceInitializerInvoker not works with multiple data sources, throw NoUniqueBeanDefinitionException Without Primary.

I think DataSourceProperties should be assigned like DataSource.

thanks.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 15, 2018
@wilkinsona
Copy link
Member

Thanks for the report but it's difficult to know what to do with it as you have described a solution to a problem without telling us what the problem is.

I think it's strange about the @Primary setting. Without this setting, throws NoUniqueBeanDefinitionException.

This example is for defining two DataSource beans. The need for @Primary is explained in the documentation:

You must, however, mark one of the DataSource instances as @Primary, because various auto-configurations down the road expect to be able to get one by type.

Why do I need to make a useless DataSourceProperties bean from "spring.datasource"?

Why is the bean useless? It's not clear what you are trying to do.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Aug 15, 2018
@kingbbode
Copy link
Author

kingbbode commented Aug 15, 2018

sorry.

i did not mention the Primary DataSource. It says Primary DataSourceProperties.

In my case, I'm running a multi-module project. And I have multiple data sources.

So, instead of 'spring.datasource', we use DataSourceProperties which receives 'app.datasource1' and 'app.datasource2' properties.

A module

@Bean(A)
@ConfigurationProperties("app.datasource1")
public DataSourceProperties dataSourceProperties() {
	return new DataSourceProperties();
}

B module

@Bean(B)
@ConfigurationProperties("app.datasource2")
public DataSourceProperties dataSourceProperties() {
	return new DataSourceProperties();
}

Some applications use only a single datasource, and some applications require multiple datasources.

The application only needs to use the required Datasource configuration module dependencies and write the properties.

There is no problem because DataSourceProperties is explicitly bound to each DataSource.

However, the problem is that the DataSourceProperties cause a NoUniqueBeanDefinitionException.

So I have come to the two conclusions mentioned above.

  1. remove EnableConfigurationProperties, and DataSourceProperties should be created with conditional beans.
  2. all DatasourceProperties inserted into the Spring Boot Auto Configuration must be wrapped in an ObjectProvider.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 15, 2018
@wilkinsona
Copy link
Member

There's a related problem here with how other components use DataSourceProperties. For example, Flyway's auto-configuration falls back to using the URL, username, or password from DataSourceProperties but those properties may have nothing to do with the configuration of the context's DataSource.

@wilkinsona wilkinsona added type: bug A general bug and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Aug 15, 2018
@kingbbode
Copy link
Author

I agree that there are issues related to how other components use DataSourceProperties.

And I changed my mind and reached the following conclusion.

  1. remove EnableConfigurationProperties from DataSourceAutoConfiguration and DataSourceProperties should be created with conditional beans.

  2. Only the DataSourceInitializerInvoker needs to be changed.

Below are the configuration classes used in AutoConfiguration that use DataSourceProperties.

FlywayAutoConfiguration
DataSourceAutoConfiguration
DataSourceBeanCreationFailureAnalyzer
DataSourceConfiguration
DataSourceInitializerInvoker
DataSourceTransactionManagerAutoConfiguration
EmbeddedDataSourceConfiguration
JndiDataSourceAutoConfiguration
XADataSourceAutoConfiguration
LiquibaseAutoConfiguration

Except for DataSourceInitializerInvoker, there is a good reason to grant an independent DataSourceProperties. I think it's right to have a NoUniqueBeanDefinitionException when using them.

I also noticed that there is a Condition that they should work with except DataSourceInitializerInvoker.

DataSourceAutoConfiguration is the most basic and most important configuration class for using the database in Spring Boot.

Therefore, it is correct that the DataSourceInitializerInvoker should be changed. DataSourceInitializerInvoker does not need to have an independent DataSourceProperties.

Below is the conclusion I think.

remove EnableConfigurationProperties from DataSourceAutoConfiguration and create DataSourceProperties as a conditional bean.

@Bean
@ConditionalOnMissingBean(DataSourceProperties.class)
public DataSourceProperties dataSourceProperties() {
    return new DataSourceProperties();
}

The DataSourceProperties in DataSourceInitializerInvoker are wrapped in ObjectProvider.

DataSourceInitializerInvoker(
    ObjectProvider<DataSource> dataSource,
		ObjectProvider<DataSourceProperties> properties, 
    ApplicationContext applicationContext) {
	this.dataSource = dataSource;
	this.properties = properties;
	this.applicationContext = applicationContext;
}
private DataSourceInitializer getDataSourceInitializer() {
	if (this.dataSourceInitializer == null) {
		DataSource ds = this.dataSource.getIfUnique();
    DataSourceProperties props = this.properties.getIfUnique();
    
		if (ds != null && props != null) {
			this.dataSourceInitializer = new DataSourceInitializer(ds, properties, this.applicationContext);
		}
	}
	return this.dataSourceInitializer;
}

@nosan
Copy link
Contributor

nosan commented May 30, 2019

@wilkinsona
May I take this bug?
If yes, Could you please clarify what exactly should be done?

@wilkinsona
Copy link
Member

@nosan Thanks for the offer, but we don't know yet. I'd like to address the problem described here but it's tied up in other related problems such as #9528. We're blocked here until we've figured out what to do there.

@wilkinsona wilkinsona added the status: blocked An issue that's blocked on an external project change label May 30, 2019
@philwebb philwebb added this to the 2.x milestone Sep 13, 2019
@wilkinsona
Copy link
Member

I think this has been addressed as part of reworking DataSource initialization in Spring Boot 2.5. An app with these two beans fails with 2.4.x:

@Bean
@ConfigurationProperties("custom.datasource")
public DataSourceProperties dataSourceProperties() {
	return new DataSourceProperties();
}
	
@Bean
public DataSource dataSource() {
	DataSourceProperties properties = dataSourceProperties();
	return DataSourceBuilder.create().url(properties.determineUrl()).build();
}

It starts successfully when using 2.5.

@wilkinsona wilkinsona removed this from the 2.x milestone Jun 14, 2021
@wilkinsona wilkinsona added status: superseded An issue that has been superseded by another and removed status: blocked An issue that's blocked on an external project change labels Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants