accomodate manually initiated rollbacks. Fixes #1137 & fixes #1136.

PiperOrigin-RevId: 525746602

Author: sameb

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 ([email protected]) */
+/**
+ * @author Dhanji R. Prasanna ([email protected])
+ */
 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,28 +133,28 @@ private boolean rollbackIfNecessary(
       Transactional transactional, Exception e, EntityTransaction txn) {
     boolean commit = true;
 
-    //check rollback clauses
+    // check rollback clauses
     for (Class<? extends Exception> 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<? extends Exception> 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;
             break;
           }
         }
 
-        //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;
       }

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 ([email protected]) */
+/**
+ * @author Dhanji R. Prasanna ([email protected])
+ */
 
 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();
+    }
   }
 }