Merge pull request #362 from sartography/improvement/misc-cleanup-and-bugfixes

Improvement/misc cleanup and bugfixes

Author: essweine

SpiffWorkflow/bpmn/FeelLikeScriptEngine.py

@@ -20,6 +20,8 @@
 import re
 import datetime
 import operator
+import warnings
+
 from datetime import timedelta
 from decimal import Decimal
 from .PythonScriptEngine import PythonScriptEngine
@@ -30,6 +32,11 @@ def feelConvertTime(datestr,parsestr):
 
 class FeelInterval():
     def __init__(self, begin, end, leftOpen=False, rightOpen=False):
+        warnings.warn(
+            'The FEEL script engine is deprecated and will be removed in the next release',
+            DeprecationWarning,
+            stacklevel=2,
+        )
         # pesky thing with python floats and Decimal comparison
         if isinstance(begin,float):
             begin = Decimal("%0.5f"%begin)
@@ -287,26 +294,26 @@ def patch_expression(self, invalid_python, lhs=''):
             proposed_python = lhs + proposed_python
         return proposed_python
 
-    def _evaluate(self, expression, context, task=None, external_methods=None):
+    def _evaluate(self, expression, context, task=None, external_context=None):
         """
         Evaluate the given expression, within the context of the given task and
         return the result.
         """
-        if external_methods is None:
-            external_methods = {}
+        if external_context is None:
+            external_context = {}
 
         revised = self.patch_expression(expression)
-        external_methods.update(externalFuncs)
-        return super()._evaluate(revised, context, external_methods=external_methods)
+        external_context.update(externalFuncs)
+        return super()._evaluate(revised, context, external_context=external_context)
 
-    def execute(self, task, script, data, external_methods=None):
+    def execute(self, task, script, data, external_context=None):
         """
         Execute the script, within the context of the specified task
         """
-        if external_methods is None:
-            external_methods = {}
-        external_methods.update(externalFuncs)
-        super().execute(task, script, external_methods)
+        if external_context is None:
+            external_context = {}
+        external_context.update(externalFuncs)
+        super().execute(task, script, external_context)
 
 
 

SpiffWorkflow/bpmn/PythonScriptEngine.py

@@ -20,7 +20,6 @@
 import ast
 import sys
 import traceback
-import warnings
 
 from SpiffWorkflow.exceptions import SpiffWorkflowException
 from SpiffWorkflow.bpmn.exceptions import WorkflowTaskException
@@ -38,41 +37,29 @@ class PythonScriptEngine(object):
     expressions in a different way.
     """
 
-    def __init__(self, default_globals=None, scripting_additions=None, environment=None):
-
-        if default_globals is not None or scripting_additions is not None:
-            warnings.warn('default_globals and scripting_additions are deprecated.  '
-                          'Please provide an environment such as TaskDataEnvrionment',
-                          DeprecationWarning, stacklevel=2)
-
-        if environment is None:
-            environment_globals = {}
-            environment_globals.update(default_globals or {})
-            environment_globals.update(scripting_additions or {})
-            self.environment = TaskDataEnvironment(environment_globals)
-        else:
-            self.environment = environment
+    def __init__(self, environment=None):
+        self.environment = environment or TaskDataEnvironment()
 
     def validate(self, expression):
         ast.parse(expression)
 
-    def evaluate(self, task, expression, external_methods=None):
+    def evaluate(self, task, expression, external_context=None):
         """
         Evaluate the given expression, within the context of the given task and
         return the result.
         """
         try:
-            return self._evaluate(expression, task.data, external_methods)
+            return self._evaluate(expression, task.data, external_context)
         except SpiffWorkflowException as se:
             se.add_note(f"Error evaluating expression '{expression}'")
             raise se
         except Exception as e:
             raise WorkflowTaskException(f"Error evaluating expression '{expression}'", task=task, exception=e)
 
-    def execute(self, task, script, external_methods=None):
+    def execute(self, task, script, external_context=None):
         """Execute the script, within the context of the specified task."""
         try:
-            return self._execute(script, task.data, external_methods or {})
+            return self._execute(script, task.data, external_context or {})
         except Exception as err:
             wte = self.create_task_exec_exception(task, script, err)
             raise wte
@@ -111,8 +98,8 @@ def get_error_line_number_and_content(self, script, err):
             error_line = script.splitlines()[line_number - 1]
         return line_number, error_line
 
-    def _evaluate(self, expression, context, external_methods=None):
-        return self.environment.evaluate(expression, context, external_methods)
+    def _evaluate(self, expression, context, external_context=None):
+        return self.environment.evaluate(expression, context, external_context)
 
-    def _execute(self, script, context, external_methods=None):
-        return self.environment.execute(script, context, external_methods)
+    def _execute(self, script, context, external_context=None):
+        return self.environment.execute(script, context, external_context)

SpiffWorkflow/bpmn/PythonScriptEngineEnvironment.py

@@ -24,54 +24,54 @@ class BasePythonScriptEngineEnvironment:
     def __init__(self, environment_globals=None):
         self.globals = environment_globals or {}
 
-    def evaluate(self, expression, context, external_methods=None):
+    def evaluate(self, expression, context, external_context=None):
         raise NotImplementedError("Subclass must implement this method")
 
-    def execute(self, script, context, external_methods=None):
+    def execute(self, script, context, external_context=None):
         raise NotImplementedError("Subclass must implement this method")
 
 
 class TaskDataEnvironment(BasePythonScriptEngineEnvironment):
 
-    def evaluate(self, expression, context, external_methods=None):
+    def evaluate(self, expression, context, external_context=None):
         my_globals = copy.copy(self.globals)  # else we pollute all later evals.
         self._prepare_context(context)
-        my_globals.update(external_methods or {})
+        my_globals.update(external_context or {})
         my_globals.update(context)
         return eval(expression, my_globals)
 
-    def execute(self, script, context, external_methods=None):
-        self.check_for_overwrite(context, external_methods or {})
+    def execute(self, script, context, external_context=None):
+        self.check_for_overwrite(context, external_context or {})
         my_globals = copy.copy(self.globals)
         self._prepare_context(context)
-        my_globals.update(external_methods or {})
+        my_globals.update(external_context or {})
         context.update(my_globals)
         try:
             exec(script, context)
         finally:
-            self._remove_globals_and_functions_from_context(context, external_methods)
+            self._remove_globals_and_functions_from_context(context, external_context)
         return True
 
     def _prepare_context(self, context):
         pass
 
-    def _remove_globals_and_functions_from_context(self, context, external_methods=None):
+    def _remove_globals_and_functions_from_context(self, context, external_context=None):
         """When executing a script, don't leave the globals, functions
         and external methods in the context that we have modified."""
         for k in list(context):
             if k == "__builtins__" or \
                     hasattr(context[k], '__call__') or \
                     k in self.globals or \
-                    external_methods and k in external_methods:
+                    external_context and k in external_context:
                 context.pop(k)
 
-    def check_for_overwrite(self, context, external_methods):
+    def check_for_overwrite(self, context, external_context):
         """It's possible that someone will define a variable with the
         same name as a pre-defined script, rendering the script un-callable.
         This results in a nearly indecipherable error.  Better to fail
         fast with a sensible error message."""
         func_overwrites = set(self.globals).intersection(context)
-        func_overwrites.update(set(external_methods).intersection(context))
+        func_overwrites.update(set(external_context).intersection(context))
         if len(func_overwrites) > 0:
             msg = f"You have task data that overwrites a predefined " \
                   f"function(s). Please change the following variable or " \

SpiffWorkflow/bpmn/serializer/default/workflow.py

@@ -80,6 +80,12 @@ def to_dict(self, workflow):
             'root': str(workflow.task_tree.id),
         }
 
+    def set_default_attributes(self, workflow, dct):
+        workflow.success = dct['success']
+        workflow.correlations = dct.pop('correlations', {})
+        if isinstance(dct['last_task'], str):
+            workflow.last_task = workflow.tasks.get(UUID(dct['last_task']))
+        workflow.data = self.registry.restore(dct.pop('data', {}))
 
 class BpmnSubWorkflowConverter(WorkflowConverter):
 
@@ -92,13 +98,9 @@ def to_dict(self, workflow):
     def from_dict(self, dct, task, top_workflow):
         spec = top_workflow.subprocess_specs.get(task.task_spec.spec)
         subprocess = self.target_class(spec, task.id, top_workflow, deserializing=True)
-        subprocess.correlations = dct.pop('correlations', {})
         subprocess.tasks = self.mapping_from_dict(dct['tasks'], UUID, workflow=subprocess)
         subprocess.task_tree = subprocess.tasks.get(UUID(dct['root']))
-        if isinstance(dct['last_task'], str):
-            subprocess.last_task = subprocess.tasks.get(UUID(dct['last_task']))
-        subprocess.success = dct['success']
-        subprocess.data = self.registry.restore(dct['data'])
+        self.set_default_attributes(subprocess, dct)
         return subprocess
 
 
@@ -127,24 +129,24 @@ def from_dict(self, dct):
         Returns:
             a BPMN Workflow object
         """
-        # Restore the top level spec and the subprocess specs
+        # Restore the specs
         spec = self.registry.restore(dct.pop('spec'))
         subprocess_specs = self.mapping_from_dict(dct.pop('subprocess_specs', {}))
 
         # Create the top-level workflow
         workflow = self.target_class(spec, subprocess_specs, deserializing=True)
 
-        # Restore the remainder of the workflow
-        workflow.bpmn_events = self.registry.restore(dct.pop('bpmn_events', []))
-        workflow.correlations = dct.pop('correlations', {})
-        workflow.data = self.registry.restore(dct.pop('data', {}))
-        workflow.success = dct.pop('success')
+        # Restore the task tree
         workflow.tasks = self.mapping_from_dict(dct['tasks'], UUID, workflow=workflow)
         workflow.task_tree = workflow.tasks.get(UUID(dct['root']))
-        if dct['last_task'] is not None:
-            workflow.last_task = workflow.tasks.get(UUID(dct['last_task']))
 
+        # Restore other default attributes
+        self.set_default_attributes(workflow, dct)
+
+        # Handle the remaining top workflow attributes
         self.subprocesses_from_dict(dct['subprocesses'], workflow)
+        workflow.bpmn_events = self.registry.restore(dct.pop('bpmn_events', []))
+
         return workflow
 
     def subprocesses_from_dict(self, dct, workflow, top_workflow=None):

SpiffWorkflow/bpmn/specs/bpmn_task_spec.py

@@ -12,7 +12,7 @@ def __init__(self, *args):
         super(_BpmnCondition, self).__init__(*args)
 
     def _matches(self, task):
-        return task.workflow.script_engine.evaluate(task, self.args[0], external_methods=task.workflow.data)
+        return task.workflow.script_engine.evaluate(task, self.args[0], external_context=task.workflow.data)
 
 
 class BpmnTaskSpec(TaskSpec):

SpiffWorkflow/bpmn/specs/event_definitions/conditional.py

@@ -9,6 +9,6 @@ def __init__(self, expression, **kwargs):
 
     def has_fired(self, my_task):
         my_task._set_internal_data(
-            has_fired=my_task.workflow.script_engine.evaluate(my_task, self.expression, external_methods=my_task.workflow.data)
+            has_fired=my_task.workflow.script_engine.evaluate(my_task, self.expression, external_context=my_task.workflow.data)
         )
         return my_task._get_internal_data('has_fired', False)

SpiffWorkflow/bpmn/specs/event_definitions/timer.py

@@ -1,12 +1,12 @@
 import re
 from datetime import datetime, timedelta, timezone
 from calendar import monthrange
-from time import timezone as tzoffset, altzone as dstoffset, daylight as isdst
+from time import timezone as tzoffset, altzone as dstoffset, struct_time, localtime
 
 from SpiffWorkflow.bpmn.event import PendingBpmnEvent
 from .base import EventDefinition
 
-seconds_from_utc = dstoffset if isdst else tzoffset
+seconds_from_utc = dstoffset if struct_time(localtime()).tm_isdst else tzoffset
 LOCALTZ = timezone(timedelta(seconds=-1 * seconds_from_utc))
 
 class TimerEventDefinition(EventDefinition):

SpiffWorkflow/bpmn/specs/mixins/events/event_types.py

@@ -45,7 +45,7 @@ def catch(self, my_task, event):
         definition, at which point we can update our task's state.
         """
         self.event_definition.catch(my_task, event)
-        my_task.last_update_time = time.time()
+        my_task.last_state_change = time.time()
         my_task._set_state(TaskState.WAITING)
 
     def _update_hook(self, my_task):

SpiffWorkflow/bpmn/workflow.py

@@ -68,15 +68,15 @@ def _next(self):
             self.task_list = []
         elif all([
             len(task._children) > 0 or subprocess is not None,
-            task.state >= self.min_state,
+            task.state >= self.min_state or subprocess is not None,
             self.depth < self.max_depth,
         ]):
             if subprocess is None:
                 next_tasks = task.children
             elif self.depth_first:
                 next_tasks = [subprocess.task_tree] + task.children
             else:
-                next_tasks = task.children = [subprocess.task_tree]
+                next_tasks = task.children + [subprocess.task_tree]
 
             if self.depth_first:
                 self.task_list = next_tasks + self.task_list

SpiffWorkflow/dmn/engine/DMNEngine.py

@@ -119,11 +119,11 @@ def evaluate(self, input_expr, match_expr, task):
         # do the replacement.
         match_expr = re.sub(r'(\?)(?=(?:[^\'"]|[\'"][^\'"]*[\'"])*$)', 'dmninputexpr', match_expr)
         if 'dmninputexpr' in match_expr:
-            external_methods = {
+            external_context = {
                 'dmninputexpr': script_engine.evaluate(task, input_expr)
             }
             return script_engine.evaluate(task, match_expr,
-                                          external_methods=external_methods)
+                                          external_context=external_context)
 
         # The input expression just has to be something that can be parsed as is by the engine.
         script_engine.validate(input_expr)

SpiffWorkflow/specs/base.py

@@ -112,10 +112,6 @@ def __init__(self, wf_spec, name, **kwargs):
         self._wf_spec._add_notify(self)
         self.data.update(self.defines)
 
-    @property
-    def spec_type(self):
-        return f'{self.__class__.__module__}.{self.__class__.__name__}'
-
     @property
     def inputs(self):
         return [self._wf_spec.task_specs.get(name) for name in self._inputs]

SpiffWorkflow/spiff/specs/mixins/service_task.py

@@ -31,10 +31,6 @@ def __init__(self, wf_spec, name, operation_name, operation_params, result_varia
         self.operation_params = operation_params
         self.result_variable = result_variable
 
-    @property
-    def spec_type(self):
-        return 'Service Task'
-
     def _result_variable(self, task):
         if self.result_variable is not None and len(self.result_variable) > 0:
             return self.result_variable

SpiffWorkflow/spiff/specs/spiff_task.py

@@ -31,26 +31,13 @@ def __init__(self, wf_spec, name, prescript=None, postscript=None, **kwargs):
         self.prescript = prescript
         self.postscript = postscript
 
-    @property
-    def spec_type(self):
-        return 'Spiff BPMN Task'
-
     def execute_script(self, my_task, script):
         try:
             my_task.workflow.script_engine.execute(my_task, script)
         except Exception as exc:
             my_task._set_state(TaskState.ERROR)
             raise exc
 
-    def get_payload(self, my_task, script, expr):
-        try:
-            data = deepcopy(my_task.data)
-            my_task.worklflow.script_engine.execute(my_task, script, data)
-            return my_task.workflow.script_engine._evaluate(expr, data)
-        except Exception as exc:
-            my_task._set_state(TaskState.WAITING)
-            raise exc
-
     def _update_hook(self, my_task):
         super()._update_hook(my_task)
         if self.prescript is not None:

SpiffWorkflow/task.py

@@ -398,9 +398,8 @@ def log_info(self, dct=None):
         extra.update({
             'workflow_spec': self.workflow.spec.name,
             'task_spec': self.task_spec.name,
-            'task_name': self.task_spec.description,
             'task_id': self.id,
-            'task_type': self.task_spec.spec_type,
+            'task_type': self.task_spec.__class__.__name__,
             'data': self.data if logger.level < 20 else None,
             'internal_data': self.internal_data if logger.level <= 10 else None,
         })