From b432f22fe072f24756a0d09da8308a2a1b843f25 Mon Sep 17 00:00:00 2001 From: Dan Funk Date: Tue, 12 Sep 2023 16:15:43 -0400 Subject: [PATCH] Bug/predicted tasks dropped (#484) * when removing predicted tasks for data storage, don't alter the in-memory version of the workflow. * run_pyl * Bump gitPython for security vulnerability. --- poetry.lock | 9 ++-- .../services/task_service.py | 11 ++++ .../services/workflow_execution_service.py | 53 ++++++++----------- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/poetry.lock b/poetry.lock index a231a6c0..617cce99 100644 --- a/poetry.lock +++ b/poetry.lock @@ -299,19 +299,22 @@ smmap = ">=3.0.1,<6" [[package]] name = "gitpython" -version = "3.1.31" +version = "3.1.36" description = "GitPython is a Python library used to interact with Git repositories" category = "dev" optional = false python-versions = ">=3.7" files = [ - {file = "GitPython-3.1.31-py3-none-any.whl", hash = "sha256:f04893614f6aa713a60cbbe1e6a97403ef633103cdd0ef5eb6efe0deb98dbe8d"}, - {file = "GitPython-3.1.31.tar.gz", hash = "sha256:8ce3bcf69adfdf7c7d503e78fd3b1c492af782d58893b650adb2ac8912ddd573"}, + {file = "GitPython-3.1.36-py3-none-any.whl", hash = "sha256:8d22b5cfefd17c79914226982bb7851d6ade47545b1735a9d010a2a4c26d8388"}, + {file = "GitPython-3.1.36.tar.gz", hash = "sha256:4bb0c2a6995e85064140d31a33289aa5dce80133a23d36fcd372d716c54d3ebf"}, ] [package.dependencies] gitdb = ">=4.0.1,<5" +[package.extras] +test = ["black", "coverage[toml]", "ddt (>=1.1.1,!=1.4.3)", "mypy", "pre-commit", "pytest", "pytest-cov", "pytest-sugar", "virtualenv"] + [[package]] name = "identify" version = "2.5.22" diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py index aaa7e8f2..97977b20 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/task_service.py @@ -259,7 +259,18 @@ class TaskService: This will NOT update start_in_seconds or end_in_seconds. It also returns the relating json_data object so they can be imported later. """ + new_properties_json = self.serializer.task_to_dict(spiff_task) + + # Only save links to children that are definite - we don't currently store predicted + # children in the database. They are filtered out in other places in the code, so we + # must remove references to them here. + modified_children = [] + for child in spiff_task.children: + if not child._has_state(TaskState.PREDICTED_MASK): + modified_children.append(str(child.id)) + new_properties_json["children"] = modified_children + if new_properties_json["task_spec"] == "Start": new_properties_json["parent"] = None spiff_task_data = new_properties_json.pop("data") diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py index 98e35345..ab8c9c04 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py @@ -1,7 +1,6 @@ from __future__ import annotations import concurrent.futures -import copy import time from abc import abstractmethod from collections.abc import Callable @@ -242,6 +241,28 @@ class TaskModelSavingDelegate(EngineStepDelegate): self.secondary_engine_step_delegate.did_complete_task(spiff_task) def save(self, bpmn_process_instance: BpmnWorkflow, _commit: bool = True) -> None: + # NOTE: process-all-tasks: All tests pass with this but it's less efficient and would be nice to replace + # excludes COMPLETED. the others were required to get PP1 to go to completion. + # process FUTURE tasks because Boundary events are not processed otherwise. + # + # ANOTHER NOTE: at one point we attempted to be smarter about what tasks we considered for persistence, + # but it didn't quite work in all cases, so we deleted it. you can find it in commit + # 1ead87b4b496525df8cc0e27836c3e987d593dc0 if you are curious. + for waiting_spiff_task in bpmn_process_instance.get_tasks( + TaskState.WAITING + | TaskState.CANCELLED + | TaskState.READY + | TaskState.MAYBE + | TaskState.LIKELY + | TaskState.FUTURE + | TaskState.STARTED + | TaskState.ERROR + ): + # these will be removed from the parent and then ignored + if waiting_spiff_task._has_state(TaskState.PREDICTED_MASK): + continue + self.task_service.update_task_model_with_spiff_task(waiting_spiff_task) + self.task_service.save_objects_to_database() if self.secondary_engine_step_delegate: @@ -249,35 +270,7 @@ class TaskModelSavingDelegate(EngineStepDelegate): db.session.commit() def after_engine_steps(self, bpmn_process_instance: BpmnWorkflow) -> None: - if self._should_update_task_model(): - # NOTE: process-all-tasks: All tests pass with this but it's less efficient and would be nice to replace - # excludes COMPLETED. the others were required to get PP1 to go to completion. - # process FUTURE tasks because Boundary events are not processed otherwise. - # - # ANOTHER NOTE: at one point we attempted to be smarter about what tasks we considered for persistence, - # but it didn't quite work in all cases, so we deleted it. you can find it in commit - # 1ead87b4b496525df8cc0e27836c3e987d593dc0 if you are curious. - for waiting_spiff_task in bpmn_process_instance.get_tasks( - TaskState.WAITING - | TaskState.CANCELLED - | TaskState.READY - | TaskState.MAYBE - | TaskState.LIKELY - | TaskState.FUTURE - | TaskState.STARTED - | TaskState.ERROR - ): - # these will be removed from the parent and then ignored - if waiting_spiff_task._has_state(TaskState.PREDICTED_MASK): - continue - - # removing elements from an array causes the loop to exit so deep copy the array first - waiting_children = copy.copy(waiting_spiff_task.children) - for waiting_child in waiting_children: - if waiting_child._has_state(TaskState.PREDICTED_MASK): - waiting_spiff_task._children.remove(waiting_child.id) - - self.task_service.update_task_model_with_spiff_task(waiting_spiff_task) + pass def on_exception(self, bpmn_process_instance: BpmnWorkflow) -> None: self.after_engine_steps(bpmn_process_instance)