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

Author: alex.guizar(a)jboss.com Date: 2010-05-12 05:43:32 -0400 (Wed, 12 May 2010) New Revision: 6329 Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/DelegationException.java 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/Node.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/node/Decision.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/svc/save/CheckUnpersistableVariablesOperation.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/taskmgmt/exe/TaskMgmtInstance.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jpdl/exe/DecisionHandlerTest.java jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/scheduler/exe/SchedulerTest.java Log: JBPM-1162: ensure decision node selects default transition after decision handler throws exception Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/DelegationException.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/DelegationException.java 2010-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/DelegationException.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -28,17 +28,20 @@ private static final long serialVersionUID = 1L; + /** @deprecated this execution context may not be in a consistent state */ protected transient ExecutionContext executionContext; public DelegationException(String message, Throwable cause) { super(message, cause); } + /** @deprecated the given execution context may not be in a consistent state */ public DelegationException(Throwable cause, ExecutionContext executionContext) { super(cause.getMessage(), cause); this.executionContext = executionContext; } + /** @deprecated the returned execution context may not be in a consistent state */ public ExecutionContext getExecutionContext() { return executionContext; } 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-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/GraphElement.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -157,13 +157,16 @@ } public void reorderExceptionHandler(int oldIndex, int newIndex) { - if (exceptionHandlers != null && Math.min(oldIndex, newIndex) >= 0 + if (exceptionHandlers != null + && Math.min(oldIndex, newIndex) >= 0 && Math.max(oldIndex, newIndex) < exceptionHandlers.size()) { Object o = exceptionHandlers.remove(oldIndex); exceptionHandlers.add(newIndex, o); } else { - throw new IndexOutOfBoundsException("could not move element from " + oldIndex + " to " + throw new IndexOutOfBoundsException("could not move element from " + + oldIndex + + " to " + newIndex); } } @@ -324,14 +327,14 @@ } /** - * throws an ActionException if no applicable exception handler is found. An ExceptionHandler - * is searched for in this graph element and then recursively up the parent hierarchy. If an - * exception handler is found, it is applied. If the exception handler does not throw an - * exception, the exception is considered handled. Otherwise the search for an applicable - * exception handler continues where it left of with the newly thrown exception. + * looks for an {@linkplain ExceptionHandler exception handler} in this graph element and then + * recursively up the parent hierarchy. If an exception handler is found, it is applied. If + * the exception handler does not rethrow, the exception is considered handled. Otherwise the + * rethrown exception is propagated up the parent hierarchy. + * + * @throws DelegationException if no applicable exception handler is found */ - public void raiseException(Throwable exception, ExecutionContext executionContext) - throws DelegationException { + public void raiseException(Throwable exception, ExecutionContext executionContext) { if (isAbleToHandleExceptions(executionContext)) { try { ExceptionHandler exceptionHandler = findExceptionHandler(exception); @@ -358,7 +361,7 @@ // if exception was not handled, throw delegation exception to client throw exception instanceof JbpmException ? (JbpmException) exception - : new DelegationException(exception, executionContext); + : new DelegationException("no applicable exception handler found", exception); } /** Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/Node.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/Node.java 2010-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/def/Node.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -272,14 +272,14 @@ * is the default leaving transition. */ public Transition getDefaultLeavingTransition() { - if (leavingTransitions != null) { + if (leavingTransitions != null && !leavingTransitions.isEmpty()) { // select the first unconditional transition for (Iterator i = leavingTransitions.iterator(); i.hasNext();) { Transition transition = (Transition) i.next(); if (transition.getCondition() == null) return transition; } // there is no unconditional transition, just pick the first one - if (!leavingTransitions.isEmpty()) return (Transition) leavingTransitions.get(0); + return (Transition) leavingTransitions.get(0); } else if (superState != null) { return superState.getDefaultLeavingTransition(); Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/node/Decision.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/node/Decision.java 2010-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/graph/node/Decision.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -91,19 +91,11 @@ Transition transition = null; if (decisionDelegation != null) { - DecisionHandler decisionHandler = (DecisionHandler) decisionDelegation.getInstance(); - try { - String transitionName = decisionHandler.decide(executionContext); - transition = getLeavingTransition(transitionName); - if (transition == null) { - throw new JbpmException("no such transition: " + transitionName); - } - } - catch (Exception e) { - raiseException(e, executionContext); - } + // call decision handler + transition = handleDecision(executionContext); } else if (decisionExpression != null) { + // evaluate expression String transitionName = (String) JbpmExpressionEvaluator .evaluate(decisionExpression, executionContext, String.class); transition = getLeavingTransition(transitionName); @@ -165,6 +157,26 @@ } } + private Transition handleDecision(ExecutionContext executionContext) { + // invoke handler to obtain transition name + DecisionHandler decisionHandler = (DecisionHandler) decisionDelegation.getInstance(); + String transitionName; + try { + transitionName = decisionHandler.decide(executionContext); + } + catch (Exception e) { + raiseException(e, executionContext); + return null; + } + + // resolve transition from name + Transition transition = getLeavingTransition(transitionName); + if (transition == null) { + throw new JbpmException("no such transition: " + transitionName); + } + return transition; + } + public List getDecisionConditions() { return decisionConditions; } Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/svc/save/CheckUnpersistableVariablesOperation.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/svc/save/CheckUnpersistableVariablesOperation.java 2010-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/svc/save/CheckUnpersistableVariablesOperation.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -37,31 +37,20 @@ private static final long serialVersionUID = 1L; public void save(ProcessInstance processInstance, JbpmContext jbpmContext) { - Collection updatedVariableContainers = VariableContainer.getUpdatedVariableContainers(processInstance); - if (updatedVariableContainers!=null) { - - // loop over all updated variable containers - Iterator iter = updatedVariableContainers.iterator(); - while (iter.hasNext()) { - VariableContainer variableContainer = (VariableContainer) iter.next(); + Collection variableContainers = processInstance.getContextInstance() + .getUpdatedVariableContainers(); + // iterate over variable containers + if (variableContainers != null) { + for (Iterator containerIter = variableContainers.iterator(); containerIter.hasNext();) { + VariableContainer variableContainer = (VariableContainer) containerIter.next(); Map variableInstances = variableContainer.getVariableInstances(); - if (variableInstances!=null) { - - // loop over all variable instances in the container - Iterator varInstancesIter = variableInstances.entrySet().iterator(); - while (varInstancesIter.hasNext()) { - Map.Entry entry = (Map.Entry) varInstancesIter.next(); - String name = (String) entry.getKey(); - VariableInstance variableInstance = (VariableInstance) entry.getValue(); - - // if the variable is of the unpersistable type... booom! + // iterate over variable instances + if (variableInstances != null) { + for (Iterator instanceIter = variableInstances.values().iterator(); instanceIter.hasNext();) { + VariableInstance variableInstance = (VariableInstance) instanceIter.next(); + // if the variable cannot be persisted... boom! if (variableInstance instanceof UnpersistableInstance) { - Object value = variableInstance.getValue(); - if (value!=null) { - throw new JbpmException("variable '"+name+"' in '"+variableContainer+"' contains '"+value+"': type '"+value.getClass().getName()+"' is unpersistable according to the jbpm.varmapping.xml configuration"); - } else { - throw new JbpmException("variable '"+name+"' in '"+variableContainer+"' was created with a non persistable value"); - } + throw new JbpmException(variableInstance + " cannot be persisted"); } } } @@ -69,5 +58,4 @@ } } - // private static Log log = LogFactory.getLog(CheckUnpersistableVariablesOperation.class); } Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/taskmgmt/exe/TaskMgmtInstance.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/taskmgmt/exe/TaskMgmtInstance.java 2010-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/main/java/org/jbpm/taskmgmt/exe/TaskMgmtInstance.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -230,17 +230,7 @@ public void performAssignment(Delegation assignmentDelegation, String actorIdExpression, String pooledActorsExpression, Assignable assignable, ExecutionContext executionContext) { if (assignmentDelegation != null) { - try { - performAssignmentDelegation(assignmentDelegation, assignable, executionContext); - } - catch (Exception e) { - GraphElement graphElement = executionContext.getEventSource(); - if (graphElement == null) { - throw e instanceof JbpmException ? (JbpmException) e - : new DelegationException(e, executionContext); - } - graphElement.raiseException(e, executionContext); - } + performAssignmentDelegation(assignmentDelegation, assignable, executionContext); } else { if (actorIdExpression != null) { @@ -253,8 +243,8 @@ } private void performAssignmentDelegation(Delegation assignmentDelegation, - Assignable assignable, ExecutionContext executionContext) throws Exception { - ClassLoader surroundingClassLoader = Thread.currentThread().getContextClassLoader(); + Assignable assignable, ExecutionContext executionContext) { + ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); try { // set context class loader correctly for delegation class // https://jira.jboss.org/jira/browse/JBPM-1448 @@ -268,15 +258,25 @@ // invoke the assignment handler UserCodeInterceptor userCodeInterceptor = UserCodeInterceptorConfig .getUserCodeInterceptor(); - if (userCodeInterceptor != null) { - userCodeInterceptor.executeAssignment(assignmentHandler, assignable, executionContext); + try { + if (userCodeInterceptor != null) { + userCodeInterceptor.executeAssignment(assignmentHandler, assignable, executionContext); + } + else { + assignmentHandler.assign(assignable, executionContext); + } } - else { - assignmentHandler.assign(assignable, executionContext); + catch (Exception e) { + GraphElement eventSource = executionContext.getEventSource(); + if (eventSource == null) { + throw e instanceof JbpmException ? (JbpmException) e + : new DelegationException("event source is null", e); + } + eventSource.raiseException(e, executionContext); } } finally { - Thread.currentThread().setContextClassLoader(surroundingClassLoader); + Thread.currentThread().setContextClassLoader(contextClassLoader); } } Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jpdl/exe/DecisionHandlerTest.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jpdl/exe/DecisionHandlerTest.java 2010-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/jpdl/exe/DecisionHandlerTest.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -22,6 +22,7 @@ package org.jbpm.jpdl.exe; import org.jbpm.AbstractJbpmTestCase; +import org.jbpm.JbpmException; import org.jbpm.graph.def.ProcessDefinition; import org.jbpm.graph.exe.ExecutionContext; import org.jbpm.graph.exe.ProcessInstance; @@ -33,51 +34,76 @@ private static final long serialVersionUID = 1L; public String decide(ExecutionContext executionContext) { - int budget = ((Number)executionContext.getContextInstance().getVariable("budget")).intValue(); - if (budget>1000) return "important lead"; - else if (budget>100) return "lead"; - return "beggars"; + Number wrapper = (Number) executionContext.getContextInstance().getVariable("budget"); + int budget = wrapper.intValue(); + return budget > 1000 ? "important lead" : budget == 777 ? "lucky number" + : budget > 100 ? "lead" : "beggars"; } } - ProcessDefinition processDefinition = ProcessDefinition.parseXmlString( - "<process-definition>" + - " <start-state>" + - " <transition to='d' />" + - " </start-state>" + - " <decision name='d'>" + - " <handler class='org.jbpm.jpdl.exe.DecisionHandlerTest$LeadEvaluator'/>" + - " <transition name='important lead' to='harras them'/>" + - " <transition name='lead' to='put it in the lead db'/>" + - " <transition name='beggars' to='forget about it'/>" + - " </decision>" + - " <state name='harras them' />" + - " <state name='put it in the lead db' />" + - " <state name='forget about it' />" + - "</process-definition>" ); - + ProcessDefinition processDefinition = ProcessDefinition.parseXmlString("<process-definition>" + + " <start-state>" + + " <transition to='d' />" + + " </start-state>" + + " <decision name='d'>" + + " <handler class='org.jbpm.jpdl.exe.DecisionHandlerTest$LeadEvaluator'/>" + + " <transition name='beggars' to='forget about it'/>" + + " <transition name='lead' to='put it in the lead db'/>" + + " <transition name='important lead' to='harass them'/>" + + " <exception-handler exception-class='java.lang.ClassCastException'/>" + + " </decision>" + + " <state name='harass them' />" + + " <state name='put it in the lead db' />" + + " <state name='forget about it' />" + + "</process-definition>"); + public void testBudgetHignerThenThousand() { ProcessInstance processInstance = new ProcessInstance(processDefinition); processInstance.getContextInstance().setVariable("budget", new Integer(3500)); processInstance.signal(); - - assertEquals(processDefinition.getNode("harras them"), processInstance.getRootToken().getNode()); + + assertEquals(processDefinition.getNode("harass them"), processInstance.getRootToken() + .getNode()); } public void testBudgetBetweenHundredAndThousand() { ProcessInstance processInstance = new ProcessInstance(processDefinition); processInstance.getContextInstance().setVariable("budget", new Integer(350)); processInstance.signal(); - - assertEquals(processDefinition.getNode("put it in the lead db"), processInstance.getRootToken().getNode()); + + assertEquals(processDefinition.getNode("put it in the lead db"), processInstance.getRootToken() + .getNode()); } public void testSmallBudget() { ProcessInstance processInstance = new ProcessInstance(processDefinition); processInstance.getContextInstance().setVariable("budget", new Integer(35)); processInstance.signal(); - - assertEquals(processDefinition.getNode("forget about it"), processInstance.getRootToken().getNode()); + + assertEquals(processDefinition.getNode("forget about it"), processInstance.getRootToken() + .getNode()); } + public void testBadTransitionName() { + ProcessInstance processInstance = new ProcessInstance(processDefinition); + processInstance.getContextInstance().setVariable("budget", new Integer(777)); + + try { + processInstance.signal(); + fail("expected exception"); + } + catch (JbpmException e) { + assert e.getMessage().indexOf("lucky number") != -1; + } + } + + public void testHandledException() { + ProcessInstance processInstance = new ProcessInstance(processDefinition); + processInstance.getContextInstance().setVariable("budget", "nothing"); + processInstance.signal(); + + // decision node should take default transition + assertEquals(processDefinition.getNode("forget about it"), processInstance.getRootToken() + .getNode()); + } } Modified: jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/scheduler/exe/SchedulerTest.java =================================================================== --- jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/scheduler/exe/SchedulerTest.java 2010-05-12 04:10:11 UTC (rev 6328) +++ jbpm3/branches/jbpm-3.2-soa/modules/core/src/test/java/org/jbpm/scheduler/exe/SchedulerTest.java 2010-05-12 09:43:32 UTC (rev 6329) @@ -689,7 +689,9 @@ processInstance.signal(); } catch (JbpmException je) { - assertEquals("improper format of duration '1 demo'", je.getMessage()); + Throwable cause = je.getCause(); + assertSame(IllegalArgumentException.class, cause.getClass()); + assert cause.getMessage().indexOf("1 demo") != -1 : cause; } finally { jbpmContext.close();