summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJim Hahn <jrh3@att.com>2018-09-17 14:41:02 -0400
committerJim Hahn <jrh3@att.com>2018-09-17 16:11:24 -0400
commit1bf0e1d6acc1df4da77cfcf315bfe16f58a12def (patch)
tree82f77afe2e7d1af95a12b018e7a8a67e48d5add8
parent02641b81ea49d3ecc7ab102d17b921a1ed164bd3 (diff)
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>
-rw-r--r--feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/EntityMgrTrans.java62
-rw-r--r--feature-session-persistence/src/test/java/org/onap/policy/drools/persistence/EntityMgrTransTest.java295
2 files changed, 248 insertions, 109 deletions
diff --git a/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/EntityMgrTrans.java b/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/EntityMgrTrans.java
index 3d276519..988cd9c2 100644
--- a/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/EntityMgrTrans.java
+++ b/feature-session-persistence/src/main/java/org/onap/policy/drools/persistence/EntityMgrTrans.java
@@ -28,27 +28,40 @@ import javax.transaction.RollbackException;
import javax.transaction.Status;
import javax.transaction.SystemException;
import javax.transaction.UserTransaction;
-
import org.onap.policy.common.utils.jpa.EntityMgrCloser;
/**
- * Wrapper for an <i>EntityManager</i> that creates a JTA transaction that is auto-rolled back when
- * closed.
+ * Wrapper for an <i>EntityManager</i> that creates a JTA transaction that is auto-rolled
+ * back when closed. Note: commit() and rollback() actions do nothing if a transaction was
+ * already in progress when this object was constructed.
*/
public class EntityMgrTrans extends EntityMgrCloser {
- /** Transaction to be rolled back. */
+ /**
+ * Transaction to be rolled back.
+ */
private static UserTransaction userTrans = com.arjuna.ats.jta.UserTransaction.userTransaction();
/**
+ * {@code True} if a transaction had already been started before this object was
+ * constructed, {@code false} otherwise.
+ */
+ private boolean transInProgress;
+
+ /**
* Constructor.
- *
- * @param em entity for which a transaction is to be begun */
+ *
+ * @param em entity for which a transaction is to be begun
+ */
public EntityMgrTrans(EntityManager em) {
super(em);
try {
- userTrans.begin();
+ transInProgress = (userTrans.getStatus() == Status.STATUS_ACTIVE);
+ if (!transInProgress) {
+ userTrans.begin();
+ }
+
em.joinTransaction();
} catch (RuntimeException | NotSupportedException | SystemException e) {
@@ -75,24 +88,32 @@ public class EntityMgrTrans extends EntityMgrCloser {
EntityMgrTrans.userTrans = userTrans;
}
- /** Commits the transaction. */
+ /**
+ * Commits the transaction.
+ */
public void commit() {
+ if (transInProgress) {
+ return;
+ }
+
try {
userTrans.commit();
- } catch (SecurityException
- | IllegalStateException
- | RollbackException
- | HeuristicMixedException
- | HeuristicRollbackException
- | SystemException e) {
+ } catch (SecurityException | IllegalStateException | RollbackException | HeuristicMixedException
+ | HeuristicRollbackException | SystemException e) {
throw new EntityMgrException(e);
}
}
- /** Rolls back the transaction. */
+ /**
+ * Rolls back the transaction.
+ */
public void rollback() {
+ if (transInProgress) {
+ return;
+ }
+
try {
userTrans.rollback();
@@ -104,7 +125,7 @@ public class EntityMgrTrans extends EntityMgrCloser {
@Override
public void close() {
try {
- if (userTrans.getStatus() == Status.STATUS_ACTIVE) {
+ if (!transInProgress && userTrans.getStatus() == Status.STATUS_ACTIVE) {
userTrans.rollback();
}
@@ -117,12 +138,17 @@ public class EntityMgrTrans extends EntityMgrCloser {
}
/**
- * Runtime exceptions generated by this class. Wraps exceptions generated by delegated operations,
- * particularly when they are not, themselves, Runtime exceptions.
+ * Runtime exceptions generated by this class. Wraps exceptions generated by delegated
+ * operations, particularly when they are not, themselves, Runtime exceptions.
*/
public static class EntityMgrException extends RuntimeException {
private static final long serialVersionUID = 1L;
+ /**
+ * Constructor.
+ *
+ * @param ex exception to be wrapped
+ */
public EntityMgrException(Exception ex) {
super(ex);
}
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();
}
}