Skip to content

Commit bca82ce

Browse files
essweineburnettk
andauthored
Bugfix/ensure boundary events checked before workflow proceeds (sartography#2668)
This checks workflow branches that potentially contain boundary events and prioritizes completing the events before proceeding with other tasks. It also gives precedence to error and escalation events that handle a matching code rather than the default action. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Ready tasks now favor boundary events that carry code definitions; when such code-boundary events are present they take precedence, non-code ready tasks are canceled, with sensible fallbacks to non-code boundary events or original selection if none exist. * **Tests** * Added a unit test validating boundary-event prioritization and escalation behavior during process execution. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: burnettk <burnettk@users.noreply.github.com>
1 parent 950290c commit bca82ce

File tree

4 files changed

+277
-5
lines changed

4 files changed

+277
-5
lines changed

spiffworkflow-backend/src/spiffworkflow_backend/services/process_model_test_runner_service.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
from RestrictedPython import safe_globals # type: ignore
2121
from SpiffWorkflow.bpmn.exceptions import WorkflowTaskException # type: ignore
2222
from SpiffWorkflow.bpmn.script_engine import PythonScriptEngine # type: ignore
23+
from SpiffWorkflow.bpmn.specs.defaults import BoundaryEvent # type: ignore
24+
from SpiffWorkflow.bpmn.specs.event_definitions.item_aware_event import CodeEventDefinition # type: ignore
2325
from SpiffWorkflow.bpmn.workflow import BpmnWorkflow # type: ignore
2426
from SpiffWorkflow.task import Task as SpiffTask # type: ignore
2527
from SpiffWorkflow.util.deep_merge import DeepMerge # type: ignore
@@ -274,9 +276,27 @@ def execute_task(self, spiff_task: SpiffTask, task_data_for_submit: dict | None
274276

275277
def get_next_task(self, bpmn_process_instance: BpmnWorkflow) -> SpiffTask | None:
276278
ready_tasks = list(bpmn_process_instance.get_tasks(state=TaskState.READY))
277-
if len(ready_tasks) > 0:
279+
code, no_code = [], []
280+
for task in ready_tasks:
281+
if isinstance(task.task_spec, BoundaryEvent):
282+
event_definition = task.task_spec.event_definition
283+
if isinstance(event_definition, CodeEventDefinition) and event_definition.code is not None:
284+
code.append(task)
285+
else:
286+
no_code.append(task)
287+
288+
# This ensures that boundary events take priority, and events that have codes assigned
289+
# take precedence over default actions
290+
if len(code) > 0:
291+
for task in no_code:
292+
task.cancel()
293+
return code[0]
294+
elif len(no_code) > 0:
295+
return no_code[0]
296+
elif len(ready_tasks) > 0:
278297
return ready_tasks[0]
279-
return None
298+
else:
299+
return None
280300

281301
def _add_bpmn_file_to_parser(self, parser: MyCustomParser, bpmn_file: str) -> None:
282302
related_file_etree = self._get_etree_from_bpmn_file(bpmn_file)

spiffworkflow-backend/src/spiffworkflow_backend/services/workflow_execution_service.py

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@
2222
from flask import g
2323
from SpiffWorkflow.bpmn.exceptions import WorkflowTaskException # type: ignore
2424
from SpiffWorkflow.bpmn.serializer.workflow import BpmnWorkflowSerializer # type: ignore
25-
from SpiffWorkflow.bpmn.specs.control import UnstructuredJoin # type: ignore
25+
from SpiffWorkflow.bpmn.specs.control import BoundaryEventJoin # type: ignore
26+
from SpiffWorkflow.bpmn.specs.control import BoundaryEventSplit
27+
from SpiffWorkflow.bpmn.specs.control import UnstructuredJoin
28+
from SpiffWorkflow.bpmn.specs.event_definitions.item_aware_event import CodeEventDefinition # type: ignore
2629
from SpiffWorkflow.bpmn.specs.event_definitions.message import MessageEventDefinition # type: ignore
2730
from SpiffWorkflow.bpmn.specs.mixins import SubWorkflowTaskMixin # type: ignore
2831
from SpiffWorkflow.bpmn.specs.mixins.events.event_types import CatchingEvent # type: ignore
32+
from SpiffWorkflow.bpmn.specs.mixins.events.intermediate_event import BoundaryEvent # type: ignore
2933
from SpiffWorkflow.bpmn.workflow import BpmnWorkflow # type: ignore
3034
from SpiffWorkflow.exceptions import SpiffWorkflowException # type: ignore
3135
from SpiffWorkflow.task import Task as SpiffTask # type: ignore
@@ -224,14 +228,29 @@ def add_object_to_db_session(self, bpmn_process_instance: BpmnWorkflow) -> None:
224228

225229
def get_ready_engine_steps(self, bpmn_process_instance: BpmnWorkflow) -> list[SpiffTask]:
226230
task_filter = TaskFilter(state=TaskState.READY, manual=False)
227-
228231
steps = list(
229232
bpmn_process_instance.get_tasks(
230233
first_task=self.delegate.last_completed_spiff_task(),
231234
task_filter=task_filter,
232235
)
233236
)
234237

238+
# This ensures that escalations and errors that have code specified take precedence over those that do not
239+
code, no_code = [], []
240+
for task in steps:
241+
spec = task.task_spec
242+
if not isinstance(spec, BoundaryEvent) or not isinstance(spec.event_definition, CodeEventDefinition):
243+
continue
244+
if spec.event_definition.code is None:
245+
no_code.append(task)
246+
else:
247+
code.append(task)
248+
249+
if len(code) > 0 and len(no_code) > 0:
250+
for task in no_code:
251+
steps.remove(task)
252+
task.cancel()
253+
235254
if not steps:
236255
steps = list(
237256
bpmn_process_instance.get_tasks(
@@ -392,7 +411,20 @@ def did_complete_task(self, spiff_task: SpiffTask) -> None:
392411

393412
LoggingService.log_event(ProcessInstanceEventType.task_completed.value, log_extras)
394413
self.process_instance.task_updated_at_in_seconds = round(time.time())
395-
self._last_completed_spiff_task = spiff_task
414+
# This ensures boundary events attached to a task are prioritized
415+
# The default is to continue along a branch as long as possible, but this can allow subprocesses to
416+
# complete before any boundary events can cancel it.
417+
# Once a boundary event split is reached, don't update the search start until the corresponding join is reached.
418+
if self._last_completed_spiff_task is None or not isinstance(
419+
self._last_completed_spiff_task.task_spec, BoundaryEventSplit
420+
):
421+
self._last_completed_spiff_task = spiff_task
422+
elif (
423+
isinstance(spiff_task.task_spec, BoundaryEventJoin)
424+
and spiff_task.task_spec.split_task == self._last_completed_spiff_task.task_spec.name
425+
):
426+
self._last_completed_spiff_task = spiff_task
427+
396428
if self.secondary_engine_step_delegate:
397429
self.secondary_engine_step_delegate.did_complete_task(spiff_task)
398430

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:spiffworkflow="http://spiffworkflow.org/bpmn/schema/1.0/core" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" id="Definitions_96f6665" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="3.0.0-dev">
3+
<bpmn:process id="test-escalation-priority" isExecutable="true">
4+
<bpmn:startEvent id="StartEvent_1">
5+
<bpmn:outgoing>Flow_0zbf5g9</bpmn:outgoing>
6+
</bpmn:startEvent>
7+
<bpmn:endEvent id="Event_0ulnllb">
8+
<bpmn:incoming>Flow_0dxz86r</bpmn:incoming>
9+
</bpmn:endEvent>
10+
<bpmn:subProcess id="Activity_0f4y776" name="TC6: No Hierarchy, select most specific">
11+
<bpmn:extensionElements>
12+
<spiffworkflow:instructionsForEndUser />
13+
</bpmn:extensionElements>
14+
<bpmn:incoming>Flow_0zbf5g9</bpmn:incoming>
15+
<bpmn:outgoing>Flow_0dxz86r</bpmn:outgoing>
16+
<bpmn:startEvent id="Event_0bmiwfz">
17+
<bpmn:outgoing>Flow_0rhvc3p</bpmn:outgoing>
18+
</bpmn:startEvent>
19+
<bpmn:endEvent id="Event_0k6ki2k" name="Failed">
20+
<bpmn:incoming>Flow_1g0anfm</bpmn:incoming>
21+
</bpmn:endEvent>
22+
<bpmn:scriptTask id="Activity_10y5vuu" name="Test Case Success">
23+
<bpmn:extensionElements>
24+
<spiffworkflow:instructionsForEndUser />
25+
</bpmn:extensionElements>
26+
<bpmn:incoming>Flow_1t6g80d</bpmn:incoming>
27+
<bpmn:outgoing>Flow_1r51h6b</bpmn:outgoing>
28+
<bpmn:script>testOk = True</bpmn:script>
29+
</bpmn:scriptTask>
30+
<bpmn:scriptTask id="Activity_18i1k4f" name="Test Case Failed">
31+
<bpmn:extensionElements>
32+
<spiffworkflow:instructionsForEndUser />
33+
</bpmn:extensionElements>
34+
<bpmn:incoming>Flow_0c2dcn9</bpmn:incoming>
35+
<bpmn:outgoing>Flow_1g0anfm</bpmn:outgoing>
36+
<bpmn:script>testOk = False</bpmn:script>
37+
</bpmn:scriptTask>
38+
<bpmn:endEvent id="Event_0j0flby" name="Success">
39+
<bpmn:incoming>Flow_1r51h6b</bpmn:incoming>
40+
</bpmn:endEvent>
41+
<bpmn:subProcess id="Activity_0i38awy">
42+
<bpmn:incoming>Flow_0rhvc3p</bpmn:incoming>
43+
<bpmn:outgoing>Flow_1hl5f8i</bpmn:outgoing>
44+
<bpmn:startEvent id="Event_0nl85sx">
45+
<bpmn:outgoing>Flow_1kbqgxt</bpmn:outgoing>
46+
</bpmn:startEvent>
47+
<bpmn:endEvent id="Event_0eh8u5t" name="Esc1">
48+
<bpmn:incoming>Flow_1kbqgxt</bpmn:incoming>
49+
<bpmn:escalationEventDefinition id="EscalationEventDefinition_0eclyn7" escalationRef="Escalation1" />
50+
</bpmn:endEvent>
51+
<bpmn:sequenceFlow id="Flow_1kbqgxt" sourceRef="Event_0nl85sx" targetRef="Event_0eh8u5t" />
52+
</bpmn:subProcess>
53+
<bpmn:boundaryEvent id="Event_0wpdw73" name="Esc1" attachedToRef="Activity_0i38awy">
54+
<bpmn:outgoing>Flow_1t6g80d</bpmn:outgoing>
55+
<bpmn:escalationEventDefinition id="EscalationEventDefinition_1wr8072" escalationRef="Escalation1" />
56+
</bpmn:boundaryEvent>
57+
<bpmn:sequenceFlow id="Flow_0rhvc3p" sourceRef="Event_0bmiwfz" targetRef="Activity_0i38awy" />
58+
<bpmn:sequenceFlow id="Flow_1g0anfm" sourceRef="Activity_18i1k4f" targetRef="Event_0k6ki2k" />
59+
<bpmn:sequenceFlow id="Flow_1t6g80d" sourceRef="Event_0wpdw73" targetRef="Activity_10y5vuu" />
60+
<bpmn:sequenceFlow id="Flow_1r51h6b" sourceRef="Activity_10y5vuu" targetRef="Event_0j0flby" />
61+
<bpmn:sequenceFlow id="Flow_1hl5f8i" sourceRef="Activity_0i38awy" targetRef="Gateway_0cl3uhr" />
62+
<bpmn:boundaryEvent id="Event_0rg9g4i" name="Esc2" attachedToRef="Activity_0i38awy">
63+
<bpmn:outgoing>Flow_0mfryhh</bpmn:outgoing>
64+
<bpmn:escalationEventDefinition id="EscalationEventDefinition_1l2ok01" escalationRef="Escalation2" />
65+
</bpmn:boundaryEvent>
66+
<bpmn:exclusiveGateway id="Gateway_0cl3uhr">
67+
<bpmn:incoming>Flow_1hl5f8i</bpmn:incoming>
68+
<bpmn:incoming>Flow_0mfryhh</bpmn:incoming>
69+
<bpmn:incoming>Flow_1jzm10m</bpmn:incoming>
70+
<bpmn:outgoing>Flow_0c2dcn9</bpmn:outgoing>
71+
</bpmn:exclusiveGateway>
72+
<bpmn:sequenceFlow id="Flow_0c2dcn9" sourceRef="Gateway_0cl3uhr" targetRef="Activity_18i1k4f" />
73+
<bpmn:sequenceFlow id="Flow_0mfryhh" sourceRef="Event_0rg9g4i" targetRef="Gateway_0cl3uhr" />
74+
<bpmn:boundaryEvent id="Event_1rdjvpi" attachedToRef="Activity_0i38awy">
75+
<bpmn:outgoing>Flow_1jzm10m</bpmn:outgoing>
76+
<bpmn:escalationEventDefinition id="EscalationEventDefinition_1q9sefw" />
77+
</bpmn:boundaryEvent>
78+
<bpmn:sequenceFlow id="Flow_1jzm10m" sourceRef="Event_1rdjvpi" targetRef="Gateway_0cl3uhr" />
79+
</bpmn:subProcess>
80+
<bpmn:sequenceFlow id="Flow_0zbf5g9" sourceRef="StartEvent_1" targetRef="Activity_0f4y776" />
81+
<bpmn:sequenceFlow id="Flow_0dxz86r" sourceRef="Activity_0f4y776" targetRef="Event_0ulnllb" />
82+
</bpmn:process>
83+
<bpmn:escalation id="Escalation1" name="Escalation1" escalationCode="Escalation1" />
84+
<bpmn:escalation id="Escalation2" name="Escalation2" escalationCode="Escalation2" />
85+
<bpmndi:BPMNDiagram id="BPMNDiagram_1">
86+
<bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="test-escalation-priority">
87+
<bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
88+
<dc:Bounds x="-128" y="159" width="36" height="36" />
89+
</bpmndi:BPMNShape>
90+
<bpmndi:BPMNShape id="Event_0ulnllb_di" bpmnElement="Event_0ulnllb">
91+
<dc:Bounds x="122" y="159" width="36" height="36" />
92+
</bpmndi:BPMNShape>
93+
<bpmndi:BPMNShape id="BPMNShape_1j8knis" bpmnElement="Activity_0f4y776">
94+
<dc:Bounds x="-20" y="137" width="100" height="80" />
95+
<bpmndi:BPMNLabel />
96+
</bpmndi:BPMNShape>
97+
<bpmndi:BPMNEdge id="Flow_0zbf5g9_di" bpmnElement="Flow_0zbf5g9">
98+
<di:waypoint x="-92" y="177" />
99+
<di:waypoint x="-20" y="177" />
100+
</bpmndi:BPMNEdge>
101+
<bpmndi:BPMNEdge id="Flow_0dxz86r_di" bpmnElement="Flow_0dxz86r">
102+
<di:waypoint x="80" y="177" />
103+
<di:waypoint x="122" y="177" />
104+
</bpmndi:BPMNEdge>
105+
</bpmndi:BPMNPlane>
106+
</bpmndi:BPMNDiagram>
107+
<bpmndi:BPMNDiagram id="BPMNDiagram_0q38sm9">
108+
<bpmndi:BPMNPlane id="BPMNPlane_1sp1uwy" bpmnElement="Activity_0f4y776">
109+
<bpmndi:BPMNShape id="BPMNShape_19oih50" bpmnElement="Event_0bmiwfz">
110+
<dc:Bounds x="180" y="222" width="36" height="36" />
111+
</bpmndi:BPMNShape>
112+
<bpmndi:BPMNShape id="BPMNShape_0pl2gwb" bpmnElement="Event_0k6ki2k">
113+
<dc:Bounds x="782" y="222" width="36" height="36" />
114+
<bpmndi:BPMNLabel>
115+
<dc:Bounds x="785" y="265" width="31" height="14" />
116+
</bpmndi:BPMNLabel>
117+
</bpmndi:BPMNShape>
118+
<bpmndi:BPMNShape id="BPMNShape_1br6tft" bpmnElement="Activity_10y5vuu">
119+
<dc:Bounds x="520" y="420" width="100" height="80" />
120+
<bpmndi:BPMNLabel />
121+
</bpmndi:BPMNShape>
122+
<bpmndi:BPMNShape id="BPMNShape_1oawm7g" bpmnElement="Activity_18i1k4f">
123+
<dc:Bounds x="620" y="200" width="100" height="80" />
124+
<bpmndi:BPMNLabel />
125+
</bpmndi:BPMNShape>
126+
<bpmndi:BPMNShape id="BPMNShape_0hjbdst" bpmnElement="Event_0j0flby">
127+
<dc:Bounds x="680" y="442" width="36" height="36" />
128+
<bpmndi:BPMNLabel>
129+
<dc:Bounds x="669" y="545" width="43" height="14" />
130+
</bpmndi:BPMNLabel>
131+
</bpmndi:BPMNShape>
132+
<bpmndi:BPMNShape id="BPMNShape_11kw4rt" bpmnElement="Activity_0i38awy" isExpanded="true">
133+
<dc:Bounds x="268" y="160" width="230" height="160" />
134+
</bpmndi:BPMNShape>
135+
<bpmndi:BPMNShape id="BPMNShape_1nzhrfc" bpmnElement="Event_0nl85sx">
136+
<dc:Bounds x="300" y="222" width="36" height="36" />
137+
</bpmndi:BPMNShape>
138+
<bpmndi:BPMNShape id="BPMNShape_0jmzg7p" bpmnElement="Event_0eh8u5t">
139+
<dc:Bounds x="390" y="222" width="36" height="36" />
140+
<bpmndi:BPMNLabel>
141+
<dc:Bounds x="395" y="265" width="26" height="14" />
142+
</bpmndi:BPMNLabel>
143+
</bpmndi:BPMNShape>
144+
<bpmndi:BPMNEdge id="BPMNEdge_19jcv3h" bpmnElement="Flow_1kbqgxt">
145+
<di:waypoint x="336" y="240" />
146+
<di:waypoint x="390" y="240" />
147+
</bpmndi:BPMNEdge>
148+
<bpmndi:BPMNShape id="Gateway_0cl3uhr_di" bpmnElement="Gateway_0cl3uhr" isMarkerVisible="true">
149+
<dc:Bounds x="535" y="215" width="50" height="50" />
150+
</bpmndi:BPMNShape>
151+
<bpmndi:BPMNShape id="Event_0cjo72r_di" bpmnElement="Event_1rdjvpi">
152+
<dc:Bounds x="422" y="302" width="36" height="36" />
153+
</bpmndi:BPMNShape>
154+
<bpmndi:BPMNShape id="Event_1lrrdie_di" bpmnElement="Event_0rg9g4i">
155+
<dc:Bounds x="362" y="302" width="36" height="36" />
156+
<bpmndi:BPMNLabel>
157+
<dc:Bounds x="367" y="345" width="26" height="14" />
158+
</bpmndi:BPMNLabel>
159+
</bpmndi:BPMNShape>
160+
<bpmndi:BPMNShape id="BPMNShape_0vb41ze" bpmnElement="Event_0wpdw73">
161+
<dc:Bounds x="300" y="302" width="36" height="36" />
162+
<bpmndi:BPMNLabel>
163+
<dc:Bounds x="305" y="345" width="26" height="14" />
164+
</bpmndi:BPMNLabel>
165+
</bpmndi:BPMNShape>
166+
<bpmndi:BPMNEdge id="BPMNEdge_0pfv51t" bpmnElement="Flow_0rhvc3p">
167+
<di:waypoint x="216" y="240" />
168+
<di:waypoint x="268" y="240" />
169+
</bpmndi:BPMNEdge>
170+
<bpmndi:BPMNEdge id="BPMNEdge_0mtnsrn" bpmnElement="Flow_1g0anfm">
171+
<di:waypoint x="720" y="240" />
172+
<di:waypoint x="782" y="240" />
173+
</bpmndi:BPMNEdge>
174+
<bpmndi:BPMNEdge id="BPMNEdge_0l2czqj" bpmnElement="Flow_1t6g80d">
175+
<di:waypoint x="318" y="338" />
176+
<di:waypoint x="318" y="460" />
177+
<di:waypoint x="520" y="460" />
178+
</bpmndi:BPMNEdge>
179+
<bpmndi:BPMNEdge id="BPMNEdge_0n1udah" bpmnElement="Flow_1r51h6b">
180+
<di:waypoint x="620" y="460" />
181+
<di:waypoint x="680" y="460" />
182+
</bpmndi:BPMNEdge>
183+
<bpmndi:BPMNEdge id="BPMNEdge_0h8aseb" bpmnElement="Flow_1hl5f8i">
184+
<di:waypoint x="498" y="240" />
185+
<di:waypoint x="535" y="240" />
186+
</bpmndi:BPMNEdge>
187+
<bpmndi:BPMNEdge id="Flow_0c2dcn9_di" bpmnElement="Flow_0c2dcn9">
188+
<di:waypoint x="585" y="240" />
189+
<di:waypoint x="620" y="240" />
190+
</bpmndi:BPMNEdge>
191+
<bpmndi:BPMNEdge id="Flow_0mfryhh_di" bpmnElement="Flow_0mfryhh">
192+
<di:waypoint x="380" y="338" />
193+
<di:waypoint x="380" y="400" />
194+
<di:waypoint x="560" y="400" />
195+
<di:waypoint x="560" y="265" />
196+
</bpmndi:BPMNEdge>
197+
<bpmndi:BPMNEdge id="Flow_1jzm10m_di" bpmnElement="Flow_1jzm10m">
198+
<di:waypoint x="440" y="338" />
199+
<di:waypoint x="440" y="358" />
200+
<di:waypoint x="560" y="358" />
201+
<di:waypoint x="560" y="265" />
202+
</bpmndi:BPMNEdge>
203+
</bpmndi:BPMNPlane>
204+
</bpmndi:BPMNDiagram>
205+
</bpmn:definitions>

spiffworkflow-backend/tests/spiffworkflow_backend/unit/test_workflow_execution_service.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,18 @@ def test_saves_last_milestone_appropriately(
2828
self.complete_next_manual_task(processor)
2929
assert process_instance.last_milestone_bpmn_name == "Completed"
3030
assert process_instance.status == "complete"
31+
32+
def test_boundary_event_priority(
33+
self,
34+
app: Flask,
35+
with_db_and_bpmn_file_cleanup: None,
36+
) -> None:
37+
process_model = load_test_spec(
38+
"test_group/prioritize-boundary-event",
39+
process_model_source_directory="prioritize-boundary-event",
40+
)
41+
process_instance = self.create_process_instance_from_process_model(process_model)
42+
processor = ProcessInstanceProcessor(process_instance)
43+
processor.do_engine_steps(save=True, execution_strategy_name="greedy")
44+
assert process_instance.status == "complete"
45+
assert processor.bpmn_process_instance.data == {"testOk": True}

0 commit comments

Comments
 (0)