From c4a5a347028b827ea53975d1ec80f52a14223c5c Mon Sep 17 00:00:00 2001 From: Asha Somayajula Date: Wed, 14 Oct 2020 18:36:58 -0500 Subject: [PATCH] Fix detection logic for embedded databases Closes gh-23721 --- .../jdbc/DataSourceProperties.java | 4 +- .../jdbc/DataSourcePropertiesTests.java | 36 +++++++ .../src/docs/asciidoc/howto.adoc | 4 +- .../boot/jdbc/EmbeddedDatabaseConnection.java | 66 ++++++++++--- .../jdbc/EmbeddedDatabaseConnectionTests.java | 95 +++++++++++++++++++ 5 files changed, 188 insertions(+), 17 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java index adfe5bcfab..4c44683659 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java @@ -325,7 +325,7 @@ public class DataSourceProperties implements BeanClassLoaderAware, InitializingB if (StringUtils.hasText(this.username)) { return this.username; } - if (EmbeddedDatabaseConnection.isEmbedded(determineDriverClassName())) { + if (EmbeddedDatabaseConnection.isEmbedded(determineDriverClassName(), determineUrl())) { return "sa"; } return null; @@ -353,7 +353,7 @@ public class DataSourceProperties implements BeanClassLoaderAware, InitializingB if (StringUtils.hasText(this.password)) { return this.password; } - if (EmbeddedDatabaseConnection.isEmbedded(determineDriverClassName())) { + if (EmbeddedDatabaseConnection.isEmbedded(determineDriverClassName(), determineUrl())) { return ""; } return null; diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourcePropertiesTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourcePropertiesTests.java index 6f47aecb95..0195c23d9b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourcePropertiesTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourcePropertiesTests.java @@ -98,6 +98,24 @@ class DataSourcePropertiesTests { assertThat(properties.determineUsername()).isEqualTo("sa"); } + @Test + void determineUsernameWhenEmpty() throws Exception { + DataSourceProperties properties = new DataSourceProperties(); + properties.setUsername(""); + properties.afterPropertiesSet(); + assertThat(properties.getUsername()); + assertThat(properties.determineUsername()).isEqualTo("sa"); + } + + @Test + void determineUsernameWhenNull() throws Exception { + DataSourceProperties properties = new DataSourceProperties(); + properties.setUsername(null); + properties.afterPropertiesSet(); + assertThat(properties.getUsername()); + assertThat(properties.determineUsername()).isEqualTo("sa"); + } + @Test void determineUsernameWithExplicitConfig() throws Exception { DataSourceProperties properties = new DataSourceProperties(); @@ -107,6 +125,15 @@ class DataSourcePropertiesTests { assertThat(properties.determineUsername()).isEqualTo("foo"); } + @Test + void determineUsernameWithNonEmbeddedUrl() throws Exception { + DataSourceProperties properties = new DataSourceProperties(); + properties.setUrl("jdbc:h2:~/test"); + properties.afterPropertiesSet(); + assertThat(properties.getPassword()).isNull(); + assertThat(properties.determineUsername()).isNull(); + } + @Test void determinePassword() throws Exception { DataSourceProperties properties = new DataSourceProperties(); @@ -124,6 +151,15 @@ class DataSourcePropertiesTests { assertThat(properties.determinePassword()).isEqualTo("bar"); } + @Test + void determinePasswordWithNonEmbeddedUrl() throws Exception { + DataSourceProperties properties = new DataSourceProperties(); + properties.setUrl("jdbc:h2:~/test"); + properties.afterPropertiesSet(); + assertThat(properties.getPassword()).isNull(); + assertThat(properties.determinePassword()).isNull(); + } + @Test void determineCredentialsForSchemaScripts() { DataSourceProperties properties = new DataSourceProperties(); diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/howto.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/howto.adoc index eb2e4be55d..e627ba01f3 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/howto.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/howto.adoc @@ -1927,8 +1927,8 @@ This is controlled through two external properties: You can set `spring.jpa.hibernate.ddl-auto` explicitly and the standard Hibernate property values are `none`, `validate`, `update`, `create`, and `create-drop`. Spring Boot chooses a default value for you based on whether it thinks your database is embedded. It defaults to `create-drop` if no schema manager has been detected or `none` in all other cases. -An embedded database is detected by looking at the `Connection` type. -`hsqldb`, `h2`, and `derby` are embedded, and others are not. +An embedded database is detected by looking at the `Connection` type and JDBC url. +`hsqldb`, `h2`, and `derby` are candidates, and others are not. Be careful when switching from in-memory to a '`real`' database that you do not make assumptions about the existence of the tables and data in the new platform. You either have to set `ddl-auto` explicitly or use one of the other mechanisms to initialize the database. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnection.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnection.java index 3eb8505c3d..a9daabb2a3 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnection.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnection.java @@ -17,8 +17,11 @@ package org.springframework.boot.jdbc; import java.sql.Connection; +import java.sql.DatabaseMetaData; import java.sql.SQLException; import java.util.Locale; +import java.util.function.Predicate; +import java.util.stream.Stream; import javax.sql.DataSource; @@ -43,24 +46,25 @@ public enum EmbeddedDatabaseConnection { /** * No Connection. */ - NONE(null, null, null), + NONE(null, null, null, (url) -> false), /** * H2 Database Connection. */ H2(EmbeddedDatabaseType.H2, DatabaseDriver.H2.getDriverClassName(), - "jdbc:h2:mem:%s;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE"), + "jdbc:h2:mem:%s;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=FALSE", (url) -> url.contains(":h2:mem")), /** * Derby Database Connection. */ - DERBY(EmbeddedDatabaseType.DERBY, DatabaseDriver.DERBY.getDriverClassName(), "jdbc:derby:memory:%s;create=true"), + DERBY(EmbeddedDatabaseType.DERBY, DatabaseDriver.DERBY.getDriverClassName(), "jdbc:derby:memory:%s;create=true", + (url) -> true), /** * HSQL Database Connection. */ HSQL(EmbeddedDatabaseType.HSQL, DatabaseDriver.HSQLDB.getDriverClassName(), "org.hsqldb.jdbcDriver", - "jdbc:hsqldb:mem:%s"); + "jdbc:hsqldb:mem:%s", (url) -> url.contains(":hsqldb:mem:")); private final EmbeddedDatabaseType type; @@ -70,15 +74,20 @@ public enum EmbeddedDatabaseConnection { private final String url; - EmbeddedDatabaseConnection(EmbeddedDatabaseType type, String driverClass, String url) { - this(type, driverClass, null, url); + private final Predicate embeddedUrl; + + EmbeddedDatabaseConnection(EmbeddedDatabaseType type, String driverClass, String url, + Predicate embeddedUrl) { + this(type, driverClass, null, url, embeddedUrl); } - EmbeddedDatabaseConnection(EmbeddedDatabaseType type, String driverClass, String fallbackDriverClass, String url) { + EmbeddedDatabaseConnection(EmbeddedDatabaseType type, String driverClass, String fallbackDriverClass, String url, + Predicate embeddedUrl) { this.type = type; this.driverClass = driverClass; this.alternativeDriverClass = fallbackDriverClass; this.url = url; + this.embeddedUrl = embeddedUrl; } /** @@ -107,19 +116,48 @@ public enum EmbeddedDatabaseConnection { return (this.url != null) ? String.format(this.url, databaseName) : null; } + boolean isEmbeddedUrl(String url) { + return this.embeddedUrl.test(url); + } + + boolean isDriverCompatible(String driverClass) { + return (driverClass != null + && (driverClass.equals(this.driverClass) || driverClass.equals(this.alternativeDriverClass))); + } + /** * Convenience method to determine if a given driver class name represents an embedded * database type. * @param driverClass the driver class * @return true if the driver class is one of the embedded types + * @deprecated since 2.3.5 in favor of {@link #isEmbedded(String, String)} */ + @Deprecated public static boolean isEmbedded(String driverClass) { - return driverClass != null - && (matches(HSQL, driverClass) || matches(H2, driverClass) || matches(DERBY, driverClass)); + return isEmbedded(driverClass, null); } - private static boolean matches(EmbeddedDatabaseConnection candidate, String driverClass) { - return driverClass.equals(candidate.driverClass) || driverClass.equals(candidate.alternativeDriverClass); + /** + * Convenience method to determine if a given driver class name and url represent an + * embedded database type. + * @param driverClass the driver class + * @param url the jdbc url (can be {@code null)} + * @return true if the driver class and url refer to an embedded database + */ + public static boolean isEmbedded(String driverClass, String url) { + if (driverClass == null) { + return false; + } + EmbeddedDatabaseConnection connection = getEmbeddedDatabaseConnection(driverClass); + if (connection == NONE) { + return false; + } + return (url == null || connection.isEmbeddedUrl(url)); + } + + private static EmbeddedDatabaseConnection getEmbeddedDatabaseConnection(String driverClass) { + return Stream.of(H2, HSQL, DERBY).filter((connection) -> connection.isDriverCompatible(driverClass)).findFirst() + .orElse(NONE); } /** @@ -160,7 +198,8 @@ public enum EmbeddedDatabaseConnection { @Override public Boolean doInConnection(Connection connection) throws SQLException, DataAccessException { - String productName = connection.getMetaData().getDatabaseProductName(); + DatabaseMetaData metaData = connection.getMetaData(); + String productName = metaData.getDatabaseProductName(); if (productName == null) { return false; } @@ -168,7 +207,8 @@ public enum EmbeddedDatabaseConnection { EmbeddedDatabaseConnection[] candidates = EmbeddedDatabaseConnection.values(); for (EmbeddedDatabaseConnection candidate : candidates) { if (candidate != NONE && productName.contains(candidate.name())) { - return true; + String url = metaData.getURL(); + return (url == null || candidate.isEmbeddedUrl(url)); } } return false; diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnectionTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnectionTests.java index bf7511c549..9258916c12 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnectionTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/EmbeddedDatabaseConnectionTests.java @@ -16,10 +16,24 @@ package org.springframework.boot.jdbc; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.SQLException; + +import javax.sql.DataSource; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; /** * Tests for {@link EmbeddedDatabaseConnection}. @@ -57,4 +71,85 @@ class EmbeddedDatabaseConnectionTests { .withMessageContaining("DatabaseName must not be empty"); } + @ParameterizedTest(name = "{0} - {1}") + @MethodSource("embeddedDriverAndUrlParameters") + void isEmbeddedWithDriverAndUrl(String driverClassName, String url, boolean embedded) { + assertThat(EmbeddedDatabaseConnection.isEmbedded(driverClassName, url)).isEqualTo(embedded); + } + + static Object[] embeddedDriverAndUrlParameters() { + return new Object[] { + new Object[] { EmbeddedDatabaseConnection.H2.getDriverClassName(), "jdbc:h2:~/test", false }, + new Object[] { EmbeddedDatabaseConnection.H2.getDriverClassName(), "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1", + true }, + new Object[] { EmbeddedDatabaseConnection.H2.getDriverClassName(), null, true }, + new Object[] { EmbeddedDatabaseConnection.HSQL.getDriverClassName(), "jdbc:hsqldb:hsql://localhost", + false }, + new Object[] { EmbeddedDatabaseConnection.HSQL.getDriverClassName(), "jdbc:hsqldb:mem:test", true }, + new Object[] { EmbeddedDatabaseConnection.HSQL.getDriverClassName(), null, true }, + new Object[] { EmbeddedDatabaseConnection.DERBY.getDriverClassName(), "jdbc:derby:memory:test", true }, + new Object[] { EmbeddedDatabaseConnection.DERBY.getDriverClassName(), null, true }, + new Object[] { "com.mysql.cj.jdbc.Driver", "jdbc:mysql:mem:test", false }, + new Object[] { "com.mysql.cj.jdbc.Driver", null, false }, + new Object[] { null, "jdbc:none:mem:test", false }, new Object[] { null, null, false } }; + } + + @Test + void isEmbeddedWithH2DataSource() { + testEmbeddedDatabase(new EmbeddedDatabaseBuilder().setType(EmbeddedDatabaseType.H2).build()); + } + + @Test + void isEmbeddedWithHsqlDataSource() { + testEmbeddedDatabase(new EmbeddedDatabaseBuilder().setType(EmbeddedDatabaseType.HSQL).build()); + } + + @Test + void isEmbeddedWithDerbyDataSource() { + testEmbeddedDatabase(new EmbeddedDatabaseBuilder().setType(EmbeddedDatabaseType.DERBY).build()); + } + + void testEmbeddedDatabase(EmbeddedDatabase database) { + try { + assertThat(EmbeddedDatabaseConnection.isEmbedded(database)).isTrue(); + } + finally { + database.shutdown(); + } + } + + @Test + void isEmbeddedWithUnknownDataSource() throws SQLException { + assertThat(EmbeddedDatabaseConnection.isEmbedded(mockDataSource("unknown-db", null))).isFalse(); + } + + @Test + void isEmbeddedWithH2File() throws SQLException { + assertThat(EmbeddedDatabaseConnection + .isEmbedded(mockDataSource(EmbeddedDatabaseConnection.H2.getDriverClassName(), "jdbc:h2:~/test"))) + .isFalse(); + } + + @Test + void isEmbeddedWithMissingDriverClassMetadata() throws SQLException { + assertThat(EmbeddedDatabaseConnection.isEmbedded(mockDataSource(null, "jdbc:h2:meme:test"))).isFalse(); + } + + @Test + void isEmbeddedWithMissingUrlMetadata() throws SQLException { + assertThat(EmbeddedDatabaseConnection + .isEmbedded(mockDataSource(EmbeddedDatabaseConnection.H2.getDriverClassName(), null))).isTrue(); + } + + DataSource mockDataSource(String productName, String connectionUrl) throws SQLException { + DatabaseMetaData metaData = mock(DatabaseMetaData.class); + given(metaData.getDatabaseProductName()).willReturn(productName); + given(metaData.getURL()).willReturn(connectionUrl); + Connection connection = mock(Connection.class); + given(connection.getMetaData()).willReturn(metaData); + DataSource dataSource = mock(DataSource.class); + given(dataSource.getConnection()).willReturn(connection); + return dataSource; + } + }