From 57bc23284ea84c93c015e883198b9d876fa1cb4f Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 24 Sep 2020 18:06:18 +0200 Subject: [PATCH] Apply DataSource aliases only when necessary This commit makes sure that aliases are only applied when they match the DataSource being bound. This prevent an alias targetted to a DataSource to accidently match an unexpected property of another DataSource. Closes gh-23480 --- .../boot/jdbc/DataSourceBuilder.java | 143 +++++++++++++++--- .../boot/jdbc/DataSourceBuilderTests.java | 67 +++++++- 2 files changed, 186 insertions(+), 24 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/DataSourceBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/DataSourceBuilder.java index 2880103577a..9f5f2e698ee 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/DataSourceBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/DataSourceBuilder.java @@ -16,8 +16,13 @@ package org.springframework.boot.jdbc; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.function.Consumer; +import java.util.function.Function; import javax.sql.DataSource; @@ -46,14 +51,11 @@ import org.springframework.util.ClassUtils; */ public final class DataSourceBuilder { - private static final String[] DATA_SOURCE_TYPE_NAMES = new String[] { "com.zaxxer.hikari.HikariDataSource", - "org.apache.tomcat.jdbc.pool.DataSource", "org.apache.commons.dbcp2.BasicDataSource" }; - private Class type; - private ClassLoader classLoader; + private final DataSourceSettingsResolver settingsResolver; - private Map properties = new HashMap<>(); + private final Map properties = new HashMap<>(); public static DataSourceBuilder create() { return new DataSourceBuilder<>(null); @@ -64,7 +66,7 @@ public final class DataSourceBuilder { } private DataSourceBuilder(ClassLoader classLoader) { - this.classLoader = classLoader; + this.settingsResolver = new DataSourceSettingsResolver(classLoader); } @SuppressWarnings("unchecked") @@ -87,9 +89,7 @@ public final class DataSourceBuilder { private void bind(DataSource result) { ConfigurationPropertySource source = new MapConfigurationPropertySource(this.properties); ConfigurationPropertyNameAliases aliases = new ConfigurationPropertyNameAliases(); - aliases.addAliases("driver-class-name", "driver-class"); - aliases.addAliases("url", "jdbc-url"); - aliases.addAliases("username", "user"); + this.settingsResolver.registerAliases(result, aliases); Binder binder = new Binder(source.withAliases(aliases)); binder.bind(ConfigurationPropertyName.EMPTY, Bindable.ofInstance(result)); } @@ -120,25 +120,124 @@ public final class DataSourceBuilder { return this; } - @SuppressWarnings("unchecked") public static Class findType(ClassLoader classLoader) { - for (String name : DATA_SOURCE_TYPE_NAMES) { - try { - return (Class) ClassUtils.forName(name, classLoader); - } - catch (Exception ex) { - // Swallow and continue - } - } - return null; + DataSourceSettings preferredDataSourceSettings = new DataSourceSettingsResolver(classLoader) + .getPreferredDataSourceSettings(); + return (preferredDataSourceSettings != null) ? preferredDataSourceSettings.getType() : null; } private Class getType() { - Class type = (this.type != null) ? this.type : findType(this.classLoader); - if (type != null) { - return type; + if (this.type != null) { + return this.type; + } + DataSourceSettings preferredDataSourceSettings = this.settingsResolver.getPreferredDataSourceSettings(); + if (preferredDataSourceSettings != null) { + return preferredDataSourceSettings.getType(); } throw new IllegalStateException("No supported DataSource type found"); } + private static class DataSourceSettings { + + private final Class type; + + private final Consumer aliasesCustomizer; + + DataSourceSettings(Class type, + Consumer aliasesCustomizer) { + this.type = type; + this.aliasesCustomizer = aliasesCustomizer; + } + + DataSourceSettings(Class type) { + this(type, (aliases) -> { + }); + } + + Class getType() { + return this.type; + } + + void registerAliases(DataSource candidate, ConfigurationPropertyNameAliases aliases) { + if (this.type != null && this.type.isInstance(candidate)) { + this.aliasesCustomizer.accept(aliases); + } + } + + } + + private static class OracleDataSourceSettings extends DataSourceSettings { + + OracleDataSourceSettings(Class type) { + super(type, (aliases) -> aliases.addAliases("username", "user")); + } + + @Override + public Class getType() { + return null; // Base interface + } + + } + + private static class DataSourceSettingsResolver { + + private final DataSourceSettings preferredDataSourceSettings; + + private final List allDataSourceSettings; + + DataSourceSettingsResolver(ClassLoader classLoader) { + List supportedProviders = resolveAvailableDataSourceSettings(classLoader); + this.preferredDataSourceSettings = (!supportedProviders.isEmpty()) ? supportedProviders.get(0) : null; + this.allDataSourceSettings = new ArrayList<>(supportedProviders); + addIfAvailable(this.allDataSourceSettings, + create(classLoader, "org.springframework.jdbc.datasource.SimpleDriverDataSource", + (type) -> new DataSourceSettings(type, + (aliases) -> aliases.addAliases("driver-class-name", "driver-class")))); + addIfAvailable(this.allDataSourceSettings, create(classLoader, + "oracle.jdbc.datasource.OracleCommonDataSource", OracleDataSourceSettings::new)); + } + + private static List resolveAvailableDataSourceSettings(ClassLoader classLoader) { + List providers = new ArrayList<>(); + addIfAvailable(providers, create(classLoader, "com.zaxxer.hikari.HikariDataSource", + (type) -> new DataSourceSettings(type, (aliases) -> aliases.addAliases("url", "jdbc-url")))); + addIfAvailable(providers, + create(classLoader, "org.apache.tomcat.jdbc.pool.DataSource", DataSourceSettings::new)); + addIfAvailable(providers, + create(classLoader, "org.apache.commons.dbcp2.BasicDataSource", DataSourceSettings::new)); + return providers; + } + + @SuppressWarnings("unchecked") + private static DataSourceSettings create(ClassLoader classLoader, String target, + Function, DataSourceSettings> factory) { + if (ClassUtils.isPresent(target, classLoader)) { + try { + Class type = (Class) ClassUtils.forName(target, + classLoader); + return factory.apply(type); + } + catch (Exception ex) { + // Ignore + } + } + return null; + } + + private static void addIfAvailable(Collection list, DataSourceSettings dataSourceSettings) { + if (dataSourceSettings != null) { + list.add(dataSourceSettings); + } + } + + DataSourceSettings getPreferredDataSourceSettings() { + return this.preferredDataSourceSettings; + } + + void registerAliases(DataSource result, ConfigurationPropertyNameAliases aliases) { + this.allDataSourceSettings.forEach((settings) -> settings.registerAliases(result, aliases)); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/DataSourceBuilderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/DataSourceBuilderTests.java index d5ce84eb77a..779a02be42d 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/DataSourceBuilderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/DataSourceBuilderTests.java @@ -20,11 +20,13 @@ import java.io.Closeable; import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; +import java.sql.SQLException; import java.util.Arrays; import javax.sql.DataSource; import com.zaxxer.hikari.HikariDataSource; +import oracle.jdbc.pool.OracleDataSource; import org.apache.commons.dbcp2.BasicDataSource; import org.h2.Driver; import org.junit.jupiter.api.AfterEach; @@ -54,6 +56,8 @@ class DataSourceBuilderTests { void defaultToHikari() { this.dataSource = DataSourceBuilder.create().url("jdbc:h2:test").build(); assertThat(this.dataSource).isInstanceOf(HikariDataSource.class); + HikariDataSource hikariDataSource = (HikariDataSource) this.dataSource; + assertThat(hikariDataSource.getJdbcUrl()).isEqualTo("jdbc:h2:test"); } @Test @@ -81,8 +85,33 @@ class DataSourceBuilderTests { void dataSourceCanBeCreatedWithSimpleDriverDataSource() { this.dataSource = DataSourceBuilder.create().url("jdbc:h2:test").type(SimpleDriverDataSource.class).build(); assertThat(this.dataSource).isInstanceOf(SimpleDriverDataSource.class); - assertThat(((SimpleDriverDataSource) this.dataSource).getUrl()).isEqualTo("jdbc:h2:test"); - assertThat(((SimpleDriverDataSource) this.dataSource).getDriver()).isInstanceOf(Driver.class); + SimpleDriverDataSource simpleDriverDataSource = (SimpleDriverDataSource) this.dataSource; + assertThat(simpleDriverDataSource.getUrl()).isEqualTo("jdbc:h2:test"); + assertThat(simpleDriverDataSource.getDriver()).isInstanceOf(Driver.class); + } + + @Test + void dataSourceCanBeCreatedWithOracleDataSource() throws SQLException { + this.dataSource = DataSourceBuilder.create().url("jdbc:oracle:thin:@localhost:1521:xe") + .type(OracleDataSource.class).username("test").build(); + assertThat(this.dataSource).isInstanceOf(OracleDataSource.class); + OracleDataSource oracleDataSource = (OracleDataSource) this.dataSource; + assertThat(oracleDataSource.getURL()).isEqualTo("jdbc:oracle:thin:@localhost:1521:xe"); + assertThat(oracleDataSource.getUser()).isEqualTo("test"); + } + + @Test + void dataSourceAliasesAreOnlyAppliedToRelevantDataSource() { + this.dataSource = DataSourceBuilder.create().url("jdbc:h2:test").type(TestDataSource.class).username("test") + .build(); + assertThat(this.dataSource).isInstanceOf(TestDataSource.class); + TestDataSource testDataSource = (TestDataSource) this.dataSource; + assertThat(testDataSource.getUrl()).isEqualTo("jdbc:h2:test"); + assertThat(testDataSource.getJdbcUrl()).isNull(); + assertThat(testDataSource.getUsername()).isEqualTo("test"); + assertThat(testDataSource.getUser()).isNull(); + assertThat(testDataSource.getDriverClassName()).isEqualTo(Driver.class.getName()); + assertThat(testDataSource.getDriverClass()).isNull(); } final class HidePackagesClassLoader extends URLClassLoader { @@ -104,4 +133,38 @@ class DataSourceBuilderTests { } + public static class TestDataSource extends org.apache.tomcat.jdbc.pool.DataSource { + + private String jdbcUrl; + + private String user; + + private String driverClass; + + public String getJdbcUrl() { + return this.jdbcUrl; + } + + public void setJdbcUrl(String jdbcUrl) { + this.jdbcUrl = jdbcUrl; + } + + public String getUser() { + return this.user; + } + + public void setUser(String user) { + this.user = user; + } + + public String getDriverClass() { + return this.driverClass; + } + + public void setDriverClass(String driverClass) { + this.driverClass = driverClass; + } + + } + }