From 89c7797ffbe2f50c4087a9ada1ebf2fbd10462d9 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Wed, 13 Oct 2021 19:09:53 +0200 Subject: [PATCH] Commit in DatabasePopulatorUtils if Connection has auto-commit=false Prior to this commit, DatabasePopulatorUtils.execute(...) did not perform a commit for the current Connection. This works for most use cases; however, when DatabasePopulatorUtils is used to execute initialization scripts without a managed transaction -- for example, via a DataSourceInitializer configured as a bean in the ApplicationContext or via Spring Boot configuration in application.properties -- if the underlying database is configured with auto-commit=false, the results of executing the SQL scripts are not committed to the database which can lead to data being silently lost. This commit addresses this issue by committing the Connection for the supplied DataSource if the connection is not configured for auto-commit and is not transactional. Existing use cases running with a managed transaction should therefore not be affected by this change. Closes gh-27008 --- .../jdbc/datasource/DataSourceUtils.java | 6 +- .../init/DatabasePopulatorUtils.java | 10 +- ...tDisabledH2EmbeddedDatabaseConfigurer.java | 51 ++++++++ .../H2DatabasePopulatorIntegrationTests.java | 54 --------- .../init/H2DatabasePopulatorTests.java | 111 ++++++++++++++++++ .../init/HsqlDatabasePopulatorTests.java | 4 +- 6 files changed, 177 insertions(+), 59 deletions(-) create mode 100644 spring-jdbc/src/test/java/org/springframework/jdbc/datasource/embedded/AutoCommitDisabledH2EmbeddedDatabaseConfigurer.java delete mode 100644 spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorIntegrationTests.java create mode 100644 spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorTests.java diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java index 67c8338394db..ae708030bcc1 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-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. @@ -72,7 +72,8 @@ public abstract class DataSourceUtils { * @return a JDBC Connection from the given DataSource * @throws org.springframework.jdbc.CannotGetJdbcConnectionException * if the attempt to get a Connection failed - * @see #releaseConnection + * @see #releaseConnection(Connection, DataSource) + * @see #isConnectionTransactional(Connection, DataSource) */ public static Connection getConnection(DataSource dataSource) throws CannotGetJdbcConnectionException { try { @@ -298,6 +299,7 @@ public static void resetConnectionAfterTransaction(Connection con, @Nullable Int * @param dataSource the DataSource that the Connection was obtained from * (may be {@code null}) * @return whether the Connection is transactional + * @see #getConnection(DataSource) */ public static boolean isConnectionTransactional(Connection con, @Nullable DataSource dataSource) { if (dataSource == null) { diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulatorUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulatorUtils.java index 3861cb5e54b2..f3f0d5780f92 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulatorUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/init/DatabasePopulatorUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-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. @@ -36,9 +36,14 @@ public abstract class DatabasePopulatorUtils { /** * Execute the given {@link DatabasePopulator} against the given {@link DataSource}. + *

As of Spring Framework 5.3.11, the {@link Connection} for the supplied + * {@code DataSource} will be {@linkplain Connection#commit() committed} if + * it is not configured for {@link Connection#getAutoCommit() auto-commit} and + * is not {@linkplain DataSourceUtils#isConnectionTransactional transactional}. * @param populator the {@code DatabasePopulator} to execute * @param dataSource the {@code DataSource} to execute against * @throws DataAccessException if an error occurs, specifically a {@link ScriptException} + * @see DataSourceUtils#isConnectionTransactional(Connection, DataSource) */ public static void execute(DatabasePopulator populator, DataSource dataSource) throws DataAccessException { Assert.notNull(populator, "DatabasePopulator must not be null"); @@ -47,6 +52,9 @@ public static void execute(DatabasePopulator populator, DataSource dataSource) t Connection connection = DataSourceUtils.getConnection(dataSource); try { populator.populate(connection); + if (!connection.getAutoCommit() && !DataSourceUtils.isConnectionTransactional(connection, dataSource)) { + connection.commit(); + } } finally { DataSourceUtils.releaseConnection(connection, dataSource); diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/embedded/AutoCommitDisabledH2EmbeddedDatabaseConfigurer.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/embedded/AutoCommitDisabledH2EmbeddedDatabaseConfigurer.java new file mode 100644 index 000000000000..723d6d915c4e --- /dev/null +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/embedded/AutoCommitDisabledH2EmbeddedDatabaseConfigurer.java @@ -0,0 +1,51 @@ +/* + * Copyright 2002-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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.jdbc.datasource.embedded; + +import java.sql.Driver; + +import org.springframework.util.ClassUtils; + +/** + * {@link EmbeddedDatabaseConfigurer} for an H2 embedded database instance + * with auto-commit disabled. + * + * @author Sam Brannen + * @since 5.3.11 + */ +public class AutoCommitDisabledH2EmbeddedDatabaseConfigurer extends AbstractEmbeddedDatabaseConfigurer { + + private final Class driverClass; + + + @SuppressWarnings("unchecked") + public AutoCommitDisabledH2EmbeddedDatabaseConfigurer() throws Exception { + this.driverClass = (Class) ClassUtils.forName("org.h2.Driver", + AutoCommitDisabledH2EmbeddedDatabaseConfigurer.class.getClassLoader()); + } + + @Override + public void configureConnectionProperties(ConnectionProperties properties, String databaseName) { + String url = String.format("jdbc:h2:mem:%s;DB_CLOSE_DELAY=-1;DB_CLOSE_ON_EXIT=false;AUTOCOMMIT=false", databaseName); + + properties.setDriverClass(this.driverClass); + properties.setUrl(url); + properties.setUsername("sa"); + properties.setPassword(""); + } + +} diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorIntegrationTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorIntegrationTests.java deleted file mode 100644 index 37ea29bbf85c..000000000000 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorIntegrationTests.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2002-2019 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.jdbc.datasource.init; - -import org.junit.jupiter.api.Test; - -import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * @author Sam Brannen - * @since 4.0.3 - */ -class H2DatabasePopulatorIntegrationTests extends AbstractDatabasePopulatorTests { - - @Override - protected EmbeddedDatabaseType getEmbeddedDatabaseType() { - return EmbeddedDatabaseType.H2; - } - - /** - * https://jira.spring.io/browse/SPR-15896 - * - * @since 5.0 - */ - @Test - void scriptWithH2Alias() throws Exception { - databasePopulator.addScript(usersSchema()); - databasePopulator.addScript(resource("db-test-data-h2-alias.sql")); - // Set statement separator to double newline so that ";" is not - // considered a statement separator within the source code of the - // aliased function 'REVERSE'. - databasePopulator.setSeparator("\n\n"); - DatabasePopulatorUtils.execute(databasePopulator, db); - String sql = "select REVERSE(first_name) from users where last_name='Brannen'"; - assertThat(jdbcTemplate.queryForObject(sql, String.class)).isEqualTo("maS"); - } - -} diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorTests.java new file mode 100644 index 000000000000..6a67619bc893 --- /dev/null +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/H2DatabasePopulatorTests.java @@ -0,0 +1,111 @@ +/* + * Copyright 2002-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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.jdbc.datasource.init; + +import java.sql.Connection; +import java.util.List; + +import javax.sql.DataSource; + +import org.junit.jupiter.api.Test; + +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.datasource.DataSourceUtils; +import org.springframework.jdbc.datasource.embedded.AutoCommitDisabledH2EmbeddedDatabaseConfigurer; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseFactory; +import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Sam Brannen + * @since 4.0.3 + */ +class H2DatabasePopulatorTests extends AbstractDatabasePopulatorTests { + + @Override + protected EmbeddedDatabaseType getEmbeddedDatabaseType() { + return EmbeddedDatabaseType.H2; + } + + /** + * https://jira.spring.io/browse/SPR-15896 + * + * @since 5.0 + */ + @Test + void scriptWithH2Alias() throws Exception { + databasePopulator.addScript(usersSchema()); + databasePopulator.addScript(resource("db-test-data-h2-alias.sql")); + // Set statement separator to double newline so that ";" is not + // considered a statement separator within the source code of the + // aliased function 'REVERSE'. + databasePopulator.setSeparator("\n\n"); + DatabasePopulatorUtils.execute(databasePopulator, db); + String sql = "select REVERSE(first_name) from users where last_name='Brannen'"; + assertThat(jdbcTemplate.queryForObject(sql, String.class)).isEqualTo("maS"); + } + + /** + * https://github.com/spring-projects/spring-framework/issues/27008 + * + * @since 5.3.11 + */ + @Test + void automaticallyCommitsIfAutoCommitIsDisabled() throws Exception { + EmbeddedDatabase database = null; + try { + EmbeddedDatabaseFactory databaseFactory = new EmbeddedDatabaseFactory(); + databaseFactory.setDatabaseConfigurer(new AutoCommitDisabledH2EmbeddedDatabaseConfigurer()); + database = databaseFactory.getDatabase(); + + assertAutoCommitDisabledPreconditions(database); + + // Set up schema + databasePopulator.setScripts(usersSchema()); + DatabasePopulatorUtils.execute(databasePopulator, database); + assertThat(selectFirstNames(database)).isEmpty(); + + // Insert data + databasePopulator.setScripts(resource("users-data.sql")); + DatabasePopulatorUtils.execute(databasePopulator, database); + assertThat(selectFirstNames(database)).containsExactly("Sam"); + } + finally { + if (database != null) { + database.shutdown(); + } + } + } + + /** + * DatabasePopulatorUtils.execute() will obtain a new Connection, so we're + * really just testing the configuration of the database here. + */ + private void assertAutoCommitDisabledPreconditions(DataSource dataSource) throws Exception { + Connection connection = DataSourceUtils.getConnection(dataSource); + assertThat(connection.getAutoCommit()).as("auto-commit").isFalse(); + assertThat(DataSourceUtils.isConnectionTransactional(connection, dataSource)).as("transactional").isFalse(); + connection.close(); + } + + private List selectFirstNames(DataSource dataSource) { + return new JdbcTemplate(dataSource).queryForList("select first_name from users", String.class); + } + +} diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/HsqlDatabasePopulatorTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/HsqlDatabasePopulatorTests.java index 0b43cd74a3cf..9135c2055856 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/HsqlDatabasePopulatorTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/init/HsqlDatabasePopulatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-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. @@ -22,7 +22,7 @@ * @author Sam Brannen * @since 4.0.3 */ -public class HsqlDatabasePopulatorTests extends AbstractDatabasePopulatorTests { +class HsqlDatabasePopulatorTests extends AbstractDatabasePopulatorTests { @Override protected EmbeddedDatabaseType getEmbeddedDatabaseType() {