Skip to content

Commit

Permalink
Allow driverClassName to be optional
Browse files Browse the repository at this point in the history 
Update `DataSourceBuilder` so that the `driverClassName` may be optional
and silently ignored if it set but the underlying type does not have
a getter/setter.

This restores Spring Boot 2.4 behavior.

Fixes gh-26631

Co-authored-by: Phillip Webb <pwebb@vmware.com>
  • Loading branch information
@scottfrederick @philwebb
scottfrederick and philwebb committed May 26, 2021
1 parent c679b4c commit eed620f
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 31 deletions.
62 changes: 44 additions & 18 deletions ...ot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/DataSourceBuilder.java
View file
Expand Up @@ -253,20 +253,27 @@ public static Class<? extends DataSource> findType(ClassLoader classLoader) {
*/
private enum DataSourceProperty {

URL("url", "URL"),
URL(false, "url", "URL"),

DRIVER_CLASS_NAME("driverClassName"),
DRIVER_CLASS_NAME(true, "driverClassName"),

USERNAME("username", "user"),
USERNAME(false, "username", "user"),

PASSWORD("password");
PASSWORD(false, "password");

private boolean optional;

private final String[] names;

DataSourceProperty(String... names) {
DataSourceProperty(boolean optional, String... names) {
this.optional = optional;
this.names = names;
}

boolean isOptional() {
return this.optional;
}

@Override
public String toString() {
return this.names[0];
Expand Down Expand Up @@ -343,18 +350,23 @@ public boolean canSet(DataSourceProperty property) {
@Override
public void set(T dataSource, DataSourceProperty property, String value) {
MappedDataSourceProperty<T, ?> mappedProperty = getMapping(property);
mappedProperty.set(dataSource, value);
if (mappedProperty != null) {
mappedProperty.set(dataSource, value);
}
}

@Override
public String get(T dataSource, DataSourceProperty property) {
MappedDataSourceProperty<T, ?> mappedProperty = getMapping(property);
return mappedProperty.get(dataSource);
if (mappedProperty != null) {
return mappedProperty.get(dataSource);
}
return null;
}

private MappedDataSourceProperty<T, ?> getMapping(DataSourceProperty property) {
MappedDataSourceProperty<T, ?> mappedProperty = this.mappedProperties.get(property);
UnsupportedDataSourcePropertyException.throwIf(mappedProperty == null,
UnsupportedDataSourcePropertyException.throwIf(!property.isOptional() && mappedProperty == null,
() -> "No mapping found for " + property);
return mappedProperty;
}
Expand Down Expand Up @@ -439,8 +451,11 @@ private static class MappedDataSourceProperty<T extends DataSource, V> {

void set(T dataSource, String value) {
try {
UnsupportedDataSourcePropertyException.throwIf(this.setter == null,
() -> "No setter mapped for '" + this.property + "' property");
if (this.setter == null) {
UnsupportedDataSourcePropertyException.throwIf(!this.property.isOptional(),
() -> "No setter mapped for '" + this.property + "' property");
return;
}
this.setter.set(dataSource, convertFromString(value));
}
catch (SQLException ex) {
Expand All @@ -450,8 +465,11 @@ void set(T dataSource, String value) {

String get(T dataSource) {
try {
UnsupportedDataSourcePropertyException.throwIf(this.getter == null,
() -> "No getter mapped for '" + this.property + "' property");
if (this.getter == null) {
UnsupportedDataSourcePropertyException.throwIf(!this.property.isOptional(),
() -> "No getter mapped for '" + this.property + "' property");
return null;
}
return convertToString(this.getter.get(dataSource));
}
catch (SQLException ex) {
Expand Down Expand Up @@ -522,19 +540,27 @@ public boolean canSet(DataSourceProperty property) {
@Override
public void set(T dataSource, DataSourceProperty property, String value) {
Method method = getMethod(property, this.setters);
ReflectionUtils.invokeMethod(method, dataSource, value);
if (method != null) {
ReflectionUtils.invokeMethod(method, dataSource, value);
}
}

@Override
public String get(T dataSource, DataSourceProperty property) {
Method method = getMethod(property, this.getters);
return (String) ReflectionUtils.invokeMethod(method, dataSource);
if (method != null) {
return (String) ReflectionUtils.invokeMethod(method, dataSource);
}
return null;
}

private Method getMethod(DataSourceProperty property, Map<DataSourceProperty, Method> setters2) {
Method method = setters2.get(property);
UnsupportedDataSourcePropertyException.throwIf(method == null,
() -> "Unable to find suitable method for " + property);
private Method getMethod(DataSourceProperty property, Map<DataSourceProperty, Method> methods) {
Method method = methods.get(property);
if (method == null) {
UnsupportedDataSourcePropertyException.throwIf(!property.isOptional(),
() -> "Unable to find suitable method for " + property);
return null;
}
ReflectionUtils.makeAccessible(method);
return method;
}
Expand Down
64 changes: 51 additions & 13 deletions ...oject/spring-boot/src/test/java/org/springframework/boot/jdbc/DataSourceBuilderTests.java
View file
Expand Up @@ -46,6 +46,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatNoException;

/**
* Tests for {@link DataSourceBuilder}.
Expand Down Expand Up @@ -143,6 +144,17 @@ void buildWhenOracleTypeSpecifiedReturnsExpectedDataSource() throws SQLException
assertThat(oracleDataSource.getUser()).isEqualTo("test");
}

@Test // gh-26631
void buildWhenOracleTypeSpecifiedWithDriverClassReturnsExpectedDataSource() throws SQLException {
this.dataSource = DataSourceBuilder.create().url("jdbc:oracle:thin:@localhost:1521:xe")
.type(OracleDataSource.class).driverClassName("oracle.jdbc.pool.OracleDataSource").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 buildWhenOracleUcpTypeSpecifiedReturnsExpectedDataSource() {
this.dataSource = DataSourceBuilder.create().driverClassName("org.hsqldb.jdbc.JDBCDriver")
Expand All @@ -163,6 +175,16 @@ void buildWhenH2TypeSpecifiedReturnsExpectedDataSource() {
assertThat(h2DataSource.getPassword()).isEqualTo("secret");
}

@Test // gh-26631
void buildWhenH2TypeSpecifiedWithDriverClassReturnsExpectedDataSource() {
this.dataSource = DataSourceBuilder.create().url("jdbc:h2:test").type(JdbcDataSource.class)
.driverClassName("org.h2.jdbcx.JdbcDataSource").username("test").password("secret").build();
assertThat(this.dataSource).isInstanceOf(JdbcDataSource.class);
JdbcDataSource h2DataSource = (JdbcDataSource) this.dataSource;
assertThat(h2DataSource.getUser()).isEqualTo("test");
assertThat(h2DataSource.getPassword()).isEqualTo("secret");
}

@Test
void buildWhenPostgresTypeSpecifiedReturnsExpectedDataSource() {
this.dataSource = DataSourceBuilder.create().url("jdbc:postgresql://localhost/test")
Expand All @@ -172,6 +194,16 @@ void buildWhenPostgresTypeSpecifiedReturnsExpectedDataSource() {
assertThat(pgDataSource.getUser()).isEqualTo("test");
}

@Test // gh-26631
void buildWhenPostgresTypeSpecifiedWithDriverClassReturnsExpectedDataSource() {
this.dataSource = DataSourceBuilder.create().url("jdbc:postgresql://localhost/test")
.type(PGSimpleDataSource.class).driverClassName("org.postgresql.ds.PGSimpleDataSource").username("test")
.build();
assertThat(this.dataSource).isInstanceOf(PGSimpleDataSource.class);
PGSimpleDataSource pgDataSource = (PGSimpleDataSource) this.dataSource;
assertThat(pgDataSource.getUser()).isEqualTo("test");
}

@Test // gh-26647
void buildWhenSqlServerTypeSpecifiedReturnsExpectedDataSource() {
this.dataSource = DataSourceBuilder.create().url("jdbc:sqlserver://localhost/test")
Expand All @@ -182,8 +214,8 @@ void buildWhenSqlServerTypeSpecifiedReturnsExpectedDataSource() {
}

@Test
void buildWhenMappedTypeSpecifiedAndNoSuitableMappingThrowsException() {
assertThatExceptionOfType(UnsupportedDataSourcePropertyException.class).isThrownBy(
void buildWhenMappedTypeSpecifiedAndNoSuitableOptionalMappingBuilds() {
assertThatNoException().isThrownBy(
() -> DataSourceBuilder.create().type(OracleDataSource.class).driverClassName("com.example").build());
}

Expand Down Expand Up @@ -214,9 +246,15 @@ void buildWhenCustomTypeSpecifiedReturnsDataSourceWithPropertiesSetViaReflection
}

@Test
void buildWhenCustomTypeSpecifiedAndNoSuitableSetterThrowsException() {
assertThatExceptionOfType(UnsupportedDataSourcePropertyException.class).isThrownBy(() -> DataSourceBuilder
.create().type(LimitedCustomDataSource.class).driverClassName("com.example").build());
void buildWhenCustomTypeSpecifiedAndNoSuitableOptionalSetterBuilds() {
assertThatNoException().isThrownBy(() -> DataSourceBuilder.create().type(LimitedCustomDataSource.class)
.driverClassName("com.example").build());
}

@Test
void buildWhenCustomTypeSpecifiedAndNoSuitableMandatorySetterThrowsException() {
assertThatExceptionOfType(UnsupportedDataSourcePropertyException.class).isThrownBy(
() -> DataSourceBuilder.create().type(LimitedCustomDataSource.class).url("jdbc:com.example").build());
}

@Test
Expand Down Expand Up @@ -339,8 +377,6 @@ static class LimitedCustomDataSource extends AbstractDataSource {

private String password;

private String url;

@Override
public Connection getConnection() throws SQLException {
throw new UnsupportedOperationException();
Expand All @@ -367,6 +403,14 @@ void setPassword(String password) {
this.password = password;
}

}

static class CustomDataSource extends LimitedCustomDataSource {

private String url;

private String driverClassName;

String getUrl() {
return this.url;
}
Expand All @@ -375,12 +419,6 @@ void setUrl(String url) {
this.url = url;
}

}

static class CustomDataSource extends LimitedCustomDataSource {

private String driverClassName;

String getDriverClassName() {
return this.driverClassName;
}
Expand Down

0 comments on commit eed620f

Please sign in to comment.