diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml index 50fb6f87..59d156e6 100755 --- a/spiffworkflow-backend/src/spiffworkflow_backend/api.yml +++ b/spiffworkflow-backend/src/spiffworkflow_backend/api.yml @@ -397,6 +397,12 @@ paths: description: the modified process model id schema: type: string + - name: include_file_references + in: query + required: false + description: include all file references in the return + schema: + type: boolean get: operationId: spiffworkflow_backend.routes.process_models_controller.process_model_show summary: Returns a single process model diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py index 4e129deb..2703a311 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/routes/process_models_controller.py @@ -15,6 +15,7 @@ from flask import jsonify from flask import make_response from flask.wrappers import Response from flask_bpmn.api.api_error import ApiError +from werkzeug.datastructures import FileStorage from spiffworkflow_backend.interfaces import IdToProcessGroupMapping from spiffworkflow_backend.models.file import FileSchema @@ -38,6 +39,9 @@ from spiffworkflow_backend.services.process_instance_report_service import ( ProcessInstanceReportService, ) from spiffworkflow_backend.services.process_model_service import ProcessModelService +from spiffworkflow_backend.services.spec_file_service import ( + ProcessModelFileInvalidError, +) from spiffworkflow_backend.services.spec_file_service import SpecFileService @@ -119,7 +123,9 @@ def process_model_update( return ProcessModelInfoSchema().dump(process_model) -def process_model_show(modified_process_model_identifier: str) -> Any: +def process_model_show( + modified_process_model_identifier: str, include_file_references: bool = False +) -> Any: """Process_model_show.""" process_model_identifier = modified_process_model_identifier.replace(":", "/") process_model = _get_process_model(process_model_identifier) @@ -128,8 +134,12 @@ def process_model_show(modified_process_model_identifier: str) -> Any: key=lambda f: "" if f.name == process_model.primary_file_name else f.sort_index, ) process_model.files = files - for file in process_model.files: - file.references = SpecFileService.get_references_for_file(file, process_model) + + if include_file_references: + for file in process_model.files: + file.references = SpecFileService.get_references_for_file( + file, process_model + ) process_model.parent_groups = ProcessModelService.get_parent_group_array( process_model.id @@ -222,26 +232,11 @@ def process_model_file_update( modified_process_model_identifier: str, file_name: str ) -> flask.wrappers.Response: """Process_model_file_update.""" - process_model_identifier = modified_process_model_identifier.replace(":", "/") - process_model = _get_process_model(process_model_identifier) - - request_file = _get_file_from_request() - request_file_contents = request_file.stream.read() - if not request_file_contents: - raise ApiError( - error_code="file_contents_empty", - message="Given request file does not have any content", - status_code=400, - ) - - SpecFileService.update_file(process_model, file_name, request_file_contents) - _commit_and_push_to_git( - f"User: {g.user.username} clicked save for" - f" {process_model_identifier}/{file_name}" + message = f"User: {g.user.username} clicked save for" + return _create_or_update_process_model_file( + modified_process_model_identifier, message, 200 ) - return Response(json.dumps({"ok": True}), status=200, mimetype="application/json") - def process_model_file_delete( modified_process_model_identifier: str, file_name: str @@ -271,28 +266,9 @@ def process_model_file_create( modified_process_model_identifier: str, ) -> flask.wrappers.Response: """Process_model_file_create.""" - process_model_identifier = modified_process_model_identifier.replace(":", "/") - process_model = _get_process_model(process_model_identifier) - request_file = _get_file_from_request() - if not request_file.filename: - raise ApiError( - error_code="could_not_get_filename", - message="Could not get filename from request", - status_code=400, - ) - - file = SpecFileService.add_file( - process_model, request_file.filename, request_file.stream.read() - ) - file_contents = SpecFileService.get_data(process_model, file.name) - file.file_contents = file_contents - file.process_model_id = process_model.id - _commit_and_push_to_git( - f"User: {g.user.username} added process model file" - f" {process_model_identifier}/{file.name}" - ) - return Response( - json.dumps(FileSchema().dump(file)), status=201, mimetype="application/json" + message = f"User: {g.user.username} added process model file" + return _create_or_update_process_model_file( + modified_process_model_identifier, message, 201 ) @@ -462,9 +438,9 @@ def process_model_create_with_natural_language( ) -def _get_file_from_request() -> Any: +def _get_file_from_request() -> FileStorage: """Get_file_from_request.""" - request_file = connexion.request.files.get("file") + request_file: FileStorage = connexion.request.files.get("file") if not request_file: raise ApiError( error_code="no_file_given", @@ -502,3 +478,58 @@ def _get_process_group_from_modified_identifier( status_code=400, ) return process_group + + +def _create_or_update_process_model_file( + modified_process_model_identifier: str, + message_for_git_commit: str, + http_status_to_return: int, +) -> flask.wrappers.Response: + """_create_or_update_process_model_file.""" + process_model_identifier = modified_process_model_identifier.replace(":", "/") + process_model = _get_process_model(process_model_identifier) + request_file = _get_file_from_request() + + # for mypy + request_file_contents = request_file.stream.read() + if not request_file_contents: + raise ApiError( + error_code="file_contents_empty", + message="Given request file does not have any content", + status_code=400, + ) + if not request_file.filename: + raise ApiError( + error_code="could_not_get_filename", + message="Could not get filename from request", + status_code=400, + ) + + file = None + try: + file = SpecFileService.update_file( + process_model, request_file.filename, request_file_contents + ) + except ProcessModelFileInvalidError as exception: + raise ( + ApiError( + error_code="process_model_file_invalid", + message=( + f"Invalid Process model file cannot be save: {request_file.name}." + f" Received error: {str(exception)}" + ), + status_code=400, + ) + ) from exception + file_contents = SpecFileService.get_data(process_model, file.name) + file.file_contents = file_contents + file.process_model_id = process_model.id + _commit_and_push_to_git( + f"{message_for_git_commit} {process_model_identifier}/{file.name}" + ) + + return Response( + json.dumps(FileSchema().dump(file)), + status=http_status_to_return, + mimetype="application/json", + ) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py index 4fdfbd6d..868d0fcd 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/spec_file_service.py @@ -6,7 +6,8 @@ from typing import List from typing import Optional from flask_bpmn.models.db import db -from SpiffWorkflow.bpmn.parser.ValidationException import ValidationException # type: ignore +from lxml import etree # type: ignore +from SpiffWorkflow.bpmn.parser.BpmnParser import BpmnValidator # type: ignore from spiffworkflow_backend.models.file import File from spiffworkflow_backend.models.file import FileType @@ -29,6 +30,10 @@ class ProcessModelFileNotFoundError(Exception): """ProcessModelFileNotFoundError.""" +class ProcessModelFileInvalidError(Exception): + """ProcessModelFileInvalidError.""" + + class SpecFileService(FileSystemService): """SpecFileService.""" @@ -44,7 +49,6 @@ class SpecFileService(FileSystemService): extension_filter: str = "", ) -> List[File]: """Return all files associated with a workflow specification.""" - # path = SpecFileService.workflow_path(process_model_info) path = os.path.join( FileSystemService.root_path(), process_model_info.id_for_file_path() ) @@ -76,9 +80,22 @@ class SpecFileService(FileSystemService): ) return references - @staticmethod + @classmethod def get_references_for_file( - file: File, process_model_info: ProcessModelInfo + cls, file: File, process_model_info: ProcessModelInfo + ) -> list[SpecReference]: + """Get_references_for_file.""" + full_file_path = SpecFileService.full_file_path(process_model_info, file.name) + file_contents: bytes = b"" + with open(full_file_path) as f: + file_contents = f.read().encode() + return cls.get_references_for_file_contents( + process_model_info, file.name, file_contents + ) + + @classmethod + def get_references_for_file_contents( + cls, process_model_info: ProcessModelInfo, file_name: str, binary_data: bytes ) -> list[SpecReference]: """Uses spiffworkflow to parse BPMN and DMN files to determine how they can be externally referenced. @@ -89,8 +106,8 @@ class SpecFileService(FileSystemService): type = {str} 'process' / 'decision' """ references: list[SpecReference] = [] - full_file_path = SpecFileService.full_file_path(process_model_info, file.name) - file_path = os.path.join(process_model_info.id_for_file_path(), file.name) + file_path = os.path.join(process_model_info.id_for_file_path(), file_name) + file_type = FileSystemService.file_type(file_name) parser = MyCustomParser() parser_type = None sub_parser = None @@ -100,14 +117,14 @@ class SpecFileService(FileSystemService): messages = {} correlations = {} start_messages = [] - if file.type == FileType.bpmn.value: - parser.add_bpmn_file(full_file_path) + if file_type.value == FileType.bpmn.value: + parser.add_bpmn_xml(etree.fromstring(binary_data)) parser_type = "process" sub_parsers = list(parser.process_parsers.values()) messages = parser.messages correlations = parser.correlations - elif file.type == FileType.dmn.value: - parser.add_dmn_file(full_file_path) + elif file_type.value == FileType.dmn.value: + parser.add_dmn_xml(etree.fromstring(binary_data)) sub_parsers = list(parser.dmn_parsers.values()) parser_type = "decision" else: @@ -127,7 +144,7 @@ class SpecFileService(FileSystemService): display_name=sub_parser.get_name(), process_model_id=process_model_info.id, type=parser_type, - file_name=file.name, + file_name=file_name, relative_path=file_path, has_lanes=has_lanes, is_executable=is_executable, @@ -147,23 +164,36 @@ class SpecFileService(FileSystemService): # Same as update return SpecFileService.update_file(process_model_info, file_name, binary_data) - @staticmethod + @classmethod + def validate_bpmn_xml(cls, file_name: str, binary_data: bytes) -> None: + """Validate_bpmn_xml.""" + file_type = FileSystemService.file_type(file_name) + if file_type.value == FileType.bpmn.value: + validator = BpmnValidator() + parser = MyCustomParser(validator=validator) + try: + parser.add_bpmn_xml(etree.fromstring(binary_data), filename=file_name) + except etree.XMLSyntaxError as exception: + raise ProcessModelFileInvalidError( + f"Received error trying to parse bpmn xml: {str(exception)}" + ) from exception + + @classmethod def update_file( - process_model_info: ProcessModelInfo, file_name: str, binary_data: bytes + cls, process_model_info: ProcessModelInfo, file_name: str, binary_data: bytes ) -> File: """Update_file.""" SpecFileService.assert_valid_file_name(file_name) - full_file_path = SpecFileService.full_file_path(process_model_info, file_name) - SpecFileService.write_file_data_to_system(full_file_path, binary_data) - file = SpecFileService.to_file_object(file_name, full_file_path) + cls.validate_bpmn_xml(file_name, binary_data) - references = SpecFileService.get_references_for_file(file, process_model_info) + references = cls.get_references_for_file_contents( + process_model_info, file_name, binary_data + ) primary_process_ref = next( (ref for ref in references if ref.is_primary and ref.is_executable), None ) SpecFileService.clear_caches_for_file(file_name, process_model_info) - for ref in references: # If no valid primary process is defined, default to the first process in the # updated file. @@ -184,7 +214,11 @@ class SpecFileService(FileSystemService): update_hash, ) SpecFileService.update_caches(ref) - return file + + # make sure we save the file as the last thing we do to ensure validations have run + full_file_path = SpecFileService.full_file_path(process_model_info, file_name) + SpecFileService.write_file_data_to_system(full_file_path, binary_data) + return SpecFileService.to_file_object(file_name, full_file_path) @staticmethod def get_data(process_model_info: ProcessModelInfo, file_name: str) -> bytes: @@ -282,7 +316,7 @@ class SpecFileService(FileSystemService): # if the old relative bpmn file no longer exists, then assume things were moved around # on the file system. Otherwise, assume it is a duplicate process id and error. if os.path.isfile(full_bpmn_file_path): - raise ValidationException( + raise ProcessModelFileInvalidError( f"Process id ({ref.identifier}) has already been used for " f"{process_id_lookup.relative_path}. It cannot be reused." ) @@ -314,7 +348,7 @@ class SpecFileService(FileSystemService): identifier=message_model_identifier ).first() if message_model is None: - raise ValidationException( + raise ProcessModelFileInvalidError( "Could not find message model with identifier" f" '{message_model_identifier}'Required by a Start Event in :" f" {ref.file_name}" @@ -336,7 +370,7 @@ class SpecFileService(FileSystemService): message_triggerable_process_model.process_model_identifier != ref.process_model_id ): - raise ValidationException( + raise ProcessModelFileInvalidError( "Message model is already used to start process model" f" {ref.process_model_id}" ) @@ -355,7 +389,7 @@ class SpecFileService(FileSystemService): identifier=message_model_identifier ).first() if message_model is None: - raise ValidationException( + raise ProcessModelFileInvalidError( "Could not find message model with identifier" f" '{message_model_identifier}'specified by correlation" f" property: {cpre}" diff --git a/spiffworkflow-backend/tests/data/dot_notation/diagram.bpmn b/spiffworkflow-backend/tests/data/dot_notation/diagram.bpmn index cb623748..96bdce89 100644 --- a/spiffworkflow-backend/tests/data/dot_notation/diagram.bpmn +++ b/spiffworkflow-backend/tests/data/dot_notation/diagram.bpmn @@ -2,10 +2,10 @@ - to + to - from.name + from.name @@ -20,7 +20,7 @@ - new + new diff --git a/spiffworkflow-backend/tests/data/message_send_one_conversation/message_receiver.bpmn b/spiffworkflow-backend/tests/data/message_send_one_conversation/message_receiver.bpmn index c82aa363..828f7d2e 100644 --- a/spiffworkflow-backend/tests/data/message_send_one_conversation/message_receiver.bpmn +++ b/spiffworkflow-backend/tests/data/message_send_one_conversation/message_receiver.bpmn @@ -12,18 +12,18 @@ - topica + topica - the_payload.topica + the_payload.topica - topicb + topicb - the_payload.topicb + the_payload.topicb diff --git a/spiffworkflow-backend/tests/data/message_send_one_conversation/message_sender.bpmn b/spiffworkflow-backend/tests/data/message_send_one_conversation/message_sender.bpmn index a735d1ad..7bda31eb 100644 --- a/spiffworkflow-backend/tests/data/message_send_one_conversation/message_sender.bpmn +++ b/spiffworkflow-backend/tests/data/message_send_one_conversation/message_sender.bpmn @@ -12,18 +12,18 @@ - topica + topica - the_payload.topica + the_payload.topica - topicb + topicb - the_payload.topicb + the_payload.topicb diff --git a/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_one.bpmn b/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_one.bpmn index 7a2356d6..62236cc4 100644 --- a/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_one.bpmn +++ b/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_one.bpmn @@ -12,18 +12,18 @@ - topica_one + topica_one - topica_one + topica_one - topicb_one + topicb_one - topicb_one + topicb_one diff --git a/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_two.bpmn b/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_two.bpmn index 354f3aad..67e856a5 100644 --- a/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_two.bpmn +++ b/spiffworkflow-backend/tests/data/message_send_two_conversations/message_receiver_two.bpmn @@ -12,18 +12,18 @@ - topica_two + topica_two - topica_two + topica_two - topicb_two + topicb_two - topicb_two + topicb_two diff --git a/spiffworkflow-backend/tests/data/message_send_two_conversations/message_sender.bpmn b/spiffworkflow-backend/tests/data/message_send_two_conversations/message_sender.bpmn index 38359c92..16051705 100644 --- a/spiffworkflow-backend/tests/data/message_send_two_conversations/message_sender.bpmn +++ b/spiffworkflow-backend/tests/data/message_send_two_conversations/message_sender.bpmn @@ -1,6 +1,6 @@ - + @@ -19,18 +19,18 @@ - topica_one + topica_one - payload_var_one.topica_one + payload_var_one.topica_one - topicb_one + topicb_one - payload_var_one.topicb + payload_var_one.topicb @@ -117,18 +117,18 @@ del time - topica_two + topica_two - topica_two + topica_two - topicb_two + topicb_two - topicb_two + topicb_two diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py index f5248050..150d033f 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/integration/test_process_api.py @@ -860,7 +860,7 @@ class TestProcessApi(BaseTest): assert response.status_code == 200 assert response.json is not None - assert response.json["ok"] + assert response.json["file_contents"] is not None response = client.get( f"/v1.0/process-models/{modified_process_model_id}/files/random_fact.svg", diff --git a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py index 3cc353b5..2ce011b2 100644 --- a/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py +++ b/spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_spec_file_service.py @@ -5,13 +5,15 @@ import pytest from flask import Flask from flask.testing import FlaskClient from flask_bpmn.models.db import db -from SpiffWorkflow.bpmn.parser.ValidationException import ValidationException # type: ignore from tests.spiffworkflow_backend.helpers.base_test import BaseTest from tests.spiffworkflow_backend.helpers.test_data import load_test_spec from spiffworkflow_backend.models.spec_reference import SpecReferenceCache from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.services.process_model_service import ProcessModelService +from spiffworkflow_backend.services.spec_file_service import ( + ProcessModelFileInvalidError, +) from spiffworkflow_backend.services.spec_file_service import SpecFileService @@ -74,7 +76,7 @@ class TestSpecFileService(BaseTest): bpmn_process_id_lookups[0].relative_path == self.call_activity_nested_relative_file_path ) - with pytest.raises(ValidationException) as exception: + with pytest.raises(ProcessModelFileInvalidError) as exception: load_test_spec( "call_activity_nested_duplicate", process_model_source_directory="call_activity_duplicate", @@ -85,6 +87,14 @@ class TestSpecFileService(BaseTest): in str(exception.value) ) + process_model = ProcessModelService.get_process_model( + "call_activity_nested_duplicate" + ) + full_file_path = SpecFileService.full_file_path( + process_model, "call_activity_nested_duplicate.bpmn" + ) + assert not os.path.isfile(full_file_path) + def test_updates_relative_file_path_when_appropriate( self, app: Flask, @@ -206,3 +216,23 @@ class TestSpecFileService(BaseTest): assert dmn1[0].display_name == "Decision 1" assert dmn1[0].identifier == "Decision_0vrtcmk" assert dmn1[0].type == "decision" + + def test_validate_bpmn_xml_with_invalid_xml( + self, + app: Flask, + client: FlaskClient, + with_db_and_bpmn_file_cleanup: None, + ) -> None: + """Test_validate_bpmn_xml_with_invalid_xml.""" + process_model = load_test_spec( + process_model_id="group/invalid_xml", + bpmn_file_name="script_error_with_task_data.bpmn", + process_model_source_directory="error", + ) + with pytest.raises(ProcessModelFileInvalidError): + SpecFileService.update_file( + process_model, "bad_xml.bpmn", b"THIS_IS_NOT_VALID_XML" + ) + + full_file_path = SpecFileService.full_file_path(process_model, "bad_xml.bpmn") + assert not os.path.isfile(full_file_path) diff --git a/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx b/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx index bd995bc3..b7debc6b 100644 --- a/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx +++ b/spiffworkflow-frontend/src/components/ProcessModelSearch.tsx @@ -37,7 +37,9 @@ export default function ProcessModelSearch({ const shouldFilterProcessModel = (options: any) => { const processModel: ProcessModel = options.item; const { inputValue } = options; - return getFullProcessModelLabel(processModel).includes(inputValue); + return getFullProcessModelLabel(processModel) + .toLowerCase() + .includes((inputValue || '').toLowerCase()); }; return ( ); diff --git a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx index a9b1224c..cdbe38a9 100644 --- a/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx +++ b/spiffworkflow-frontend/src/routes/ProcessModelShow.tsx @@ -374,6 +374,7 @@ export default function ProcessModelShow() { const doFileUpload = (event: any) => { event.preventDefault(); + setErrorObject(null); const url = `/process-models/${modifiedProcessModelId}/files`; const formData = new FormData(); formData.append('file', filesToUpload[0]); @@ -383,6 +384,7 @@ export default function ProcessModelShow() { successCallback: onUploadedCallback, httpMethod: 'POST', postBody: formData, + failureCallback: setErrorObject, }); setFilesToUpload(null); };