Extract JdbcConnectionDetails from DataSource conditionality #46314
+6
−2
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 aFor 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 thePooledDataSourceConfiguration
which is@ConditionalOnMissingBean({ DataSource.class, XADataSource.class })
. However we want to have the JdbcConnectionDetails in order to properly define our ownDataSource
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.The way the rest of this class is structured I should probably wrap this@Bean
in aprotected 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.