diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationEngine.java b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationEngine.java index 7490799..17f5c95 100644 --- a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationEngine.java +++ b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationEngine.java @@ -5,7 +5,6 @@ import io.ebean.migration.MigrationException; import io.ebean.migration.MigrationResource; -import java.io.IOException; import java.sql.Connection; import java.sql.SQLException; import java.util.Collections; @@ -44,7 +43,6 @@ public List run(Connection connection) { } long startMs = System.currentTimeMillis(); - connection.setAutoCommit(false); MigrationTable table = initialiseMigrationTable(connection); try { List result = runMigrations(resources.versions(), table, checkStateOnly); @@ -58,30 +56,34 @@ public List run(Connection connection) { } } return result; + } catch (MigrationException e) { + rollback(connection); + throw e; + } catch (Exception e) { + log.log(ERROR, "Perform rollback due to DB migration error", e); + rollback(connection); + throw new MigrationException("Error running DB migrations", e); } finally { table.unlockMigrationTable(); } - - } catch (MigrationException e) { - rollback(connection); - throw e; - - } catch (Exception e) { - rollback(connection); - throw new MigrationException("Error running DB migrations", e); - } finally { close(connection); } } - private MigrationTable initialiseMigrationTable(Connection connection) throws SQLException, IOException { - MigrationPlatform platform = derivePlatformName(migrationConfig, connection); - new MigrationSchema(migrationConfig, connection).createAndSetIfNeeded(); + private MigrationTable initialiseMigrationTable(Connection connection) { + try { + connection.setAutoCommit(false); + MigrationPlatform platform = derivePlatformName(migrationConfig, connection); + new MigrationSchema(migrationConfig, connection).createAndSetIfNeeded(); - final MigrationTable table = new MigrationTable(migrationConfig, connection, checkStateOnly, platform); - table.createIfNeededAndLock(); - return table; + final MigrationTable table = new MigrationTable(migrationConfig, connection, checkStateOnly, platform); + table.createIfNeededAndLock(); + return table; + } catch (Exception e) { + rollback(connection); + throw new MigrationException("Error initialising db migrations table", e); + } } /** @@ -144,7 +146,7 @@ private void close(Connection connection) { /** * Rollback the connection logging if an error occurs. */ - private void rollback(Connection connection) { + static void rollback(Connection connection) { try { if (connection != null) { connection.rollback(); diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationPlatform.java b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationPlatform.java index e0e267e..1c41987 100644 --- a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationPlatform.java +++ b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationPlatform.java @@ -1,6 +1,7 @@ package io.ebean.migration.runner; import io.ebean.ddlrunner.DdlDetect; +import io.ebean.migration.MigrationException; import java.sql.*; import java.util.ArrayList; @@ -11,6 +12,7 @@ /** * Handle database platform specific locking on db migration table. */ +@SuppressWarnings({"SqlDialectInspection", "SqlSourceToSinkFlow"}) class MigrationPlatform { private static final System.Logger log = MigrationTable.log; @@ -30,7 +32,7 @@ DdlDetect ddlDetect() { return DdlDetect.NONE; } - void unlockMigrationTable(String sqlTable, Connection connection) throws SQLException { + void unlockMigrationTable(String sqlTable, Connection connection) { // do nothing by default for select for update case } @@ -118,9 +120,14 @@ void lockMigrationTable(String sqlTable, Connection connection) throws SQLExcept } @Override - void unlockMigrationTable(String sqlTable, Connection connection) throws SQLException { - releaseLogicalLock(sqlTable, connection); - connection.commit(); + void unlockMigrationTable(String sqlTable, Connection connection) { + try { + releaseLogicalLock(sqlTable, connection); + connection.commit(); + } catch (SQLException e) { + MigrationEngine.rollback(connection); + throw new MigrationException("Error releasing logical lock for ebean migrations"); + } } private boolean obtainLogicalLock(String sqlTable, Connection connection) throws SQLException { @@ -191,10 +198,14 @@ private boolean obtainNamedLock(Connection connection) throws SQLException { } @Override - void unlockMigrationTable(String sqlTable, Connection connection) throws SQLException { - String hash = Integer.toHexString(connection.getMetaData().getURL().hashCode()); - try (Statement query = connection.createStatement()) { - query.execute("select release_lock('ebean_migration-" + hash + "')"); + void unlockMigrationTable(String sqlTable, Connection connection) { + try { + String hash = Integer.toHexString(connection.getMetaData().getURL().hashCode()); + try (Statement query = connection.createStatement()) { + query.execute("select release_lock('ebean_migration-" + hash + "')"); + } + } catch (SQLException e) { + throw new MigrationException("Error releasing lock for ebean_migration", e); } } } diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationTable.java b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationTable.java index 1ea127f..c708eae 100644 --- a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationTable.java +++ b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationTable.java @@ -176,7 +176,7 @@ private void obtainLockWithWait() throws SQLException { /** * Release a lock on the migration table (MySql, MariaDB only). */ - void unlockMigrationTable() throws SQLException { + void unlockMigrationTable() { platform.unlockMigrationTable(sqlTable, connection); } diff --git a/ebean-migration/src/test/java/io/ebean/migration/MigrationRunnerTest.java b/ebean-migration/src/test/java/io/ebean/migration/MigrationRunnerTest.java index 24bc6bf..eb1df6f 100644 --- a/ebean-migration/src/test/java/io/ebean/migration/MigrationRunnerTest.java +++ b/ebean-migration/src/test/java/io/ebean/migration/MigrationRunnerTest.java @@ -56,6 +56,37 @@ public void run_when_fileSystemResources() { runner.run(); } + @Test + public void run_when_error() throws SQLException { + + DataSourceConfig dataSourceConfig = new DataSourceConfig(); + dataSourceConfig.setDriver("org.h2.Driver"); + dataSourceConfig.setUrl("jdbc:h2:mem:err.db"); + dataSourceConfig.setUsername("sa"); + dataSourceConfig.setPassword(""); + + DataSourcePool dataSource = DataSourceFactory.create("test", dataSourceConfig); + + MigrationConfig config = createMigrationConfig(); + config.setMigrationPath("dbmig_error"); + MigrationRunner runner = new MigrationRunner(config); + try { + runner.run(dataSource); + } catch (Exception expected) { + try (Connection connection = dataSource.getConnection()) { + try (var pstmt = connection.prepareStatement("select count(*) from m1")) { + try (var resultSet = pstmt.executeQuery()) { + assertThat(resultSet.next()).isTrue(); + int val = resultSet.getInt(1); + assertThat(val).isEqualTo(0); + } + } catch (SQLException ex) { + fail(ex); + } + } + } + } + @Test public void run_when_suppliedConnection() { diff --git a/ebean-migration/src/test/resources/dbmig_error/1.1__initial.sql b/ebean-migration/src/test/resources/dbmig_error/1.1__initial.sql new file mode 100644 index 0000000..7d68a2e --- /dev/null +++ b/ebean-migration/src/test/resources/dbmig_error/1.1__initial.sql @@ -0,0 +1 @@ +create table m1 (id integer, acol varchar(20)); diff --git a/ebean-migration/src/test/resources/dbmig_error/1.2__insert.sql b/ebean-migration/src/test/resources/dbmig_error/1.2__insert.sql new file mode 100644 index 0000000..3bedec8 --- /dev/null +++ b/ebean-migration/src/test/resources/dbmig_error/1.2__insert.sql @@ -0,0 +1 @@ +insert into m1 (id, acol) values (1,'hi'); diff --git a/ebean-migration/src/test/resources/dbmig_error/1.3__error.sql b/ebean-migration/src/test/resources/dbmig_error/1.3__error.sql new file mode 100644 index 0000000..8679eef --- /dev/null +++ b/ebean-migration/src/test/resources/dbmig_error/1.3__error.sql @@ -0,0 +1 @@ +not valid sql