From 80aca3e5409451325d1e5769a7f6523ee373ee2d Mon Sep 17 00:00:00 2001 From: Ondra Chaloupka Date: Mon, 12 Apr 2021 23:32:19 +0200 Subject: [PATCH] [#16455] java.lang.Error should rollback the JTA transaction --- .../narayana/interceptor/TestException.java | 4 + .../narayana/interceptor/TestXAResource.java | 60 +++++ .../interceptor/TransactionalTest.java | 210 ++++++++++++++++++ .../narayana/interceptor/TxAssertionData.java | 32 +++ .../TransactionalInterceptorBase.java | 35 ++- 5 files changed, 330 insertions(+), 11 deletions(-) create mode 100644 extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestException.java create mode 100644 extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestXAResource.java create mode 100644 extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TransactionalTest.java create mode 100644 extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TxAssertionData.java diff --git a/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestException.java b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestException.java new file mode 100644 index 0000000000000..efe2606a6ff12 --- /dev/null +++ b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestException.java @@ -0,0 +1,4 @@ +package io.quarkus.narayana.interceptor; + +public class TestException extends Exception { +} diff --git a/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestXAResource.java b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestXAResource.java new file mode 100644 index 0000000000000..2dbc5a1efe5db --- /dev/null +++ b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TestXAResource.java @@ -0,0 +1,60 @@ +package io.quarkus.narayana.interceptor; + +import javax.transaction.xa.XAException; +import javax.transaction.xa.XAResource; +import javax.transaction.xa.Xid; + +public class TestXAResource implements XAResource { + final TxAssertionData txAssertionData; + + TestXAResource(TxAssertionData txAssertionData) { + this.txAssertionData = txAssertionData; + } + + @Override + public void commit(Xid xid, boolean b) throws XAException { + txAssertionData.addCommit(); + } + + @Override + public void end(Xid xid, int i) throws XAException { + } + + @Override + public void forget(Xid xid) throws XAException { + } + + @Override + public int getTransactionTimeout() throws XAException { + return 0; + } + + @Override + public boolean isSameRM(XAResource xaResource) throws XAException { + return false; + } + + @Override + public int prepare(Xid xid) throws XAException { + return XA_OK; + } + + @Override + public Xid[] recover(int i) throws XAException { + return new Xid[0]; + } + + @Override + public void rollback(Xid xid) throws XAException { + txAssertionData.addRollback(); + } + + @Override + public boolean setTransactionTimeout(int i) throws XAException { + return false; + } + + @Override + public void start(Xid xid, int i) throws XAException { + } +} diff --git a/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TransactionalTest.java b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TransactionalTest.java new file mode 100644 index 0000000000000..29386dd33126c --- /dev/null +++ b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TransactionalTest.java @@ -0,0 +1,210 @@ +package io.quarkus.narayana.interceptor; + +import javax.enterprise.context.ApplicationScoped; +import javax.inject.Inject; +import javax.transaction.RollbackException; +import javax.transaction.Status; +import javax.transaction.SystemException; +import javax.transaction.TransactionManager; +import javax.transaction.Transactional; +import javax.transaction.UserTransaction; + +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; + +public class TransactionalTest { + + @RegisterExtension + static final QuarkusUnitTest config = new QuarkusUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(TransactionalTest.TransactionalBean.class, TestXAResource.class, + TxAssertionData.class, TestException.class)); + + @Inject + private TransactionManager tm; + + @Inject + private UserTransaction userTransaction; + + @Inject + private TransactionalTest.TransactionalBean testTransactionalBean; + + @Inject + private TxAssertionData txAssertionData; + + @AfterEach + public void tearDown() { + try { + userTransaction.rollback(); + } catch (Exception e) { + // do nothing + } finally { + txAssertionData.reset(); + } + } + + @Test + public void transactionalRequiresToCommit() throws Exception { + assertTransactionInactive(); + testTransactionalBean.executeTransactional(); + assertTransactionInactive(); + Assertions.assertEquals(1, txAssertionData.getCommit()); + Assertions.assertEquals(0, txAssertionData.getRollback()); + } + + @Test + public void transactionalThrowRuntimeException() { + assertTransactionInactive(); + try { + testTransactionalBean.executeTransactionalThrowException(RuntimeException.class); + Assertions.fail("Expecting RuntimeException to be thrown and the execution does not reach this point"); + } catch (Throwable expected) { + } + assertTransactionInactive(); + Assertions.assertEquals(0, txAssertionData.getCommit()); + Assertions.assertEquals(1, txAssertionData.getRollback()); + } + + @Test + public void transactionalThrowApplicationException() { + assertTransactionInactive(); + try { + testTransactionalBean.executeTransactionalThrowException(TestException.class); + Assertions.fail("Expecting TestException to be thrown and the execution does not reach this point"); + } catch (Throwable expected) { + } + assertTransactionInactive(); + Assertions.assertEquals(1, txAssertionData.getCommit()); + Assertions.assertEquals(0, txAssertionData.getRollback()); + } + + @Test + public void transactionalThrowError() { + assertTransactionInactive(); + try { + testTransactionalBean.executeTransactionalThrowException(Error.class); + Assertions.fail("Expecting Error to be thrown and the execution does not reach this point"); + } catch (Throwable expected) { + } + assertTransactionInactive(); + Assertions.assertEquals(0, txAssertionData.getCommit()); + Assertions.assertEquals(1, txAssertionData.getRollback()); + } + + @Test + public void transactionalThrowApplicationExceptionWithRollbackOn() { + assertTransactionInactive(); + try { + testTransactionalBean.executeTransactionalRollbackOnException(TestException.class); + Assertions.fail("Expecting TestException to be thrown and the execution does not reach this point"); + } catch (Throwable expected) { + } + assertTransactionInactive(); + Assertions.assertEquals(0, txAssertionData.getCommit()); + Assertions.assertEquals(1, txAssertionData.getRollback()); + } + + @Test + public void transactionalThrowRuntimeExceptionWithDontRollbackOn() { + assertTransactionInactive(); + try { + testTransactionalBean.executeTransactionalDontRollbackOnRuntimeException(RuntimeException.class); + Assertions.fail("Expecting RuntimeException to be thrown and the execution does not reach this point"); + } catch (Throwable expected) { + } + assertTransactionInactive(); + Assertions.assertEquals(1, txAssertionData.getCommit()); + Assertions.assertEquals(0, txAssertionData.getRollback()); + } + + @Test + public void transactionalThrowErrorWithDontRollbackOn() { + assertTransactionInactive(); + try { + testTransactionalBean.executeTransactionalDontRollbackOnError(Error.class); + Assertions.fail("Expecting Error to be thrown and the execution does not reach this point"); + } catch (Throwable expected) { + } + assertTransactionInactive(); + Assertions.assertEquals(1, txAssertionData.getCommit()); + Assertions.assertEquals(0, txAssertionData.getRollback()); + } + + @Test + public void transactionalThrowApplicationExceptionDontRollbackOnPriority() { + assertTransactionInactive(); + try { + testTransactionalBean.executeTransactionalRollbackOnPriority(TestException.class); + Assertions.fail("Expecting TestException to be thrown and the execution does not reach this point"); + } catch (Throwable expected) { + } + assertTransactionInactive(); + Assertions.assertEquals(1, txAssertionData.getCommit()); + Assertions.assertEquals(0, txAssertionData.getRollback()); + } + + private void assertTransactionInactive() { + try { + if (tm.getTransaction() != null) { + Assertions.assertNotEquals(Status.STATUS_ACTIVE, tm.getTransaction().getStatus()); + } + } catch (Exception e) { + Assertions.fail(e.getMessage()); + } + } + + @ApplicationScoped + static class TransactionalBean { + @Inject + private TransactionManager transactionManager; + + @Inject + private TxAssertionData txAssertionData; + + private void enlist() throws SystemException, RollbackException { + transactionManager.getTransaction() + .enlistResource(new TestXAResource(txAssertionData)); + } + + @Transactional + public void executeTransactional() throws Exception { + enlist(); + } + + @Transactional + public void executeTransactionalThrowException(Class throwable) throws Throwable { + enlist(); + throw throwable.newInstance(); + } + + @Transactional(rollbackOn = Exception.class) + public void executeTransactionalRollbackOnException(Class throwable) throws Throwable { + enlist(); + throw throwable.newInstance(); + } + + @Transactional(dontRollbackOn = RuntimeException.class) + public void executeTransactionalDontRollbackOnRuntimeException(Class throwable) throws Throwable { + enlist(); + throw throwable.newInstance(); + } + + @Transactional(dontRollbackOn = Error.class) + public void executeTransactionalDontRollbackOnError(Class throwable) throws Throwable { + enlist(); + throw throwable.newInstance(); + } + + @Transactional(dontRollbackOn = Exception.class, rollbackOn = Exception.class) + public void executeTransactionalRollbackOnPriority(Class throwable) throws Throwable { + enlist(); + throw throwable.newInstance(); + } + } +} diff --git a/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TxAssertionData.java b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TxAssertionData.java new file mode 100644 index 0000000000000..2ed5fc1e621f1 --- /dev/null +++ b/extensions/narayana-jta/deployment/src/test/java/io/quarkus/narayana/interceptor/TxAssertionData.java @@ -0,0 +1,32 @@ +package io.quarkus.narayana.interceptor; + +import java.util.concurrent.atomic.AtomicInteger; + +import javax.enterprise.context.ApplicationScoped; + +@ApplicationScoped +public class TxAssertionData { + private AtomicInteger commitNumber = new AtomicInteger(); + private AtomicInteger rollbackNumber = new AtomicInteger(); + + public void reset() { + commitNumber.set(0); + rollbackNumber.set(0); + } + + public int addCommit() { + return commitNumber.incrementAndGet(); + } + + public int addRollback() { + return rollbackNumber.incrementAndGet(); + } + + public int getCommit() { + return commitNumber.get(); + } + + public int getRollback() { + return rollbackNumber.get(); + } +} diff --git a/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/interceptor/TransactionalInterceptorBase.java b/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/interceptor/TransactionalInterceptorBase.java index 24b599c3e6e32..fe79dfe920312 100644 --- a/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/interceptor/TransactionalInterceptorBase.java +++ b/extensions/narayana-jta/runtime/src/main/java/io/quarkus/narayana/jta/runtime/interceptor/TransactionalInterceptorBase.java @@ -125,9 +125,9 @@ protected Object invokeInOurTx(InvocationContext ic, TransactionManager tm, Runn try { ret = ic.proceed(); - } catch (Exception e) { + } catch (Throwable t) { throwing = true; - handleException(ic, e, tx); + handleException(ic, t, tx); } finally { // handle asynchronously if not throwing if (!throwing && ret != null) { @@ -250,8 +250,8 @@ protected Object invokeInCallerTx(InvocationContext ic, Transaction tx) throws E try { checkConfiguration(ic); return ic.proceed(); - } catch (Exception e) { - handleException(ic, e, tx); + } catch (Throwable t) { + handleException(ic, t, tx); } throw new RuntimeException("UNREACHABLE"); } @@ -269,34 +269,35 @@ private void checkConfiguration(InvocationContext ic) { } } - protected void handleExceptionNoThrow(InvocationContext ic, Throwable e, Transaction tx) + protected void handleExceptionNoThrow(InvocationContext ic, Throwable t, Transaction tx) throws IllegalStateException, SystemException { Transactional transactional = getTransactional(ic); for (Class dontRollbackOnClass : transactional.dontRollbackOn()) { - if (dontRollbackOnClass.isAssignableFrom(e.getClass())) { + if (dontRollbackOnClass.isAssignableFrom(t.getClass())) { return; } } for (Class rollbackOnClass : transactional.rollbackOn()) { - if (rollbackOnClass.isAssignableFrom(e.getClass())) { + if (rollbackOnClass.isAssignableFrom(t.getClass())) { tx.setRollbackOnly(); return; } } - if (e instanceof RuntimeException) { + // RuntimeException and Error are un-checked exceptions and rollback is expected + if (t instanceof RuntimeException || t instanceof Error) { tx.setRollbackOnly(); return; } } - protected void handleException(InvocationContext ic, Exception e, Transaction tx) throws Exception { + protected void handleException(InvocationContext ic, Throwable t, Transaction tx) throws Exception { - handleExceptionNoThrow(ic, e, tx); - throw e; + handleExceptionNoThrow(ic, t, tx); + sneakyThrow(t); } protected void endTransaction(TransactionManager tm, Transaction tx, RunnableWithException afterEndTransaction) @@ -327,4 +328,16 @@ protected boolean setUserTransactionAvailable(boolean available) { protected void resetUserTransactionAvailability(boolean previousUserTransactionAvailability) { ServerVMClientUserTransaction.setAvailability(previousUserTransactionAvailability); } + + /** + * An utility method to throw any exception as a {@link RuntimeException}. + * We may throw a checked exception (subtype of {@code Throwable} or {@code Exception}) as un-checked exception. + * This considers the Java 8 inference rule that states that a {@code throws E} is inferred as {@code RuntimeException}. + * + * This method can be used in {@code throw} statement such as: {@code throw sneakyThrow(exception);}. + */ + @SuppressWarnings("unchecked") + private static void sneakyThrow(Throwable e) throws E { + throw (E) e; + } }