From 1378d3100e7bc4700120eb9fbdebd669d52dfcab Mon Sep 17 00:00:00 2001 From: Rsl1122 Date: Sun, 17 Feb 2019 22:52:30 +0200 Subject: [PATCH] Transaction rollback fail no longer hides original exception --- .../exceptions/database/DBOpException.java | 2 +- .../java/com/djrapitops/plan/db/MySQLDB.java | 7 ++-- .../db/access/transactions/Transaction.java | 40 +++++++------------ .../plan/db/DBPatchMySQLRegressionTest.java | 13 +++++- 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/Plan/common/src/main/java/com/djrapitops/plan/api/exceptions/database/DBOpException.java b/Plan/common/src/main/java/com/djrapitops/plan/api/exceptions/database/DBOpException.java index de3b398d1..5e8fc3448 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/api/exceptions/database/DBOpException.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/api/exceptions/database/DBOpException.java @@ -23,7 +23,7 @@ import java.sql.SQLException; * * @author Rsl1122 */ -public class DBOpException extends RuntimeException { +public class DBOpException extends IllegalStateException { public DBOpException(String message) { super(message); diff --git a/Plan/common/src/main/java/com/djrapitops/plan/db/MySQLDB.java b/Plan/common/src/main/java/com/djrapitops/plan/db/MySQLDB.java index 035538fd5..4f6ab35c1 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/db/MySQLDB.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/db/MySQLDB.java @@ -108,9 +108,7 @@ public class MySQLDB extends SQLDB { hikariConfig.setLeakDetectionThreshold(TimeUnit.MINUTES.toMillis(10L)); this.dataSource = new HikariDataSource(hikariConfig); - - getConnection(); - } catch (HikariPool.PoolInitializationException | SQLException e) { + } catch (HikariPool.PoolInitializationException e) { throw new DBInitException("Failed to set-up HikariCP Datasource: " + e.getMessage(), e); } } @@ -126,11 +124,12 @@ public class MySQLDB extends SQLDB { try { setupDataSource(); // get new connection after restarting pool - return dataSource.getConnection(); + connection = dataSource.getConnection(); } catch (DBInitException e) { throw new DBOpException("Failed to restart DataSource after a connection was invalid: " + e.getMessage(), e); } } + if (connection.getAutoCommit()) connection.setAutoCommit(false); return connection; } diff --git a/Plan/common/src/main/java/com/djrapitops/plan/db/access/transactions/Transaction.java b/Plan/common/src/main/java/com/djrapitops/plan/db/access/transactions/Transaction.java index a4f6cb49c..25651fe1e 100644 --- a/Plan/common/src/main/java/com/djrapitops/plan/db/access/transactions/Transaction.java +++ b/Plan/common/src/main/java/com/djrapitops/plan/db/access/transactions/Transaction.java @@ -64,12 +64,25 @@ public abstract class Transaction { try { initializeTransaction(db); performOperations(); + if (connection != null) connection.commit(); success = true; + } catch (Exception statementFail) { + manageFailure(statementFail); // Throws a DBOpException. } finally { - finalizeTransaction(); + db.returnToPool(connection); } } + private void manageFailure(Exception statementFail) { + String failMsg = getClass().getSimpleName() + " failed: " + statementFail.getMessage(); + try { + connection.rollback(savepoint); + } catch (SQLException rollbackFail) { + throw new DBOpException(failMsg + ", additionally Transaction rollback failed: " + rollbackFail.getMessage(), statementFail); + } + throw new DBOpException(failMsg + ", Transaction was rolled back.", statementFail); + } + /** * Override this method for conditional execution. *

@@ -90,37 +103,12 @@ public abstract class Transaction { private void initializeTransaction(SQLDB db) { try { this.connection = db.getConnection(); - // Temporary fix for MySQL Patch task test failing, TODO remove after Auto commit is off for MySQL - if (connection.getAutoCommit()) connection.setAutoCommit(false); this.savepoint = connection.setSavepoint(); } catch (SQLException e) { throw new DBOpException(getClass().getSimpleName() + " initialization failed: " + e.getMessage(), e); } } - private void finalizeTransaction() { - try { - handleSavepoint(); - // Temporary fix for MySQL Patch task test failing, TODO remove after Auto commit is off for MySQL - if (db.getType() == DBType.MYSQL) connection.setAutoCommit(true); - } catch (SQLException e) { - throw new DBOpException(getClass().getSimpleName() + " finalization failed: " + e.getMessage(), e); - } - if (db != null) db.returnToPool(connection); - } - - private void handleSavepoint() throws SQLException { - if (connection == null) { - return; - } - // Commit or rollback the transaction - if (success) { - connection.commit(); - } else { - connection.rollback(savepoint); - } - } - protected T query(Query query) { return query.executeQuery(db); } diff --git a/Plan/common/src/test/java/com/djrapitops/plan/db/DBPatchMySQLRegressionTest.java b/Plan/common/src/test/java/com/djrapitops/plan/db/DBPatchMySQLRegressionTest.java index ca23a3e03..819da1b77 100644 --- a/Plan/common/src/test/java/com/djrapitops/plan/db/DBPatchMySQLRegressionTest.java +++ b/Plan/common/src/test/java/com/djrapitops/plan/db/DBPatchMySQLRegressionTest.java @@ -94,7 +94,7 @@ public class DBPatchMySQLRegressionTest extends DBPatchRegressionTest { underTest.setTransactionExecutorServiceProvider(MoreExecutors::newDirectExecutorService); underTest.init(); - dropAllTables(underTest); + dropAllTables(); // Initialize database with the old table schema underTest.executeTransaction(new Transaction() { @@ -122,6 +122,17 @@ public class DBPatchMySQLRegressionTest extends DBPatchRegressionTest { insertData(underTest); } + private void dropAllTables() { + underTest.executeTransaction(new Transaction() { + @Override + protected void performOperations() { + execute("DROP DATABASE Plan"); + execute("CREATE DATABASE Plan"); + execute("USE Plan"); + } + }); + } + @After public void closeDatabase() { underTest.close();