[ARIES-1497] properly handle RollbackException

git-svn-id: https://svn.apache.org/repos/asf/aries/trunk/jpa@1777105 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java b/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java
index f8f984e..e17637d 100644
--- a/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java
+++ b/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java
@@ -19,6 +19,7 @@
 package org.apache.aries.jpa.support.impl;
 
 import javax.persistence.EntityManager;
+import javax.transaction.RollbackException;
 import javax.transaction.Status;
 import javax.transaction.Transaction;
 import javax.transaction.TransactionManager;
@@ -61,6 +62,9 @@
             R result = (R)code.apply(em);
             safeFinish(tranToken, ta, coord);
             return result;
+        } catch (RollbackException ex) {
+            safeRollback(tranToken, ta, coord, ex);
+            throw wrapThrowable(ex, "RollbackException is propagating");  
         } catch (Exception ex) {
             safeRollback(tranToken, ta, coord, ex);
             throw wrapThrowable(ex, "Exception occured in transactional code");
@@ -76,9 +80,12 @@
         }
     }
 
-    private void safeFinish(TransactionToken tranToken, TransactionAttribute ta, Coordination coord) {
+    private void safeFinish(TransactionToken tranToken, TransactionAttribute ta, Coordination coord) throws RollbackException {
         try {
             ta.finish(tm, tranToken);
+        } catch (RollbackException e) {
+            // just rethrow these as they indicate a very special case
+            throw e;
         } catch (Exception e) {
             // We are throwing an exception, so we don't error it out
             LOGGER.debug("Exception during finish of transaction", e);
@@ -88,9 +95,11 @@
     }
 
     private void safeRollback(TransactionToken token, TransactionAttribute ta, Coordination coord, Throwable ex) {
+        LOGGER.warn("Beginning rollback logic due to exception", ex);
         try {
             Transaction tran = token.getActiveTransaction();
             if (tran != null && shouldRollback(ex)) {
+                LOGGER.info("Rolling back TX due to exception", ex);
                 tran.setRollbackOnly();
             }
         } catch (Exception e) {
@@ -98,7 +107,12 @@
             // need to log it
             LOGGER.warn("Exception during transaction rollback", e);
         }
-        safeFinish(token, ta, coord);
+        
+        try {
+            safeFinish(token, ta, coord);
+        } catch (RollbackException e) {
+            LOGGER.warn("RollbackException during safeFinish attempt for already running safeRollback", e);
+        }
     }
 
     private static boolean shouldRollback(Throwable ex) {
diff --git a/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java b/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java
index cb61e5a..ceea963 100644
--- a/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java
+++ b/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java
@@ -101,7 +101,7 @@
         if (tranToken.isCompletionAllowed()) {
           if (man.getStatus() == Status.STATUS_MARKED_ROLLBACK) {
             man.rollback();
-          } else {
+          } else if (man.getStatus() != Status.STATUS_NO_TRANSACTION) {
             man.commit();
           }
         }
@@ -144,7 +144,7 @@
         if (tranToken.isCompletionAllowed()) {
           if (man.getStatus() == Status.STATUS_MARKED_ROLLBACK) {
             man.rollback();
-          } else {
+          } else if (man.getStatus() != Status.STATUS_NO_TRANSACTION) {
             man.commit();
           }
         }
diff --git a/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java b/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java
new file mode 100644
index 0000000..1a9ddb5
--- /dev/null
+++ b/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java
@@ -0,0 +1,91 @@
+package org.apache.aries.jpa.support.impl;
+
+import static org.mockito.Mockito.*;
+
+import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.OptimisticLockException;
+import javax.transaction.HeuristicMixedException;
+import javax.transaction.HeuristicRollbackException;
+import javax.transaction.RollbackException;
+import javax.transaction.Status;
+import javax.transaction.SystemException;
+import javax.transaction.TransactionManager;
+
+import org.apache.aries.jpa.impl.DummyCoordinator;
+import org.apache.aries.jpa.template.EmConsumer;
+import org.apache.aries.jpa.template.TransactionType;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class XAJpaTemplateTest
+{
+    private static EntityManagerFactory mockEmf() {
+        EntityManagerFactory emf = mock(EntityManagerFactory.class);
+        EntityManager em = mock(EntityManager.class);
+        when(emf.createEntityManager()).thenReturn(em);
+        return emf;
+    }
+
+    private EntityManagerFactory emf;
+    private DummyCoordinator coordinator;
+    private EMSupplierImpl emSupplier;
+
+    @Before
+    public void setup() throws IllegalStateException, SecurityException, HeuristicMixedException,
+            HeuristicRollbackException, RollbackException, SystemException {
+        this.emf = mockEmf();
+        this.coordinator = new DummyCoordinator();
+        this.emSupplier = new EMSupplierImpl("myunit", emf, coordinator);
+
+    }
+    
+    @After
+    public void cleanup() {
+        this.emSupplier.close();
+    }
+
+    private TransactionManager mockTm() {
+        TransactionManager tm = mock(TransactionManager.class);
+        return tm;
+    }
+
+    @Test
+    public void test_RollbackExceptionHandling_rollbackiscalledonmarkedrollback() throws Exception {
+        TransactionManager tm = mockTm();
+        when(tm.getStatus()).thenReturn(Status.STATUS_NO_TRANSACTION,
+                Status.STATUS_MARKED_ROLLBACK);
+        XAJpaTemplate tx = new XAJpaTemplate(emSupplier, tm, coordinator);
+        tx.tx(TransactionType.Required, new EmConsumer() {
+            public void accept(EntityManager em) {
+                em.persist(new Object());
+            }
+        });
+        verify(tm, times(3)).getStatus();
+        verify(tm, never()).commit();
+        verify(tm, times(1)).rollback();
+    }
+
+    @Test
+    public void test_RollbackExceptionHandling_rollbackafterthrown()
+            throws Exception {
+        TransactionManager tm = mockTm();
+        when(tm.getStatus()).thenReturn(Status.STATUS_NO_TRANSACTION, Status.STATUS_ACTIVE, Status.STATUS_ACTIVE, Status.STATUS_MARKED_ROLLBACK);
+        doThrow(new RollbackException().initCause(new OptimisticLockException())).when(tm).commit();
+        XAJpaTemplate tx = new XAJpaTemplate(emSupplier, tm, coordinator);
+        try {
+            tx.tx(TransactionType.Required, new EmConsumer() {
+                public void accept(EntityManager em) {
+                    em.persist(new Object());
+                }
+            });
+        } catch (RuntimeException e) {
+            // this is ok
+        }
+        verify(tm, times(5)).getStatus();
+        verify(tm, times(1)).commit();
+        verify(tm, times(1)).rollback();
+    }
+
+}