Merge pull request #23693 from somayaj

* pr/23693:
  Polish "Fix detection logic for embedded databases"
  Fix detection logic for embedded databases

Closes gh-23693

Author: snicoll

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/jdbc/DataSourceProperties.java

@@ -325,7 +325,7 @@ public String determineUsername() {
 		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 String determinePassword() {
 		if (StringUtils.hasText(this.password)) {
 			return this.password;
 		}
-		if (EmbeddedDatabaseConnection.isEmbedded(determineDriverClassName())) {
+		if (EmbeddedDatabaseConnection.isEmbedded(determineDriverClassName(), determineUrl())) {
 			return "";
 		}
 		return null;

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/jdbc/DataSourcePropertiesTests.java

@@ -98,6 +98,24 @@ void determineUsername() throws Exception {
 		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 @@ void determineUsernameWithExplicitConfig() throws Exception {
 		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 @@ void determinePasswordWithExplicitConfig() throws Exception {
 		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();

spring-boot-project/spring-boot-docs/src/docs/asciidoc/howto.adoc

@@ -1982,8 +1982,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.
 

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;
 
@@ -44,32 +47,33 @@ 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.
 	 * @deprecated since 2.4.0 in favor of {@link EmbeddedDatabaseConnection#HSQLDB}.
 	 */
 	@Deprecated
 	HSQL(EmbeddedDatabaseType.HSQL, DatabaseDriver.HSQLDB.getDriverClassName(), "org.hsqldb.jdbcDriver",
-			"jdbc:hsqldb:mem:%s"),
+			"jdbc:hsqldb:mem:%s", (url) -> url.contains(":hsqldb:mem:")),
 
 	/**
 	 * HSQL Database Connection.
 	 */
 	HSQLDB(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;
 
@@ -79,15 +83,20 @@ public enum EmbeddedDatabaseConnection {
 
 	private final String url;
 
-	EmbeddedDatabaseConnection(EmbeddedDatabaseType type, String driverClass, String url) {
-		this(type, driverClass, null, url);
+	private final Predicate<String> embeddedUrl;
+
+	EmbeddedDatabaseConnection(EmbeddedDatabaseType type, String driverClass, String url,
+			Predicate<String> 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<String> embeddedUrl) {
 		this.type = type;
 		this.driverClass = driverClass;
 		this.alternativeDriverClass = fallbackDriverClass;
 		this.url = url;
+		this.embeddedUrl = embeddedUrl;
 	}
 
 	/**
@@ -116,19 +125,41 @@ public String getUrl(String databaseName) {
 		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.4.0 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) || matches(HSQLDB, driverClass));
+		return isEmbedded(driverClass, null);
+	}
+
+	/**
+	 * 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) {
+		return driverClass != null && getEmbeddedDatabaseConnection(driverClass).isEmbeddedUrl(url);
 	}
 
-	private static boolean matches(EmbeddedDatabaseConnection candidate, String driverClass) {
-		return driverClass.equals(candidate.driverClass) || driverClass.equals(candidate.alternativeDriverClass);
+	private static EmbeddedDatabaseConnection getEmbeddedDatabaseConnection(String driverClass) {
+		return Stream.of(H2, HSQLDB, DERBY).filter((connection) -> connection.isDriverCompatible(driverClass))
+				.findFirst().orElse(NONE);
 	}
 
 	/**
@@ -169,15 +200,16 @@ private static class IsEmbedded implements ConnectionCallback<Boolean> {
 
 		@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;
 			}
 			productName = productName.toUpperCase(Locale.ENGLISH);
 			EmbeddedDatabaseConnection[] candidates = EmbeddedDatabaseConnection.values();
 			for (EmbeddedDatabaseConnection candidate : candidates) {
 				if (candidate != NONE && productName.contains(candidate.name())) {
-					return true;
+					return candidate.isEmbeddedUrl(metaData.getURL());
 				}
 			}
 			return false;

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}.
@@ -78,4 +92,65 @@ void getUrlWithEmptyDatabaseNameForHsqldb() {
 				.withMessageContaining("DatabaseName must not be empty");
 	}
 
+	@ParameterizedTest(name = "{1}")
+	@MethodSource("embeddedDriverAndUrlParameters")
+	void isEmbeddedWithDriverAndUrl(EmbeddedDatabaseConnection connection, String url, boolean embedded) {
+		assertThat(EmbeddedDatabaseConnection.isEmbedded(connection.getDriverClassName(), url)).isEqualTo(embedded);
+	}
+
+	static Object[] embeddedDriverAndUrlParameters() {
+		return new Object[] { new Object[] { EmbeddedDatabaseConnection.H2, "jdbc:h2:~/test", false },
+				new Object[] { EmbeddedDatabaseConnection.H2, "jdbc:h2:mem:test;DB_CLOSE_DELAY=-1", true },
+				new Object[] { EmbeddedDatabaseConnection.HSQLDB, "jdbc:hsqldb:hsql://localhost", false },
+				new Object[] { EmbeddedDatabaseConnection.HSQLDB, "jdbc:hsqldb:mem:test", true },
+				new Object[] { EmbeddedDatabaseConnection.DERBY, "jdbc:derby:memory:test", true } };
+	}
+
+	@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();
+	}
+
+	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;
+	}
+
 }