From 8ba45cc6b5e05957c698980c63f95d3c12f56ec8 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 16 Mar 2021 19:47:54 -0700 Subject: [PATCH] Allow DB migrations without DataSourceProperties Update `FlywayAutoConfiguration`, `LiquibaseAutoConfiguration` and `DataSourceInitializer` classes so that they no longer depend on `DataSourceProperties`. DB migrations can now be performed against a `@Bean` defined primary `DataSource` with an alternative username/password. This update also removed using fallback properties when a custom connection `url` is defined with Flyway or Liquibase. We now assume that `username`, `password` and `driver-class-name` will be provided if the default values are unacceptable. Our previous logic was particularly flawed if a custom URL caused a change of driver type. Closes gh-25643 --- .../flyway/FlywayAutoConfiguration.java | 64 ++++++---- .../flyway/FlywayProperties.java | 47 +++++--- .../jdbc/DataSourceInitializer.java | 8 +- .../liquibase/LiquibaseAutoConfiguration.java | 74 +++++------- .../flyway/FlywayAutoConfigurationTests.java | 92 +++++++++++++- .../flyway/FlywayPropertiesTests.java | 3 +- ...aSourceInitializationIntegrationTests.java | 15 ++- .../jdbc/MultiDataSourceConfiguration.java | 6 +- ...tiDataSourceUsingPrimaryConfiguration.java | 6 +- .../autoconfigure/jdbc/TestDataSource.java | 44 +++++-- .../LiquibaseAutoConfigurationTests.java | 113 +++++++++++++----- src/checkstyle/import-control.xml | 1 + 12 files changed, 332 insertions(+), 141 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfiguration.java index 5b5358f064..f7a5fbd0ea 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfiguration.java @@ -23,7 +23,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Supplier; import java.util.stream.Collectors; import javax.sql.DataSource; @@ -44,12 +43,12 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.flyway.FlywayAutoConfiguration.FlywayDataSourceCondition; import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; -import org.springframework.boot.autoconfigure.jdbc.DataSourceProperties; import org.springframework.boot.autoconfigure.jdbc.JdbcTemplateAutoConfiguration; import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; import org.springframework.boot.context.properties.ConfigurationPropertiesBinding; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.jdbc.DataSourceBuilder; import org.springframework.boot.jdbc.DatabaseDriver; import org.springframework.boot.jdbc.init.DataSourceInitializationDependencyConfigurer; import org.springframework.context.annotation.Bean; @@ -59,8 +58,10 @@ import org.springframework.context.annotation.Import; import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.converter.GenericConverter; import org.springframework.core.io.ResourceLoader; +import org.springframework.jdbc.datasource.SimpleDriverDataSource; import org.springframework.jdbc.support.JdbcUtils; import org.springframework.jdbc.support.MetaDataAccessException; +import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -103,19 +104,17 @@ public class FlywayAutoConfiguration { @Configuration(proxyBeanMethods = false) @ConditionalOnMissingBean(Flyway.class) - @EnableConfigurationProperties({ DataSourceProperties.class, FlywayProperties.class }) + @EnableConfigurationProperties(FlywayProperties.class) public static class FlywayConfiguration { @Bean - public Flyway flyway(FlywayProperties properties, DataSourceProperties dataSourceProperties, - ResourceLoader resourceLoader, ObjectProvider dataSource, - @FlywayDataSource ObjectProvider flywayDataSource, + public Flyway flyway(FlywayProperties properties, ResourceLoader resourceLoader, + ObjectProvider dataSource, @FlywayDataSource ObjectProvider flywayDataSource, ObjectProvider fluentConfigurationCustomizers, ObjectProvider javaMigrations, ObjectProvider callbacks) { FluentConfiguration configuration = new FluentConfiguration(resourceLoader.getClassLoader()); - DataSource dataSourceToMigrate = configureDataSource(configuration, properties, dataSourceProperties, - flywayDataSource.getIfAvailable(), dataSource.getIfUnique()); - checkLocationExists(dataSourceToMigrate, properties, resourceLoader); + configureDataSource(configuration, properties, flywayDataSource.getIfAvailable(), dataSource.getIfUnique()); + checkLocationExists(configuration.getDataSource(), properties, resourceLoader); configureProperties(configuration, properties); List orderedCallbacks = callbacks.orderedStream().collect(Collectors.toList()); configureCallbacks(configuration, orderedCallbacks); @@ -126,21 +125,39 @@ public class FlywayAutoConfiguration { return configuration.load(); } - private DataSource configureDataSource(FluentConfiguration configuration, FlywayProperties properties, - DataSourceProperties dataSourceProperties, DataSource flywayDataSource, DataSource dataSource) { - if (properties.isCreateDataSource()) { - String url = getProperty(properties::getUrl, dataSourceProperties::determineUrl); - String user = getProperty(properties::getUser, dataSourceProperties::determineUsername); - String password = getProperty(properties::getPassword, dataSourceProperties::determinePassword); - configuration.dataSource(url, user, password); + private void configureDataSource(FluentConfiguration configuration, FlywayProperties properties, + DataSource flywayDataSource, DataSource dataSource) { + DataSource migrationDataSource = getMigrationDataSource(properties, flywayDataSource, dataSource); + configuration.dataSource(migrationDataSource); + } + + private DataSource getMigrationDataSource(FlywayProperties properties, DataSource flywayDataSource, + DataSource dataSource) { + if (flywayDataSource != null) { + return flywayDataSource; } - else if (flywayDataSource != null) { - configuration.dataSource(flywayDataSource); + if (properties.getUrl() != null) { + DataSourceBuilder builder = DataSourceBuilder.create().type(SimpleDriverDataSource.class); + builder.url(properties.getUrl()); + applyCommonBuilderProperties(properties, builder); + return builder.build(); } - else { - configuration.dataSource(dataSource); + if (properties.getUser() != null && dataSource != null) { + DataSourceBuilder builder = DataSourceBuilder.derivedFrom(dataSource) + .type(SimpleDriverDataSource.class); + applyCommonBuilderProperties(properties, builder); + return builder.build(); + } + Assert.state(dataSource != null, "Flyway migration DataSource missing"); + return dataSource; + } + + private void applyCommonBuilderProperties(FlywayProperties properties, DataSourceBuilder builder) { + builder.username(properties.getUser()); + builder.password(properties.getPassword()); + if (StringUtils.hasText(properties.getDriverClassName())) { + builder.driverClassName(properties.getDriverClassName()); } - return configuration.getDataSource(); } @SuppressWarnings("deprecation") @@ -279,11 +296,6 @@ public class FlywayAutoConfiguration { } } - private String getProperty(Supplier property, Supplier defaultValue) { - String value = property.get(); - return (value != null) ? value : defaultValue.get(); - } - private boolean hasAtLeastOneLocation(ResourceLoader resourceLoader, Collection locations) { for (String location : locations) { if (resourceLoader.getResource(normalizePrefix(location)).exists()) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayProperties.java index 09df1c6006..edfc094700 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayProperties.java @@ -161,12 +161,6 @@ public class FlywayProperties { */ private String target; - /** - * JDBC url of the database to migrate. If not set, the primary configured data source - * is used. - */ - private String url; - /** * Login user of the database to migrate. */ @@ -177,6 +171,17 @@ public class FlywayProperties { */ private String password; + /** + * Fully qualified name of the JDBC driver. Auto-detected based on the URL by default. + */ + private String driverClassName; + + /** + * JDBC url of the database to migrate. If not set, the primary configured data source + * is used. + */ + private String url; + /** * SQL statements to execute to initialize a connection immediately after obtaining * it. @@ -538,18 +543,16 @@ public class FlywayProperties { this.target = target; } + /** + * Return if a new datasource is being created. + * @return {@code true} if a new datasource is created + * @deprecated since 2.5.0 in favor of directly checking user and url. + */ + @Deprecated public boolean isCreateDataSource() { return this.url != null || this.user != null; } - public String getUrl() { - return this.url; - } - - public void setUrl(String url) { - this.url = url; - } - public String getUser() { return this.user; } @@ -566,6 +569,22 @@ public class FlywayProperties { this.password = password; } + public String getDriverClassName() { + return this.driverClassName; + } + + public void setDriverClassName(String driverClassName) { + this.driverClassName = driverClassName; + } + + public String getUrl() { + return this.url; + } + + public void setUrl(String url) { + this.url = url; + } + public List getInitSqls() { return this.initSqls; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializer.java index 28b5845199..d95c11b0fb 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializer.java @@ -33,6 +33,7 @@ import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; import org.springframework.jdbc.config.SortedResourcesFactoryBean; +import org.springframework.jdbc.datasource.SimpleDriverDataSource; import org.springframework.jdbc.datasource.init.DatabasePopulatorUtils; import org.springframework.jdbc.datasource.init.ResourceDatabasePopulator; import org.springframework.util.StringUtils; @@ -185,10 +186,9 @@ public class DataSourceInitializer { populator.addScript(resource); } DataSource dataSource = this.dataSource; - if (StringUtils.hasText(username) && StringUtils.hasText(password)) { - dataSource = DataSourceBuilder.create(this.properties.getClassLoader()) - .driverClassName(this.properties.determineDriverClassName()).url(this.properties.determineUrl()) - .username(username).password(password).build(); + if (StringUtils.hasText(username) && dataSource != null) { + dataSource = DataSourceBuilder.derivedFrom(dataSource).type(SimpleDriverDataSource.class).username(username) + .password(password).build(); } DatabasePopulatorUtils.execute(populator, dataSource); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java index 79a75930ee..97f7f38382 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfiguration.java @@ -16,8 +16,6 @@ package org.springframework.boot.autoconfigure.liquibase; -import java.util.function.Supplier; - import javax.sql.DataSource; import liquibase.change.DatabaseChange; @@ -32,18 +30,17 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration; -import org.springframework.boot.autoconfigure.jdbc.DataSourceProperties; import org.springframework.boot.autoconfigure.liquibase.LiquibaseAutoConfiguration.LiquibaseDataSourceCondition; import org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaAutoConfiguration; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jdbc.DataSourceBuilder; -import org.springframework.boot.jdbc.DatabaseDriver; import org.springframework.boot.jdbc.init.DataSourceInitializationDependencyConfigurer; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.jdbc.datasource.SimpleDriverDataSource; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** @@ -77,7 +74,7 @@ public class LiquibaseAutoConfiguration { @Configuration(proxyBeanMethods = false) @ConditionalOnMissingBean(SpringLiquibase.class) - @EnableConfigurationProperties({ DataSourceProperties.class, LiquibaseProperties.class }) + @EnableConfigurationProperties(LiquibaseProperties.class) public static class LiquibaseConfiguration { private final LiquibaseProperties properties; @@ -87,11 +84,10 @@ public class LiquibaseAutoConfiguration { } @Bean - public SpringLiquibase liquibase(DataSourceProperties dataSourceProperties, - ObjectProvider dataSource, + public SpringLiquibase liquibase(ObjectProvider dataSource, @LiquibaseDataSource ObjectProvider liquibaseDataSource) { SpringLiquibase liquibase = createSpringLiquibase(liquibaseDataSource.getIfAvailable(), - dataSource.getIfUnique(), dataSourceProperties); + dataSource.getIfUnique()); liquibase.setChangeLog(this.properties.getChangeLog()); liquibase.setClearCheckSums(this.properties.isClearChecksums()); liquibase.setContexts(this.properties.getContexts()); @@ -110,51 +106,43 @@ public class LiquibaseAutoConfiguration { return liquibase; } - private SpringLiquibase createSpringLiquibase(DataSource liquibaseDatasource, DataSource dataSource, - DataSourceProperties dataSourceProperties) { - DataSource liquibaseDataSource = getDataSource(liquibaseDatasource, dataSource); - if (liquibaseDataSource != null) { - SpringLiquibase liquibase = new SpringLiquibase(); - liquibase.setDataSource(liquibaseDataSource); - return liquibase; - } - SpringLiquibase liquibase = new DataSourceClosingSpringLiquibase(); - liquibase.setDataSource(createNewDataSource(dataSourceProperties)); + private SpringLiquibase createSpringLiquibase(DataSource liquibaseDataSource, DataSource dataSource) { + LiquibaseProperties properties = this.properties; + DataSource migrationDataSource = getMigrationDataSource(liquibaseDataSource, dataSource, properties); + SpringLiquibase liquibase = (migrationDataSource == liquibaseDataSource + || migrationDataSource == dataSource) ? new SpringLiquibase() + : new DataSourceClosingSpringLiquibase(); + liquibase.setDataSource(migrationDataSource); return liquibase; } - private DataSource getDataSource(DataSource liquibaseDataSource, DataSource dataSource) { + private DataSource getMigrationDataSource(DataSource liquibaseDataSource, DataSource dataSource, + LiquibaseProperties properties) { if (liquibaseDataSource != null) { return liquibaseDataSource; } - if (this.properties.getUrl() == null && this.properties.getUser() == null) { - return dataSource; + if (properties.getUrl() != null) { + DataSourceBuilder builder = DataSourceBuilder.create().type(SimpleDriverDataSource.class); + builder.url(properties.getUrl()); + applyCommonBuilderProperties(properties, builder); + return builder.build(); } - return null; - } - - private DataSource createNewDataSource(DataSourceProperties dataSourceProperties) { - String url = getProperty(this.properties::getUrl, dataSourceProperties::determineUrl); - String user = getProperty(this.properties::getUser, dataSourceProperties::determineUsername); - String password = getProperty(this.properties::getPassword, dataSourceProperties::determinePassword); - String driverClassName = determineDriverClassName(dataSourceProperties, url); - return DataSourceBuilder.create().type(SimpleDriverDataSource.class).url(url).username(user) - .password(password).driverClassName(driverClassName).build(); - } - - private String determineDriverClassName(DataSourceProperties dataSourceProperties, String url) { - if (StringUtils.hasText(this.properties.getDriverClassName())) { - return this.properties.getDriverClassName(); + if (properties.getUser() != null && dataSource != null) { + DataSourceBuilder builder = DataSourceBuilder.derivedFrom(dataSource) + .type(SimpleDriverDataSource.class); + applyCommonBuilderProperties(properties, builder); + return builder.build(); } - if (StringUtils.hasText(dataSourceProperties.getDriverClassName())) { - return dataSourceProperties.getDriverClassName(); - } - return StringUtils.hasText(url) ? DatabaseDriver.fromJdbcUrl(url).getDriverClassName() : null; + Assert.state(dataSource != null, "Liquibase migration DataSource missing"); + return dataSource; } - private String getProperty(Supplier property, Supplier defaultValue) { - String value = property.get(); - return (value != null) ? value : defaultValue.get(); + private void applyCommonBuilderProperties(LiquibaseProperties properties, DataSourceBuilder builder) { + builder.username(properties.getUser()); + builder.password(properties.getPassword()); + if (StringUtils.hasText(properties.getDriverClassName())) { + builder.driverClassName(properties.getDriverClassName()); + } } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfigurationTests.java index ca9cef7cd0..8ab4be1d79 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayAutoConfigurationTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.autoconfigure.flyway; +import java.sql.Connection; +import java.sql.SQLException; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -42,7 +44,9 @@ import org.mockito.InOrder; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.boot.autoconfigure.AutoConfigurations; +import org.springframework.boot.autoconfigure.jdbc.DataSourceProperties; import org.springframework.boot.autoconfigure.jdbc.EmbeddedDataSourceConfiguration; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.jdbc.DataSourceBuilder; import org.springframework.boot.jdbc.SchemaManagement; import org.springframework.boot.orm.jpa.EntityManagerFactoryBuilder; @@ -62,6 +66,10 @@ import org.springframework.jdbc.core.JdbcOperations; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import org.springframework.jdbc.datasource.SimpleDriverDataSource; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean; import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; import org.springframework.stereotype.Component; @@ -128,25 +136,42 @@ class FlywayAutoConfigurationTests { } @Test - void createDataSourceFallbackToEmbeddedProperties() { + void createDataSourceDoesNotFallbackToEmbeddedProperties() { this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) .withPropertyValues("spring.flyway.url:jdbc:hsqldb:mem:flywaytest").run((context) -> { assertThat(context).hasSingleBean(Flyway.class); DataSource dataSource = context.getBean(Flyway.class).getConfiguration().getDataSource(); assertThat(dataSource).isNotNull(); - assertThat(dataSource).hasFieldOrPropertyWithValue("user", "sa"); + assertThat(dataSource).hasFieldOrPropertyWithValue("username", null); assertThat(dataSource).hasFieldOrPropertyWithValue("password", ""); }); } @Test void createDataSourceWithUserAndFallbackToEmbeddedProperties() { - this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) - .withPropertyValues("spring.flyway.user:sa").run((context) -> { + this.contextRunner.withUserConfiguration(PropertiesBackedH2DataSourceConfiguration.class) + .withPropertyValues("spring.flyway.user:test", "spring.flyway.password:secret").run((context) -> { assertThat(context).hasSingleBean(Flyway.class); DataSource dataSource = context.getBean(Flyway.class).getConfiguration().getDataSource(); assertThat(dataSource).isNotNull(); assertThat(dataSource).extracting("url").asString().startsWith("jdbc:h2:mem:"); + assertThat(dataSource).extracting("username").asString().isEqualTo("test"); + }); + } + + @Test + void createDataSourceWithUserAndCustomEmbeddedProperties() { + this.contextRunner.withUserConfiguration(CustomBackedH2DataSourceConfiguration.class) + .withPropertyValues("spring.flyway.user:test", "spring.flyway.password:secret").run((context) -> { + assertThat(context).hasSingleBean(Flyway.class); + String expectedName = context.getBean(CustomBackedH2DataSourceConfiguration.class).name; + String propertiesName = context.getBean(DataSourceProperties.class).determineDatabaseName(); + assertThat(expectedName).isNotEqualTo(propertiesName); + DataSource dataSource = context.getBean(Flyway.class).getConfiguration().getDataSource(); + assertThat(dataSource).isNotNull(); + assertThat(dataSource).extracting("url").asString().startsWith("jdbc:h2:mem:") + .contains(expectedName); + assertThat(dataSource).extracting("username").asString().isEqualTo("test"); }); } @@ -239,6 +264,20 @@ class FlywayAutoConfigurationTests { }); } + @Test + void overrideDataSourceAndDriverClassName() { + String jdbcUrl = "jdbc:hsqldb:mem:flyway" + UUID.randomUUID(); + String driverClassName = "org.hsqldb.jdbcDriver"; + this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class).withPropertyValues( + "spring.flyway.url:" + jdbcUrl, "spring.flyway.driver-class-name:" + driverClassName).run((context) -> { + Flyway flyway = context.getBean(Flyway.class); + SimpleDriverDataSource dataSource = (SimpleDriverDataSource) flyway.getConfiguration() + .getDataSource(); + assertThat(dataSource.getUrl()).isEqualTo(jdbcUrl); + assertThat(dataSource.getDriver().getClass().getName()).isEqualTo(driverClassName); + }); + } + @Test void changeLogDoesNotExist() { this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) @@ -842,6 +881,51 @@ class FlywayAutoConfigurationTests { } + @Configuration(proxyBeanMethods = false) + @EnableConfigurationProperties(DataSourceProperties.class) + abstract static class AbstractUserH2DataSourceConfiguration { + + @Bean(destroyMethod = "shutdown") + EmbeddedDatabase dataSource(DataSourceProperties properties) throws SQLException { + EmbeddedDatabase database = new EmbeddedDatabaseBuilder().setType(EmbeddedDatabaseType.H2) + .setName(getDatabaseName(properties)).build(); + insertUser(database); + return database; + } + + protected abstract String getDatabaseName(DataSourceProperties properties); + + private void insertUser(EmbeddedDatabase database) throws SQLException { + try (Connection connection = database.getConnection()) { + connection.prepareStatement("CREATE USER test password 'secret'").execute(); + connection.prepareStatement("ALTER USER test ADMIN TRUE").execute(); + } + } + + } + + @Configuration(proxyBeanMethods = false) + static class PropertiesBackedH2DataSourceConfiguration extends AbstractUserH2DataSourceConfiguration { + + @Override + protected String getDatabaseName(DataSourceProperties properties) { + return properties.determineDatabaseName(); + } + + } + + @Configuration(proxyBeanMethods = false) + static class CustomBackedH2DataSourceConfiguration extends AbstractUserH2DataSourceConfiguration { + + private final String name = UUID.randomUUID().toString(); + + @Override + protected String getDatabaseName(DataSourceProperties properties) { + return this.name; + } + + } + static final class CustomClassLoader extends ClassLoader { private CustomClassLoader(ClassLoader parent) { diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayPropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayPropertiesTests.java index 5a1ae774cd..7ff514f900 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayPropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/flyway/FlywayPropertiesTests.java @@ -100,7 +100,8 @@ class FlywayPropertiesTests { Map configuration = indexProperties( PropertyAccessorFactory.forBeanPropertyAccess(new ClassicConfiguration())); // Properties specific settings - ignoreProperties(properties, "url", "user", "password", "enabled", "checkLocation", "createDataSource"); + ignoreProperties(properties, "url", "driverClassName", "user", "password", "enabled", "checkLocation", + "createDataSource"); // High level object we can't set with properties ignoreProperties(configuration, "callbacks", "classLoader", "dataSource", "javaMigrations", "javaMigrationClassProvider", "resourceProvider", "resolvers"); diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializationIntegrationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializationIntegrationTests.java index f674acbb3d..be62afcfd0 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializationIntegrationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourceInitializationIntegrationTests.java @@ -92,6 +92,17 @@ class DataSourceInitializationIntegrationTests { }); } + @Test + void initializationWithUsernameAndPasswordAppliesToCustomDataSource() { + this.contextRunner.withUserConfiguration(OneDataSource.class) + .withPropertyValues("spring.datasource.initialization-mode:always", + "spring.datasource.schema-username=test", "spring.datasource.schema-password=secret") + .run((context) -> { + assertThat(context).hasSingleBean(DataSource.class); + assertDataSourceIsInitialized(context.getBean(DataSource.class)); + }); + } + private void assertDataSourceIsInitialized(DataSource dataSource) { JdbcOperations template = new JdbcTemplate(dataSource); assertThat(template.queryForObject("SELECT COUNT(*) from BAR", Integer.class)).isEqualTo(1); @@ -286,7 +297,7 @@ class DataSourceInitializationIntegrationTests { @Bean DataSource oneDataSource() { - return new TestDataSource(); + return new TestDataSource(true); } } @@ -296,7 +307,7 @@ class DataSourceInitializationIntegrationTests { @Bean DataSource twoDataSource() { - return new TestDataSource(); + return new TestDataSource(true); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceConfiguration.java index eed6efcd52..86427ae518 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,12 +32,12 @@ class MultiDataSourceConfiguration { @Bean DataSource test1DataSource() { - return new TestDataSource("test1"); + return new TestDataSource("test1", false); } @Bean DataSource test2DataSource() { - return new TestDataSource("test2"); + return new TestDataSource("test2", false); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceUsingPrimaryConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceUsingPrimaryConfiguration.java index 05b5f2c45e..9800a38984 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceUsingPrimaryConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/MultiDataSourceUsingPrimaryConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,12 +34,12 @@ class MultiDataSourceUsingPrimaryConfiguration { @Bean @Primary DataSource test1DataSource() { - return new TestDataSource("test1"); + return new TestDataSource("test1", false); } @Bean DataSource test2DataSource() { - return new TestDataSource("test2"); + return new TestDataSource("test2", false); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TestDataSource.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TestDataSource.java index efc8ec139a..37c5f666cf 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TestDataSource.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/TestDataSource.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,10 +16,14 @@ package org.springframework.boot.autoconfigure.jdbc; +import java.sql.Connection; +import java.sql.SQLException; import java.util.UUID; import org.apache.commons.dbcp2.BasicDataSource; +import org.springframework.jdbc.datasource.SimpleDriverDataSource; + /** * {@link BasicDataSource} used for testing. * @@ -27,23 +31,45 @@ import org.apache.commons.dbcp2.BasicDataSource; * @author Kazuki Shimizu * @author Stephane Nicoll */ -public class TestDataSource extends BasicDataSource { +public class TestDataSource extends SimpleDriverDataSource { + + /** + * Create an in-memory database with a random name. + */ + public TestDataSource() { + this(false); + } + + /** + * Create an in-memory database with a random name. + * @param addTestUser if a test user should be added + */ + public TestDataSource(boolean addTestUser) { + this(UUID.randomUUID().toString(), addTestUser); + } /** * Create an in-memory database with the specified name. * @param name the name of the database + * @param addTestUser if a test user should be added */ - public TestDataSource(String name) { - setDriverClassName("org.hsqldb.jdbcDriver"); + public TestDataSource(String name, boolean addTestUser) { + setDriverClass(org.hsqldb.jdbc.JDBCDriver.class); setUrl("jdbc:hsqldb:mem:" + name); setUsername("sa"); + setupDatabase(addTestUser); + setUrl(getUrl() + ";create=false"); } - /** - * Create an in-memory database with a random name. - */ - public TestDataSource() { - this(UUID.randomUUID().toString()); + private void setupDatabase(boolean addTestUser) { + try (Connection connection = getConnection()) { + if (addTestUser) { + connection.prepareStatement("CREATE USER \"test\" password \"secret\" ADMIN").execute(); + } + } + catch (SQLException ex) { + throw new IllegalStateException(ex); + } } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java index bd349ce9fd..029a4aa0c1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/liquibase/LiquibaseAutoConfigurationTests.java @@ -20,7 +20,10 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.sql.Connection; +import java.sql.SQLException; import java.util.Map; +import java.util.UUID; import java.util.function.Consumer; import javax.sql.DataSource; @@ -50,6 +53,9 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.jdbc.datasource.SimpleDriverDataSource; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -82,11 +88,11 @@ class LiquibaseAutoConfigurationTests { @Test void createsDataSourceWithNoDataSourceBeanAndLiquibaseUrl() { - this.contextRunner.withPropertyValues("spring.liquibase.url:jdbc:hsqldb:mem:liquibase") - .run(assertLiquibase((liquibase) -> { - SimpleDriverDataSource dataSource = (SimpleDriverDataSource) liquibase.getDataSource(); - assertThat(dataSource.getUrl()).isEqualTo("jdbc:hsqldb:mem:liquibase"); - })); + String jdbcUrl = "jdbc:hsqldb:mem:liquibase" + UUID.randomUUID(); + this.contextRunner.withPropertyValues("spring.liquibase.url:" + jdbcUrl).run(assertLiquibase((liquibase) -> { + SimpleDriverDataSource dataSource = (SimpleDriverDataSource) liquibase.getDataSource(); + assertThat(dataSource.getUrl()).isEqualTo(jdbcUrl); + })); } @Test @@ -191,18 +197,18 @@ class LiquibaseAutoConfigurationTests { @Test void overrideDataSource() { + String jdbcUrl = "jdbc:hsqldb:mem:liquibase" + UUID.randomUUID(); this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) - .withPropertyValues("spring.liquibase.url:jdbc:hsqldb:mem:liquibase") - .run(assertLiquibase((liquibase) -> { + .withPropertyValues("spring.liquibase.url:" + jdbcUrl).run(assertLiquibase((liquibase) -> { SimpleDriverDataSource dataSource = (SimpleDriverDataSource) liquibase.getDataSource(); - assertThat(dataSource.getUrl()).isEqualTo("jdbc:hsqldb:mem:liquibase"); + assertThat(dataSource.getUrl()).isEqualTo(jdbcUrl); assertThat(dataSource.getDriver().getClass().getName()).isEqualTo("org.hsqldb.jdbc.JDBCDriver"); })); } @Test void overrideDataSourceAndDriverClassName() { - String jdbcUrl = "jdbc:hsqldb:mem:liquibase"; + String jdbcUrl = "jdbc:hsqldb:mem:liquibase" + UUID.randomUUID(); String driverClassName = "org.hsqldb.jdbcDriver"; this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) .withPropertyValues("spring.liquibase.url:" + jdbcUrl, @@ -215,40 +221,39 @@ class LiquibaseAutoConfigurationTests { } @Test - void overrideDataSourceWithFallbackDriverClassName() { - String jdbcUrl = "jdbc:hsqldb:mem:liquibase"; - String driverClassName = "org.hsqldb.jdbcDriver"; + void overrideUser() { + String databaseName = "normal" + UUID.randomUUID(); this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) - .withPropertyValues("spring.liquibase.url:" + jdbcUrl, - "spring.datasource.driver-class-name:" + driverClassName) + .withPropertyValues("spring.datasource.generate-unique-name:false", + "spring.datasource.name:" + databaseName, "spring.datasource.username:not-sa", + "spring.liquibase.user:sa") .run(assertLiquibase((liquibase) -> { SimpleDriverDataSource dataSource = (SimpleDriverDataSource) liquibase.getDataSource(); - assertThat(dataSource.getUrl()).isEqualTo(jdbcUrl); - assertThat(dataSource.getDriver().getClass().getName()).isEqualTo(driverClassName); + assertThat(dataSource.getUrl()).contains("jdbc:h2:mem:" + databaseName); + assertThat(dataSource.getUsername()).isEqualTo("sa"); })); } @Test - void overrideUser() { - String jdbcUrl = "jdbc:hsqldb:mem:normal"; - this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) - .withPropertyValues("spring.datasource.url:" + jdbcUrl, "spring.datasource.username:not-sa", - "spring.liquibase.user:sa") - .run(assertLiquibase((liquibase) -> { + void overrideUserWhenCustom() { + this.contextRunner.withUserConfiguration(CustomDataSourceConfiguration.class) + .withPropertyValues("spring.liquibase.user:test", "spring.liquibase.password:secret").run((context) -> { + String expectedName = context.getBean(CustomDataSourceConfiguration.class).name; + SpringLiquibase liquibase = context.getBean(SpringLiquibase.class); SimpleDriverDataSource dataSource = (SimpleDriverDataSource) liquibase.getDataSource(); - assertThat(dataSource.getUrl()).isEqualTo(jdbcUrl); - assertThat(dataSource.getUsername()).isEqualTo("sa"); - })); + assertThat(dataSource.getUrl()).contains(expectedName); + assertThat(dataSource.getUsername()).isEqualTo("test"); + }); } @Test - void overrideDataSourceAndFallbackToEmbeddedProperties() { + void createDataSourceDoesNotFallbackToEmbeddedProperties() { + String jdbcUrl = "jdbc:hsqldb:mem:liquibase" + UUID.randomUUID(); this.contextRunner.withUserConfiguration(EmbeddedDataSourceConfiguration.class) - .withPropertyValues("spring.liquibase.url:jdbc:hsqldb:mem:liquibase") - .run(assertLiquibase((liquibase) -> { + .withPropertyValues("spring.liquibase.url:" + jdbcUrl).run(assertLiquibase((liquibase) -> { SimpleDriverDataSource dataSource = (SimpleDriverDataSource) liquibase.getDataSource(); - assertThat(dataSource.getUsername()).isEqualTo("sa"); - assertThat(dataSource.getPassword()).isEqualTo(""); + assertThat(dataSource.getUsername()).isNull(); + assertThat(dataSource.getPassword()).isNull(); })); } @@ -404,13 +409,14 @@ class LiquibaseAutoConfigurationTests { @Bean @Primary DataSource normalDataSource() { - return DataSourceBuilder.create().url("jdbc:hsqldb:mem:normal").username("sa").build(); + return DataSourceBuilder.create().url("jdbc:hsqldb:mem:normal" + UUID.randomUUID()).username("sa").build(); } @LiquibaseDataSource @Bean DataSource liquibaseDataSource() { - return DataSourceBuilder.create().url("jdbc:hsqldb:mem:liquibasetest").username("sa").build(); + return DataSourceBuilder.create().url("jdbc:hsqldb:mem:liquibasetest" + UUID.randomUUID()).username("sa") + .build(); } } @@ -429,4 +435,47 @@ class LiquibaseAutoConfigurationTests { } + @Configuration(proxyBeanMethods = false) + static class CustomDataSourceConfiguration { + + private String name = UUID.randomUUID().toString(); + + @Bean(destroyMethod = "shutdown") + EmbeddedDatabase dataSource() throws SQLException { + EmbeddedDatabase database = new EmbeddedDatabaseBuilder().setType(EmbeddedDatabaseType.H2) + .setName(this.name).build(); + insertUser(database); + return database; + } + + private void insertUser(EmbeddedDatabase database) throws SQLException { + try (Connection connection = database.getConnection()) { + connection.prepareStatement("CREATE USER test password 'secret'").execute(); + connection.prepareStatement("ALTER USER test ADMIN TRUE").execute(); + } + } + + } + + @Configuration(proxyBeanMethods = false) + static class CustomDriverConfiguration { + + private String name = UUID.randomUUID().toString(); + + @Bean + SimpleDriverDataSource dataSource() throws SQLException { + SimpleDriverDataSource dataSource = new SimpleDriverDataSource(); + dataSource.setDriverClass(CustomH2Driver.class); + dataSource.setUrl(String.format("jdbc:h2:mem:%s;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=false", this.name)); + dataSource.setUsername("sa"); + dataSource.setPassword(""); + return dataSource; + } + + } + + static class CustomH2Driver extends org.h2.Driver { + + } + } diff --git a/src/checkstyle/import-control.xml b/src/checkstyle/import-control.xml index 53ba5f5e99..54c3ed043f 100644 --- a/src/checkstyle/import-control.xml +++ b/src/checkstyle/import-control.xml @@ -4,6 +4,7 @@ +