Mailing-List: contact commits-help@aries.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@aries.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1777105 - in /aries/trunk/jpa/jpa-support/src:
 main/java/org/apache/aries/jpa/support/impl/
 main/java/org/apache/aries/jpa/support/xa/impl/
 test/java/org/apache/aries/jpa/support/impl/
Date: Tue, 03 Jan 2017 10:21:48 -0000
To: commits@aries.apache.org
From: cschneider@apache.org
Message-Id: <20170103102149.864D33A00E5@svn01-us-west.apache.org>
archived-at: Tue, 03 Jan 2017 10:21:52 -0000

Author: cschneider
Date: Tue Jan  3 10:21:48 2017
New Revision: 1777105

URL: http://svn.apache.org/viewvc?rev=1777105&view=rev
Log:
[ARIES-1497] properly handle RollbackException

Added:
    aries/trunk/jpa/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/
    aries/trunk/jpa/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java
Modified:
    aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java
    aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java

Modified: aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java
URL: http://svn.apache.org/viewvc/aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java?rev=1777105&r1=1777104&r2=1777105&view=diff
==============================================================================
--- aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java (original)
+++ aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/impl/XAJpaTemplate.java Tue Jan  3 10:21:48 2017
@@ -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 @@ public class XAJpaTemplate extends Abstr
             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 @@ public class XAJpaTemplate extends Abstr
         }
     }
 
-    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 @@ public class XAJpaTemplate extends Abstr
     }
 
     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 @@ public class XAJpaTemplate extends Abstr
             // 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) {

Modified: aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java
URL: http://svn.apache.org/viewvc/aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java?rev=1777105&r1=1777104&r2=1777105&view=diff
==============================================================================
--- aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java (original)
+++ aries/trunk/jpa/jpa-support/src/main/java/org/apache/aries/jpa/support/xa/impl/TransactionAttribute.java Tue Jan  3 10:21:48 2017
@@ -101,7 +101,7 @@ public enum TransactionAttribute {
         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 @@ public enum TransactionAttribute {
         if (tranToken.isCompletionAllowed()) {
           if (man.getStatus() == Status.STATUS_MARKED_ROLLBACK) {
             man.rollback();
-          } else {
+          } else if (man.getStatus() != Status.STATUS_NO_TRANSACTION) {
             man.commit();
           }
         }

Added: aries/trunk/jpa/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java
URL: http://svn.apache.org/viewvc/aries/trunk/jpa/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java?rev=1777105&view=auto
==============================================================================
--- aries/trunk/jpa/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java (added)
+++ aries/trunk/jpa/jpa-support/src/test/java/org/apache/aries/jpa/support/impl/XAJpaTemplateTest.java Tue Jan  3 10:21:48 2017
@@ -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();
+    }
+
+}