don't begin new transaction

Persistence code was not checking to see if a transaction was already
in progress before beginning a transaction.
Fix checkstyle issue with comments and parameter for EntityMgrException.
Restore other checkstyle fixes.
More checkstyle fixes.
Rename some variables.

Change-Id: Ic820944781571ba2ef411cb86d12fa32fb206124
Issue-ID: POLICY-1106
Signed-off-by: Jim Hahn <jrh3@att.com>

1 files changed, 204 insertions, 91 deletions

diff --git a/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/EntityMgrTransTest.java b/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/EntityMgrTransTest.java
index aba1d80e..c622a933 100644
--- a/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/EntityMgrTransTest.java
+++ b/feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/EntityMgrTransTest.java
@@ -1,4 +1,4 @@
-/*-
+/*
  * ============LICENSE_START=======================================================
  * feature-session-persistence
  * ================================================================================
@@ -21,7 +21,7 @@
 package org.onap.policy.drools.persistence;
 
 import static org.junit.Assert.assertEquals;
-
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -29,7 +29,11 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.util.HashMap;
+import java.util.Map;
 import javax.persistence.EntityManager;
+import javax.persistence.EntityManagerFactory;
+import javax.persistence.Persistence;
 import javax.transaction.HeuristicMixedException;
 import javax.transaction.HeuristicRollbackException;
 import javax.transaction.NotSupportedException;
@@ -37,7 +41,6 @@ import javax.transaction.RollbackException;
 import javax.transaction.Status;
 import javax.transaction.SystemException;
 import javax.transaction.UserTransaction;
-
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -56,8 +59,7 @@ public class EntityMgrTransTest {
     private EntityManager mgr;
 
     /**
-     * Setup before the class.
-     * 
+     * Configure properties for JTA.
      */
     @BeforeClass
     public static void setUpBeforeClass() {
@@ -73,40 +75,68 @@ public class EntityMgrTransTest {
     }
 
     /**
-     * Setup.
-     * 
-     * @throws Exception exception
+     * Creates a mock transaction and entity manager. Resets the "userTrans" field of the
+     * class under test.
+     *
+     * @throws Exception if an error occurs
      */
     @Before
     public void setUp() throws Exception {
         trans = mock(UserTransaction.class);
         mgr = mock(EntityManager.class);
 
+        when(trans.getStatus()).thenReturn(Status.STATUS_NO_TRANSACTION, Status.STATUS_ACTIVE);
+
         EntityMgrTrans.setUserTrans(trans);
     }
 
     /**
-     * Verifies that the constructor starts a transaction, but does not do anything extra before being
-     * closed.
+     * Verifies that the constructor starts a transaction, but does not do anything extra
+     * before being closed.
+     *
+     * @throws Exception if an error occurs
+     */
+    @Test
+    public void testEntityMgrTrans_Inactive() throws Exception {
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
+
+        // verify that transaction was started
+        verify(trans).begin();
+        verify(mgr).joinTransaction();
+
+        // verify not closed, committed, or rolled back yet
+        verify(trans, never()).commit();
+        verify(trans, never()).rollback();
+        verify(mgr, never()).close();
+
+        emt.close();
+    }
+
+    /**
+     * Verifies that the constructor does not start a transaction, because one is already
+     * active.
      *
-     * @throws Exception exception
+     * @throws Exception if an error occurs
      */
     @Test
-    public void testEntityMgrTrans() throws Exception {
+    public void testEntityMgrTrans_Active() throws Exception {
 
         when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
 
-        final EntityMgrTrans newTrans = new EntityMgrTrans(mgr);
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
 
-        // verify that transaction was started
-        verify(trans).begin();
+        // verify that transaction was not re-started started
+        verify(trans, never()).begin();
+
+        // verify that transaction was joined
+        verify(mgr).joinTransaction();
 
         // verify not closed, committed, or rolled back yet
         verify(trans, never()).commit();
         verify(trans, never()).rollback();
         verify(mgr, never()).close();
 
-        newTrans.close();
+        emt.close();
     }
 
     @Test(expected = EntityMgrException.class)
@@ -114,7 +144,7 @@ public class EntityMgrTransTest {
 
         doThrow(new IllegalArgumentException("expected exception")).when(trans).begin();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
     }
@@ -124,7 +154,7 @@ public class EntityMgrTransTest {
 
         doThrow(new NotSupportedException("expected exception")).when(trans).begin();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
     }
@@ -134,22 +164,37 @@ public class EntityMgrTransTest {
 
         doThrow(new SystemException("expected exception")).when(trans).begin();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
     }
 
     /**
-     * Verifies that the transaction is rolled back and the manager is closed when and a transaction
-     * is active.
+     * Verifies that the transaction is not rolled back, but the manager is closed when a
+     * transaction is already active.
      */
     @Test
     public void testClose_Active() throws Exception {
-        EntityMgrTrans newTrans = new EntityMgrTrans(mgr);
-
         when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
 
-        newTrans.close();
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
+        emt.close();
+
+        // closed and rolled back, but not committed
+        verify(trans, never()).commit();
+        verify(trans, never()).rollback();
+        verify(mgr).close();
+    }
+
+    /**
+     * Verifies that the transaction is rolled back and the manager is closed when a
+     * transaction is begun by the constructor.
+     */
+    @Test
+    public void testClose_Begun() throws Exception {
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
+
+        emt.close();
 
         // closed and rolled back, but not committed
         verify(trans, never()).commit();
@@ -158,16 +203,16 @@ public class EntityMgrTransTest {
     }
 
     /**
-     * Verifies that the manager is closed, but that the transaction is <i>not</i> rolled back when
-     * and no transaction is active.
+     * Verifies that the manager is closed, but that the transaction is <i>not</i> rolled
+     * back when no transaction is active.
      */
     @Test
     public void testClose_Inactive() throws Exception {
-        EntityMgrTrans newTrans = new EntityMgrTrans(mgr);
-
         when(trans.getStatus()).thenReturn(Status.STATUS_NO_TRANSACTION);
 
-        newTrans.close();
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
+
+        emt.close();
 
         // closed, but not committed or rolled back
         verify(mgr).close();
@@ -178,10 +223,9 @@ public class EntityMgrTransTest {
     @Test(expected = EntityMgrException.class)
     public void testClose_IllStateEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new IllegalStateException("expected exception")).when(trans).rollback();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
     }
@@ -189,10 +233,9 @@ public class EntityMgrTransTest {
     @Test(expected = EntityMgrException.class)
     public void testClose_SecEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new SecurityException("expected exception")).when(trans).rollback();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
     }
@@ -200,23 +243,20 @@ public class EntityMgrTransTest {
     @Test(expected = EntityMgrException.class)
     public void testClose_SysEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new SystemException("expected exception")).when(trans).rollback();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
     }
 
     /**
-     * Verifies that the manager is closed and the transaction rolled back when "try" block exits
-     * normally and a transaction is active.
+     * Verifies that the manager is closed and the transaction rolled back when "try"
+     * block exits normally and a transaction is active.
      */
     @Test
     public void testClose_TryWithoutExcept_Active() throws Exception {
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
-
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
 
@@ -227,15 +267,15 @@ public class EntityMgrTransTest {
     }
 
     /**
-     * Verifies that the manager is closed, but that the transaction is <i>not</i> rolled back when
-     * "try" block exits normally and no transaction is active.
+     * Verifies that the manager is closed, but that the transaction is <i>not</i> rolled
+     * back when "try" block exits normally and no transaction is active.
      */
     @Test
     public void testClose_TryWithoutExcept_Inactive() throws Exception {
 
         when(trans.getStatus()).thenReturn(Status.STATUS_NO_TRANSACTION);
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
             // Empty
         }
 
@@ -246,16 +286,13 @@ public class EntityMgrTransTest {
     }
 
     /**
-     * Verifies that the manager is closed and the transaction rolled back when "try" block throws an
-     * exception and a transaction is active.
+     * Verifies that the manager is closed and the transaction rolled back when "try"
+     * block throws an exception and a transaction is active.
      */
     @Test
     public void testClose_TryWithExcept_Active() throws Exception {
-
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
-
         try {
-            try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+            try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
                 throw new SystemException("expected exception");
             }
 
@@ -270,8 +307,8 @@ public class EntityMgrTransTest {
     }
 
     /**
-     * Verifies that the manager is closed, but that the transaction is <i>not</i> rolled back when
-     * "try" block throws an exception and no transaction is active.
+     * Verifies that the manager is closed, but that the transaction is <i>not</i> rolled
+     * back when "try" block throws an exception and no transaction is active.
      */
     @Test
     public void testClose_TryWithExcept_Inactive() throws Exception {
@@ -279,7 +316,7 @@ public class EntityMgrTransTest {
         when(trans.getStatus()).thenReturn(Status.STATUS_NO_TRANSACTION);
 
         try {
-            try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
+            try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
                 throw new SystemException("expected exception");
             }
 
@@ -293,13 +330,15 @@ public class EntityMgrTransTest {
         verify(mgr).close();
     }
 
-    /** Verifies that commit() only commits, and that the subsequent close() does not re-commit. */
+    /**
+     * Verifies that commit() only commits, and that the subsequent close() does not
+     * re-commit.
+     */
     @Test
     public void testCommit() throws Exception {
-        EntityMgrTrans newTrans = new EntityMgrTrans(mgr);
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
 
-        newTrans.commit();
+        emt.commit();
 
         when(trans.getStatus()).thenReturn(Status.STATUS_COMMITTED);
 
@@ -309,88 +348,107 @@ public class EntityMgrTransTest {
         verify(mgr, never()).close();
 
         // closed, but not re-committed
-        newTrans.close();
+        emt.close();
 
         verify(trans, times(1)).commit();
         verify(mgr).close();
     }
 
+    /**
+     * Verifies that commit() does nothing, and that the subsequent close() does not
+     * re-commit when a transaction is already active.
+     */
+    @Test
+    public void testCommit_Active() throws Exception {
+        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
+
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
+
+        emt.commit();
+
+        // nothing happened yet
+        verify(trans, never()).commit();
+        verify(trans, never()).rollback();
+        verify(mgr, never()).close();
+
+        // closed, but not re-committed
+        emt.close();
+
+        // still no commit or rollback
+        verify(trans, never()).commit();
+        verify(trans, never()).rollback();
+        verify(mgr).close();
+    }
+
     @Test(expected = EntityMgrException.class)
     public void testCommit_SecEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new SecurityException("expected exception")).when(trans).commit();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.commit();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.commit();
         }
     }
 
     @Test(expected = EntityMgrException.class)
     public void testCommit_IllStateEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new IllegalStateException("expected exception")).when(trans).commit();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.commit();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.commit();
         }
     }
 
     @Test(expected = EntityMgrException.class)
     public void testCommit_RbEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new RollbackException("expected exception")).when(trans).commit();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.commit();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.commit();
         }
     }
 
     @Test(expected = EntityMgrException.class)
     public void testCommit_HmEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new HeuristicMixedException("expected exception")).when(trans).commit();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.commit();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.commit();
         }
     }
 
     @Test(expected = EntityMgrException.class)
     public void testCommit_HrbEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new HeuristicRollbackException("expected exception")).when(trans).commit();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.commit();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.commit();
         }
     }
 
     @Test(expected = EntityMgrException.class)
     public void testCommit_SysEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new SystemException("expected exception")).when(trans).commit();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.commit();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.commit();
         }
     }
 
     /**
-     * Verifies that rollback() only rolls back, and that the subsequent close() does not re-roll
-     * back.
+     * Verifies that rollback() only rolls back, and that the subsequent close() does not
+     * re-roll back.
      */
     @Test
     public void testRollback() throws Exception {
-        EntityMgrTrans newTrans = new EntityMgrTrans(mgr);
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
 
-        newTrans.rollback();
+        emt.rollback();
 
         when(trans.getStatus()).thenReturn(Status.STATUS_ROLLEDBACK);
 
@@ -400,42 +458,65 @@ public class EntityMgrTransTest {
         verify(mgr, never()).close();
 
         // closed, but not re-rolled back
-        newTrans.close();
+        emt.close();
 
-        verify(trans, times(1)).rollback();
+        // still no commit or rollback
+        verify(trans, never()).commit();
+        verify(trans).rollback();
+        verify(mgr).close();
+    }
+
+    /**
+     * Verifies that rollback() does nothing, and that the subsequent close() does not
+     * re-roll back when a transaction is already active.
+     */
+    @Test
+    public void testRollback_Active() throws Exception {
+        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
+        EntityMgrTrans emt = new EntityMgrTrans(mgr);
+
+        emt.rollback();
+
+        // nothing happens
+        verify(trans, never()).commit();
+        verify(trans, never()).rollback();
+        verify(mgr, never()).close();
+
+        emt.close();
+
+        // still no commit or rollback
+        verify(trans, never()).commit();
+        verify(trans, never()).rollback();
         verify(mgr).close();
     }
 
     @Test(expected = EntityMgrException.class)
     public void testRollback_IllStateEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new IllegalStateException("expected exception")).when(trans).rollback();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.rollback();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.rollback();
         }
     }
 
     @Test(expected = EntityMgrException.class)
     public void testRollback_SecEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new SecurityException("expected exception")).when(trans).rollback();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.rollback();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.rollback();
         }
     }
 
     @Test(expected = EntityMgrException.class)
     public void testRollback_SysEx() throws Exception {
 
-        when(trans.getStatus()).thenReturn(Status.STATUS_ACTIVE);
         doThrow(new SystemException("expected exception")).when(trans).rollback();
 
-        try (EntityMgrTrans t = new EntityMgrTrans(mgr)) {
-            t.rollback();
+        try (EntityMgrTrans emt = new EntityMgrTrans(mgr)) {
+            emt.rollback();
         }
     }
 
@@ -445,5 +526,37 @@ public class EntityMgrTransTest {
         EntityMgrException ex = new EntityMgrException(secex);
 
         assertEquals(secex, ex.getCause());
+
+    }
+
+    /**
+     * Tests using real (i.e., not mocked) Persistence classes.
+     */
+    @Test
+    public void testReal() {
+        EntityMgrTrans.setUserTrans(savetrans);
+
+        Map<String, Object> propMap = new HashMap<>();
+
+        propMap.put("javax.persistence.jdbc.driver", "org.h2.Driver");
+        propMap.put("javax.persistence.jdbc.url", "jdbc:h2:mem:EntityMgrTransTest");
+
+        EntityManagerFactory emf = Persistence.createEntityManagerFactory("junitDroolsSessionEntityPU", propMap);
+
+        try (EntityMgrTrans trans1 = new EntityMgrTrans(emf.createEntityManager())) {
+
+            // nest a transaction - should still be OK
+            
+            try (EntityMgrTrans trans2 = new EntityMgrTrans(emf.createEntityManager())) {                
+                // Empty
+            }
+
+        } catch (Exception e) {
+            logger.info("persistence error", e);
+            emf.close();
+            fail("persistence error");
+        }
+
+        emf.close();
     }
 }