Skip to content

Commit

Permalink
accomodate manually initiated rollbacks. Fixes #1137 & fixes #1136.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 525815800
  • Loading branch information
@Vogel612
Vogel612 authored and Guice Team committed Apr 20, 2023
1 parent 7ed6624 commit 70ad639
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 38 deletions.
34 changes: 21 additions & 13 deletions extensions/persist/src/com/google/inject/persist/jpa/JpaLocalTxnInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;

/** @author Dhanji R. Prasanna (dhanji@gmail.com) */
/**
* @author Dhanji R. Prasanna (dhanji@gmail.com)
*/
class JpaLocalTxnInterceptor implements MethodInterceptor {

// TODO(gak): Move these args to the cxtor & make these final.
Expand Down Expand Up @@ -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).
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand Down
102 changes: 77 additions & 25 deletions extensions/persist/test/com/google/inject/persist/jpa/ManagedLocalTransactionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,7 +31,9 @@
import javax.persistence.NoResultException;
import junit.framework.TestCase;

/** @author Dhanji R. Prasanna (dhanji@gmail.com) */
/**
* @author Dhanji R. Prasanna (dhanji@gmail.com)
*/

public class ManagedLocalTransactionsTest extends TestCase {
private Injector injector;
Expand All @@ -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();
}

Expand All @@ -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)
Expand All @@ -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 =
Expand All @@ -100,7 +104,7 @@ public void testSimpleTransactionRollbackOnChecked() {
try {
injector.getInstance(TransactionalObject.class).runOperationInTxnThrowingChecked();
} catch (IOException e) {
//ignore
// ignore
injector.getInstance(UnitOfWork.class).end();
}

Expand All @@ -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();
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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();
}
}
}

0 comments on commit 70ad639

Please sign in to comment.