diff --git a/extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java b/extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java index 43b7a1eaad..41acf3c199 100644 --- a/extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java +++ b/extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java @@ -25,7 +25,9 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -/** @author Dhanji R. Prasanna (dhanji@gmail.com) */ +/** + * @author Dhanji R. Prasanna (dhanji@gmail.com) + */ class JpaLocalTxnInterceptor implements MethodInterceptor { // TODO(gak): Move these args to the cxtor & make these final. @@ -64,12 +66,12 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable { result = methodInvocation.proceed(); } catch (Exception e) { - //commit transaction only if rollback didnt occur + // commit transaction only if rollback didnt occur if (rollbackIfNecessary(transactional, e, txn)) { txn.commit(); } - //propagate whatever exception is thrown anyway + // propagate whatever exception is thrown anyway throw e; } finally { // Close the em if necessary (guarded so this code doesn't run unless catch fired). @@ -79,19 +81,25 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable { } } - //everything was normal so commit the txn (do not move into try block above as it + // everything was normal so commit the txn (do not move into try block above as it // interferes with the advised method's throwing semantics) try { - txn.commit(); + if (txn.isActive()) { + if (txn.getRollbackOnly()) { + txn.rollback(); + } else { + txn.commit(); + } + } } finally { - //close the em if necessary + // close the em if necessary if (null != didWeStartWork.get()) { didWeStartWork.remove(); unitOfWork.end(); } } - //or return result + // or return result return result; } @@ -125,16 +133,16 @@ private boolean rollbackIfNecessary( Transactional transactional, Exception e, EntityTransaction txn) { boolean commit = true; - //check rollback clauses + // check rollback clauses for (Class rollBackOn : transactional.rollbackOn()) { - //if one matched, try to perform a rollback + // if one matched, try to perform a rollback if (rollBackOn.isInstance(e)) { commit = false; - //check ignore clauses (supercedes rollback clause) + // check ignore clauses (supercedes rollback clause) for (Class exceptOn : transactional.ignore()) { - //An exception to the rollback clause was found, DON'T rollback + // An exception to the rollback clause was found, DON'T rollback // (i.e. commit and throw anyway) if (exceptOn.isInstance(e)) { commit = true; @@ -142,11 +150,11 @@ private boolean rollbackIfNecessary( } } - //rollback only if nothing matched the ignore check + // rollback only if nothing matched the ignore check if (!commit) { txn.rollback(); } - //otherwise continue to commit + // otherwise continue to commit break; } diff --git a/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java b/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java index 167d6a4c14..343c66dee4 100644 --- a/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java +++ b/extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java @@ -16,6 +16,8 @@ package com.google.inject.persist.jpa; +import static org.junit.Assert.assertThrows; + import com.google.inject.Guice; import com.google.inject.Inject; import com.google.inject.Injector; @@ -29,7 +31,9 @@ import javax.persistence.NoResultException; import junit.framework.TestCase; -/** @author Dhanji R. Prasanna (dhanji@gmail.com) */ +/** + * @author Dhanji R. Prasanna (dhanji@gmail.com) + */ public class ManagedLocalTransactionsTest extends TestCase { private Injector injector; @@ -41,7 +45,7 @@ public class ManagedLocalTransactionsTest extends TestCase { public void setUp() { injector = Guice.createInjector(new JpaPersistModule("testUnit")); - //startup persistence + // startup persistence injector.getInstance(PersistService.class).start(); } @@ -57,7 +61,7 @@ public void testSimpleTransaction() { EntityManager em = injector.getInstance(EntityManager.class); assertFalse("txn was not closed by transactional service", em.getTransaction().isActive()); - //test that the data has been stored + // test that the data has been stored Object result = em.createQuery("from JpaTestEntity where text = :text") .setParameter("text", UNIQUE_TEXT) @@ -79,7 +83,7 @@ public void testSimpleTransactionWithMerge() { EntityManager em = injector.getInstance(EntityManager.class); assertFalse("txn was not closed by transactional service", em.getTransaction().isActive()); - //test that the data has been stored + // test that the data has been stored assertTrue("Em was closed after txn!", em.isOpen()); Object result = @@ -100,7 +104,7 @@ public void testSimpleTransactionRollbackOnChecked() { try { injector.getInstance(TransactionalObject.class).runOperationInTxnThrowingChecked(); } catch (IOException e) { - //ignore + // ignore injector.getInstance(UnitOfWork.class).end(); } @@ -110,23 +114,21 @@ public void testSimpleTransactionRollbackOnChecked() { "Previous EM was not closed by transactional service (rollback didnt happen?)", em.getTransaction().isActive()); - //test that the data has been stored - try { - Object result = - em.createQuery("from JpaTestEntity where text = :text") - .setParameter("text", TRANSIENT_UNIQUE_TEXT) - .getSingleResult(); - injector.getInstance(UnitOfWork.class).end(); - fail("a result was returned! rollback sure didnt happen!!!"); - } catch (NoResultException e) { - } + // test that the data has been stored + assertThrows( + NoResultException.class, + () -> + em.createQuery("from JpaTestEntity where text = :text") + .setParameter("text", TRANSIENT_UNIQUE_TEXT) + .getSingleResult()); + injector.getInstance(UnitOfWork.class).end(); } public void testSimpleTransactionRollbackOnUnchecked() { try { injector.getInstance(TransactionalObject.class).runOperationInTxnThrowingUnchecked(); } catch (RuntimeException re) { - //ignore + // ignore injector.getInstance(UnitOfWork.class).end(); } @@ -135,15 +137,47 @@ public void testSimpleTransactionRollbackOnUnchecked() { "Session was not closed by transactional service (rollback didnt happen?)", em.getTransaction().isActive()); - try { - Object result = - em.createQuery("from JpaTestEntity where text = :text") - .setParameter("text", TRANSIENT_UNIQUE_TEXT) - .getSingleResult(); - injector.getInstance(UnitOfWork.class).end(); - fail("a result was returned! rollback sure didnt happen!!!"); - } catch (NoResultException e) { - } + assertThrows( + NoResultException.class, + () -> + em.createQuery("from JpaTestEntity where text = :text") + .setParameter("text", TRANSIENT_UNIQUE_TEXT) + .getSingleResult()); + injector.getInstance(UnitOfWork.class).end(); + } + + public void testSimpleTransactionRollbackPerformedManuallyWithoutException() { + injector.getInstance(TransactionalObject.class).runOperationInTxnWithManualRollback(); + + EntityManager em = injector.getInstance(EntityManager.class); + assertFalse( + "Session was not closed by transactional service (rollback didnt happen?)", + em.getTransaction().isActive()); + + assertThrows( + NoResultException.class, + () -> + em.createQuery("from JpaTestEntity where text = :text") + .setParameter("text", TRANSIENT_UNIQUE_TEXT) + .getSingleResult()); + injector.getInstance(UnitOfWork.class).end(); + } + + public void testSimpleTransactionRollbackOnlySetWithoutException() { + injector.getInstance(TransactionalObject.class).runOperationInTxnWithRollbackOnlySet(); + + EntityManager em = injector.getInstance(EntityManager.class); + assertFalse( + "Session was not closed by transactional service (rollback didnt happen?)", + em.getTransaction().isActive()); + + assertThrows( + NoResultException.class, + () -> + em.createQuery("from JpaTestEntity where text = :text") + .setParameter("text", TRANSIENT_UNIQUE_TEXT) + .getSingleResult()); + injector.getInstance(UnitOfWork.class).end(); } public static class TransactionalObject { @@ -185,5 +219,23 @@ public void runOperationInTxnThrowingUnchecked() { throw new IllegalStateException(); } + + @Transactional + public void runOperationInTxnWithManualRollback() { + JpaTestEntity entity = new JpaTestEntity(); + entity.setText(TRANSIENT_UNIQUE_TEXT); + em.persist(entity); + + em.getTransaction().rollback(); + } + + @Transactional + public void runOperationInTxnWithRollbackOnlySet() { + JpaTestEntity entity = new JpaTestEntity(); + entity.setText(TRANSIENT_UNIQUE_TEXT); + em.persist(entity); + + em.getTransaction().setRollbackOnly(); + } } }