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;
}
-
}