From 7df168f2320b341a7cd3101a2f46d5186a0880b8 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Thu, 1 Sep 2022 17:08:11 +1200 Subject: [PATCH] ENH: Add Transaction.rollbackAndContinue() Typically useful for handling DuplicateKeyException where we expect DuplicateKeyException to be thrown and catch it with the intention of continuing processing using the same transaction. Note that some databases like Oracle do not require this explicit rollback() and would work without the rollbackAndContinue(). Postgres in particular requires the rollback() call on the underlying connection such that we can continue using that transaction/java.sql.Connection. Note that in the existing test we can see that rollbackAndContinue() is pretty close to being syntactic sugar. I think adding rollbackAndContinue() is justified and complements the existing commitAndContinue(). --- .../java/io/ebean/DuplicateKeyException.java | 22 ++++++++++ .../src/main/java/io/ebean/Transaction.java | 29 ++++++++++++++ .../ImplicitReadOnlyTransaction.java | 17 +++++--- .../server/transaction/JdbcTransaction.java | 40 +++++++++++++------ .../server/transaction/TChangeLogHolder.java | 3 ++ .../tests/insert/TestInsertDuplicateKey.java | 13 ++---- 6 files changed, 96 insertions(+), 28 deletions(-) diff --git a/ebean-api/src/main/java/io/ebean/DuplicateKeyException.java b/ebean-api/src/main/java/io/ebean/DuplicateKeyException.java index 681b4dd27..0495015da 100644 --- a/ebean-api/src/main/java/io/ebean/DuplicateKeyException.java +++ b/ebean-api/src/main/java/io/ebean/DuplicateKeyException.java @@ -2,6 +2,28 @@ package io.ebean; /** * Thrown when a duplicate is attempted on a unique constraint. + *

+ * In terms of catching this exception with the view of continuing processing + * using the same transaction look to use {@link Transaction#rollbackAndContinue()}. + * + * 

{@code
+ *
+ *   try (Transaction txn = database.beginTransaction()) {
+ *
+ *     try {
+ *       ...
+ *       database.save(bean);
+ *       database.flush();
+ *     } catch (DuplicateKeyException e) {
+ *       // carry on processing using the transaction
+ *       txn.rollbackAndContinue();
+ *       ...
+ *     }
+ *
+ *     txn.commit();
+ *   }
+ *
+ * }
*/ public class DuplicateKeyException extends DataIntegrityException { private static final long serialVersionUID = -4771932723285724817L; diff --git a/ebean-api/src/main/java/io/ebean/Transaction.java b/ebean-api/src/main/java/io/ebean/Transaction.java index 08c584231..065396a92 100644 --- a/ebean-api/src/main/java/io/ebean/Transaction.java +++ b/ebean-api/src/main/java/io/ebean/Transaction.java @@ -144,6 +144,35 @@ public interface Transaction extends AutoCloseable { */ void rollback(Throwable e) throws PersistenceException; + /** + * Performs a rollback on the underlying JDBC connection with the intention of + * continuing to use this same transaction and performing a commit or rollback + * later to complete the transaction. + *

+ * Typically used when catching {@link DuplicateKeyException} where we wish to + * rollback work done at that point but carry on processing using the transaction. + * + * 

{@code
+   *
+   *   try (Transaction txn = database.beginTransaction()) {
+   *
+   *     try {
+   *       ...
+   *       database.save(bean);
+   *       database.flush();
+   *     } catch (DuplicateKeyException e) {
+   *       // carry on processing using the transaction
+   *       txn.rollbackAndContinue();
+   *       ...
+   *     }
+   *
+   *     txn.commit();
+   *   }
+   *
+   * }
+ */ + void rollbackAndContinue(); + /** * Set when we want nested transactions to use Savepoint's. *

diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java index 2dfa2a454..7ffa36f8d 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/ImplicitReadOnlyTransaction.java @@ -240,7 +240,7 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve @Override public boolean isReadOnly() { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } try { @@ -252,7 +252,7 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve @Override public void setReadOnly(boolean readOnly) { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } try { @@ -403,7 +403,7 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve */ @Override public void setPersistenceContext(SpiPersistenceContext context) { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } this.persistenceContext = context; @@ -465,7 +465,7 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve */ @Override public Connection getInternalConnection() { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } return connection; @@ -508,7 +508,7 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve */ @Override public void commit() { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } // expect AutoCommit so just deactivate / put back into pool @@ -532,6 +532,11 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve throw new IllegalStateException(notExpectedMessage); } + @Override + public void rollbackAndContinue() { + // do nothing + } + /** * Rollback the transaction. */ @@ -546,7 +551,7 @@ final class ImplicitReadOnlyTransaction implements SpiTransaction, TxnProfileEve */ @Override public void rollback(Throwable cause) throws PersistenceException { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } // expect AutoCommit so it really has already committed diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java index 0ad958e25..5d3f9c213 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/JdbcTransaction.java @@ -424,7 +424,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { @Override public boolean isReadOnly() { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } try { @@ -436,7 +436,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { @Override public void setReadOnly(boolean readOnly) { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } try { @@ -459,7 +459,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { @Override public final void setBatchMode(boolean batchMode) { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } this.batchMode = batchMode; @@ -472,7 +472,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { @Override public final void setBatchOnCascade(boolean batchMode) { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } this.batchOnCascadeMode = batchMode; @@ -666,7 +666,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { */ @Override public final void flush() { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } internalBatchFlush(); @@ -707,7 +707,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { */ @Override public final void setPersistenceContext(SpiPersistenceContext context) { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } this.persistenceContext = context; @@ -776,7 +776,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { */ @Override public Connection getInternalConnection() { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } return connection; @@ -920,7 +920,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { if (rollbackOnly) { return; } - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } try { @@ -945,7 +945,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { rollback(); return; } - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } try { @@ -1014,6 +1014,22 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { this.nestedUseSavepoint = true; } + @Override + public void rollbackAndContinue() { + if (!active) { + throw new IllegalStateException(illegalStateMessage); + } + internalBatchClear(); + if (changeLogHolder != null) { + changeLogHolder.clear(); + } + try { + performRollback(); + } catch (SQLException ex) { + throw new PersistenceException(ex); + } + } + /** * Rollback the transaction. */ @@ -1028,7 +1044,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { */ @Override public void rollback(Throwable cause) throws PersistenceException { - if (!isActive()) { + if (!active) { throw new IllegalStateException(illegalStateMessage); } try { @@ -1066,7 +1082,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { */ @Override public void end() throws PersistenceException { - if (isActive()) { + if (active) { rollback(); } } @@ -1075,7 +1091,7 @@ class JdbcTransaction implements SpiTransaction, TxnProfileEventCodes { * Return true if the transaction is active. */ @Override - public boolean isActive() { + public final boolean isActive() { return active; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/TChangeLogHolder.java b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/TChangeLogHolder.java index b2a7269d0..73a497fad 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/transaction/TChangeLogHolder.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/transaction/TChangeLogHolder.java @@ -96,4 +96,7 @@ public final class TChangeLogHolder { owner.sendChangeLog(changes); } + void clear() { + changes.getChanges().clear(); + } } diff --git a/ebean-test/src/test/java/org/tests/insert/TestInsertDuplicateKey.java b/ebean-test/src/test/java/org/tests/insert/TestInsertDuplicateKey.java index 4ffa8f8e9..b894d4de6 100644 --- a/ebean-test/src/test/java/org/tests/insert/TestInsertDuplicateKey.java +++ b/ebean-test/src/test/java/org/tests/insert/TestInsertDuplicateKey.java @@ -1,16 +1,15 @@ package org.tests.insert; -import io.ebean.xtest.BaseTestCase; import io.ebean.DB; import io.ebean.DuplicateKeyException; import io.ebean.annotation.Transactional; +import io.ebean.xtest.BaseTestCase; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.tests.model.draftable.Document; -import java.sql.SQLException; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -92,14 +91,8 @@ public class TestInsertDuplicateKey extends BaseTestCase { DB.getDefault().currentTransaction().flush(); } catch (DuplicateKeyException e) { log.info("duplicate failed but just continue" + e.getMessage()); - try { - // typically we would use transaction.commitAndContinue() - // ... this is a rollback and continue type scenario - // ... more sensible to use a second transaction that do this - DB.getDefault().currentTransaction().connection().rollback(); - } catch (SQLException e1) { - e1.printStackTrace(); - } + // rollback and continue using the transaction + DB.getDefault().currentTransaction().rollbackAndContinue(); } Document doc0 = new Document();