diff --git a/SpiffWorkflow/bpmn/workflow.py b/SpiffWorkflow/bpmn/workflow.py index 4b68e7ef..4b5f8479 100644 --- a/SpiffWorkflow/bpmn/workflow.py +++ b/SpiffWorkflow/bpmn/workflow.py @@ -61,27 +61,32 @@ def _next(self): if len(self.task_list) == 0: raise StopIteration() - task = self.task_list.pop(-1) + task = self.task_list.pop(0) subprocess = task.workflow.top_workflow.subprocesses.get(task.id) - if all([ + if task.task_spec.name == self.end_at_spec: + self.task_list = [] + elif all([ len(task._children) > 0 or subprocess is not None, task.state >= self.min_state, self.depth < self.max_depth, - task.task_spec.name != self.end_at_spec, ]): - add_tasks = [t for t in reversed(task.children)] + 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] + if self.depth_first: - if subprocess is not None: - add_tasks.append(subprocess.task_tree) - self.task_list.extend(add_tasks) + self.task_list = next_tasks + self.task_list else: - if subprocess is not None: - add_tasks = [subprocess.task_tree] + add_tasks - self.task_list = add_tasks + self.task_list - self.depth += 1 - elif len(self.task_list) > 0 and task.parent != self.task_list[0].parent: - self.depth -= 1 + self.task_list.extend(next_tasks) + self._update_depth(task) + + elif self.depth_first and len(self.task_list) > 0: + self._handle_leaf_depth(task) + return task diff --git a/SpiffWorkflow/task.py b/SpiffWorkflow/task.py index 1bdec52e..d47eaefe 100644 --- a/SpiffWorkflow/task.py +++ b/SpiffWorkflow/task.py @@ -272,7 +272,7 @@ def _sync_children(self, task_specs, state=TaskState.MAYBE): # Update children accordingly for child in unneeded_children: - self._children.remove(child.id) + self.workflow._remove_task(child.id) for task_spec in new_children: self._add_child(task_spec, state) @@ -290,8 +290,9 @@ def _drop_children(self, force=False): drop.append(child) else: child._drop_children() + for task in drop: - self._children.remove(task.id) + self.workflow._remove_task(task.id) def _set_state(self, value): """Force set the state on a task""" diff --git a/SpiffWorkflow/util/task.py b/SpiffWorkflow/util/task.py index ee77e1c1..d164db29 100644 --- a/SpiffWorkflow/util/task.py +++ b/SpiffWorkflow/util/task.py @@ -204,19 +204,43 @@ def _next(self): if len(self.task_list) == 0: raise StopIteration() - task = self.task_list.pop(-1) - if all([ + task = self.task_list.pop(0) + + if task.task_spec.name == self.end_at_spec: + self.task_list = [] + elif all([ len(task._children) > 0, task.state >= self.min_state, self.depth < self.max_depth, - task.task_spec.name != self.end_at_spec, ]): if self.depth_first: - self.task_list.extend(reversed(task.children)) + self.task_list = task.children + self.task_list else: - self.task_list = reversed(task.children) + self.task_list - self.depth += 1 - elif len(self.task_list) > 0 and task.parent != self.task_list[0].parent: - self.depth -= 1 + self.task_list.extend(task.children) + self._update_depth(task) + elif self.depth_first and len(self.task_list) > 0: + self._handle_leaf_depth(task) + return task + def _update_depth(self, task): + + if self.depth_first: + # Since we visit the children before siblings, we always increment depth when adding children + self.depth += 1 + else: + # In this case, we have to check for a common ancestor at the same depth + first, second = task, self.task_list[0] + for i in range(self.depth): + first = first.parent if first is not None else None + second = second.parent if second is not None else None + if first != second: + self.depth += 1 + + def _handle_leaf_depth(self, task): + + ancestor = self.task_list[0].parent + current = task.parent + while current is not None and current != ancestor: + current = current.parent + self.depth -= 1 diff --git a/SpiffWorkflow/workflow.py b/SpiffWorkflow/workflow.py index 88c86128..d8d7d146 100644 --- a/SpiffWorkflow/workflow.py +++ b/SpiffWorkflow/workflow.py @@ -277,6 +277,13 @@ def _task_completed_notify(self, task): # Since is_completed() is expensive it makes sense to bail out if calling it is not necessary. self.completed_event(self) + def _remove_task(self, task_id): + task = self.tasks[task_id] + for child in task.children: + self._remove_task(child.id) + task.parent._children.remove(task.id) + self.tasks.pop(task_id) + def _get_mutex(self, name): """Get or create a mutex""" if name not in self.locks: diff --git a/tests/SpiffWorkflow/core/IteratorTest.py b/tests/SpiffWorkflow/core/IteratorTest.py new file mode 100644 index 00000000..f13a3440 --- /dev/null +++ b/tests/SpiffWorkflow/core/IteratorTest.py @@ -0,0 +1,79 @@ +import unittest +import os +from datetime import datetime + +from lxml import etree + +from SpiffWorkflow.workflow import Workflow +from SpiffWorkflow.specs.Cancel import Cancel +from SpiffWorkflow.specs.Simple import Simple +from SpiffWorkflow.specs.WorkflowSpec import WorkflowSpec +from SpiffWorkflow.util.task import TaskState, TaskIterator, TaskFilter +from SpiffWorkflow.serializer.prettyxml import XmlSerializer + +data_dir = os.path.join(os.path.dirname(__file__), 'data') + +class IterationTest(unittest.TestCase): + + def setUp(self): + xml_file = os.path.join(data_dir, 'iteration_test.xml') + with open(xml_file) as fp: + xml = etree.parse(fp).getroot() + wf_spec = WorkflowSpec.deserialize(XmlSerializer(), xml) + self.workflow = Workflow(wf_spec) + + def get_tasks_updated_after(self): + start = self.workflow.get_next_task(end_at_spec='Start') + start.run() + updated = datetime.now().timestamp() + for task in self.workflow.get_tasks(state=TaskState.READY): + task.run() + return updated + +class DepthFirstTest(IterationTest): + + def test_get_tasks_updated_after(self): + updated = super().get_tasks_updated_after() + tasks = self.workflow.get_tasks(updated_ts=updated) + self.assertListEqual( + [t.task_spec.name for t in tasks], + ['a', 'a1', 'a2', 'c', 'b', 'b1', 'b2'] + ) + + def test_get_tasks_end_at(self): + tasks = self.workflow.get_tasks(end_at_spec='c') + self.assertEqual( + [t.task_spec.name for t in tasks], + ['Start', 'a', 'a1', 'last', 'End', 'a2', 'last', 'End', 'c'] + ) + + def test_get_tasks_max_depth(self): + tasks = self.workflow.get_tasks(max_depth=2) + self.assertEqual( + [t.task_spec.name for t in tasks], + ['Start', 'a', 'a1', 'a2', 'c', 'b', 'b1', 'b2'] + ) + +class BreadthFirstTest(IterationTest): + + def test_get_tasks_updated_after(self): + updated = super().get_tasks_updated_after() + tasks = self.workflow.get_tasks(updated_ts=updated, depth_first=False) + self.assertListEqual( + [t.task_spec.name for t in tasks], + ['a', 'b', 'a1', 'a2', 'c', 'b1', 'b2'] + ) + + def test_get_tasks_end_at(self): + tasks = self.workflow.get_tasks(end_at_spec='c', depth_first=False) + self.assertEqual( + [t.task_spec.name for t in tasks], + ['Start', 'a', 'b', 'a1', 'a2', 'c'] + ) + + def test_get_tasks_max_depth(self): + tasks = self.workflow.get_tasks(max_depth=2, depth_first=False) + self.assertEqual( + [t.task_spec.name for t in tasks], + ['Start', 'a', 'b', 'a1', 'a2', 'c', 'b1', 'b2'] + ) diff --git a/tests/SpiffWorkflow/core/WorkflowTest.py b/tests/SpiffWorkflow/core/WorkflowTest.py index ed433f93..35c60591 100644 --- a/tests/SpiffWorkflow/core/WorkflowTest.py +++ b/tests/SpiffWorkflow/core/WorkflowTest.py @@ -15,8 +15,6 @@ class WorkflowTest(unittest.TestCase): - ready_task_filter = TaskFilter(state=TaskState.READY) - def setUp(self): xml_file = os.path.join(data_dir, 'workflow1.xml') with open(xml_file) as fp: @@ -27,13 +25,13 @@ def setUp(self): def test_interactive_calls(self): """Simulates interactive calls, as would be issued by a user.""" - tasks = self.workflow.get_tasks(task_filter=self.ready_task_filter) + tasks = self.workflow.get_tasks(state=TaskState.READY) self.assertEqual(len(tasks), 1) self.assertEqual(tasks[0].task_spec.name, 'Start') self.workflow.run_task_from_id(tasks[0].id) self.assertEqual(tasks[0].state, TaskState.COMPLETED) - tasks = self.workflow.get_tasks(task_filter=self.ready_task_filter) + tasks = self.workflow.get_tasks(state=TaskState.READY) self.assertEqual(len(tasks), 2) task_a1 = tasks[0] task_b1 = tasks[1] @@ -44,7 +42,7 @@ def test_interactive_calls(self): self.workflow.run_task_from_id(task_a1.id) self.assertEqual(task_a1.state, TaskState.COMPLETED) - tasks = self.workflow.get_tasks(task_filter=self.ready_task_filter) + tasks = self.workflow.get_tasks(state=TaskState.READY) self.assertEqual(len(tasks), 2) self.assertTrue(task_b1 in tasks) task_a2 = tasks[0] @@ -52,38 +50,15 @@ def test_interactive_calls(self): self.assertEqual(task_a2.task_spec.name, 'task_a2') self.workflow.run_task_from_id(task_a2.id) - tasks = self.workflow.get_tasks(task_filter=self.ready_task_filter) + tasks = self.workflow.get_tasks(state=TaskState.READY) self.assertEqual(len(tasks), 1) self.assertTrue(task_b1 in tasks) self.workflow.run_task_from_id(task_b1.id) - tasks = self.workflow.get_tasks(task_filter=self.ready_task_filter) + tasks = self.workflow.get_tasks(state=TaskState.READY) self.assertEqual(len(tasks), 1) self.workflow.run_task_from_id(tasks[0].id) - tasks = self.workflow.get_tasks(task_filter=self.ready_task_filter) + tasks = self.workflow.get_tasks(state=TaskState.READY) self.assertEqual(len(tasks), 1) self.assertEqual(tasks[0].task_spec.name, 'synch_1') - - def test_get_tasks_updated_after(self): - - start = self.workflow.get_next_task(end_at_spec='Start') - start.run() - updated = datetime.now().timestamp() - for task in self.workflow.get_tasks(task_filter=self.ready_task_filter): - task.run() - tasks = self.workflow.get_tasks(task_filter=TaskFilter(updated_ts=updated)) - self.assertListEqual([t.task_spec.name for t in tasks], ['task_a1', 'task_a2', 'task_b1', 'task_b2']) - - def test_get_tasks_end_at(self): - - tasks = self.workflow.get_tasks(end_at_spec='excl_choice_1') - spec_names = [t.task_spec.name for t in tasks] - self.assertEqual(len([name for name in spec_names if name == 'excl_choice_1']), 2) - self.assertNotIn('task_c1', spec_names) - self.assertNotIn('task_c2', spec_names) - self.assertNotIn('task_c3', spec_names) - - def test_get_tasks_max_depth(self): - tasks = [t for t in self.workflow.get_tasks(max_depth=2)] - self.assertListEqual([t.task_spec.name for t in tasks], ['Start', 'task_a1', 'task_a2', 'task_b1', 'task_b2']) diff --git a/tests/SpiffWorkflow/core/data/iteration_test.xml b/tests/SpiffWorkflow/core/data/iteration_test.xml new file mode 100644 index 00000000..c6e9e472 --- /dev/null +++ b/tests/SpiffWorkflow/core/data/iteration_test.xml @@ -0,0 +1,53 @@ + + + + A test workflow to be used to test task iteration. + + + + + a + b + + + + a1 + a2 + c + + + + b1 + b2 + + + + c1 + c2 + + + + last + + + last + + + + last + + + last + + + + last + + + last + + + + end + +