From e5971f11ff2e542d3ba251e31f326e247ec9c33c Mon Sep 17 00:00:00 2001 From: jbirddog Date: Fri, 24 Apr 2026 11:54:12 +0000 Subject: [PATCH 1/2] Optimize BPMN start message lookups Refactor ProcessParser.start_messages() to build a message-id lookup table once instead of rescanning all BPMN message nodes for each message start event. This preserves the existing behavior while reducing the lookup path from O(m*n) to O(m+n), where m is the number of messages and n is the number of message start events. Add a focused regression test that counts id lookups so the improvement is verified without relying on noisy timing assertions. --- SpiffWorkflow/bpmn/parser/ProcessParser.py | 7 ++-- tests/SpiffWorkflow/bpmn/ProcessParserTest.py | 34 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/SpiffWorkflow/bpmn/parser/ProcessParser.py b/SpiffWorkflow/bpmn/parser/ProcessParser.py index badbb070..5cbe97d9 100644 --- a/SpiffWorkflow/bpmn/parser/ProcessParser.py +++ b/SpiffWorkflow/bpmn/parser/ProcessParser.py @@ -67,7 +67,10 @@ def start_messages(self): """ This returns a list of message names that would cause this process to start. """ message_names = [] - messages = self.xpath("//bpmn:message") + messages_by_id = { + message.attrib.get('id'): message + for message in self.xpath("//bpmn:message") + } message_event_definitions = self.xpath( "//bpmn:startEvent/bpmn:messageEventDefinition") for message_event_definition in message_event_definitions: @@ -80,7 +83,7 @@ def start_messages(self): f"Could not find messageRef from message event definition: {med_id}" ) # Convert the id into a Message Name - message_name = next((m for m in messages if m.attrib.get('id') == message_model_identifier), None) + message_name = messages_by_id.get(message_model_identifier) message_names.append(message_name.attrib.get('name')) return message_names diff --git a/tests/SpiffWorkflow/bpmn/ProcessParserTest.py b/tests/SpiffWorkflow/bpmn/ProcessParserTest.py index a091559d..18421ebe 100644 --- a/tests/SpiffWorkflow/bpmn/ProcessParserTest.py +++ b/tests/SpiffWorkflow/bpmn/ProcessParserTest.py @@ -4,6 +4,7 @@ from SpiffWorkflow.dmn.parser.BpmnDmnParser import BpmnDmnParser from SpiffWorkflow.bpmn.parser.BpmnParser import BpmnParser +from SpiffWorkflow.bpmn.parser.ProcessParser import ProcessParser def _process_parser(bpmn_filename, process_id): parser = BpmnParser() @@ -12,6 +13,39 @@ def _process_parser(bpmn_filename, process_id): return parser.get_process_parser(process_id) class ProcessParserTest(unittest.TestCase): + def testStartMessagesAvoidsRepeatedMessageIdScans(self): + class CountingAttrib(dict): + id_gets = 0 + + def get(self, key, default=None): + if key == 'id': + CountingAttrib.id_gets += 1 + return super().get(key, default) + + class FakeNode: + def __init__(self, attrib): + self.attrib = attrib + + messages = [ + FakeNode(CountingAttrib(id=f'message_{idx}', name=f'Message {idx}')) + for idx in range(10) + ] + message_event_definitions = [ + FakeNode({'messageRef': f'message_{idx}'}) + for idx in range(10) + ] + parser = ProcessParser.__new__(ProcessParser) + parser.xpath = lambda expr: ( + message_event_definitions + if expr == "//bpmn:startEvent/bpmn:messageEventDefinition" + else messages + ) + + message_names = parser.start_messages() + + self.assertEqual([f'Message {idx}' for idx in range(10)], message_names) + self.assertLessEqual(CountingAttrib.id_gets, 10) + def testReturnsEmptyListIfNoCallActivities(self): parser = _process_parser("no-tasks.bpmn", "no_tasks") assert parser.called_element_ids() == [] From c4a2b425827387b5ce0ada4c0b5f8985d0f85463 Mon Sep 17 00:00:00 2001 From: jbirddog Date: Fri, 24 Apr 2026 14:45:53 +0000 Subject: [PATCH 2/2] Optimize BPMN parser XML lookups Collapse repeated BPMN parser XPath scans into document- and process-level indexes while preserving existing parser behavior. This adds one-time indexes for messages, signals, errors, escalations, correlations, outgoing flows, boundary events, task nodes, data references, and BPMN DI lane/position metadata, and avoids fallback root scans when indexed absence is already known. Focused BPMN parser tests pass, and the full suite passed in both serial and parallel runs (681 tests, 1 skipped). On a large production workflow set of about 1.4 MB of BPMN/DMN XML, specs_from_xml improved from roughly 1.0s before this indexing work to a 10-run median of 0.195s, with a 0.161s minimum and 0.208s mean. --- SpiffWorkflow/bpmn/parser/BpmnParser.py | 87 +++++++++++++++++++ SpiffWorkflow/bpmn/parser/ProcessParser.py | 46 +++++++++- SpiffWorkflow/bpmn/parser/TaskParser.py | 12 ++- SpiffWorkflow/bpmn/parser/event_parsers.py | 47 ++++------- SpiffWorkflow/bpmn/parser/node_parser.py | 27 ++++-- tests/SpiffWorkflow/bpmn/EventParserTest.py | 93 +++++++++++++++++++++ tests/SpiffWorkflow/bpmn/NodeParserTest.py | 80 ++++++++++++++++++ tests/SpiffWorkflow/bpmn/TaskParserTest.py | 66 +++++++++++++++ 8 files changed, 413 insertions(+), 45 deletions(-) create mode 100644 tests/SpiffWorkflow/bpmn/EventParserTest.py create mode 100644 tests/SpiffWorkflow/bpmn/NodeParserTest.py create mode 100644 tests/SpiffWorkflow/bpmn/TaskParserTest.py diff --git a/SpiffWorkflow/bpmn/parser/BpmnParser.py b/SpiffWorkflow/bpmn/parser/BpmnParser.py index 2f4911ed..7b269af8 100644 --- a/SpiffWorkflow/bpmn/parser/BpmnParser.py +++ b/SpiffWorkflow/bpmn/parser/BpmnParser.py @@ -47,6 +47,7 @@ ) from SpiffWorkflow.bpmn.specs.event_definitions.simple import NoneEventDefinition from SpiffWorkflow.bpmn.specs.event_definitions.timer import TimerEventDefinition +from SpiffWorkflow.bpmn.specs.event_definitions.message import CorrelationProperty from SpiffWorkflow.bpmn.specs.mixins.subworkflow_task import SubWorkflowTask as SubWorkflowTaskMixin from SpiffWorkflow.bpmn.specs.mixins.events.start_event import StartEvent as StartEventMixin @@ -161,8 +162,14 @@ def __init__(self, namespaces=None, validator=None, spec_descriptions=SPEC_DESCR self.collaborations = {} self.process_dependencies = set() self.messages = {} + self.signals = {} + self.errors = {} + self.escalations = {} self.correlations = {} + self.message_correlations = {} self.data_stores = {} + self.document_lanes = {} + self.document_positions = {} def _get_parser_class(self, tag): if tag in self.OVERRIDE_PARSER_CLASSES: @@ -231,10 +238,14 @@ def add_bpmn_xml(self, bpmn, filename=None): # the parser instances, which need to know about the data stores to # resolve data references. self._add_data_stores(bpmn) + self._add_diagram_indexes(bpmn) self._add_processes(bpmn, filename) self._add_collaborations(bpmn) self._add_messages(bpmn) + self._add_signals(bpmn) + self._add_errors(bpmn) + self._add_escalations(bpmn) self._add_correlations(bpmn) def _add_processes(self, bpmn, filename=None): @@ -242,6 +253,40 @@ def _add_processes(self, bpmn, filename=None): self._find_dependencies(process) self.create_parser(process, filename) + def _document_key(self, bpmn): + root = bpmn.getroot() if hasattr(bpmn, 'getroot') else bpmn + return id(root) + + def _add_diagram_indexes(self, bpmn): + document_key = self._document_key(bpmn) + if document_key in self.document_lanes: + return + + lanes = {} + for flow_node_ref in bpmn.xpath('.//bpmn:flowNodeRef', namespaces=self.namespaces): + flow_node_id = flow_node_ref.text + if flow_node_id is not None: + lanes[flow_node_id] = flow_node_ref.getparent().get('name') + + positions = {} + for shape in bpmn.xpath('.//bpmndi:BPMNShape[@bpmnElement]', namespaces=self.namespaces): + node_id = shape.get('bpmnElement') + bounds = first(shape.xpath('.//dc:Bounds', namespaces=self.namespaces)) + if node_id is not None and bounds is not None: + positions[node_id] = { + 'x': float(bounds.get('x', 0)), + 'y': float(bounds.get('y', 0)), + } + + self.document_lanes[document_key] = lanes + self.document_positions[document_key] = positions + + def get_document_lanes(self, root): + return self.document_lanes.get(id(root), {}) + + def get_document_positions(self, root): + return self.document_positions.get(id(root), {}) + def _add_collaborations(self, bpmn): collaboration = first(bpmn.xpath('.//bpmn:collaboration', namespaces=self.namespaces)) if collaboration is not None: @@ -257,7 +302,41 @@ def _add_messages(self, bpmn): ) self.messages[message.attrib.get("id")] = message.attrib.get("name") + def _add_signals(self, bpmn): + for signal in bpmn.xpath('.//bpmn:signal', namespaces=self.namespaces): + signal_identifier = signal.attrib.get("id") + if signal_identifier is None: + raise ValidationException("Signal identifier is missing from bpmn xml") + self.signals[signal_identifier] = signal.attrib.get("name") + + def _add_errors(self, bpmn): + for error in bpmn.xpath('.//bpmn:error', namespaces=self.namespaces): + error_identifier = error.attrib.get("id") + if error_identifier is None: + raise ValidationException("Error identifier is missing from bpmn xml") + self.errors[error_identifier] = { + "name": error.attrib.get("name"), + "error_code": error.attrib.get("errorCode"), + } + + def _add_escalations(self, bpmn): + for escalation in bpmn.xpath('.//bpmn:escalation', namespaces=self.namespaces): + escalation_identifier = escalation.attrib.get("id") + if escalation_identifier is None: + raise ValidationException("Escalation identifier is missing from bpmn xml") + self.escalations[escalation_identifier] = { + "name": escalation.attrib.get("name"), + "escalation_code": escalation.attrib.get("escalationCode"), + } + def _add_correlations(self, bpmn): + property_id_to_used_by = {} + for correlation_key in bpmn.xpath('.//bpmn:correlationKey', namespaces=self.namespaces): + used_by = correlation_key.attrib.get("name") + for property_ref in correlation_key.xpath('./bpmn:correlationPropertyRef', namespaces=self.namespaces): + if property_ref.text is not None: + property_id_to_used_by.setdefault(property_ref.text, []).append(used_by) + for correlation in bpmn.xpath('.//bpmn:correlationProperty', namespaces=self.namespaces): correlation_identifier = correlation.attrib.get("id") if correlation_identifier is None: @@ -279,6 +358,14 @@ def _add_correlations(self, bpmn): expression = children[0].text if len(children) > 0 else None retrieval_expressions.append({"messageRef": message_model_identifier, "expression": expression}) + if expression is not None: + self.message_correlations.setdefault(message_model_identifier, []).append( + CorrelationProperty( + correlation_identifier, + expression, + property_id_to_used_by.get(correlation_identifier, []), + ) + ) self.correlations[correlation_identifier] = { "name": correlation.attrib.get("name"), "retrieval_expressions": retrieval_expressions diff --git a/SpiffWorkflow/bpmn/parser/ProcessParser.py b/SpiffWorkflow/bpmn/parser/ProcessParser.py index 5cbe97d9..b04feb84 100644 --- a/SpiffWorkflow/bpmn/parser/ProcessParser.py +++ b/SpiffWorkflow/bpmn/parser/ProcessParser.py @@ -41,13 +41,22 @@ def __init__(self, p, node, nsmap, data_stores, filename=None, lane=None): :param filename: the source BPMN filename (optional) :param lane: the lane of a subprocess (optional) """ - super().__init__(node, nsmap, filename=filename, lane=lane) self.parser = p + super().__init__(node, nsmap, filename=filename, lane=lane) self.lane = lane self.spec = None self.process_executable = node.get('isExecutable', 'true') == 'true' self.data_stores = data_stores self.parent = None + self.document_root = node.getroottree().getroot() + self.nodes_by_id = {} + self.outgoing_sequence_flows = {} + self.boundary_events_by_attached = {} + self.data_object_references = {} + self.data_store_references = {} + self.positions_by_node_id = self.parser.get_document_positions(self.document_root) + self.lanes_by_node_id = self.parser.get_document_lanes(self.document_root) + self._index_process_nodes() def get_name(self): """ @@ -55,6 +64,41 @@ def get_name(self): """ return self.node.get('name', default=self.bpmn_id) + def _index_process_nodes(self): + for node in self.xpath('.//*[@id]'): + node_id = node.get('id') + if node_id is not None: + self.nodes_by_id[node_id] = node + if node.tag.endswith('sequenceFlow'): + self.outgoing_sequence_flows.setdefault(node.get('sourceRef'), []).append(node) + elif node.tag.endswith('boundaryEvent'): + self.boundary_events_by_attached.setdefault(node.get('attachedToRef'), []).append(node) + elif node.tag.endswith('dataObjectReference'): + self.data_object_references[node_id] = node + elif node.tag.endswith('dataStoreReference'): + self.data_store_references[node_id] = node + + def get_boundary_events(self, attached_to_ref): + return self.boundary_events_by_attached.get(attached_to_ref, []) + + def get_outgoing_sequence_flows(self, source_ref): + return self.outgoing_sequence_flows.get(source_ref, []) + + def get_node_by_id(self, node_id): + return self.nodes_by_id.get(node_id) + + def get_data_object_reference(self, reference_id): + return self.data_object_references.get(reference_id) + + def get_data_store_reference(self, reference_id): + return self.data_store_references.get(reference_id) + + def get_position(self, node_id): + return self.positions_by_node_id.get(node_id, {'x': 0.0, 'y': 0.0}) + + def get_lane_name(self, node_id): + return self.lanes_by_node_id.get(node_id) + def has_lanes(self) -> bool: """Returns true if this process has one or more named lanes """ elements = self.xpath("//bpmn:lane") diff --git a/SpiffWorkflow/bpmn/parser/TaskParser.py b/SpiffWorkflow/bpmn/parser/TaskParser.py index 1edd9d67..0f96649e 100644 --- a/SpiffWorkflow/bpmn/parser/TaskParser.py +++ b/SpiffWorkflow/bpmn/parser/TaskParser.py @@ -59,9 +59,9 @@ def __init__(self, process_parser, spec_class, node, nsmap=None, lane=None): extending the TaskParser. :param node: the XML node for this task """ - super().__init__(node, nsmap, filename=process_parser.filename, lane=lane) self.process_parser = process_parser self.spec_class = spec_class + super().__init__(node, nsmap, filename=process_parser.filename, lane=lane) self.spec = self.process_parser.spec def _copy_task_attrs(self, original, loop_characteristics=None): @@ -199,19 +199,18 @@ def parse_node(self): if len(mi_loop_characteristics) > 0: self._add_multiinstance_task(mi_loop_characteristics[0]) - boundary_event_nodes = self.doc_xpath('.//bpmn:boundaryEvent[@attachedToRef="%s"]' % self.bpmn_id) + boundary_event_nodes = self.process_parser.get_boundary_events(self.bpmn_id) if boundary_event_nodes: parent = self._add_boundary_event(boundary_event_nodes) children = [] - outgoing = self.doc_xpath('.//bpmn:sequenceFlow[@sourceRef="%s"]' % self.bpmn_id) + outgoing = self.process_parser.get_outgoing_sequence_flows(self.bpmn_id) if len(outgoing) > 1 and not self.handles_multiple_outgoing(): self.raise_validation_exception('Multiple outgoing flows are not supported for tasks of type') for sequence_flow in outgoing: target_ref = sequence_flow.get('targetRef') - try: - target_node = one(self.doc_xpath('.//bpmn:*[@id="%s"]'% target_ref)) - except Exception: + target_node = self.process_parser.get_node_by_id(target_ref) + if target_node is None: self.raise_validation_exception('When looking for a task spec, we found two items, ' 'perhaps a form has the same ID? (%s)' % target_ref) @@ -268,4 +267,3 @@ def handles_multiple_outgoing(self): outgoing sequence flows. """ return False - diff --git a/SpiffWorkflow/bpmn/parser/event_parsers.py b/SpiffWorkflow/bpmn/parser/event_parsers.py index 0f2f197d..6db5ebe2 100644 --- a/SpiffWorkflow/bpmn/parser/event_parsers.py +++ b/SpiffWorkflow/bpmn/parser/event_parsers.py @@ -21,7 +21,7 @@ from .ValidationException import ValidationException from .TaskParser import TaskParser -from .util import first, one +from .util import first from SpiffWorkflow.bpmn.specs.event_definitions.simple import ( NoneEventDefinition, @@ -38,10 +38,7 @@ ErrorEventDefinition, EscalationEventDefinition ) -from SpiffWorkflow.bpmn.specs.event_definitions.message import ( - MessageEventDefinition, - CorrelationProperty -) +from SpiffWorkflow.bpmn.specs.event_definitions.message import MessageEventDefinition from SpiffWorkflow.bpmn.specs.event_definitions.multiple import MultipleEventDefinition from SpiffWorkflow.bpmn.specs.event_definitions.conditional import ConditionalEventDefinition @@ -89,11 +86,10 @@ def parse_error_event(self, error_event): """Parse the errorEventDefinition node and return an instance of ErrorEventDefinition.""" error_ref = error_event.get('errorRef') if error_ref: - try: - error = one(self.doc_xpath('.//bpmn:error[@id="%s"]' % error_ref)) - except Exception: + error = self.process_parser.parser.errors.get(error_ref) + if error is None: self.raise_validation_exception('Expected an error node', node=error_event) - error_code = error.get('errorCode') + error_code = error.get('error_code') name = error.get('name') else: name, error_code = 'None Error Event', None @@ -104,11 +100,10 @@ def parse_escalation_event(self, escalation_event): escalation_ref = escalation_event.get('escalationRef') if escalation_ref: - try: - escalation = one(self.doc_xpath('.//bpmn:escalation[@id="%s"]' % escalation_ref)) - except Exception: + escalation = self.process_parser.parser.escalations.get(escalation_ref) + if escalation is None: self.raise_validation_exception('Expected an Escalation node', node=escalation_event) - escalation_code = escalation.get('escalationCode') + escalation_code = escalation.get('escalation_code') name = escalation.get('name') else: name, escalation_code = 'None Escalation Event', None @@ -118,11 +113,10 @@ def parse_message_event(self, message_event): message_ref = message_event.get('messageRef') if message_ref is not None: - try: - message = one(self.doc_xpath('.//bpmn:message[@id="%s"]' % message_ref)) - except Exception: + message = self.process_parser.parser.messages.get(message_ref) + if message is None: self.raise_validation_exception('Expected a Message node', node=message_event) - name = message.get('name') + name = message description = self.get_event_description(message_event) correlations = self.get_message_correlations(message_ref) else: @@ -136,11 +130,10 @@ def parse_signal_event(self, signal_event): signal_ref = signal_event.get('signalRef') if signal_ref: - try: - signal = one(self.doc_xpath('.//bpmn:signal[@id="%s"]' % signal_ref)) - except Exception: + signal = self.process_parser.parser.signals.get(signal_ref) + if signal is None: self.raise_validation_exception('Expected a Signal node', node=signal_event) - name = signal.get('name') + name = signal else: name = signal_event.getparent().get('name') return SignalEventDefinition(name, description=self.get_event_description(signal_event)) @@ -168,17 +161,7 @@ def parse_timer_event(self, event): raise ValidationException("Time Specification Error. " + str(e), node=self.node, file_name=self.filename) def get_message_correlations(self, message_ref): - - correlations = [] - for correlation in self.doc_xpath(f".//bpmn:correlationPropertyRetrievalExpression[@messageRef='{message_ref}']"): - key = correlation.getparent().get('id') - children = correlation.getchildren() - expression = children[0].text if len(children) > 0 else None - used_by = [ e.getparent().get('name') for e in - self.doc_xpath(f".//bpmn:correlationKey/bpmn:correlationPropertyRef[text()='{key}']") ] - if key is not None and expression is not None: - correlations.append(CorrelationProperty(key, expression, used_by)) - return correlations + return self.process_parser.parser.message_correlations.get(message_ref, []) def _create_task(self, event_definition, cancel_activity=None, parallel=None): diff --git a/SpiffWorkflow/bpmn/parser/node_parser.py b/SpiffWorkflow/bpmn/parser/node_parser.py index 98b12b62..5885ae18 100644 --- a/SpiffWorkflow/bpmn/parser/node_parser.py +++ b/SpiffWorkflow/bpmn/parser/node_parser.py @@ -36,7 +36,9 @@ def __init__(self, node, nsmap=None, filename=None, lane=None): self.node = node self.nsmap = nsmap or DEFAULT_NSMAP self.filename = filename - self.lane = self._get_lane() or lane + self.lane = lane + if self.lane is None and hasattr(self, 'process_parser'): + self.lane = self._get_lane() @property def bpmn_id(self): @@ -81,12 +83,16 @@ def parse_documentation(self, sequence_flow=None): def parse_incoming_data_references(self): specs = [] for name in self.xpath('./bpmn:dataInputAssociation/bpmn:sourceRef'): - ref = first(self.doc_xpath(f".//bpmn:dataObjectReference[@id='{name.text}']")) + ref = self.process_parser.get_data_object_reference(name.text) + if ref is None: + ref = first(self.doc_xpath(f".//bpmn:dataObjectReference[@id='{name.text}']")) data_obj = self._resolve_data_object_ref(ref) if data_obj is not None: specs.append(data_obj) else: - ref = first(self.doc_xpath(f".//bpmn:dataStoreReference[@id='{name.text}']")) + ref = self.process_parser.get_data_store_reference(name.text) + if ref is None: + ref = first(self.doc_xpath(f".//bpmn:dataStoreReference[@id='{name.text}']")) if ref is not None and ref.get('dataStoreRef') in self.process_parser.data_stores: specs.append(self.process_parser.data_stores[ref.get('dataStoreRef')]) else: @@ -96,12 +102,16 @@ def parse_incoming_data_references(self): def parse_outgoing_data_references(self): specs = [] for name in self.xpath('./bpmn:dataOutputAssociation/bpmn:targetRef'): - ref = first(self.doc_xpath(f".//bpmn:dataObjectReference[@id='{name.text}']")) + ref = self.process_parser.get_data_object_reference(name.text) + if ref is None: + ref = first(self.doc_xpath(f".//bpmn:dataObjectReference[@id='{name.text}']")) data_obj = self._resolve_data_object_ref(ref) if data_obj is not None: specs.append(data_obj) else: - ref = first(self.doc_xpath(f".//bpmn:dataStoreReference[@id='{name.text}']")) + ref = self.process_parser.get_data_store_reference(name.text) + if ref is None: + ref = first(self.doc_xpath(f".//bpmn:dataStoreReference[@id='{name.text}']")) if ref is not None and ref.get('dataStoreRef') in self.process_parser.data_stores: specs.append(self.process_parser.data_stores[ref.get('dataStoreRef')]) else: @@ -146,12 +156,19 @@ def get_position(self, node=None): node = node if node is not None else self.node nodeid = node.get('id') if nodeid is not None: + position = self.process_parser.get_position(nodeid) + if position != {'x': 0.0, 'y': 0.0} or hasattr(self.process_parser, 'positions_by_node_id'): + return position bounds = first(self.doc_xpath(f".//bpmndi:BPMNShape[@bpmnElement='{nodeid}']//dc:Bounds")) if bounds is not None: return {'x': float(bounds.get('x', 0)), 'y': float(bounds.get('y', 0))} return {'x': 0.0, 'y': 0.0} def _get_lane(self): + if hasattr(self, 'process_parser'): + lane_name = self.process_parser.get_lane_name(self.bpmn_id) + if lane_name is not None or hasattr(self.process_parser, 'lanes_by_node_id'): + return lane_name noderef = first(self.doc_xpath(f".//bpmn:flowNodeRef[text()='{self.bpmn_id}']")) if noderef is not None: return noderef.getparent().get('name') diff --git a/tests/SpiffWorkflow/bpmn/EventParserTest.py b/tests/SpiffWorkflow/bpmn/EventParserTest.py new file mode 100644 index 00000000..a16b1ef5 --- /dev/null +++ b/tests/SpiffWorkflow/bpmn/EventParserTest.py @@ -0,0 +1,93 @@ +import unittest +from types import SimpleNamespace + +from SpiffWorkflow.bpmn.parser.event_parsers import EventDefinitionParser +from SpiffWorkflow.bpmn.specs.event_definitions.message import CorrelationProperty + + +class FakeNode: + def __init__(self, attrib=None, text=None, parent=None, children=None): + self.attrib = attrib or {} + self.text = text + self._parent = parent + self._children = children or [] + + def get(self, key, default=None): + return self.attrib.get(key, default) + + def getparent(self): + return self._parent + + def getchildren(self): + return list(self._children) + + +class EventParserTest(unittest.TestCase): + def _parser(self): + parser = EventDefinitionParser.__new__(EventDefinitionParser) + parser.filename = "test.bpmn" + parser.node = FakeNode({"id": "Task_1", "name": "Task"}) + parser.process_parser = SimpleNamespace( + parser=SimpleNamespace( + messages={"message_1": "Message 1"}, + message_correlations={ + "message_1": [CorrelationProperty("prop_1", "payload.value", ["Conversation 1"])] + }, + signals={"signal_1": "Signal 1"}, + errors={"error_1": {"name": "Error 1", "error_code": "E-1"}}, + escalations={"escalation_1": {"name": "Escalation 1", "escalation_code": "ESC-1"}}, + ) + ) + parser.get_event_description = lambda event: "Event Description" + parser.raise_validation_exception = lambda message, node=None: (_ for _ in ()).throw(AssertionError(message)) + parser.doc_xpath_count = 0 + + def doc_xpath(path): + parser.doc_xpath_count += 1 + if path == './/bpmn:message[@id="message_1"]': + return [FakeNode({"id": "message_1", "name": "Message 1"})] + if path == ".//bpmn:correlationPropertyRetrievalExpression[@messageRef='message_1']": + correlation_key = FakeNode({"id": "prop_1", "name": "Conversation 1"}) + return [ + FakeNode( + {"messageRef": "message_1"}, + parent=correlation_key, + children=[FakeNode(text="payload.value")], + ) + ] + if path == ".//bpmn:correlationKey/bpmn:correlationPropertyRef[text()='prop_1']": + return [FakeNode(text="prop_1", parent=FakeNode({"name": "Conversation 1"}))] + if path == './/bpmn:signal[@id="signal_1"]': + return [FakeNode({"id": "signal_1", "name": "Signal 1"})] + if path == './/bpmn:error[@id="error_1"]': + return [FakeNode({"id": "error_1", "name": "Error 1", "errorCode": "E-1"})] + if path == './/bpmn:escalation[@id="escalation_1"]': + return [FakeNode({"id": "escalation_1", "name": "Escalation 1", "escalationCode": "ESC-1"})] + return [] + + parser.doc_xpath = doc_xpath + return parser + + def testIndexedMessageDefinitionsAvoidDocumentXPathLookups(self): + parser = self._parser() + + message_event = FakeNode({"messageRef": "message_1"}, parent=FakeNode({"name": "Parent Event"})) + event_definition = parser.parse_message_event(message_event) + + self.assertEqual("Message 1", event_definition.name) + self.assertEqual(["Conversation 1"], event_definition.correlation_properties[0].correlation_keys) + self.assertEqual(0, parser.doc_xpath_count) + + def testIndexedSignalErrorAndEscalationAvoidDocumentXPathLookups(self): + parser = self._parser() + + signal = parser.parse_signal_event(FakeNode({"signalRef": "signal_1"}, parent=FakeNode({"name": "Parent Event"}))) + error = parser.parse_error_event(FakeNode({"errorRef": "error_1"})) + escalation = parser.parse_escalation_event(FakeNode({"escalationRef": "escalation_1"})) + + self.assertEqual("Signal 1", signal.name) + self.assertEqual("Error 1", error.name) + self.assertEqual("E-1", error.code) + self.assertEqual("Escalation 1", escalation.name) + self.assertEqual("ESC-1", escalation.code) + self.assertEqual(0, parser.doc_xpath_count) diff --git a/tests/SpiffWorkflow/bpmn/NodeParserTest.py b/tests/SpiffWorkflow/bpmn/NodeParserTest.py new file mode 100644 index 00000000..460ddc7d --- /dev/null +++ b/tests/SpiffWorkflow/bpmn/NodeParserTest.py @@ -0,0 +1,80 @@ +import unittest +from types import SimpleNamespace + +from SpiffWorkflow.bpmn.parser.node_parser import NodeParser + + +class FakeNode: + def __init__(self, attrib=None, text=None): + self.attrib = attrib or {} + self.text = text + + def get(self, key, default=None): + return self.attrib.get(key, default) + + +class NodeParserTest(unittest.TestCase): + def testConstructorSkipsLaneLookupWithoutProcessParser(self): + class TrackingNodeParser(NodeParser): + def __init__(self, *args, **kwargs): + self.get_lane_calls = 0 + super().__init__(*args, **kwargs) + + def _get_lane(self): + self.get_lane_calls += 1 + return "Lane A" + + parser = TrackingNodeParser(FakeNode({"id": "Task_1"})) + + self.assertIsNone(parser.lane) + self.assertEqual(0, parser.get_lane_calls) + + def testIndexedDataReferencesAndPositionsAvoidDocumentXPathLookups(self): + parser = NodeParser.__new__(NodeParser) + parser.node = FakeNode({"id": "Task_1"}) + parser.filename = "test.bpmn" + parser.process_parser = SimpleNamespace( + spec=SimpleNamespace(data_objects={"Data_1": "data-object"}), + parent=None, + data_stores={}, + get_data_object_reference=lambda reference_id: FakeNode({"dataObjectRef": "Data_1"}) if reference_id == "Ref_1" else None, + get_data_store_reference=lambda reference_id: None, + get_position=lambda node_id: {"x": 10.0, "y": 20.0} if node_id == "Task_1" else {"x": 0.0, "y": 0.0}, + get_lane_name=lambda node_id: "Lane A" if node_id == "Task_1" else None, + ) + parser.xpath = lambda path: [FakeNode(text="Ref_1")] + parser.doc_xpath_count = 0 + + def doc_xpath(path): + parser.doc_xpath_count += 1 + return [] + + parser.doc_xpath = doc_xpath + + self.assertEqual(["data-object"], parser.parse_incoming_data_references()) + self.assertEqual(["data-object"], parser.parse_outgoing_data_references()) + self.assertEqual({"x": 10.0, "y": 20.0}, parser.get_position()) + self.assertEqual("Lane A", parser._get_lane()) + self.assertEqual(0, parser.doc_xpath_count) + + def testIndexedEmptyLaneAndPositionAvoidFallbackDocumentXPathLookups(self): + parser = NodeParser.__new__(NodeParser) + parser.node = FakeNode({"id": "Task_1"}) + parser.filename = "test.bpmn" + parser.process_parser = SimpleNamespace( + positions_by_node_id={}, + lanes_by_node_id={}, + get_position=lambda node_id: {"x": 0.0, "y": 0.0}, + get_lane_name=lambda node_id: None, + ) + parser.doc_xpath_count = 0 + + def doc_xpath(path): + parser.doc_xpath_count += 1 + return [] + + parser.doc_xpath = doc_xpath + + self.assertEqual({"x": 0.0, "y": 0.0}, parser.get_position()) + self.assertIsNone(parser._get_lane()) + self.assertEqual(0, parser.doc_xpath_count) diff --git a/tests/SpiffWorkflow/bpmn/TaskParserTest.py b/tests/SpiffWorkflow/bpmn/TaskParserTest.py new file mode 100644 index 00000000..70b37c23 --- /dev/null +++ b/tests/SpiffWorkflow/bpmn/TaskParserTest.py @@ -0,0 +1,66 @@ +import unittest +from types import SimpleNamespace + +from lxml import etree + +from SpiffWorkflow.bpmn.parser.TaskParser import TaskParser + + +class FakeTask: + def __init__(self, name="task"): + self.name = name + self.extensions = {} + + def connect(self, other): + return None + + +class IndexedTaskParser(TaskParser): + def create_task(self): + return FakeTask(self.bpmn_id) + + def connect_outgoing(self, outgoing_task, sequence_flow_node, is_default): + return None + + def get_position(self, node=None): + return {"x": 0.0, "y": 0.0} + + +class TaskParserTest(unittest.TestCase): + def testParseNodeAvoidsDocumentXPathForOutgoingLookupsWhenIndexed(self): + process = etree.fromstring( + """ + + + + + + """ + ) + task_node = process.xpath('./bpmn:task', namespaces={'bpmn': 'http://www.omg.org/spec/BPMN/20100524/MODEL'})[0] + target_node = process.xpath('./bpmn:task', namespaces={'bpmn': 'http://www.omg.org/spec/BPMN/20100524/MODEL'})[1] + sequence_flow = process.xpath('./bpmn:sequenceFlow', namespaces={'bpmn': 'http://www.omg.org/spec/BPMN/20100524/MODEL'})[0] + + process_parser = SimpleNamespace( + spec=SimpleNamespace(task_specs={}), + filename="test.bpmn", + parser=SimpleNamespace(spec_descriptions={}), + parse_node=lambda node: FakeTask(node.get("id")), + get_lane_name=lambda node_id: None, + get_boundary_events=lambda attached_to_ref: [], + get_outgoing_sequence_flows=lambda source_ref: [sequence_flow], + get_node_by_id=lambda node_id: target_node if node_id == "Task_2" else None, + ) + + parser = IndexedTaskParser(process_parser, FakeTask, task_node) + parser.doc_xpath_count = 0 + original_doc_xpath = parser.doc_xpath + + def counting_doc_xpath(path): + parser.doc_xpath_count += 1 + return original_doc_xpath(path) + + parser.doc_xpath = counting_doc_xpath + parser.parse_node() + + self.assertEqual(0, parser.doc_xpath_count)