Extract JdbcConnectionDetails from DataSource conditionality

[benkelaar]

Context

In most of our applications we use flyway to migrate databases. Since migration data sources usually have a very different connection usage profile from the regular process of processing HTTP calls or message queues, we configure a different datasource for flyway from the one we use for regular traffic.

For this custom configuration of a flyway datasource we need to mark the regular datasource as being the "primary" datasource, therefor we basically copy the AutoConfiguration from DataSourceConfiguration.Hikari#dataSource and we slap a @Primary on top of that, which we then use with a

    @Bean
    @FlywayDataSource
    @ConfigurationProperties(prefix = "spring.flyway.datasource.hikari")
    fun migrationDataSource(primary: HikariDataSource): HikariDataSource =
        DataSourceBuilder.derivedFrom(primary).type(HikariDataSource::class.java).build()

For the flyway datasource.

Problem

However we also like to use testcontainers for our database integration tests, which relies on the JdbcConnectionDetails abstraction to override the connection to the database to its newly started postgres container using the test containers @ServiceConnection mechanism.

This mechanism is not working as I'd expect it to work, since the PropertiesJdbcConnectionDetails bean is defined in the PooledDataSourceConfiguration which is @ConditionalOnMissingBean({ DataSource.class, XADataSource.class }). However we want to have the JdbcConnectionDetails in order to properly define our own DataSource class (Which seems to me to be the expected way those abstractions should interact).

Workaround

Of course we can fix it with an optional bean inject (which is what I've done now) but conceptually I think it makes more sense (and would lead to simpler code) if PropertiesJdbcConnectionDetails is only @ConditionalOnMissingBean(JdbcConnectionDetails.class) and not also @ConditionalOnMissingBean({ DataSource.class, XADataSource.class }).

Notes on merge request

• For now I've left the @Conditional(PooledDataSourceCondition.class) on it as well, but I think it should go as well. Was just minimizing the amount of changes.
• I've shadowed this class in one of our applications to test and it seems to work well.
• I figured this counts as "more than cosmetic changes", but probably barely so, so feel free to tell me to drop the @author line
• If you give me some tips on how and where this sort of stuff is usually tested I'll happily add some tests, didn't seem to get any consistently failing tests with this change locally
• The way the rest of this class is structured I should probably wrap this @Bean in a protected static class JdbcConnectionDetailsConfiguration which has the conditional annotation, should I do that?

Edit: I did the two crossed out changes in an extra commit, can easily roll it back or squash if you prefer.

[wilkinsona]

For this custom configuration of a flyway datasource we need to mark the regular datasource as being the "primary" datasource

As of Spring Boot 3.4, that should no longer be necessary. Instead, marking your Flyway bean as not being a default candidate should be sufficient:

@Bean(defaultCandidate = false)
@FlywayDataSource
@ConfigurationProperties(prefix = "spring.flyway.datasource.hikari")
fun migrationDataSource(primary: HikariDataSource): HikariDataSource =
    DataSourceBuilder.derivedFrom(primary).type(HikariDataSource::class.java).build()

[benkelaar]

Nice! Good tip, will give it a go right now!

Does that also mean that the @ConditionalOnMissingBean({DataSource.class}) will not fire?

Still think this improvement is a good one though. I can imagine several scenarios where you'd want to define your own dataSource that still uses the JdbcConnectionDetails abstraction.

But the defaultCandidate = false solution seems to be working well! Thanks!

[wilkinsona]

Does that also mean that the @ConditionalOnMissingBean({DataSource.class}) will not fire?

I'm not sure exactly what you mean by "not fire". Non-default beans are not considered when checking for the presence or absence of a bean. In other words, @ConditionalOnMissingBean(DataSource.class) will match if there is a DataSource bean in the context that is marked with defaultCandidate=false. Similarly, @ConditionalOnBean will not match in the presence of such a bean.

Still think this improvement is a good one though. I can imagine several scenarios where you'd want to define your own dataSource that still uses the JdbcConnectionDetails abstraction.

The pattern of the properties-based connection details backing off when something like a DataSource or JmsConnectionFactory is defined is pretty common throughout Boot's auto-configurations. I'd prefer to keep it that way until we have a definite need to change it. Thanks anyway for the PR.

[benkelaar]

No worries, good to stay consistent indeed. Thanks for all your hard work on Spring!