Updated Branches: refs/heads/master 804d3bce8 -> e62058be6
DELTASPIKE-462 don't close the EM if we didn't open it This happens when we use BeanManagedUserTransactionStrategy and have a Stateless EJB call our transactional CDI beans. Project: http://git-wip-us.apache.org/repos/asf/deltaspike/repo Commit: http://git-wip-us.apache.org/repos/asf/deltaspike/commit/e62058be Tree: http://git-wip-us.apache.org/repos/asf/deltaspike/tree/e62058be Diff: http://git-wip-us.apache.org/repos/asf/deltaspike/diff/e62058be Branch: refs/heads/master Commit: e62058be691ad452be79c96d0738207d34d55ea3 Parents: 804d3bc Author: Mark Struberg <[email protected]> Authored: Fri Dec 6 13:14:03 2013 +0100 Committer: Mark Struberg <[email protected]> Committed: Fri Dec 6 13:15:09 2013 +0100 ---------------------------------------------------------------------- .../ResourceLocalTransactionStrategy.java | 157 +++++++++++-------- .../transaction/context/EntityManagerEntry.java | 1 - 2 files changed, 94 insertions(+), 64 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/deltaspike/blob/e62058be/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/ResourceLocalTransactionStrategy.java ---------------------------------------------------------------------- diff --git a/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/ResourceLocalTransactionStrategy.java b/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/ResourceLocalTransactionStrategy.java index c363d72..b7bd1fa 100644 --- a/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/ResourceLocalTransactionStrategy.java +++ b/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/ResourceLocalTransactionStrategy.java @@ -79,6 +79,7 @@ public class ResourceLocalTransactionStrategy implements TransactionStrategy TransactionBeanStorage transactionBeanStorage = TransactionBeanStorage.getInstance(); boolean isOutermostInterceptor = transactionBeanStorage.isEmpty(); + boolean outermostTransactionAlreadyExisted = false; if (isOutermostInterceptor) { @@ -107,6 +108,10 @@ public class ResourceLocalTransactionStrategy implements TransactionStrategy { transaction.begin(); } + else if (isOutermostInterceptor) + { + outermostTransactionAlreadyExisted = true; + } //don't move it before EntityTransaction#begin() and invoke it in any case beforeProceed(entityManagerEntry); @@ -125,25 +130,15 @@ public class ResourceLocalTransactionStrategy implements TransactionStrategy Set<EntityManagerEntry> entityManagerEntryList = transactionBeanStorage.getUsedEntityManagerEntries(); - for (EntityManagerEntry currentEntityManagerEntry : entityManagerEntryList) + if (!outermostTransactionAlreadyExisted) { - EntityTransaction transaction = getTransaction(currentEntityManagerEntry); - if (transaction != null && transaction.isActive()) - { - try - { - transaction.rollback(); - } - catch (Exception eRollback) - { - if (LOGGER.isLoggable(Level.SEVERE)) - { - LOGGER.log(Level.SEVERE, - "Got additional Exception while subsequently " + - "rolling back other SQL transactions", eRollback); - } - } - } + // We only commit transactions we opened ourselfs. + // If the transaction got opened outside of our interceptor chain + // we must not handle it. + // This e.g. happens if a Stateless EJB invokes a Transactional CDI bean + // which uses the BeanManagedUserTransactionStrategy. + + rollbackAllTransactions(entityManagerEntryList); } // drop all EntityManagers from the request-context cache @@ -163,73 +158,85 @@ public class ResourceLocalTransactionStrategy implements TransactionStrategy boolean commitFailed = false; // commit all open transactions in the outermost interceptor! - // this is a 'JTA for poor men' only, and will not guaranty + // For Resource-local this is a 'JTA for poor men' only, and will not guaranty // commit stability over various databases! + // In case of JTA we will just commit the UserTransaction. if (isOutermostInterceptor) { - // only commit all transactions if we didn't rollback - // them already - if (firstException == null) + if (!outermostTransactionAlreadyExisted) { - Set<EntityManagerEntry> entityManagerEntryList = - transactionBeanStorage.getUsedEntityManagerEntries(); + // We only commit transactions we opened ourselfs. + // If the transaction got opened outside of our interceptor chain + // we must not handle it. + // This e.g. happens if a Stateless EJB invokes a Transactional CDI bean + // which uses the BeanManagedUserTransactionStrategy. - boolean rollbackOnly = false; - // but first try to flush all the transactions and write the updates to the database - for (EntityManagerEntry currentEntityManagerEntry : entityManagerEntryList) + if (firstException == null) { - EntityTransaction transaction = getTransaction(currentEntityManagerEntry); - if (transaction != null && transaction.isActive()) + // only commit all transactions if we didn't rollback + // them already + Set<EntityManagerEntry> entityManagerEntryList = + transactionBeanStorage.getUsedEntityManagerEntries(); + + boolean rollbackOnly = false; + // but first try to flush all the transactions and write the updates to the database + for (EntityManagerEntry currentEntityManagerEntry : entityManagerEntryList) { - try + EntityTransaction transaction = getTransaction(currentEntityManagerEntry); + if (transaction != null && transaction.isActive()) { - if (!commitFailed) + try { - currentEntityManagerEntry.getEntityManager().flush(); - - if (!rollbackOnly && transaction.getRollbackOnly()) + if (!commitFailed) { - //don't set commitFailed to true directly - //(the order of the entity-managers isn't deterministic -> tests would break) - rollbackOnly = true; + currentEntityManagerEntry.getEntityManager().flush(); + + if (!rollbackOnly && transaction.getRollbackOnly()) + { + // don't set commitFailed to true directly + // (the order of the entity-managers isn't deterministic + // -> tests would break) + rollbackOnly = true; + } } } - } - catch (Exception e) - { - firstException = e; - commitFailed = true; - break; + catch (Exception e) + { + firstException = e; + commitFailed = true; + break; + } } } - } - if (rollbackOnly) - { - commitFailed = true; - } + if (rollbackOnly) + { + commitFailed = true; + } - // and now either commit or rollback all transactions - for (EntityManagerEntry currentEntityManagerEntry : entityManagerEntryList) - { - EntityTransaction transaction = getTransaction(currentEntityManagerEntry); - if (transaction != null && transaction.isActive()) + // and now either commit or rollback all transactions + for (EntityManagerEntry currentEntityManagerEntry : entityManagerEntryList) { - try + EntityTransaction transaction = getTransaction(currentEntityManagerEntry); + if (transaction != null && transaction.isActive()) { - if (commitFailed || transaction.getRollbackOnly() /*last chance to check it (again)*/) + try { - transaction.rollback(); + // last chance to check it (again) + if (commitFailed || transaction.getRollbackOnly()) + { + transaction.rollback(); + } + else + { + transaction.commit(); + } } - else + catch (Exception e) { - transaction.commit(); + firstException = e; + commitFailed = true; } } - catch (Exception e) - { - firstException = e; - commitFailed = true; - } } } } @@ -248,6 +255,30 @@ public class ResourceLocalTransactionStrategy implements TransactionStrategy } } + private void rollbackAllTransactions(Set<EntityManagerEntry> entityManagerEntryList) + { + for (EntityManagerEntry currentEntityManagerEntry : entityManagerEntryList) + { + EntityTransaction transaction = getTransaction(currentEntityManagerEntry); + if (transaction != null && transaction.isActive()) + { + try + { + transaction.rollback(); + } + catch (Exception eRollback) + { + if (LOGGER.isLoggable(Level.SEVERE)) + { + LOGGER.log(Level.SEVERE, + "Got additional Exception while subsequently " + + "rolling back other SQL transactions", eRollback); + } + } + } + } + } + protected EntityManagerEntry createEntityManagerEntry( EntityManager entityManager, Class<? extends Annotation> qualifier) { http://git-wip-us.apache.org/repos/asf/deltaspike/blob/e62058be/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/context/EntityManagerEntry.java ---------------------------------------------------------------------- diff --git a/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/context/EntityManagerEntry.java b/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/context/EntityManagerEntry.java index 811bf61..c37dc72 100644 --- a/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/context/EntityManagerEntry.java +++ b/deltaspike/modules/jpa/impl/src/main/java/org/apache/deltaspike/jpa/impl/transaction/context/EntityManagerEntry.java @@ -24,7 +24,6 @@ import java.lang.annotation.Annotation; /** * Stores a {@link EntityManager} and the qualifier - * @deprecated we shall rather introduce an own QualifierMap or a ComparableQualifier which checks Nonbinding */ @Typed() public class EntityManagerEntry
