[jbpm-commits] JBoss JBPM SVN: r6636 - in jbpm3/branches/jbpm-3.2-soa/modules: core/src/main/java/org/jbpm/persistence/db and 4 other directories.

Author: alex.guizar(a)jboss.com Date: 2010-08-31 01:05:48 -0400 (Tue, 31 Aug 2010) New Revision: 6636 Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/GraphElement.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/db/DbPersistenceService.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceService.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceServiceFactory.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jbpm1775/JBPM1775Test.java jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/AbstractEnterpriseTestCase.java jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/jta/JtaDbPersistenceTest.java Log: JBPM-2918 do not consider a jta transaction marked for rollback as active; query transaction manager to determine whether a jta transaction is present Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/GraphElement.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/GraphElement.java 2010-08-30 16:34:53 UTC (rev 6635) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/GraphElement.java 2010-08-31 05:05:48 UTC (rev 6636) @@ -373,29 +373,29 @@ * Tells whether the given context can handle exception by checking for: * <ul> * <li>the absence of a previous exception</li> - * <li>an active transaction, or no transaction at all</li> + * <li>an active transaction, or no transaction present</li> * </ul> */ private static boolean isAbleToHandleExceptions(ExecutionContext executionContext) { // if executionContext has an exception set, it is already handling an exception // in this case, do not offer exception to handlers; throw at client - // see https://jira.jboss.org/jira/browse/JBPM-1887 + // see https://jira.jboss.org/browse/JBPM-1887 if (executionContext.getException() != null) return false; // check whether transaction is still active before scanning exception handlers // this way, the exception handlers can be loaded lazily - // see https://jira.jboss.org/jira/browse/JBPM-1775 + // see https://jira.jboss.org/browse/JBPM-1775 JbpmContext jbpmContext = executionContext.getJbpmContext(); if (jbpmContext != null) { Service service = jbpmContext.getServices().getPersistenceService(); if (service instanceof DbPersistenceService) { DbPersistenceService persistenceService = (DbPersistenceService) service; return persistenceService.isTransactionActive() - || persistenceService.getTransaction() == null; + || !persistenceService.isTransactionPresent(); } } - // no transaction detected, probably running in memory only + // no transaction present; likely running in memory return true; } Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/db/DbPersistenceService.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/db/DbPersistenceService.java 2010-08-30 16:34:53 UTC (rev 6635) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/db/DbPersistenceService.java 2010-08-31 05:05:48 UTC (rev 6636) @@ -96,25 +96,20 @@ } public Session getSession() { - if (session == null && getSessionFactory() != null) { - Connection connection = getConnection(false); + if (session == null) { + SessionFactory sessionFactory = persistenceServiceFactory.getSessionFactory(); + if (isCurrentSessionEnabled) { - session = getSessionFactory().getCurrentSession(); + session = sessionFactory.getCurrentSession(); mustSessionBeFlushed = false; mustSessionBeClosed = false; - mustConnectionBeClosed = false; } - else if (connection != null) { - session = getSessionFactory().openSession(connection); - mustSessionBeFlushed = true; - mustSessionBeClosed = true; - mustConnectionBeClosed = false; - } else { - session = getSessionFactory().openSession(); + Connection connection = getConnection(false); + session = connection != null ? sessionFactory.openSession(connection) + : sessionFactory.openSession(); mustSessionBeFlushed = true; mustSessionBeClosed = true; - mustConnectionBeClosed = false; } if (isTransactionEnabled) beginTransaction(); @@ -138,7 +133,7 @@ throw new JbpmPersistenceException("transaction commit failed", commitException); } } - else { // isRollbackOnly==true + else { Exception rollbackException = rollback(); if (rollbackException != null) { closeSession(); @@ -167,7 +162,7 @@ } } else { - if (resolveSession) { + if (session == null && resolveSession) { // initializes the session member getSession(); } @@ -198,6 +193,10 @@ return transaction != null && transaction.isActive(); } + public boolean isTransactionPresent() { + return transaction != null; + } + protected boolean isTransactionManagedExternally() { return !isTransactionEnabled; } @@ -240,7 +239,6 @@ transaction.commit(); } catch (HibernateException e) { - // avoid log and throw antipattern return e; } } @@ -257,7 +255,6 @@ transaction.rollback(); } catch (HibernateException e) { - // avoid log and throw antipattern return e; } } @@ -271,7 +268,6 @@ session.flush(); } catch (HibernateException e) { - // avoid log and throw antipattern return e; } } @@ -289,7 +285,6 @@ session.close(); } catch (HibernateException e) { - // avoid log and throw antipattern return e; } } @@ -307,7 +302,6 @@ connection.close(); } catch (SQLException e) { - // avoid log and throw antipattern return e; } } Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceService.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceService.java 2010-08-30 16:34:53 UTC (rev 6635) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceService.java 2010-08-31 05:05:48 UTC (rev 6636) @@ -32,8 +32,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.hibernate.Session; import org.hibernate.engine.SessionFactoryImplementor; -import org.hibernate.util.JTAHelper; import org.jbpm.JbpmException; import org.jbpm.persistence.JbpmPersistenceException; @@ -41,35 +41,38 @@ public class JtaDbPersistenceService extends DbPersistenceService { - private UserTransaction transaction; + private UserTransaction userTransaction; private static final long serialVersionUID = 1L; private static final Log log = LogFactory.getLog(JtaDbPersistenceService.class); public JtaDbPersistenceService(JtaDbPersistenceServiceFactory persistenceServiceFactory) { super(persistenceServiceFactory); - if (!isTransactionActive()) beginTransaction(); } public boolean isTransactionActive() { - SessionFactoryImplementor sessionFactory = (SessionFactoryImplementor) getSessionFactory(); - return JTAHelper.isTransactionInProgress(sessionFactory); + return getTransactionStatus() == Status.STATUS_ACTIVE; } + public boolean isTransactionPresent() { + int status = getTransactionStatus(); + return status != Status.STATUS_NO_TRANSACTION && status != Status.STATUS_UNKNOWN; + } + protected boolean isTransactionManagedExternally() { - return transaction == null; + return userTransaction == null; } protected boolean isTransactionRollbackOnly() { return super.isTransactionRollbackOnly() - || JTAHelper.isMarkedForRollback(getTransactionStatus()); + || getTransactionStatus() == Status.STATUS_MARKED_ROLLBACK; } public void beginTransaction() { try { JtaDbPersistenceServiceFactory jtaFactory = (JtaDbPersistenceServiceFactory) persistenceServiceFactory; - transaction = jtaFactory.getUserTransaction(); - transaction.begin(); + userTransaction = jtaFactory.getUserTransaction(); + userTransaction.begin(); } catch (NotSupportedException e) { throw new JbpmException("transaction is already in course", e); @@ -79,27 +82,37 @@ } } + public Session getSession() { + if (!isTransactionPresent()) beginTransaction(); + return super.getSession(); + } + private int getTransactionStatus() { try { - if (transaction != null) { - return transaction.getStatus(); + if (userTransaction != null) { + return userTransaction.getStatus(); } else { - SessionFactoryImplementor sessionFactory = (SessionFactoryImplementor) getSessionFactory(); - TransactionManager transactionManager = sessionFactory.getTransactionManager(); - if (transactionManager != null) return transactionManager.getStatus(); + TransactionManager transactionManager = getTransactionManager(); + return transactionManager != null ? transactionManager.getStatus() + : Status.STATUS_NO_TRANSACTION; } } catch (SystemException e) { log.error("could not get transaction status", e); + return Status.STATUS_UNKNOWN; } - return Status.STATUS_UNKNOWN; } + private TransactionManager getTransactionManager() { + SessionFactoryImplementor sessionFactory = (SessionFactoryImplementor) getSessionFactory(); + return sessionFactory.getTransactionManager(); + } + protected Exception commit() { - if (transaction != null) { + if (userTransaction != null) { try { - transaction.commit(); + userTransaction.commit(); } catch (RollbackException e) { return e; @@ -118,22 +131,21 @@ } protected Exception rollback() { - if (transaction != null) { + if (userTransaction != null) { try { - transaction.rollback(); + userTransaction.rollback(); } catch (SystemException e) { return e; } } else { - SessionFactoryImplementor sessionFactory = (SessionFactoryImplementor) getSessionFactory(); - TransactionManager transactionManager = sessionFactory.getTransactionManager(); + TransactionManager transactionManager = getTransactionManager(); if (transactionManager == null) { - throw new JbpmPersistenceException("cannot honor rollback request under external transaction manager"); + throw new JbpmPersistenceException("cannot honor rollback request in transaction manager absence"); } try { - transactionManager.getTransaction().setRollbackOnly(); + transactionManager.setRollbackOnly(); } catch (SystemException e) { return e; @@ -142,6 +154,7 @@ return null; } + /** @deprecated use !{@link #isTransactionManagedExternally()} instead */ public boolean isJtaTxCreated() { return !isTransactionManagedExternally(); } Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceServiceFactory.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceServiceFactory.java 2010-08-30 16:34:53 UTC (rev 6635) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/persistence/jta/JtaDbPersistenceServiceFactory.java 2010-08-31 05:05:48 UTC (rev 6636) @@ -68,7 +68,7 @@ /* * EJB 2.1 section 20.9 The container must make the UserTransaction interface available * to the enterprise beans that are allowed to use this interface (only session and - * message- driven beans with bean-managed transaction demarcation are allowed to use + * message-driven beans with bean-managed transaction demarcation are allowed to use * this interface) in JNDI under the name java:comp/UserTransaction. */ /* Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jbpm1775/JBPM1775Test.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jbpm1775/JBPM1775Test.java 2010-08-30 16:34:53 UTC (rev 6635) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jbpm1775/JBPM1775Test.java 2010-08-31 05:05:48 UTC (rev 6636) @@ -21,6 +21,8 @@ */ package org.jbpm.jbpm1775; +import org.hibernate.TransactionException; + import org.jbpm.db.AbstractDbTestCase; import org.jbpm.graph.def.ActionHandler; import org.jbpm.graph.def.DelegationException; @@ -40,17 +42,13 @@ public void testExceptionInAction() { ProcessDefinition processDefinition = ProcessDefinition.parseXmlString("<?xml version='1.0'?>" - + "<process-definition name='" - + getName() - + "'>" - + " <exception-handler exception-class='java.lang.IllegalStateException'>" + + "<process-definition name='jbpm1775'>" + + " <exception-handler exception-class='" + TransactionException.class.getName() + "'>" + " <action class='org.example.NoSuchAction' />" + " </exception-handler>" + " <start-state name='start'>" + " <transition to='end'>" - + " <action class='" - + ExceptionAction.class.getName() - + "' />" + + " <action class='" + RollbackAction.class.getName() + "' />" + " </transition>" + " </start-state>" + " <end-state name='end' />" @@ -63,7 +61,7 @@ } catch (DelegationException e) { jbpmContext.setRollbackOnly(); - assertSame(IllegalStateException.class, e.getCause().getClass()); + assert e.getCause() instanceof TransactionException : e.getCause(); try { closeJbpmContext(); @@ -75,7 +73,7 @@ } } - public static class ExceptionAction implements ActionHandler { + public static class RollbackAction implements ActionHandler { private static final long serialVersionUID = 1L; public void execute(ExecutionContext executionContext) throws Exception { @@ -86,7 +84,7 @@ .getPersistenceService(); persistenceService.getTransaction().rollback(); // throw exception - throw new IllegalStateException("transaction was forcefully rolled back"); + throw new TransactionException("transaction rolled back"); } } } Modified: jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/AbstractEnterpriseTestCase.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/AbstractEnterpriseTestCase.java 2010-08-30 16:34:53 UTC (rev 6635) +++ jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/AbstractEnterpriseTestCase.java 2010-08-31 05:05:48 UTC (rev 6636) @@ -83,22 +83,19 @@ protected CommandService createCommandService() throws Exception { if (commandServiceHome == null) { - commandServiceHome = (LocalCommandServiceHome) getEnvironment().lookup( - "ejb/CommandServiceBean"); + commandServiceHome = (LocalCommandServiceHome) getEnvironment().lookup("ejb/CommandServiceBean"); } return new RetryCommandService(commandServiceHome.create()); } protected ProcessDefinition deployProcessDefinition(String xml) { - ProcessDefinition processDefinition = (ProcessDefinition) commandService - .execute(new DeployProcessCommand(xml)); + ProcessDefinition processDefinition = (ProcessDefinition) commandService.execute(new DeployProcessCommand(xml)); processDefinitions.add(processDefinition); return processDefinition; } protected ProcessDefinition deployProcessDefinition(byte[] processArchive) { - ProcessDefinition processDefinition = (ProcessDefinition) commandService - .execute(new DeployProcessCommand(processArchive)); + ProcessDefinition processDefinition = (ProcessDefinition) commandService.execute(new DeployProcessCommand(processArchive)); processDefinitions.add(processDefinition); return processDefinition; } @@ -115,8 +112,7 @@ } protected boolean hasProcessInstanceEnded(final long processInstanceId) { - ProcessInstance processInstance = (ProcessInstance) commandService - .execute(new GetProcessInstanceCommand(processInstanceId)); + ProcessInstance processInstance = (ProcessInstance) commandService.execute(new GetProcessInstanceCommand(processInstanceId)); return processInstance.hasEnded(); } @@ -136,8 +132,7 @@ private static final long serialVersionUID = 1L; public Object execute(JbpmContext jbpmContext) throws Exception { - DbPersistenceServiceFactory factory = (DbPersistenceServiceFactory) jbpmContext - .getServiceFactory(Services.SERVICENAME_PERSISTENCE); + DbPersistenceServiceFactory factory = (DbPersistenceServiceFactory) jbpmContext.getServiceFactory(Services.SERVICENAME_PERSISTENCE); return factory.getConfiguration().getProperty(Environment.DIALECT); } }); @@ -160,7 +155,7 @@ return environment; } - static final class RetryCommandService implements CommandService { + private static final class RetryCommandService implements CommandService { private final CommandService delegate; @@ -178,8 +173,8 @@ catch (RuntimeException e) { if (!DbPersistenceService.isLockingException(e)) throw e; // if this is a locking exception, keep it quiet - StaleObjectLogConfigurer.getStaleObjectExceptionsLog().error( - "failed to execute " + command); + StaleObjectLogConfigurer.getStaleObjectExceptionsLog().error("failed to execute " + + command); lockingException = e; } } Modified: jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/jta/JtaDbPersistenceTest.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/jta/JtaDbPersistenceTest.java 2010-08-30 16:34:53 UTC (rev 6635) +++ jbpm3/branches/jbpm-3.2-soa/modules/enterprise/src/test/java/org/jbpm/enterprise/jta/JtaDbPersistenceTest.java 2010-08-31 05:05:48 UTC (rev 6636) @@ -23,9 +23,8 @@ import junit.framework.Test; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.hibernate.HibernateException; +import org.hibernate.TransactionException; import org.jbpm.JbpmConfiguration; import org.jbpm.JbpmContext; @@ -37,64 +36,106 @@ public class JtaDbPersistenceTest extends AbstractEnterpriseTestCase { - private static final Log log = LogFactory.getLog(JtaDbPersistenceTest.class); - - public static Test suite() throws Exception { + public static Test suite() { return new IntegrationTestSetup(JtaDbPersistenceTest.class, "enterprise-test.war"); } protected CommandService createCommandService() throws Exception { - return getName().indexOf("Container") != -1 ? super.createCommandService() : new CommandServiceImpl( - JbpmConfiguration.getInstance()); + return getName().indexOf("Container") != -1 ? super.createCommandService() + : new CommandServiceImpl(JbpmConfiguration.getInstance()); } - public void testContainerTxSuccess() throws Exception { - playTransaction(false); + public void testContainerTxSuccess() { + executeProcess(); } - public void testContainerTxFailure() throws Exception { - playTransaction(true); + public void testContainerTxFailure() { + executeProcess(); } - public void testUserTxSuccess() throws Exception { - playTransaction(false); + public void testContainerTxRollback() { + executeProcess(); } - public void testUserTxFailure() throws Exception { - playTransaction(true); + public void testContainerTxExceptionHandler() { + executeExceptionProcess(); } - private void playTransaction(boolean fail) throws Exception { + public void testUserTxSuccess() { + executeProcess(); + } + + public void testUserTxFailure() { + executeProcess(); + } + + public void testUserTxRollback() { + executeProcess(); + } + + public void testUserTxExceptionHandler() { + executeExceptionProcess(); + } + + private void executeProcess() { deployProcessDefinition("<process-definition name='jta'>" - + " <start-state name='start'>" - + " <transition to='midway' />" - + " </start-state>" - + " <state name='midway'>" - + " <transition to='end' />" - + " </state>" - + " <end-state name='end' />" - + "</process-definition>"); - long processInstanceId = startProcessInstance("jta").getId(); + + " <start-state name='start'>" + + " <transition to='midway' />" + + " </start-state>" + + " <state name='midway'>" + + " <transition to='end' />" + + " </state>" + + " <end-state name='end' />" + + "</process-definition>"); + final long processInstanceId = startProcessInstance("jta").getId(); + final String testName = getName(); try { - signal(processInstanceId, fail); + commandService.execute(new Command() { + private static final long serialVersionUID = 1L; + + public Object execute(JbpmContext jbpmContext) { + jbpmContext.loadProcessInstance(processInstanceId).signal(); + if (testName.endsWith("Failure")) throw new HibernateException("simulated failure"); + if (testName.endsWith("Rollback")) jbpmContext.setRollbackOnly(); + return null; + } + }); } catch (RuntimeException e) { - log.debug("signal failed on process instance #" + processInstanceId, e); + assertSame(HibernateException.class, getUltimateCause(e).getClass()); } - assertEquals(!fail, hasProcessInstanceEnded(processInstanceId)); + assertEquals(testName.endsWith("Success"), hasProcessInstanceEnded(processInstanceId)); } - private void signal(final long processInstanceId, final boolean fail) throws Exception { - commandService.execute(new Command() { + private void executeExceptionProcess() { + deployProcessDefinition("<process-definition name='jbpm2918'>" + + " <exception-handler exception-class='" + TransactionException.class.getName() + "'>" + + " <action class='org.example.NoSuchAction' />" + + " </exception-handler>" + + " <start-state name='start'>" + + " <transition to='end'>" + + " <script>" + + " executionContext.getJbpmContext().getSessionFactory().getTransactionManager().setRollbackOnly();" + + " throw new org.hibernate.TransactionException(\"transaction marked for rollback\");" + + " </script>" + + " </transition>" + + " </start-state>" + + " <end-state name='end' />" + + "</process-definition>"); + try { + startProcessInstance("jbpm2918"); + fail("expected exception"); + } + catch (RuntimeException e) { + assertSame(TransactionException.class, getUltimateCause(e).getClass()); + } + } - private static final long serialVersionUID = 1L; - - public Object execute(JbpmContext jbpmContext) throws Exception { - jbpmContext.loadProcessInstance(processInstanceId).signal(); - if (fail) throw new HibernateException("simulated failure"); - return null; - } - }); + private static Throwable getUltimateCause(Throwable exception) { + Throwable cause = exception; + while (cause.getCause() != null) { + cause = cause.getCause(); + } + return cause; } - }