diff --git a/crc/scripts/study_info.py b/crc/scripts/study_info.py index 6dac8444..6d168af5 100644 --- a/crc/scripts/study_info.py +++ b/crc/scripts/study_info.py @@ -41,3 +41,18 @@ class StudyInfo(Script): if cmd == 'details': study_info["details"] = self.pb.get_study_details(study_id) task.data["study"] = study_info + + + def get_required_docs(self, study_id): + required_docs = self.pb.get_required_docs(study_id) + return required_docs + + + + + + + + + + diff --git a/crc/services/workflow_processor.py b/crc/services/workflow_processor.py index 34915236..b15f048f 100644 --- a/crc/services/workflow_processor.py +++ b/crc/services/workflow_processor.py @@ -4,6 +4,7 @@ import xml.etree.ElementTree as ElementTree from SpiffWorkflow import Task as SpiffTask, Workflow from SpiffWorkflow.bpmn.BpmnScriptEngine import BpmnScriptEngine +from SpiffWorkflow.bpmn.parser.ValidationException import ValidationException from SpiffWorkflow.bpmn.serializer.BpmnSerializer import BpmnSerializer from SpiffWorkflow.bpmn.specs.EndEvent import EndEvent from SpiffWorkflow.bpmn.workflow import BpmnWorkflow @@ -98,6 +99,7 @@ class WorkflowProcessor(object): completed task in the previous workflow. If neither flag is set, it will use the same version of the specification that was used to originally create the workflow model. """ + orig_version = workflow_model.spec_version if soft_reset: spec = self.get_spec(workflow_model.workflow_spec_id) workflow_model.spec_version = spec.description @@ -105,7 +107,17 @@ class WorkflowProcessor(object): spec = self.get_spec(workflow_model.workflow_spec_id, workflow_model.spec_version) self.workflow_spec_id = workflow_model.workflow_spec_id - self.bpmn_workflow = self._serializer.deserialize_workflow(workflow_model.bpmn_workflow_json, workflow_spec=spec) + try: + self.bpmn_workflow = self._serializer.deserialize_workflow(workflow_model.bpmn_workflow_json, workflow_spec=spec) + except KeyError as ke: + if soft_reset: + # Undo the soft-reset. + workflow_model.spec_version = orig_version + orig_version = workflow_model.spec_version + 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, workflow_model.spec_version, str(ke)) + + " This is very likely due to a soft reset where there was a structural change.") self.bpmn_workflow.script_engine = self._script_engine if hard_reset: @@ -192,8 +204,14 @@ class WorkflowProcessor(object): dmn: ElementTree.Element = ElementTree.fromstring(file_data.data) parser.add_dmn_xml(dmn, filename=file_data.file_model.name) if process_id is None: - raise(Exception("There is no primary BPMN model defined for workflow %s" % workflow_spec_id)) - spec = parser.get_spec(process_id) + raise(ApiError(code="no_primary_bpmn_error", + message="There is no primary BPMN model defined for workflow %s" % workflow_spec_id)) + try: + spec = parser.get_spec(process_id) + except ValidationException as ve: + raise ApiError(code="workflow_validation_error", + message="Failed to parse Workflow Specification '%s' %s." % (workflow_spec_id, version) + + "Error is %s" % str(ve)) spec.description = version return spec @@ -220,7 +238,7 @@ class WorkflowProcessor(object): session.add(workflow_model) session.commit() # Need to commit twice, first to get a unique id for the workflow model, and - # a second time to store the serilaization so we can maintain this link within + # a second time to store the serialization so we can maintain this link within # the spiff-workflow process. bpmn_workflow.data[WorkflowProcessor.WORKFLOW_ID_KEY] = workflow_model.id @@ -320,7 +338,7 @@ class WorkflowProcessor(object): process_elements.append(child) if len(process_elements) == 0: - raise Exception('No executable process tag found') + raise ValidationException('No executable process tag found') # There are multiple root elements if len(process_elements) > 1: @@ -332,6 +350,6 @@ class WorkflowProcessor(object): if child_element.tag.endswith('startEvent'): return this_element.attrib['id'] - raise Exception('No start event found in %s' % et_root.attrib['id']) + raise ValidationException('No start event found in %s' % et_root.attrib['id']) return process_elements[0].attrib['id'] diff --git a/tests/data/invalid_spec/invalid_spec.bpmn b/tests/data/invalid_spec/invalid_spec.bpmn new file mode 100644 index 00000000..30a75fb4 --- /dev/null +++ b/tests/data/invalid_spec/invalid_spec.bpmn @@ -0,0 +1,116 @@ + + + + + + + + + + + SequenceFlow_1pnq3kg + SequenceFlow_1lmkn99 + + + + SequenceFlow_1lmkn99 + SequenceFlow_No_Bananas + SequenceFlow_Yes_Bananas + + + lower_case_true==true + + + + + + + + SequenceFlow_Yes_Bananas + SequenceFlow_02z84p5 + + + + + + + + SequenceFlow_No_Bananas + SequenceFlow_08djf6q + + + SequenceFlow_02z84p5 + + + + SequenceFlow_08djf6q + + + + + has_bananas == True + + + This start event doesn't go anywhere!  that should raise a sensible error to the ui + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_tasks_api.py b/tests/test_tasks_api.py index af1a105c..db2b33f0 100644 --- a/tests/test_tasks_api.py +++ b/tests/test_tasks_api.py @@ -31,6 +31,7 @@ class TestTasksApi(BaseTest): (workflow.id, str(soft_reset), str(hard_reset)), headers=self.logged_in_headers(), content_type="application/json") + self.assert_success(rv) json_data = json.loads(rv.get_data(as_text=True)) workflow_api = WorkflowApiSchema().load(json_data) self.assertEqual(workflow.workflow_spec_id, workflow_api.workflow_spec_id) @@ -243,6 +244,32 @@ 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. + self.load_example_data() + workflow = self.create_workflow('two_forms') + workflow_api = self.get_workflow_api(workflow) + self.complete_form(workflow, workflow_api.user_tasks[0], {"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', 'mods', '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_get_workflow_stats(self): self.load_example_data() workflow = self.create_workflow('exclusive_gateway') diff --git a/tests/test_workflow_processor.py b/tests/test_workflow_processor.py index 258ab195..c4ff3f21 100644 --- a/tests/test_workflow_processor.py +++ b/tests/test_workflow_processor.py @@ -159,6 +159,42 @@ class TestWorkflowProcessor(BaseTest): self.assertIn("details", task.data) self.assertIsInstance(task.task_spec, EndEvent) + def test_workflow_validation_error_is_properly_raised(self): + self.load_example_data() + workflow_spec_model = self.load_test_spec("invalid_spec") + study = session.query(StudyModel).first() + with self.assertRaises(ApiError) as context: + WorkflowProcessor.create(study.id, workflow_spec_model.id) + self.assertEquals("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 = WorkflowProcessor.create(study.id, workflow_spec_model.id) + workflow_model = db.session.query(WorkflowModel).filter(WorkflowModel.study_id == study.id).first() + self.assertEqual(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', 'mods', 'two_forms_struc_mod.bpmn') + self.replace_file("two_forms.bpmn", file_path) + + # Attemping a soft update on a structural change should raise a sensible error. + with self.assertRaises(ApiError) as context: + processor3 = WorkflowProcessor(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()