DataSourceBuilder should only alias a property when the expected DataSource is configured

[snicoll]

DataSourceBuilder has a number of aliases to accommodate for small changes in API. For instance, Hikari has a jdbc-url rather than the more common url property so we have a mapping for that. This mapping is useless if the DataSource we configure isn't using Hikari.

So far we got away with it, but adding support for Oracle UCP means we need to add yet another alias that's actually a valid property for DBCP2 (yet referring to something completely different).

[snicoll]

Taking a step back, it's rather ConfigurationPropertyNameAliases that attempts to bind to any alias that is configured irrespective to the fact that the "main property" has been bound.

[snicoll]

To reproduce quickly the issue, you need to patch DataSourceBuilder as follows and the run CommonsDbcp2DataSourcePoolMetadataTests. Both setDriverClassName and setConnectionFactoryClassName will be invoked because those two properties are present in the environment.

[philwebb]

Unfortunately we might not be able to fix this one by changing the alias logic. The alias feature is part of ConfigurationPropertySource and the Binder isn't aware of it. I think we'll need to go back to only adding aliases when we know the DataSource type supports them.

[fabio-grassi-gbs]

I committed a change in pull request #23403 that doesn't use ConfigurationPropertySource in favour of plain BeanWrapper plus some custom logic local to DataSourceBuilder. It's not super-clean, but does the job.

[snicoll]

@fabio-grassi-gbs I saw that, that's an interesting approach. I think we'd prefer something more declarative and I've been working on that Friday.

[snicoll]

Taking that back, that's rather a type safety issue as we have a test to cover that use case. Looking at the hierarchy, I'll settle for OracleDataSource anyway.

[snicoll]

I've refined things in ef2fee2