Small changes to skip expression pull request

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

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

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

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

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()}'

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

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

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<FieldDeclaration> fieldDeclarations) {
+    this(className, fieldDeclarations, null);
+  }
+  
+  public ClassDelegate(Class<?> clazz, List<FieldDeclaration> fieldDeclarations) {
+    this(clazz.getName(), fieldDeclarations, null);
+  }
 
-  public ClassDelegate(Class< ? > clazz, List<FieldDeclaration> fieldDeclarations, Expression skipExpression) {
+  public ClassDelegate(Class<?> clazz, List<FieldDeclaration> 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) {

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

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());

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) {

modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/handler/SequenceFlowParseHandler.java

@@ -44,13 +44,10 @@ public class SequenceFlowParseHandler extends AbstractBpmnParseHandler<SequenceF
     ActivityImpl destinationActivity = scope.findActivity(sequenceFlow.getTargetRef());
 
     Expression skipExpression;
-    if(StringUtils.isNotEmpty(sequenceFlow.getSkipExpression()))
-    {
+    if (StringUtils.isNotEmpty(sequenceFlow.getSkipExpression())) {
       ExpressionManager expressionManager = bpmnParse.getExpressionManager();
       skipExpression = expressionManager.createExpression(sequenceFlow.getSkipExpression());
-    }
-    else
-    {
+    } else {
       skipExpression = null;
     }
     

modules/activiti-engine/src/main/java/org/activiti/engine/impl/pvm/ProcessDefinitionBuilder.java

@@ -81,7 +81,7 @@ public class ProcessDefinitionBuilder {
       throw new PvmException("destinationActivityId is null");
     }
     ActivityImpl activity = getActivity();
-    transition = activity.createOutgoingTransition(transitionId, null);
+    transition = activity.createOutgoingTransition(transitionId);
     unresolvedTransitions.add(new Object[]{transition, destinationActivityId});
     processElement = transition;
     return this;

modules/activiti-engine/src/main/java/org/activiti/engine/impl/pvm/process/ActivityImpl.java

@@ -51,23 +51,29 @@ public class ActivityImpl extends ScopeImpl implements PvmActivity, HasDIBounds
   public ActivityImpl(String id, ProcessDefinitionImpl processDefinition) {
     super(id, processDefinition);
   }
-
-  public TransitionImpl createOutgoingTransition() {
-    return createOutgoingTransition(null, null);
-  }
+  
   public String getFailedJobRetryTimeCycleValue() {
 		return failedJobRetryTimeCycleValue;
   }
+  
   public void setFailedJobRetryTimeCycleValue(String failedJobRetryTimeCycleValue) {
 	  this.failedJobRetryTimeCycleValue = failedJobRetryTimeCycleValue;
   }
+  
+  public TransitionImpl createOutgoingTransition() {
+    return createOutgoingTransition(null);
+  }
+  
+  public TransitionImpl createOutgoingTransition(String transitionId) {
+    return createOutgoingTransition(transitionId, null);
+  }
 
   public TransitionImpl createOutgoingTransition(String transitionId, Expression skipExpression) {
     TransitionImpl transition = new TransitionImpl(transitionId, skipExpression, processDefinition);
     transition.setSource(this);
     outgoingTransitions.add(transition);
     
-    if (transitionId!=null) {
+    if (transitionId != null) {
       if (namedOutgoingTransitions.containsKey(transitionId)) {
         throw new PvmException("activity '"+id+" has duplicate transition '"+transitionId+"'");
       }

modules/activiti-engine/src/main/java/org/activiti/engine/test/TestActivityBehaviorFactory.java

@@ -165,7 +165,7 @@ public class TestActivityBehaviorFactory extends AbstractBehaviorFactory impleme
 		} else if (serviceTask.getImplementation() != null && mockedClassDelegatesMapping.containsKey(serviceTask.getImplementation())) {
 				
 			return new ClassDelegate(mockedClassDelegatesMapping.get(serviceTask.getImplementation()),
-					createFieldDeclarations(serviceTask.getFieldExtensions()), null);
+					createFieldDeclarations(serviceTask.getFieldExtensions()));
 			
 		}
 		
@@ -176,7 +176,7 @@ public class TestActivityBehaviorFactory extends AbstractBehaviorFactory impleme
 	  List<FieldDeclaration> fieldDeclarations = new ArrayList<FieldDeclaration>();
 	  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