Skip to content

Commit

Permalink
Commit in DatabasePopulatorUtils if Connection has auto-commit=false
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sbrannen committed Oct 13, 2021
1 parent 4dac833 commit 89c7797
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 59 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -36,9 +36,14 @@ public abstract class DatabasePopulatorUtils {

/**
* Execute the given {@link DatabasePopulator} against the given {@link DataSource}.
* <p>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");
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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<? extends Driver> driverClass;


@SuppressWarnings("unchecked")
public AutoCommitDisabledH2EmbeddedDatabaseConfigurer() throws Exception {
this.driverClass = (Class<? extends Driver>) 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("");
}

}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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<String> selectFirstNames(DataSource dataSource) {
return new JdbcTemplate(dataSource).queryForList("select first_name from users", String.class);
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -22,7 +22,7 @@
* @author Sam Brannen
* @since 4.0.3
*/
public class HsqlDatabasePopulatorTests extends AbstractDatabasePopulatorTests {
class HsqlDatabasePopulatorTests extends AbstractDatabasePopulatorTests {

@Override
protected EmbeddedDatabaseType getEmbeddedDatabaseType() {
Expand Down

0 comments on commit 89c7797

Please sign in to comment.