From feaf06407d9f558f111769d0b4125d7b2abc82c8 Mon Sep 17 00:00:00 2001 From: Tijs Rademakers Date: Wed, 26 Nov 2014 21:27:44 +0100 Subject: [PATCH] Small changes to skip expression pull request --- .../converter/SequenceFlowXMLConverter.java | 3 +- .../converter/ServiceTaskXMLConverter.java | 3 +- .../bpmn/converter/UserTaskXMLConverter.java | 3 +- .../bpmn/behavior/BpmnActivityBehavior.java | 16 ++++----- ...askDelegateExpressionActivityBehavior.java | 3 +- ...ServiceTaskExpressionActivityBehavior.java | 3 +- .../behavior/UserTaskActivityBehavior.java | 3 +- .../impl/bpmn/helper/ClassDelegate.java | 16 +++++++-- .../impl/bpmn/helper/SkipExpressionUtil.java | 36 +++++++++---------- .../DefaultActivityBehaviorFactory.java | 18 +++------- .../factory/DefaultListenerFactory.java | 4 +-- .../handler/SequenceFlowParseHandler.java | 7 ++-- .../impl/pvm/ProcessDefinitionBuilder.java | 2 +- .../engine/impl/pvm/process/ActivityImpl.java | 16 ++++++--- .../test/TestActivityBehaviorFactory.java | 4 +-- 15 files changed, 68 insertions(+), 69 deletions(-) diff --git a/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/SequenceFlowXMLConverter.java b/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/SequenceFlowXMLConverter.java index 7c92a00743..b3c109a30d 100644 --- a/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/SequenceFlowXMLConverter.java +++ b/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/SequenceFlowXMLConverter.java @@ -54,8 +54,7 @@ public class SequenceFlowXMLConverter extends BaseBpmnXMLConverter { SequenceFlow sequenceFlow = (SequenceFlow) element; writeDefaultAttribute(ATTRIBUTE_FLOW_SOURCE_REF, sequenceFlow.getSourceRef(), xtw); writeDefaultAttribute(ATTRIBUTE_FLOW_TARGET_REF, sequenceFlow.getTargetRef(), xtw); - if(StringUtils.isNotEmpty(sequenceFlow.getSkipExpression())) - { + if (StringUtils.isNotEmpty(sequenceFlow.getSkipExpression())) { writeDefaultAttribute(ATTRIBUTE_FLOW_SKIP_EXPRESSION, sequenceFlow.getSkipExpression(), xtw); } } diff --git a/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/ServiceTaskXMLConverter.java b/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/ServiceTaskXMLConverter.java index 183ba45099..bfb0b61485 100644 --- a/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/ServiceTaskXMLConverter.java +++ b/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/ServiceTaskXMLConverter.java @@ -97,8 +97,7 @@ public class ServiceTaskXMLConverter extends BaseBpmnXMLConverter { if (StringUtils.isNotEmpty(serviceTask.getExtensionId())) { writeQualifiedAttribute(ATTRIBUTE_TASK_SERVICE_EXTENSIONID, serviceTask.getExtensionId(), xtw); } - if(StringUtils.isNotEmpty(serviceTask.getSkipExpression())) - { + if (StringUtils.isNotEmpty(serviceTask.getSkipExpression())) { writeQualifiedAttribute(ATTRIBUTE_TASK_SERVICE_SKIP_EXPRESSION, serviceTask.getSkipExpression(), xtw); } } diff --git a/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/UserTaskXMLConverter.java b/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/UserTaskXMLConverter.java index 2b3804f75e..9073daad86 100644 --- a/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/UserTaskXMLConverter.java +++ b/modules/activiti-bpmn-converter/src/main/java/org/activiti/bpmn/converter/UserTaskXMLConverter.java @@ -128,8 +128,7 @@ public class UserTaskXMLConverter extends BaseBpmnXMLConverter { if (userTask.getPriority() != null) { writeQualifiedAttribute(ATTRIBUTE_TASK_USER_PRIORITY, userTask.getPriority().toString(), xtw); } - if(null != userTask.getSkipExpression()) - { + if (userTask.getSkipExpression() != null) { writeQualifiedAttribute(ATTRIBUTE_TASK_USER_SKIP_EXPRESSION, userTask.getSkipExpression(), xtw); } // write custom attributes diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/BpmnActivityBehavior.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/BpmnActivityBehavior.java index 05025a4739..5b630f9044 100755 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/BpmnActivityBehavior.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/BpmnActivityBehavior.java @@ -13,6 +13,11 @@ package org.activiti.engine.impl.bpmn.behavior; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.activiti.engine.ActivitiException; import org.activiti.engine.delegate.Expression; import org.activiti.engine.delegate.event.ActivitiEventType; @@ -30,11 +35,6 @@ import org.activiti.engine.impl.pvm.runtime.InterpretableExecution; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.Serializable; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - /** * Helper class for implementing BPMN 2.0 activities, offering convenience * methods specific to BPMN 2.0. @@ -122,15 +122,15 @@ public class BpmnActivityBehavior implements Serializable { for (PvmTransition outgoingTransition : outgoingTransitions) { Expression skipExpression = outgoingTransition.getSkipExpression(); - if(! SkipExpressionUtil.isSkipExpressionEnabled(execution, skipExpression)) { + if (!SkipExpressionUtil.isSkipExpressionEnabled(execution, skipExpression)) { if (defaultSequenceFlow == null || !outgoingTransition.getId().equals(defaultSequenceFlow)) { Condition condition = (Condition) outgoingTransition.getProperty(BpmnParse.PROPERTYNAME_CONDITION); if (condition == null || !checkConditions || condition.evaluate(execution)) { transitionsToTake.add(outgoingTransition); } } - } - else if(SkipExpressionUtil.skipFlowElement(execution, skipExpression)){ + + } else if (SkipExpressionUtil.shouldSkipFlowElement(execution, skipExpression)){ transitionsToTake.add(outgoingTransition); } } diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskDelegateExpressionActivityBehavior.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskDelegateExpressionActivityBehavior.java index 047fd9f984..c5599ad435 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskDelegateExpressionActivityBehavior.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskDelegateExpressionActivityBehavior.java @@ -65,7 +65,8 @@ public class ServiceTaskDelegateExpressionActivityBehavior extends TaskActivityB try { boolean isSkipExpressionEnabled = SkipExpressionUtil.isSkipExpressionEnabled(execution, skipExpression); if (!isSkipExpressionEnabled || - (isSkipExpressionEnabled && !SkipExpressionUtil.skipFlowElement(execution, skipExpression))) { + (isSkipExpressionEnabled && !SkipExpressionUtil.shouldSkipFlowElement(execution, skipExpression))) { + // Note: we can't cache the result of the expression, because the // execution can change: eg. // delegateExpression='${mySpringBeanFactory.randomSpringBean()}' diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskExpressionActivityBehavior.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskExpressionActivityBehavior.java index bdd03d2223..916b68bb53 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskExpressionActivityBehavior.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/ServiceTaskExpressionActivityBehavior.java @@ -13,7 +13,6 @@ package org.activiti.engine.impl.bpmn.behavior; -import org.activiti.engine.ActivitiIllegalArgumentException; import org.activiti.engine.delegate.BpmnError; import org.activiti.engine.delegate.Expression; import org.activiti.engine.impl.bpmn.helper.ErrorPropagation; @@ -47,7 +46,7 @@ public class ServiceTaskExpressionActivityBehavior extends TaskActivityBehavior try { boolean isSkipExpressionEnabled = SkipExpressionUtil.isSkipExpressionEnabled(execution, skipExpression); if (!isSkipExpressionEnabled || - (isSkipExpressionEnabled && !SkipExpressionUtil.skipFlowElement(execution, skipExpression))) { + (isSkipExpressionEnabled && !SkipExpressionUtil.shouldSkipFlowElement(execution, skipExpression))) { value = expression.getValue(execution); if (resultVariable != null) { execution.setVariable(resultVariable, value); diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java index fb9f66e864..d862b87f1b 100755 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/behavior/UserTaskActivityBehavior.java @@ -135,7 +135,8 @@ public class UserTaskActivityBehavior extends TaskActivityBehavior { Expression skipExpression = taskDefinition.getSkipExpression(); if (SkipExpressionUtil.isSkipExpressionEnabled(execution, skipExpression) && - SkipExpressionUtil.skipFlowElement(execution, skipExpression)) { + SkipExpressionUtil.shouldSkipFlowElement(execution, skipExpression)) { + leave(execution); } } diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ClassDelegate.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ClassDelegate.java index 354b013d9d..f464a1c635 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ClassDelegate.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/ClassDelegate.java @@ -22,9 +22,9 @@ import org.activiti.engine.ActivitiException; import org.activiti.engine.ActivitiIllegalArgumentException; import org.activiti.engine.delegate.BpmnError; import org.activiti.engine.delegate.DelegateExecution; -import org.activiti.engine.delegate.Expression; import org.activiti.engine.delegate.DelegateTask; import org.activiti.engine.delegate.ExecutionListener; +import org.activiti.engine.delegate.Expression; import org.activiti.engine.delegate.JavaDelegate; import org.activiti.engine.delegate.TaskListener; import org.activiti.engine.impl.bpmn.behavior.AbstractBpmnActivityBehavior; @@ -62,8 +62,16 @@ public class ClassDelegate extends AbstractBpmnActivityBehavior implements TaskL this.fieldDeclarations = fieldDeclarations; this.skipExpression = skipExpression; } + + public ClassDelegate(String className, List fieldDeclarations) { + this(className, fieldDeclarations, null); + } + + public ClassDelegate(Class clazz, List fieldDeclarations) { + this(clazz.getName(), fieldDeclarations, null); + } - public ClassDelegate(Class< ? > clazz, List fieldDeclarations, Expression skipExpression) { + public ClassDelegate(Class clazz, List fieldDeclarations, Expression skipExpression) { this(clazz.getName(), fieldDeclarations, skipExpression); } @@ -115,10 +123,12 @@ public class ClassDelegate extends AbstractBpmnActivityBehavior implements TaskL public void execute(ActivityExecution execution) throws Exception { boolean isSkipExpressionEnabled = SkipExpressionUtil.isSkipExpressionEnabled(execution, skipExpression); if (!isSkipExpressionEnabled || - (isSkipExpressionEnabled && !SkipExpressionUtil.skipFlowElement(execution, skipExpression))) { + (isSkipExpressionEnabled && !SkipExpressionUtil.shouldSkipFlowElement(execution, skipExpression))) { + if (activityBehaviorInstance == null) { activityBehaviorInstance = getActivityBehaviorInstance(execution); } + try { activityBehaviorInstance.execute(execution); } catch (BpmnError error) { diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/SkipExpressionUtil.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/SkipExpressionUtil.java index 4c46809e2e..2cf40b3dcf 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/SkipExpressionUtil.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/helper/SkipExpressionUtil.java @@ -6,38 +6,34 @@ import org.activiti.engine.impl.pvm.delegate.ActivityExecution; public class SkipExpressionUtil { - public static boolean isSkipExpressionEnabled(ActivityExecution execution, Expression skipExpression) - { - if(null == skipExpression) - { + + public static boolean isSkipExpressionEnabled(ActivityExecution execution, Expression skipExpression) { + + if (skipExpression == null) { return false; } final String skipExpressionEnabledVariable = "_ACTIVITI_SKIP_EXPRESSION_ENABLED"; Object isSkipExpressionEnabled = execution.getVariable(skipExpressionEnabledVariable); - if(null == isSkipExpressionEnabled) - { + + if (isSkipExpressionEnabled == null) { return false; - } - else if(isSkipExpressionEnabled instanceof Boolean) - { - return ((Boolean)isSkipExpressionEnabled).booleanValue(); - } - else - { + + } else if (isSkipExpressionEnabled instanceof Boolean) { + return ((Boolean) isSkipExpressionEnabled).booleanValue(); + + } else { throw new ActivitiIllegalArgumentException(skipExpressionEnabledVariable + " variable does not resolve to a boolean. " + isSkipExpressionEnabled); } } - public static boolean skipFlowElement(ActivityExecution execution, Expression skipExpression) - { + public static boolean shouldSkipFlowElement(ActivityExecution execution, Expression skipExpression) { Object value = skipExpression.getValue(execution); - if(value instanceof Boolean) - { + + if (value instanceof Boolean) { return ((Boolean)value).booleanValue(); - } - else - { + + } else { throw new ActivitiIllegalArgumentException("Skip expression does not resolve to a boolean: " + skipExpression.getExpressionText()); } } diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultActivityBehaviorFactory.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultActivityBehaviorFactory.java index 7a91c58cc3..b532abf212 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultActivityBehaviorFactory.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultActivityBehaviorFactory.java @@ -129,12 +129,9 @@ public class DefaultActivityBehaviorFactory extends AbstractBehaviorFactory impl public ClassDelegate createClassDelegateServiceTask(ServiceTask serviceTask) { Expression skipExpression; - if(StringUtils.isNotEmpty(serviceTask.getSkipExpression())) - { + if (StringUtils.isNotEmpty(serviceTask.getSkipExpression())) { skipExpression = expressionManager.createExpression(serviceTask.getSkipExpression()); - } - else - { + } else { skipExpression = null; } return new ClassDelegate(serviceTask.getImplementation(), createFieldDeclarations(serviceTask.getFieldExtensions()), skipExpression); @@ -143,12 +140,9 @@ public class DefaultActivityBehaviorFactory extends AbstractBehaviorFactory impl public ServiceTaskDelegateExpressionActivityBehavior createServiceTaskDelegateExpressionActivityBehavior(ServiceTask serviceTask) { Expression delegateExpression = expressionManager.createExpression(serviceTask.getImplementation()); Expression skipExpression; - if(StringUtils.isNotEmpty(serviceTask.getSkipExpression())) - { + if (StringUtils.isNotEmpty(serviceTask.getSkipExpression())) { skipExpression = expressionManager.createExpression(serviceTask.getSkipExpression()); - } - else - { + } else { skipExpression = null; } return new ServiceTaskDelegateExpressionActivityBehavior(delegateExpression, skipExpression, createFieldDeclarations(serviceTask.getFieldExtensions())); @@ -159,9 +153,7 @@ public class DefaultActivityBehaviorFactory extends AbstractBehaviorFactory impl Expression skipExpression; if (StringUtils.isNotEmpty(serviceTask.getSkipExpression())) { skipExpression = expressionManager.createExpression(serviceTask.getSkipExpression()); - } - else - { + } else { skipExpression = null; } return new ServiceTaskExpressionActivityBehavior(expression, skipExpression, serviceTask.getResultVariableName()); diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultListenerFactory.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultListenerFactory.java index b0bf6c4de8..55d437fa5b 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultListenerFactory.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/factory/DefaultListenerFactory.java @@ -65,7 +65,7 @@ public class DefaultListenerFactory extends AbstractBehaviorFactory implements L } public TaskListener createClassDelegateTaskListener(ActivitiListener activitiListener) { - return new ClassDelegate(activitiListener.getImplementation(), createFieldDeclarations(activitiListener.getFieldExtensions()), null); + return new ClassDelegate(activitiListener.getImplementation(), createFieldDeclarations(activitiListener.getFieldExtensions())); } public TaskListener createExpressionTaskListener(ActivitiListener activitiListener) { @@ -78,7 +78,7 @@ public class DefaultListenerFactory extends AbstractBehaviorFactory implements L } public ExecutionListener createClassDelegateExecutionListener(ActivitiListener activitiListener) { - return new ClassDelegate(activitiListener.getImplementation(), createFieldDeclarations(activitiListener.getFieldExtensions()), null); + return new ClassDelegate(activitiListener.getImplementation(), createFieldDeclarations(activitiListener.getFieldExtensions())); } public ExecutionListener createExpressionExecutionListener(ActivitiListener activitiListener) { diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/handler/SequenceFlowParseHandler.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/handler/SequenceFlowParseHandler.java index 19621216a8..acb9981607 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/handler/SequenceFlowParseHandler.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/handler/SequenceFlowParseHandler.java @@ -44,13 +44,10 @@ public class SequenceFlowParseHandler extends AbstractBpmnParseHandler fieldDeclarations = new ArrayList(); fieldDeclarations.add(new FieldDeclaration("name", Expression.class.getName(), new FixedValue(serviceTask.getImplementation()))); - return new ClassDelegate(NoOpServiceTask.class, fieldDeclarations, null); + return new ClassDelegate(NoOpServiceTask.class, fieldDeclarations); } @Override -- GitLab