From 99c1d1b129be9ea2a554ca85391dc861db97ac51 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Wed, 13 Jan 2021 16:12:14 -0500 Subject: [PATCH 01/13] Test for changes to hard_reset and soft_reset --- tests/workflow/test_workflow_restart.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 tests/workflow/test_workflow_restart.py diff --git a/tests/workflow/test_workflow_restart.py b/tests/workflow/test_workflow_restart.py new file mode 100644 index 00000000..7ccb10e0 --- /dev/null +++ b/tests/workflow/test_workflow_restart.py @@ -0,0 +1,18 @@ +from tests.base_test import BaseTest + + +class TestMessageEvent(BaseTest): + + def test_message_event(self): + + workflow = self.create_workflow('message_event') + + first_task = self.get_workflow_api(workflow).next_task + self.assertEqual('Activity_GetData', first_task.name) + workflow_api = self.get_workflow_api(workflow, clear_data=True) + result = self.complete_form(workflow_api, first_task, {'name': 'asdf'}) + self.assertEqual('asdf', result.next_task.data['name']) + workflow_api = self.get_workflow_api(workflow_api) + self.assertNotIn('name', workflow_api.next_task.data) + + print('Nice Test') From 6a9e6d3570c292d0f9cc6a5957846aabe89737e7 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Thu, 14 Jan 2021 15:32:14 -0500 Subject: [PATCH 02/13] Restart workflow without form data. Committing so Dan can check it out --- crc/api.yml | 8 ++--- crc/api/workflow.py | 16 ++++++--- crc/services/workflow_processor.py | 43 ++++++++++++++----------- tests/base_test.py | 11 ++++--- tests/workflow/test_workflow_restart.py | 12 ++++--- 5 files changed, 53 insertions(+), 37 deletions(-) diff --git a/crc/api.yml b/crc/api.yml index a4b799ea..f9fdecbf 100644 --- a/crc/api.yml +++ b/crc/api.yml @@ -852,16 +852,16 @@ paths: operationId: crc.api.workflow.get_workflow summary: Returns a workflow, can also be used to do a soft or hard reset on the workflow. parameters: - - name: soft_reset + - name: clear_data in: query required: false - description: Set this to true to use the latest workflow specification to load minor modifications to the spec. + description: Set this to true to clear data when starting workflow. schema: type: boolean - - name: hard_reset + - name: reload_spec in: query required: false - description: Set this to true to reset the workflow + description: Set this to true to load latest workflow spec. schema: type: boolean - name: do_engine_steps diff --git a/crc/api/workflow.py b/crc/api/workflow.py index 2b94986f..fbd6e1e1 100644 --- a/crc/api/workflow.py +++ b/crc/api/workflow.py @@ -96,27 +96,35 @@ def delete_workflow_specification(spec_id): session.commit() -def get_workflow(workflow_id, soft_reset=False, hard_reset=False, do_engine_steps=True): +def get_workflow(workflow_id, reload_spec=False, clear_data=False, do_engine_steps=True): """Soft reset will attempt to update to the latest spec without starting over, Hard reset will update to the latest spec and start from the beginning. Read Only will return the workflow in a read only state, without running any engine tasks or logging any events. """ workflow_model: WorkflowModel = session.query(WorkflowModel).filter_by(id=workflow_id).first() - processor = WorkflowProcessor(workflow_model) - if soft_reset or hard_reset: + processor = WorkflowProcessor(workflow_model, reload_spec=reload_spec, clear_data=clear_data) + if reload_spec or clear_data: try: processor.cancel_notify() except Exception as e: raise e finally: # In the event of a reset, ALWAYS allow the reset, even if the cancel_notify fails for some reason. - processor = WorkflowProcessor(workflow_model, soft_reset=soft_reset, hard_reset=hard_reset) + processor = WorkflowProcessor(workflow_model, reload_spec=reload_spec, clear_data=clear_data) if do_engine_steps: processor.do_engine_steps() processor.save() WorkflowService.update_task_assignments(processor) workflow_api_model = WorkflowService.processor_to_workflow_api(processor) + if clear_data: + remove_keys = [] + for datum in workflow_api_model.next_task.data: + if datum != 'current_user': + remove_keys.append(datum) + if len(remove_keys) > 0: + for key in remove_keys: + del(workflow_api_model.next_task.data[key]) return WorkflowApiSchema().dump(workflow_api_model) diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index b4fb5f5c..7379b588 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -21,6 +21,7 @@ import crc from crc import session, app from crc.api.common import ApiError from crc.models.file import FileDataModel, FileModel, FileType +from crc.models.task_event import TaskEventModel from crc.models.workflow import WorkflowStatus, WorkflowModel, WorkflowSpecDependencyFile from crc.scripts.script import Script from crc.services.file_service import FileService @@ -151,7 +152,7 @@ class WorkflowProcessor(object): VALIDATION_PROCESS_KEY = "validate_only" def __init__(self, workflow_model: WorkflowModel, - soft_reset=False, hard_reset=False, validate_only=False): + reload_spec=False, clear_data=False, validate_only=False): """Create a Workflow Processor based on the serialized information available in the workflow model. If soft_reset is set to true, it will try to use the latest version of the workflow specification without resetting to the beginning of the workflow. This will work for some minor changes to the spec. @@ -162,7 +163,7 @@ class WorkflowProcessor(object): self.workflow_model = workflow_model - if soft_reset or len(workflow_model.dependencies) == 0: # Depenencies of 0 means the workflow was never started. + if reload_spec or len(workflow_model.dependencies) == 0: # Depenencies of 0 means the workflow was never started. self.spec_data_files = FileService.get_spec_data_files( workflow_spec_id=workflow_model.workflow_spec_id) spec = self.get_spec(self.spec_data_files, workflow_model.workflow_spec_id) @@ -196,13 +197,17 @@ class WorkflowProcessor(object): " '%s' version %s, due to a mis-placed or missing task '%s'" % (self.workflow_spec_id, self.get_version_string(), str(ke)) + " This is very likely due to a soft reset where there was a structural change.") - if hard_reset: - # Now that the spec is loaded, get the data and rebuild the bpmn with the new details - self.hard_reset() + if clear_data: + # Clear form_data from task_events + task_events = session.query(TaskEventModel). \ + filter(TaskEventModel.workflow_id == workflow_model.id).all() + for task_event in task_events: + task_event.form_data = {} + session.add(task_event) + session.commit() + workflow_model.bpmn_workflow_json = WorkflowProcessor._serializer.serialize_workflow(self.bpmn_workflow) self.save() - if soft_reset: - self.save() # set whether this is the latest spec file. if self.spec_data_files == FileService.get_spec_data_files(workflow_spec_id=workflow_model.workflow_spec_id): @@ -355,18 +360,18 @@ class WorkflowProcessor(object): else: return WorkflowStatus.waiting - def hard_reset(self): - """Recreate this workflow. This will be useful when a workflow specification changes. - """ - self.spec_data_files = FileService.get_spec_data_files(workflow_spec_id=self.workflow_spec_id) - new_spec = WorkflowProcessor.get_spec(self.spec_data_files, self.workflow_spec_id) - new_bpmn_workflow = BpmnWorkflow(new_spec, script_engine=self._script_engine) - new_bpmn_workflow.data = self.bpmn_workflow.data - try: - new_bpmn_workflow.do_engine_steps() - except WorkflowException as we: - raise ApiError.from_task_spec("hard_reset_engine_steps_error", str(we), we.sender) - self.bpmn_workflow = new_bpmn_workflow + # def hard_reset(self): + # """Recreate this workflow. This will be useful when a workflow specification changes. + # """ + # self.spec_data_files = FileService.get_spec_data_files(workflow_spec_id=self.workflow_spec_id) + # new_spec = WorkflowProcessor.get_spec(self.spec_data_files, self.workflow_spec_id) + # new_bpmn_workflow = BpmnWorkflow(new_spec, script_engine=self._script_engine) + # new_bpmn_workflow.data = self.bpmn_workflow.data + # try: + # new_bpmn_workflow.do_engine_steps() + # except WorkflowException as we: + # raise ApiError.from_task_spec("hard_reset_engine_steps_error", str(we), we.sender) + # self.bpmn_workflow = new_bpmn_workflow def get_status(self): return self.status_of(self.bpmn_workflow) diff --git a/tests/base_test.py b/tests/base_test.py index 4f2306a2..28a62ffd 100644 --- a/tests/base_test.py +++ b/tests/base_test.py @@ -341,13 +341,14 @@ class BaseTest(unittest.TestCase): session.commit() return approval - def get_workflow_api(self, workflow, soft_reset=False, hard_reset=False, do_engine_steps=True, user_uid="dhf8r"): + def get_workflow_api(self, workflow, reload_spec=False, clear_data=False, do_engine_steps=True, user_uid="dhf8r"): user = session.query(UserModel).filter_by(uid=user_uid).first() self.assertIsNotNone(user) - rv = self.app.get(f'/v1.0/workflow/{workflow.id}' - f'?soft_reset={str(soft_reset)}' - f'&hard_reset={str(hard_reset)}' - f'&do_engine_steps={str(do_engine_steps)}', + url = (f'/v1.0/workflow/{workflow.id}' + f'?clear_data={str(clear_data)}' + f'&reload_spec={str(reload_spec)}' + f'&do_engine_steps={str(do_engine_steps)}') + rv = self.app.get(url, headers=self.logged_in_headers(user), content_type="application/json") self.assert_success(rv) diff --git a/tests/workflow/test_workflow_restart.py b/tests/workflow/test_workflow_restart.py index 7ccb10e0..bac7a361 100644 --- a/tests/workflow/test_workflow_restart.py +++ b/tests/workflow/test_workflow_restart.py @@ -5,14 +5,16 @@ class TestMessageEvent(BaseTest): def test_message_event(self): + # self.load_example_data() workflow = self.create_workflow('message_event') first_task = self.get_workflow_api(workflow).next_task self.assertEqual('Activity_GetData', first_task.name) - workflow_api = self.get_workflow_api(workflow, clear_data=True) - result = self.complete_form(workflow_api, first_task, {'name': 'asdf'}) - self.assertEqual('asdf', result.next_task.data['name']) - workflow_api = self.get_workflow_api(workflow_api) - self.assertNotIn('name', workflow_api.next_task.data) + workflow_api = self.get_workflow_api(workflow) + result = self.complete_form(workflow_api, first_task, {'formdata': 'asdf'}) + self.assertIn('formdata', result.next_task.data) + self.assertEqual('asdf', result.next_task.data['formdata']) + workflow_api = self.get_workflow_api(workflow_api, clear_data=True) + self.assertNotIn('formdata', workflow_api.next_task.data) print('Nice Test') From e1532a90c9a06b8634438385ffd0ec3c0ba426e0 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:00:39 -0500 Subject: [PATCH 03/13] New api endpoint for restarting a workflow. Unsure if I needed the responses section. **Dan, can you check this?** Also, removed clear_data and reload_spec as parameters for standard get workflow endpoint. --- crc/api.yml | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/crc/api.yml b/crc/api.yml index f9fdecbf..52762ff4 100644 --- a/crc/api.yml +++ b/crc/api.yml @@ -850,20 +850,8 @@ paths: format: int32 get: operationId: crc.api.workflow.get_workflow - summary: Returns a workflow, can also be used to do a soft or hard reset on the workflow. + summary: Returns a workflow. parameters: - - name: clear_data - in: query - required: false - description: Set this to true to clear data when starting workflow. - schema: - type: boolean - - name: reload_spec - in: query - required: false - description: Set this to true to load latest workflow spec. - schema: - type: boolean - name: do_engine_steps in: query required: false @@ -889,6 +877,35 @@ paths: responses: '204': description: The workflow was removed + /workflow/{workflow_id}/restart: + parameters: + - name: workflow_id + in: path + required: true + description: The id of the workflow + schema: + type: integer + format: int32 + get: + operationId: crc.api.workflow.restart_workflow + summary: Restarts a workflow with the latest spec. Can also clear data. + parameters: + - name: clear_data + in: query + required: false + description: Set this to true to clear data when starting workflow. + schema: + type: boolean + tags: + - Workflows and Tasks + responses: + '200': + description: Returns updated workflow, possibly without data. + content: + application/json: + schema: + $ref: "#/components/schemas/Workflow" + /workflow/{workflow_id}/task/{task_id}/data: parameters: - name: workflow_id From 9c71a503ca9e8ef34ebbe32c8aff9687c83a4ae4 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:09:20 -0500 Subject: [PATCH 04/13] Python code for new restart_workflow api endpoint. Cleaned up code and docstring in get_workflow to match. --- crc/api/workflow.py | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/crc/api/workflow.py b/crc/api/workflow.py index fbd6e1e1..d9a0b498 100644 --- a/crc/api/workflow.py +++ b/crc/api/workflow.py @@ -96,38 +96,29 @@ def delete_workflow_specification(spec_id): session.commit() -def get_workflow(workflow_id, reload_spec=False, clear_data=False, do_engine_steps=True): - """Soft reset will attempt to update to the latest spec without starting over, - Hard reset will update to the latest spec and start from the beginning. - Read Only will return the workflow in a read only state, without running any - engine tasks or logging any events. """ +def get_workflow(workflow_id, do_engine_steps=True): + """Retrieve workflow based on workflow_id, and return it in the last saved State. + If do_engine_steps is False, return the workflow without running any engine tasks or logging any events. """ workflow_model: WorkflowModel = session.query(WorkflowModel).filter_by(id=workflow_id).first() - processor = WorkflowProcessor(workflow_model, reload_spec=reload_spec, clear_data=clear_data) - if reload_spec or clear_data: - try: - processor.cancel_notify() - except Exception as e: - raise e - finally: - # In the event of a reset, ALWAYS allow the reset, even if the cancel_notify fails for some reason. - processor = WorkflowProcessor(workflow_model, reload_spec=reload_spec, clear_data=clear_data) + processor = WorkflowProcessor(workflow_model) if do_engine_steps: processor.do_engine_steps() processor.save() WorkflowService.update_task_assignments(processor) + workflow_api_model = WorkflowService.processor_to_workflow_api(processor) - if clear_data: - remove_keys = [] - for datum in workflow_api_model.next_task.data: - if datum != 'current_user': - remove_keys.append(datum) - if len(remove_keys) > 0: - for key in remove_keys: - del(workflow_api_model.next_task.data[key]) return WorkflowApiSchema().dump(workflow_api_model) +def restart_workflow(workflow_id, clear_data=False): + """Restart a workflow with the latest spec. + Clear data allows user to restart the workflow without previous data.""" + workflow_model: WorkflowModel = session.query(WorkflowModel).filter_by(id=workflow_id).first() + WorkflowProcessor.reset(workflow_model, clear_data=clear_data) + return get_workflow(workflow_model.id) + + def get_task_events(action = None, workflow = None, study = None): """Provides a way to see a history of what has happened, or get a list of tasks that need your attention.""" query = session.query(TaskEventModel).filter(TaskEventModel.user_uid == g.user.uid) From 039a1dcd7910930108ad60627ec504dd001589a1 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:14:36 -0500 Subject: [PATCH 05/13] WorkflowProcessor code for new restart_workflow api endpoint. WorkflowProcessor.reset will clear workflow_model.bpmn_workflow_json, and if clear_data is True will clear form data. WorkflowProcessor.__init__ is mostly unchanged. --- crc/services/workflow_processor.py | 39 +++++++++++++----------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 7379b588..077c0d0b 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -151,19 +151,12 @@ class WorkflowProcessor(object): STUDY_ID_KEY = "study_id" VALIDATION_PROCESS_KEY = "validate_only" - def __init__(self, workflow_model: WorkflowModel, - reload_spec=False, clear_data=False, validate_only=False): - """Create a Workflow Processor based on the serialized information available in the workflow model. - If soft_reset is set to true, it will try to use the latest version of the workflow specification - without resetting to the beginning of the workflow. This will work for some minor changes to the spec. - If hard_reset is set to true, it will use the latest spec, and start the workflow over from the beginning. - which should work in casees where a soft reset fails. - If neither flag is set, it will use the same version of the specification that was used to originally - create the workflow model. """ + def __init__(self, workflow_model: WorkflowModel, spec=None, validate_only=False): + """Create a Workflow Processor based on the serialized information available in the workflow model.""" self.workflow_model = workflow_model - if reload_spec or len(workflow_model.dependencies) == 0: # Depenencies of 0 means the workflow was never started. + if workflow_model.bpmn_workflow_json is None and spec is None: # The workflow was never started. self.spec_data_files = FileService.get_spec_data_files( workflow_spec_id=workflow_model.workflow_spec_id) spec = self.get_spec(self.spec_data_files, workflow_model.workflow_spec_id) @@ -188,15 +181,26 @@ class WorkflowProcessor(object): # can then load data as needed. self.bpmn_workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] = workflow_model.id workflow_model.bpmn_workflow_json = WorkflowProcessor._serializer.serialize_workflow( - self.bpmn_workflow,include_spec=True) + self.bpmn_workflow, include_spec=True) self.save() except MissingSpecError as ke: raise ApiError(code="unexpected_workflow_structure", message="Failed to deserialize workflow" " '%s' version %s, due to a mis-placed or missing task '%s'" % - (self.workflow_spec_id, self.get_version_string(), str(ke)) + - " This is very likely due to a soft reset where there was a structural change.") + (self.workflow_spec_id, self.get_version_string(), str(ke))) + + # set whether this is the latest spec file. + if self.spec_data_files == FileService.get_spec_data_files(workflow_spec_id=workflow_model.workflow_spec_id): + self.is_latest_spec = True + else: + self.is_latest_spec = False + + @classmethod + def reset(cls, workflow_model, clear_data=False): + print('WorkflowProcessor: reset: ') + + workflow_model.bpmn_workflow_json = None if clear_data: # Clear form_data from task_events task_events = session.query(TaskEventModel). \ @@ -206,15 +210,6 @@ class WorkflowProcessor(object): session.add(task_event) session.commit() - workflow_model.bpmn_workflow_json = WorkflowProcessor._serializer.serialize_workflow(self.bpmn_workflow) - self.save() - - # set whether this is the latest spec file. - if self.spec_data_files == FileService.get_spec_data_files(workflow_spec_id=workflow_model.workflow_spec_id): - self.is_latest_spec = True - else: - self.is_latest_spec = False - def __get_bpmn_workflow(self, workflow_model: WorkflowModel, spec: WorkflowSpec, validate_only=False): if workflow_model.bpmn_workflow_json: bpmn_workflow = self._serializer.deserialize_workflow(workflow_model.bpmn_workflow_json, From 9281a93abb58d5e61b80dbb7f3effbe59642e59d Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:15:44 -0500 Subject: [PATCH 06/13] New test for new restart_workflow endpoint --- tests/workflow/test_workflow_restart.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/workflow/test_workflow_restart.py b/tests/workflow/test_workflow_restart.py index bac7a361..dddbffeb 100644 --- a/tests/workflow/test_workflow_restart.py +++ b/tests/workflow/test_workflow_restart.py @@ -5,16 +5,30 @@ class TestMessageEvent(BaseTest): def test_message_event(self): - # self.load_example_data() workflow = self.create_workflow('message_event') first_task = self.get_workflow_api(workflow).next_task self.assertEqual('Activity_GetData', first_task.name) workflow_api = self.get_workflow_api(workflow) + result = self.complete_form(workflow_api, first_task, {'formdata': 'asdf'}) self.assertIn('formdata', result.next_task.data) self.assertEqual('asdf', result.next_task.data['formdata']) - workflow_api = self.get_workflow_api(workflow_api, clear_data=True) + + workflow_api = self.get_workflow_api(workflow) + self.assertEqual('Activity_HowMany', self.get_workflow_api(workflow_api).next_task.name) + + # restart with data. should land at beginning with data + workflow_api = self.restart_workflow_api(result) + first_task = self.get_workflow_api(workflow_api).next_task + self.assertEqual('Activity_GetData', first_task.name) + self.assertIn('formdata', workflow_api.next_task.data) + self.assertEqual('asdf', workflow_api.next_task.data['formdata']) + + # restart without data. + workflow_api = self.restart_workflow_api(workflow_api, clear_data=True) + first_task = self.get_workflow_api(workflow).next_task + self.assertEqual('Activity_GetData', first_task.name) self.assertNotIn('formdata', workflow_api.next_task.data) print('Nice Test') From bfb3871671bcd5f815380cfbe5b3ab58b21c7543 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:27:44 -0500 Subject: [PATCH 07/13] Added method for restart_workflow_api. Pulled out some common code with get_workflow_api. --- tests/base_test.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/tests/base_test.py b/tests/base_test.py index 28a62ffd..67510eb6 100644 --- a/tests/base_test.py +++ b/tests/base_test.py @@ -341,19 +341,30 @@ class BaseTest(unittest.TestCase): session.commit() return approval - def get_workflow_api(self, workflow, reload_spec=False, clear_data=False, do_engine_steps=True, user_uid="dhf8r"): - user = session.query(UserModel).filter_by(uid=user_uid).first() - self.assertIsNotNone(user) - url = (f'/v1.0/workflow/{workflow.id}' - f'?clear_data={str(clear_data)}' - f'&reload_spec={str(reload_spec)}' - f'&do_engine_steps={str(do_engine_steps)}') + def get_workflow_common(self, url, user): rv = self.app.get(url, headers=self.logged_in_headers(user), content_type="application/json") self.assert_success(rv) json_data = json.loads(rv.get_data(as_text=True)) workflow_api = WorkflowApiSchema().load(json_data) + return workflow_api + + def get_workflow_api(self, workflow, do_engine_steps=True, user_uid="dhf8r"): + user = session.query(UserModel).filter_by(uid=user_uid).first() + self.assertIsNotNone(user) + url = (f'/v1.0/workflow/{workflow.id}' + f'?do_engine_steps={str(do_engine_steps)}') + workflow_api = self.get_workflow_common(url, user) + self.assertEqual(workflow.workflow_spec_id, workflow_api.workflow_spec_id) + return workflow_api + + def restart_workflow_api(self, workflow, clear_data=False, user_uid="dhf8r"): + user = session.query(UserModel).filter_by(uid=user_uid).first() + self.assertIsNotNone(user) + url = (f'/v1.0/workflow/{workflow.id}/restart' + f'?clear_data={str(clear_data)}') + workflow_api = self.get_workflow_common(url, user) self.assertEqual(workflow.workflow_spec_id, workflow_api.workflow_spec_id) return workflow_api From 1b18b5310790b7f795381024e0147d963a90b4ff Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:32:38 -0500 Subject: [PATCH 08/13] Fixed hard_reset test. We now call restart_workflow_api. --- tests/test_user_roles.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_user_roles.py b/tests/test_user_roles.py index abd78b61..7dadbefd 100644 --- a/tests/test_user_roles.py +++ b/tests/test_user_roles.py @@ -228,7 +228,8 @@ class TestTasksApi(BaseTest): self.assertEqual(1, len(self.get_assignment_task_events(supervisor.uid))) # Resetting the workflow at this point should clear the event log. - workflow_api = self.get_workflow_api(workflow, hard_reset=True, user_uid=submitter.uid) + workflow_api = self.restart_workflow_api(workflow, user_uid=submitter.uid) + workflow_api = self.get_workflow_api(workflow, user_uid=submitter.uid) self.assertEqual(0, len(self.get_assignment_task_events(supervisor.uid))) # Re-complete first task, and awaiting tasks should shift to 0 for for submitter, and 1 for supervisor From 61b34fd6f4061d42cc570283a4c1792bc30daafd Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:34:46 -0500 Subject: [PATCH 09/13] Fixed hard_reset test. We now call restart_workflow_api. --- tests/test_tasks_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_tasks_api.py b/tests/test_tasks_api.py index f9794051..8e6d0f7b 100644 --- a/tests/test_tasks_api.py +++ b/tests/test_tasks_api.py @@ -204,7 +204,7 @@ class TestTasksApi(BaseTest): self.assertTrue(workflow_api.spec_version.startswith("v1 ")) self.assertFalse(workflow_api.is_latest_spec) - workflow_api = self.get_workflow_api(workflow, hard_reset=True) + workflow_api = self.restart_workflow_api(workflow_api, clear_data=True) self.assertTrue(workflow_api.spec_version.startswith("v2 ")) self.assertTrue(workflow_api.is_latest_spec) From 61d366c111b27d3d9fbfc9f5513a0a86001f98af Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:39:59 -0500 Subject: [PATCH 10/13] Fixed hard_reset test. Now we reset the workflow and run do_engine_steps --- tests/workflow/test_workflow_processor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/workflow/test_workflow_processor.py b/tests/workflow/test_workflow_processor.py index a9ae0cf2..67866ac1 100644 --- a/tests/workflow/test_workflow_processor.py +++ b/tests/workflow/test_workflow_processor.py @@ -301,7 +301,9 @@ class TestWorkflowProcessor(BaseTest): self.assertFalse(processor2.is_latest_spec) # Still at version 1. # Do a hard reset, which should bring us back to the beginning, but retain the data. - processor3 = WorkflowProcessor(processor.workflow_model, hard_reset=True) + WorkflowProcessor.reset(processor2.workflow_model) + processor3 = WorkflowProcessor(processor.workflow_model) + processor3.do_engine_steps() self.assertEqual("Step 1", processor3.next_task().task_spec.description) self.assertTrue(processor3.is_latest_spec) # Now at version 2. task = processor3.next_task() From 27a838389820be74a0d44d3cc2b213657ecc1540 Mon Sep 17 00:00:00 2001 From: mike cullerton Date: Tue, 19 Jan 2021 15:44:11 -0500 Subject: [PATCH 11/13] Not sure about this one. We no longer support the task this test is testing (soft_reset). I commented out that part of the test. We can discuss, if necessary. --- tests/test_tasks_api.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_tasks_api.py b/tests/test_tasks_api.py index 8e6d0f7b..6a927e13 100644 --- a/tests/test_tasks_api.py +++ b/tests/test_tasks_api.py @@ -225,12 +225,12 @@ class TestTasksApi(BaseTest): file_path = os.path.join(app.root_path, '..', 'tests', 'data', 'two_forms', 'modified', 'two_forms_struc_mod.bpmn') self.replace_file("two_forms.bpmn", file_path) - # perform a soft reset returns an error - rv = self.app.get('/v1.0/workflow/%i?soft_reset=%s&hard_reset=%s' % - (workflow.id, "true", "false"), - content_type="application/json", - headers=self.logged_in_headers()) - self.assert_failure(rv, error_code="unexpected_workflow_structure") + # # perform a soft reset returns an error + # rv = self.app.get('/v1.0/workflow/%i?soft_reset=%s&hard_reset=%s' % + # (workflow.id, "true", "false"), + # content_type="application/json", + # headers=self.logged_in_headers()) + # self.assert_failure(rv, error_code="unexpected_workflow_structure") # Try again without a soft reset, and we are still ok, and on the original version. workflow_api = self.get_workflow_api(workflow) From 6a7ee09a5dfac0b45d1985c4b0132f648284a859 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 20 Jan 2021 11:00:41 -0500 Subject: [PATCH 12/13] Updating the readme file. --- README.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/README.md b/README.md index b43dbf68..18cfdde5 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,44 @@ it should always be visible in the Docker Desktop Daskboard with a friendly litt stop button for your clicking pleasure. +### Environment Setup Part #2 +If you want to run CR-Connect in development mode you must have the following two services running: +1. `Postgres Docker`: There is a sub-directory called Postgres that contains a docker image that will set up an empty +database for CR-Connect, and for Protocol Builder Mock, mentioned below. For must systems, you can 'cd' into this +directory and just run start.sh to fire up the Postgres service, and stop.sh to shut it back down again. +create .env file in /postgres with the following two lines in it: +``` +DB_USER=crc_user +DB_PASS=crc_pass +``` +With this in place, from the command line: +```bash +cd postgres +./start.sh +``` +You can now build the database structure in the newly created database with the following lines +```baseh +cd .. (into base directory) +flask db upgrade +flask load-example-data (this creates some basic workflows for you to use) +``` + + + +2. `Protocol Builder Mock`: We created a mock of the Protocol Builder, a critical service at UVA that is a deep +dependency for CR-Connect. You can find the details here: [Protocol Builder Mock](https://github.com/sartography/protocol-builder-mock) +Be sure this is up and running on Port 5002 or you will encounter errors when the system starts up. + +With Protocol Builder Mock up and running, visit http://localhost:5001 and create a study. Set the user +and primary investigator to dhf8r - which is a user in the mock ldap service, and this will later show up when you +fire up the interface. + +### Configuration +1. `instance/config.py`: This will + + + + ### Project Initialization 1. Clone this repository. 2. In PyCharm: From 41dac33039fb47e7759767dca1cd09e9605bb877 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 20 Jan 2021 13:24:53 -0500 Subject: [PATCH 13/13] Removed two tests around soft_reset that are no longer required. Reset should construct the workflow_processor, thus completing the reset process. Removed the spec argument from init, as it is never used anywhere. --- crc/services/workflow_processor.py | 7 ++++--- tests/test_lookup_service.py | 4 +++- tests/test_tasks_api.py | 24 ----------------------- tests/workflow/test_workflow_processor.py | 22 --------------------- 4 files changed, 7 insertions(+), 50 deletions(-) diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 077c0d0b..5149fa97 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -151,12 +151,12 @@ class WorkflowProcessor(object): STUDY_ID_KEY = "study_id" VALIDATION_PROCESS_KEY = "validate_only" - def __init__(self, workflow_model: WorkflowModel, spec=None, validate_only=False): + def __init__(self, workflow_model: WorkflowModel, validate_only=False): """Create a Workflow Processor based on the serialized information available in the workflow model.""" self.workflow_model = workflow_model - if workflow_model.bpmn_workflow_json is None and spec is None: # The workflow was never started. + if workflow_model.bpmn_workflow_json is None: # The workflow was never started. self.spec_data_files = FileService.get_spec_data_files( workflow_spec_id=workflow_model.workflow_spec_id) spec = self.get_spec(self.spec_data_files, workflow_model.workflow_spec_id) @@ -208,7 +208,8 @@ class WorkflowProcessor(object): for task_event in task_events: task_event.form_data = {} session.add(task_event) - session.commit() + session.commit() + return cls(workflow_model) def __get_bpmn_workflow(self, workflow_model: WorkflowModel, spec: WorkflowSpec, validate_only=False): if workflow_model.bpmn_workflow_json: diff --git a/tests/test_lookup_service.py b/tests/test_lookup_service.py index 0b7a8ddb..e9da7fa8 100644 --- a/tests/test_lookup_service.py +++ b/tests/test_lookup_service.py @@ -53,7 +53,9 @@ class TestLookupService(BaseTest): file.close() # restart the workflow, so it can pick up the changes. - WorkflowProcessor(workflow, soft_reset=True) + + processor = WorkflowProcessor.reset(workflow) + workflow = processor.workflow_model LookupService.lookup(workflow, "sponsor", "sam", limit=10) lookup_records = session.query(LookupFileModel).all() diff --git a/tests/test_tasks_api.py b/tests/test_tasks_api.py index 6a927e13..d8d64c4d 100644 --- a/tests/test_tasks_api.py +++ b/tests/test_tasks_api.py @@ -213,30 +213,6 @@ class TestTasksApi(BaseTest): self.assertTrue(workflow_api.spec_version.startswith("v2 ")) self.assertTrue(workflow_api.is_latest_spec) - def test_soft_reset_errors_out_and_next_result_is_on_original_version(self): - # Start the basic two_forms workflow and complete a task. - workflow = self.create_workflow('two_forms') - workflow_api = self.get_workflow_api(workflow) - self.complete_form(workflow, workflow_api.next_task, {"color": "blue"}) - self.assertTrue(workflow_api.is_latest_spec) - - # Modify the specification, with a major change that alters the flow and can't be deserialized - # effectively, if it uses the latest spec files. - file_path = os.path.join(app.root_path, '..', 'tests', 'data', 'two_forms', 'modified', 'two_forms_struc_mod.bpmn') - self.replace_file("two_forms.bpmn", file_path) - - # # perform a soft reset returns an error - # rv = self.app.get('/v1.0/workflow/%i?soft_reset=%s&hard_reset=%s' % - # (workflow.id, "true", "false"), - # content_type="application/json", - # headers=self.logged_in_headers()) - # self.assert_failure(rv, error_code="unexpected_workflow_structure") - - # Try again without a soft reset, and we are still ok, and on the original version. - workflow_api = self.get_workflow_api(workflow) - self.assertTrue(workflow_api.spec_version.startswith("v1 ")) - self.assertFalse(workflow_api.is_latest_spec) - def test_manual_task_with_external_documentation(self): workflow = self.create_workflow('manual_task_with_external_documentation') diff --git a/tests/workflow/test_workflow_processor.py b/tests/workflow/test_workflow_processor.py index 67866ac1..843a4f1f 100644 --- a/tests/workflow/test_workflow_processor.py +++ b/tests/workflow/test_workflow_processor.py @@ -173,28 +173,6 @@ class TestWorkflowProcessor(BaseTest): self.assertEqual("workflow_validation_error", context.exception.code) self.assertTrue("bpmn:startEvent" in context.exception.message) - def test_workflow_spec_key_error(self): - """Frequently seeing errors in the logs about a 'Key' error, where a workflow - references something that doesn't exist in the midst of processing. Want to - make sure we produce errors to the front end that allows us to debug this.""" - # Start the two_forms workflow, and enter some data in the first form. - self.load_example_data() - study = session.query(StudyModel).first() - workflow_spec_model = self.load_test_spec("two_forms") - processor = self.get_processor(study, workflow_spec_model) - self.assertEqual(processor.workflow_model.workflow_spec_id, workflow_spec_model.id) - task = processor.next_task() - task.data = {"color": "blue"} - processor.complete_task(task) - - # Modify the specification, with a major change. - file_path = os.path.join(app.root_path, '..', 'tests', 'data', 'two_forms', 'modified', 'two_forms_struc_mod.bpmn') - self.replace_file("two_forms.bpmn", file_path) - - # Attempting a soft update on a structural change should raise a sensible error. - with self.assertRaises(ApiError) as context: - processor3 = WorkflowProcessor(processor.workflow_model, soft_reset=True) - self.assertEqual("unexpected_workflow_structure", context.exception.code) def test_workflow_with_bad_expression_raises_sensible_error(self): self.load_example_data()