From b02731df17e0120e4c85ad3c462cad0e6d0d7f65 Mon Sep 17 00:00:00 2001 From: Dan Date: Mon, 4 Oct 2021 15:07:47 -0400 Subject: [PATCH] Add some additional checks in the workflow service so we assure the data we are creating during validation can be serialized and deserialized just as it would be through the API. (Was hanging us up on dates) Assure that if we generate a default value for a date in the task data, it is stored as an ISO String. remove any unserializable data from the task_data when an error is encountered, rather than just dropping all the task_data. This case seems to happen a lot and it leaves us with nothing to go on. --- crc/api/common.py | 30 ++++++--- crc/services/workflow_service.py | 11 +++- .../date_value_expression.bpmn | 65 +++++++++++++++++++ .../test_workflow_spec_validation_api.py | 9 +++ 4 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 tests/data/date_value_expression/date_value_expression.bpmn diff --git a/crc/api/common.py b/crc/api/common.py index 1d6cbe68..73775e34 100644 --- a/crc/api/common.py +++ b/crc/api/common.py @@ -51,18 +51,30 @@ class ApiError(Exception): if "task" in task.data: task.data.pop("task") - # In the unlikely event that the API error can't be serialized, try removing the task_data, as it may - # contain some invalid data that we can't return, so we can at least get the erro rmessage. - instance.task_data = task.data - try: - json.dumps(ApiErrorSchema().dump(instance)) - except TypeError as te: - instance.task_data = { - 'task_data_hidden': 'We were unable to serialize the task data when reporting this error'} + # Assure that there is nothing in the json data that can't be serialized. + instance.task_data = ApiError.remove_unserializeable_from_dict(task.data) app.logger.error(message, exc_info=True) return instance + @staticmethod + def remove_unserializeable_from_dict(my_dict): + keys_to_delete = [] + for key, value in my_dict.items(): + if not ApiError.is_jsonable(value): + keys_to_delete.append(key) + for key in keys_to_delete: + del my_dict[key] + return my_dict + + @staticmethod + def is_jsonable(x): + try: + json.dumps(x) + return True + except (TypeError, OverflowError): + return False + @classmethod def from_task_spec(cls, code, message, task_spec, status_code=400): """Constructs an API Error with details pulled from the current task.""" @@ -89,6 +101,8 @@ class ApiError(Exception): return ApiError.from_task_spec(code, message, exp.sender) + + class ApiErrorSchema(ma.Schema): class Meta: fields = ("code", "message", "workflow_name", "file_name", "task_name", "task_id", diff --git a/crc/services/workflow_service.py b/crc/services/workflow_service.py index cba11ab1..bc22bc11 100755 --- a/crc/services/workflow_service.py +++ b/crc/services/workflow_service.py @@ -1,4 +1,5 @@ import copy +import json import string from datetime import datetime import random @@ -277,7 +278,11 @@ class WorkflowService(object): form_data[field.id] = WorkflowService.get_random_data_for_field(field, task) if task.data is None: task.data = {} - task.data.update(form_data) + + # jsonify, and de-jsonify the data to mimic how data will be returned from the front end for forms and assures + # we aren't generating something that can't be serialized. + form_data_string = json.dumps(form_data) + task.data.update(json.loads(form_data_string)) @staticmethod def check_field_id(id): @@ -435,6 +440,8 @@ class WorkflowService(object): if default == 'true' or default == 't': return True return False + elif field.type == 'date' and isinstance(default, datetime): + return default.isoformat() else: return default @@ -484,8 +491,6 @@ class WorkflowService(object): return [random_value] else: return random_value - - elif field.type == "long": return random.randint(1, 1000) elif field.type == 'boolean': diff --git a/tests/data/date_value_expression/date_value_expression.bpmn b/tests/data/date_value_expression/date_value_expression.bpmn new file mode 100644 index 00000000..9c544367 --- /dev/null +++ b/tests/data/date_value_expression/date_value_expression.bpmn @@ -0,0 +1,65 @@ + + + + + Flow_0ecke9e + + + + + + + + + + + + + + + + Flow_0ecke9e + Flow_04yzu4r + + + The Date is {{my_date}} + Flow_1hz2cs6 + + + + + Flow_04yzu4r + Flow_1hz2cs6 + dateparser.parse(my_date) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/workflow/test_workflow_spec_validation_api.py b/tests/workflow/test_workflow_spec_validation_api.py index c0e8937f..b422edfa 100644 --- a/tests/workflow/test_workflow_spec_validation_api.py +++ b/tests/workflow/test_workflow_spec_validation_api.py @@ -186,3 +186,12 @@ class TestWorkflowSpecValidation(BaseTest): api_error = json_data[0] self.assertEqual('disabled_workflow', api_error['code']) self.assertEqual('This workflow is disabled. This is my mocked disable message.', api_error['message']) + + + def test_date_generation_during_validation(self): + # We hit a bug where the date was generated as a part of a value_expression during validation, but + # it wasn't converted to an ISO String as it would be if submitted through the API. + # subsequent attempts to work with the expected date_string failed, because it was already a date. + # This can't happen in the front end code base, but it was breaking validation. + errors = self.validate_workflow("date_value_expression") + self.assertEqual(0, len(errors))