GH-804 - Polishing.

Actively reject attempts to use a dedicated schema with SQL Server. Prevent schema reset being executed if no schema is used. Properly report exception during database name detection. Original pull request: GH-808.
spring-projects · Sep 11, 2024 · 03a6700 · 03a6700
1 parent a1f2eb0
commit 03a6700
Show file tree

Hide file tree

Showing 9 changed files with 43 additions and 17 deletions.
diff --git a/...dbc/src/main/java/org/springframework/modulith/events/jdbc/DatabaseSchemaInitializer.java b/...dbc/src/main/java/org/springframework/modulith/events/jdbc/DatabaseSchemaInitializer.java
@@ -67,8 +67,9 @@ public void afterPropertiesSet() throws Exception {
 
 			var initialSchema = connection.getSchema();
 			var schemaName = properties.getSchema();
+			var useSchema = schemaName != null && !schemaName.isEmpty();
 
-			if (schemaName != null && !schemaName.isEmpty()) { // A schema name has been specified.
+			if (useSchema) { // A schema name has been specified.
 
 				if (eventPublicationTableExists(schemaName)) {
 					return;
@@ -82,7 +83,7 @@ public void afterPropertiesSet() throws Exception {
 			new ResourceDatabasePopulator(locator.getSchemaResource(databaseType)).execute(dataSource);
 
 			// Return to the initial schema.
-			if (initialSchema != null) {
+			if (initialSchema != null && useSchema) {
 				jdbcOperations.execute(databaseType.getSetSchemaSql(initialSchema));
 			}
 		}

diff --git a/...lith-events-jdbc/src/main/java/org/springframework/modulith/events/jdbc/DatabaseType.java b/...lith-events-jdbc/src/main/java/org/springframework/modulith/events/jdbc/DatabaseType.java
@@ -43,11 +43,16 @@ Object uuidToDatabase(UUID id) {
 		UUID databaseToUUID(Object id) {
 			return UUID.fromString(id.toString());
 		}
+
+		@Override
+		boolean isSchemaSupported() {
+			return false;
+		}
 	},
 
 	POSTGRES("postgresql", "PostgreSQL"),
 
-	MSSQL("sqlserver", "Microsoft SQL Server"){
+	MSSQL("sqlserver", "Microsoft SQL Server") {
 
 		@Override
 		Object uuidToDatabase(UUID id) {
@@ -58,6 +63,11 @@ Object uuidToDatabase(UUID id) {
 		UUID databaseToUUID(Object id) {
 			return UUID.fromString(id.toString());
 		}
+
+		@Override
+		boolean isSchemaSupported() {
+			return false;
+		}
 	};
 
 	static final String SCHEMA_NOT_SUPPORTED = "Setting the schema name not supported on MySQL!";
@@ -92,14 +102,21 @@ String getSchemaResourceFilename() {
 		return "/schema-" + value + ".sql";
 	}
 
+	boolean isSchemaSupported() {
+		return true;
+	}
+
 	String getSetSchemaSql(String schema) {
 
+		if (!isSchemaSupported()) {
+			throw new IllegalArgumentException(SCHEMA_NOT_SUPPORTED);
+		}
+
 		return switch (this) {
 
-			case MYSQL -> throw new IllegalArgumentException(SCHEMA_NOT_SUPPORTED);
 			case H2, HSQLDB -> "SET SCHEMA " + schema;
 			case POSTGRES -> "SET search_path TO " + schema;
-			case MSSQL -> "";  // No equivalent schema-switching command in MSSQL
+			default -> throw new IllegalArgumentException(SCHEMA_NOT_SUPPORTED);
 		};
 	}
 }
diff --git a/...c/src/main/java/org/springframework/modulith/events/jdbc/JdbcConfigurationProperties.java b/...c/src/main/java/org/springframework/modulith/events/jdbc/JdbcConfigurationProperties.java
@@ -64,7 +64,7 @@ public String getSchema() {
 
 	void verify(DatabaseType databaseType) {
 
-		if (schema != null && databaseType.equals(DatabaseType.MYSQL)) {
+		if (schema != null && !databaseType.isSchemaSupported()) {
 			throw new IllegalStateException(DatabaseType.SCHEMA_NOT_SUPPORTED);
 		}
 	}

diff --git a/.../java/org/springframework/modulith/events/jdbc/JdbcEventPublicationAutoConfiguration.java b/.../java/org/springframework/modulith/events/jdbc/JdbcEventPublicationAutoConfiguration.java
@@ -79,7 +79,9 @@ private static String fromDataSource(DataSource dataSource) {
 			var metadata = JdbcUtils.extractDatabaseMetaData(dataSource, DatabaseMetaData::getDatabaseProductName);
 			name = JdbcUtils.commonDatabaseName(metadata);
 
-		} catch (Exception o_O) {}
+		} catch (Exception o_O) {
+			throw new RuntimeException(o_O);
+		}
 
 		return name == null ? "UNKNOWN" : name;
 	}

diff --git a/spring-modulith-events/spring-modulith-events-jdbc/src/main/resources/schema-sqlserver.sql b/spring-modulith-events/spring-modulith-events-jdbc/src/main/resources/schema-sqlserver.sql
@@ -9,4 +9,4 @@ CREATE TABLE EVENT_PUBLICATION
   COMPLETION_DATE  DATETIME2(6) NULL,
   PRIMARY KEY (ID),
   INDEX EVENT_PUBLICATION_BY_COMPLETION_DATE_IDX (COMPLETION_DATE)
-);
+);
diff --git a/...est/java/org/springframework/modulith/events/jdbc/DatabaseSchemaInitializerUnitTests.java b/...est/java/org/springframework/modulith/events/jdbc/DatabaseSchemaInitializerUnitTests.java
@@ -44,11 +44,18 @@ class DatabaseSchemaInitializerUnitTests {
 	@ParameterizedTest
 	@ValueSource(strings = { "", "test" })
 	void rejectsExplicitSchemaNameForMySql(String schema) {
-		assertThatIllegalStateException().isThrownBy(() -> createInitializer(withSchema(schema)));
+		assertThatIllegalStateException().isThrownBy(() -> createInitializer(withSchema(schema), DatabaseType.MYSQL));
 	}
 
-	private DatabaseSchemaInitializer createInitializer(JdbcConfigurationProperties properties) {
-		return new DatabaseSchemaInitializer(dataSource, resourceLoader, DatabaseType.MYSQL, jdbcTemplate, properties);
+	// GH-804
+	@ParameterizedTest
+	@ValueSource(strings = { "", "test" })
+	void rejectsExplicitSchemaNameForMSSql(String schema) {
+		assertThatIllegalStateException().isThrownBy(() -> createInitializer(withSchema(schema), DatabaseType.MSSQL));
+	}
+
+	private DatabaseSchemaInitializer createInitializer(JdbcConfigurationProperties properties, DatabaseType type) {
+		return new DatabaseSchemaInitializer(dataSource, resourceLoader, type, jdbcTemplate, properties);
 	}
 
 	private static JdbcConfigurationProperties withSchema(String schema) {

diff --git a/.../springframework/modulith/events/jdbc/JdbcEventPublicationRepositoryIntegrationTests.java b/.../springframework/modulith/events/jdbc/JdbcEventPublicationRepositoryIntegrationTests.java
@@ -36,7 +36,6 @@
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.autoconfigure.jdbc.JdbcTest;
-import org.springframework.test.context.bean.override.mockito.MockitoBean;
 import org.springframework.context.annotation.Import;
 import org.springframework.jdbc.core.JdbcOperations;
 import org.springframework.modulith.events.core.EventSerializer;
@@ -46,6 +45,7 @@
 import org.springframework.modulith.testapp.TestApplication;
 import org.springframework.test.context.ActiveProfiles;
 import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.bean.override.mockito.MockitoBean;
 import org.testcontainers.junit.jupiter.Testcontainers;
 
 /**
@@ -454,11 +454,9 @@ class PostgresWithEmptySchemaName extends WithEmptySchemaName {}
 	class PostgresWithDeleteCompletion extends WithDeleteCompletion {}
 
 	// MSSQL
-	@WithMssql
-	class MssqlWithNoDefinedSchemaName extends WithNoDefinedSchemaName {}
 
 	@WithMssql
-	class MssqlWithEmptySchemaName extends WithEmptySchemaName {}
+	class MssqlWithNoDefinedSchemaName extends WithNoDefinedSchemaName {}
 
 	@WithMssql
 	class MssqlWithDeleteCompletion extends WithDeleteCompletion {}

diff --git a/...dulith-events/spring-modulith-events-jdbc/src/test/resources/application-mssql.properties b/...dulith-events/spring-modulith-events-jdbc/src/test/resources/application-mssql.properties
@@ -1,2 +1,2 @@
-spring.datasource.url=jdbc:tc:sqlserver:2022-latest:///events;encrypt=false;TC_ACCEPT_LICENSE=accept
-spring.datasource.driverClassName=org.testcontainers.jdbc.ContainerDatabaseDriver
+spring.datasource.url=jdbc:tc:sqlserver:2022-latest:///events;encrypt=false
+spring.datasource.driverClassName=org.testcontainers.jdbc.ContainerDatabaseDriver
diff --git a/...th-events/spring-modulith-events-jdbc/src/test/resources/container-license-acceptance.txt b/...th-events/spring-modulith-events-jdbc/src/test/resources/container-license-acceptance.txt
@@ -0,0 +1 @@
+mcr.microsoft.com/mssql/server:2022-latest