diff --git a/src/main/java/com/avaje/ebean/BeanState.java b/src/main/java/com/avaje/ebean/BeanState.java index 68cd12ab5..a6708a783 100644 --- a/src/main/java/com/avaje/ebean/BeanState.java +++ b/src/main/java/com/avaje/ebean/BeanState.java @@ -120,4 +120,9 @@ public interface BeanState { * for a fully loaded entity bean. */ void setLoaded(); + + /** + * Reset the bean putting it into NEW state such that a save() results in an insert. + */ + void resetForInsert(); } \ No newline at end of file diff --git a/src/main/java/com/avaje/ebean/bean/EntityBeanIntercept.java b/src/main/java/com/avaje/ebean/bean/EntityBeanIntercept.java index 1f9af8aa9..22d68e0e4 100644 --- a/src/main/java/com/avaje/ebean/bean/EntityBeanIntercept.java +++ b/src/main/java/com/avaje/ebean/bean/EntityBeanIntercept.java @@ -342,6 +342,13 @@ public final class EntityBeanIntercept implements Serializable { return state == STATE_LOADED; } + /** + * Set the bean into NEW state. + */ + public void setNew() { + this.state = STATE_NEW; + } + /** * Set the loaded state to true. *

diff --git a/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java b/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java index 44f7eb62c..6a5354061 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/api/ScopedTransaction.java @@ -29,10 +29,9 @@ public class ScopedTransaction implements SpiTransaction { public ScopedTransaction(ScopeTrans scopeTrans) { this.scopeTrans = scopeTrans; - this.transaction =scopeTrans.getTransaction(); + this.transaction = scopeTrans.getTransaction(); } - @Override public void commit() throws RollbackException { scopeTrans.commitTransaction(); @@ -49,6 +48,11 @@ public class ScopedTransaction implements SpiTransaction { scopeTrans.rollback(e); } + @Override + public void rollbackIfActive() { + transaction.rollbackIfActive(); + } + @Override public void setRollbackOnly() { scopeTrans.setRollbackOnly(); diff --git a/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java b/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java index e51719579..0f3a6ae1e 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/api/SpiTransaction.java @@ -220,6 +220,12 @@ public interface SpiTransaction extends Transaction { */ Connection getInternalConnection(); + /** + * Rollback if the transaction is active. This provides an internal + * mechanism for rollback failures occur on commit(). + */ + void rollbackIfActive(); + /** * Return true if the manyToMany intersection should be persisted for this particular relationship direction. */ diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/BeanRequest.java b/src/main/java/com/avaje/ebeaninternal/server/core/BeanRequest.java index b5907bfe4..9ba4070e7 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/BeanRequest.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/BeanRequest.java @@ -68,11 +68,11 @@ public abstract class BeanRequest { public void rollbackTransIfRequired() { if (createdTransaction) { try { - transaction.rollback(); + transaction.rollbackIfActive(); } catch (Exception e) { // Just log this and carry on. A previous exception has been // thrown and if this rollback throws exception it likely means - // that the connection is broken (and the datasource and db will cleanup) + // that the connection is broken (and the dataSource and db will cleanup) log.error("Error trying to rollback a transaction (after a prior exception thrown)", e); } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBeanState.java b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBeanState.java index d31135827..1a20d6b04 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBeanState.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/DefaultBeanState.java @@ -84,4 +84,9 @@ public class DefaultBeanState implements BeanState { public boolean isDisableLazyLoad() { return intercept.isDisableLazyLoad(); } + + @Override + public void resetForInsert() { + intercept.setNew(); + } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java b/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java index 55c997538..fa954bcff 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/PersistRequestBean.java @@ -821,7 +821,7 @@ public final class PersistRequestBean extends PersistRequest implements BeanP } /** - * Add the bean to the TransactionEvent. This will be used by TransactionManager to synch Cache, + * Add the bean to the TransactionEvent. This will be used by TransactionManager to sync Cache, * Cluster and text indexes. */ private void addEvent() { diff --git a/src/main/java/com/avaje/ebeaninternal/server/core/TransWrapper.java b/src/main/java/com/avaje/ebeaninternal/server/core/TransWrapper.java index 3ad7b74bc..36fb3de0f 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/core/TransWrapper.java +++ b/src/main/java/com/avaje/ebeaninternal/server/core/TransWrapper.java @@ -41,7 +41,7 @@ final class TransWrapper { void rollbackIfCreated() { if (wasCreated){ - transaction.rollback(); + transaction.rollbackIfActive(); } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java b/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java index 79fe82379..1cc759447 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java +++ b/src/main/java/com/avaje/ebeaninternal/server/transaction/JdbcTransaction.java @@ -913,28 +913,27 @@ public class JdbcTransaction implements SpiTransaction { throw new IllegalStateException(illegalStateMessage); } - firePreCommit(); - try { if (queryOnly) { - // can rollback or just close for performance connectionEndForQueryOnly(); + } else { - // commit if (batchControl != null && !batchControl.isEmpty()) { batchControl.flush(); } + firePreCommit(); + // only performCommit can throw an exception performCommit(); + firePostCommit(); + notifyCommit(); } } catch (Exception e) { + doRollback(e); throw new RollbackException(e); } finally { - // these will not throw an exception - firePostCommit(); deactivate(); - notifyCommit(); } } @@ -959,6 +958,16 @@ public class JdbcTransaction implements SpiTransaction { this.rollbackOnly = true; } + /** + * Perform rollback is the transaction is still active. + */ + @Override + public void rollbackIfActive() { + if (isActive()) { + rollback(null); + } + } + /** * Rollback the transaction. */ @@ -976,17 +985,26 @@ public class JdbcTransaction implements SpiTransaction { if (!isActive()) { throw new IllegalStateException(illegalStateMessage); } + try { + doRollback(cause); + } finally { + deactivate(); + } + } + + /** + * Perform the jdbc rollback and fire any registered callbacks. + */ + private void doRollback(Throwable cause) { firePreRollback(); try { performRollback(); - - } catch (Exception ex) { + } catch (SQLException ex) { throw new PersistenceException(ex); } finally { // these will not throw an exception firePostRollback(); - deactivate(); notifyRollback(cause); } } diff --git a/src/test/java/com/avaje/tests/transaction/TestBeanStateReset.java b/src/test/java/com/avaje/tests/transaction/TestBeanStateReset.java new file mode 100644 index 000000000..9825834ec --- /dev/null +++ b/src/test/java/com/avaje/tests/transaction/TestBeanStateReset.java @@ -0,0 +1,50 @@ +package com.avaje.tests.transaction; + +import com.avaje.ebean.BaseTestCase; +import com.avaje.ebean.Ebean; +import com.avaje.tests.model.m2m.MnyB; +import com.avaje.tests.model.m2m.MnyC; +import org.avaje.ebeantest.LoggedSqlCollector; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.persistence.PersistenceException; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestBeanStateReset extends BaseTestCase { + + private static final Logger logger = LoggerFactory.getLogger(TestBeanStateReset.class); + + @Test + public void resetForInsert() { + + // setup to fail foreign key constraint + MnyC c = new MnyC(); + c.setId(Long.MAX_VALUE); + + MnyB b = new MnyB(); + b.getCs().add(c); + + try { + // inserts of b succeeds but intersection insert fails FK check on c + b.save(); + + } catch (PersistenceException e) { + logger.info("expected error " + e.getMessage()); + + Ebean.getBeanState(b).resetForInsert(); + b.getCs().clear(); + + LoggedSqlCollector.start(); + b.setName("mod"); + b.save(); + + List sql = LoggedSqlCollector.stop(); + assertThat(sql.get(0)).contains("insert into mny_b (id, name, version, when_created, when_modified, a_id) values ("); + } + + } +} diff --git a/src/test/java/com/avaje/tests/transaction/TestTransactionCallback.java b/src/test/java/com/avaje/tests/transaction/TestTransactionCallback.java index e8f3ba85c..f4e7f3370 100644 --- a/src/test/java/com/avaje/tests/transaction/TestTransactionCallback.java +++ b/src/test/java/com/avaje/tests/transaction/TestTransactionCallback.java @@ -3,6 +3,7 @@ package com.avaje.tests.transaction; import com.avaje.ebean.BaseTestCase; import com.avaje.ebean.Ebean; import com.avaje.ebean.EbeanServer; +import com.avaje.ebean.Transaction; import com.avaje.ebean.TransactionCallbackAdapter; import org.junit.Test; @@ -27,8 +28,9 @@ public class TestTransactionCallback extends BaseTestCase { public void test_commitAndRollback() { - Ebean.beginTransaction(); + Transaction txn = Ebean.beginTransaction(); Ebean.register(new MyCallback()); + txn.getConnection(); Ebean.commitTransaction(); assertEquals(1, countPreCommit);