diff --git a/src/modules/Elsa.Bpmn/Hosting/BpmnCommandApplier.cs b/src/modules/Elsa.Bpmn/Hosting/BpmnCommandApplier.cs index 427fb7e35..192e5d6ee 100644 --- a/src/modules/Elsa.Bpmn/Hosting/BpmnCommandApplier.cs +++ b/src/modules/Elsa.Bpmn/Hosting/BpmnCommandApplier.cs @@ -147,6 +147,12 @@ internal sealed class BpmnCommandApplier(ActivityExecutionContext scopeContext, if (BpmnWorkTeardown.FindContext(scopeContext.WorkflowExecutionContext, record.ChildContextId) is not { } childContext) return; + // On the scope-completion path (an armed listener retired when its scope completes), this call is measured + // redundant with Elsa's own CompleteActivityAsync, which already cancels a completed container's + // non-completed children: see BpmnEventSubprocessTests.MessageEventSubprocess_RetiresTheStillArmedListenerWhenTheScopeCompletes, + // where every assertion but the ledger removal above still holds with this call skipped. The ledger removal + // is this host's own contribution and is not redundant. The fault-teardown path (BpmnScopeHost, tearing down + // a claimed fault's sibling work) is a different call site and was not part of that measurement. await BpmnWorkTeardown.CancelSubtreeAsync(childContext, $"element '{cancel.ElementId}', {cancel.Reason}"); } diff --git a/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnEventSubprocessTests.cs b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnEventSubprocessTests.cs new file mode 100644 index 000000000..ce43d9641 --- /dev/null +++ b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnEventSubprocessTests.cs @@ -0,0 +1,285 @@ +using Bpmn.Semantics; +using Elsa.Workflows; +using Elsa.Workflows.IncidentStrategies; +using Elsa.Workflows.Models; +using Xunit.Abstractions; + +namespace Elsa.Bpmn.IntegrationTests.Scenarios.HostPort; + +/// +/// Event subprocesses, both flavours, and the three things about them that are the host's to get right: a listener +/// armed at scope start and retired when the scope completes, the start-element hint reaching the body, and a +/// re-armed non-interrupting listener not colliding with the slot the fire that armed it just vacated. +/// +/// +/// +/// A dormant catcher — error or escalation — needs nothing armed: it rides the FaultSignal seam and the +/// escalation signal path, and what arrives at the host is an ordinary StartWork for the body. A +/// listener-backed catcher — message, signal or timer — additionally gets a StartWork for its +/// listenerBindingRef at scope start and a CancelWorkSubtree for it when the scope completes. Both are +/// commands the applier already handles, so these are process-level tests rather than tests of an +/// event-subprocess-specific code path: there is none. +/// +/// +/// Every process runs under . An event subprocess's failure mode is a quiet one — a body +/// that was never seeded, a listener that was never armed, a catcher that never fired — and all three leave a +/// workflow that finished and reported nothing. Faulting rather than absorbing into an incident keeps a refusal this +/// host cannot honour from being buried under work that carried on regardless. +/// +/// +public class BpmnEventSubprocessTests(ITestOutputHelper testOutputHelper) +{ + private readonly BpmnTestHost _host = new(testOutputHelper); + + [Fact(DisplayName = "A dormant error-triggered event subprocess catches a fault in its scope and routes to its body")] + public async Task ErrorEventSubprocess_CatchesTheFaultAndRunsItsBody() + { + // The whole log, not a Contains: "the body ran" is also true of a process that carried on down the sequence + // flow afterwards, and 'after' is exactly the work an error the scope only appeared to claim would reach. + // 'cancelled:risky' is in it deliberately -- a scope claiming a fault terminalizes the whole unit of work + // that failed, and that teardown is the host's own doing rather than a command the interpreter issued. + + // Act + var result = await _host.RunAsync(BpmnTestProcesses.ErrorEventSubprocess(_host.Log), typeof(FaultStrategy)); + + // Assert + Assert.Equal(["executed:risky", "cancelled:risky", "executed:handleError"], _host.Log.Entries); + + // The disposition was Caught, so the scope claimed the fault and terminalized the failed work itself. + Assert.Equal(ActivityStatus.Canceled, StatusOf(result, "risky")); + + // And a fault a container claimed is not an incident. + Assert.Empty(result.WorkflowState.Incidents); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "A dormant escalation-triggered event subprocess catches an escalation raised in a nested scope")] + public async Task EscalationEventSubprocess_CatchesTheEscalationOutOfTheNestedScope() + { + // The scope-level catcher, not a boundary event on the subprocess: the escalation crosses the scope boundary + // on the same signal path, and what claims it is an event subprocess whose body start event declares the + // matching code. Non-interrupting, so the subprocess that escalated keeps running -- which is what tells + // "the catcher fired" apart from "the subprocess was stopped". + + // Arrange + await _host.RunAsync(BpmnTestProcesses.EscalationEventSubprocessOutOfSubprocess(_host.Log), typeof(FaultStrategy)); + + // Act: the subprocess reaches its escalation throw event. + await _host.FinishWorkAsync("subWork"); + + // Assert: the event subprocess body ran... + Assert.Contains("executed:handleEscalation", _host.Log.Entries); + + // ...and the escalating subprocess carried on past the throw rather than being torn down. + Assert.Contains("executed:subMore", _host.Log.Entries); + Assert.DoesNotContain("cancelled:subMore", _host.Log.Entries); + + // And the subprocess still completes normally, so the main path continues. + var result = await _host.FinishWorkAsync("subMore"); + + Assert.Contains("executed:after", _host.Log.Entries); + Assert.Empty(result.WorkflowState.Incidents); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "A message-triggered event subprocess arms its listener at scope start and runs its body when the trigger fires")] + public async Task MessageEventSubprocess_ArmsItsListenerAtScopeStartAndRunsItsBodyWhenFired() + { + // Armed *at scope start* is the claim, so it is asserted before the trigger fires and not inferred from the + // fire having worked. The scope's own ledger -- the sole source of BpmnHostSnapshot.LiveWork -- already holds + // the listener alongside the scope's ordinary work by the time that work runs. + + // Arrange + await _host.RunAsync(BpmnTestProcesses.MessageEventSubprocess(_host.Log), typeof(FaultStrategy)); + + // Assert: armed, and nothing has fired. + Assert.Equal( + [BpmnTestProcesses.BindingRef("nudgeListener"), BpmnTestProcesses.BindingRef("work")], + _host.Log.Snapshot("liveWork@work")); + + Assert.DoesNotContain("executed:handleNudge", _host.Log.Entries); + + // Act: the trigger fires while the scope's own work is still running. + await _host.FinishWorkAsync("nudgeListener"); + + // Assert: the body ran, and the scope's own work is untouched -- non-interrupting means exactly that. + Assert.Equal(1, _host.Log.Occurrences("executed:handleNudge")); + Assert.DoesNotContain("cancelled:work", _host.Log.Entries); + + var result = await _host.FinishWorkAsync("work"); + + Assert.Empty(result.WorkflowState.Incidents); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "A scope completing with its listener still armed retires it, and the armed work does not survive")] + public async Task MessageEventSubprocess_RetiresTheStillArmedListenerWhenTheScopeCompletes() + { + // The quiet failure this pins: a listener left armed is a live child activity holding a bookmark on a scope + // that is already finished. The workflow reports Finished either way -- what differs is whether anything can + // still resume into a scope that has completed, which is a process that would run its event subprocess body + // after the process containing it ended. + + // Arrange + await _host.RunAsync(BpmnTestProcesses.MessageEventSubprocess(_host.Log), typeof(FaultStrategy)); + + Assert.Contains(BpmnTestProcesses.BindingRef("nudgeListener"), _host.LiveWorkOf("scope").Select(work => work.BindingRef)); + + // Act: the scope's own work completes, which is the last thing keeping the scope open. + var result = await _host.FinishWorkAsync("work"); + + // Assert: the listener was torn down, never fired, and left nothing live behind it. + // + // The ledger assertion is the one that carries the weight here. At the root, Elsa tears a finished workflow's + // remaining children down on its own, so 'cancelled:nudgeListener' and the empty bookmark set would both hold + // even if the retirement command were dropped on the floor; what would not is the scope's own record of what + // it still has running. NestedMessageEventSubprocess_... below is the same retirement where the workflow + // outlives the scope, which is where the rest of it stops being free. + Assert.Contains("cancelled:nudgeListener", _host.Log.Entries); + Assert.DoesNotContain("executed:handleNudge", _host.Log.Entries); + Assert.Empty(_host.LiveWorkOf("scope")); + Assert.Empty(result.WorkflowState.Bookmarks); + + Assert.Empty(result.WorkflowState.Incidents); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "A nested scope completing with its listener still armed retires it while the workflow carries on")] + public async Task NestedMessageEventSubprocess_RetiresTheStillArmedListenerWhenTheNestedScopeCompletes() + { + // The same retirement, in the only shape where it is the scope's doing rather than the workflow's: the + // enclosing process keeps running afterwards, so a listener the scope failed to retire is one that outlives + // it and could still be resumed into a subprocess that has already finished. + + // Arrange + await _host.RunAsync(BpmnTestProcesses.NestedMessageEventSubprocess(_host.Log), typeof(FaultStrategy)); + + Assert.Contains(BpmnTestProcesses.BindingRef("nudgeListener"), _host.LiveWorkOf("sub").Select(work => work.BindingRef)); + + // Act: the subprocess's own work completes, which is the last thing keeping the nested scope open. + var result = await _host.FinishWorkAsync("subWork"); + + // Assert: the nested scope retired its listener and completed, and the enclosing scope carried on. + Assert.Contains("cancelled:nudgeListener", _host.Log.Entries); + Assert.DoesNotContain("executed:handleNudge", _host.Log.Entries); + Assert.Empty(_host.LiveWorkOf("sub")); + Assert.Contains("executed:after", _host.Log.Entries); + + // Nothing is left for a stimulus to resume into. + Assert.Empty(result.WorkflowState.Bookmarks); + + Assert.Empty(result.WorkflowState.Incidents); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "A re-armed non-interrupting listener fires again cleanly, holding one live slot at a time")] + public async Task NonInterruptingListener_ReArmsWithoutCollidingWithTheSlotItJustVacated() + { + // The interpreter re-finds a parked token from (binding ref, iteration id) alone, and a re-armed listener + // keys onto the very slot the fire that armed it just vacated. It is only free because the completing + // listener was removed from the ledger before the interpreter was asked; leaving it there gives the scope two + // live records and two bookmarks for one slot, and the next teardown or completion resolves to whichever one + // it happens to find first. + // + // Firing twice rather than once is what makes that visible: the first fire is indistinguishable either way. + + // Arrange + await _host.RunAsync(BpmnTestProcesses.MessageEventSubprocess(_host.Log), typeof(FaultStrategy)); + + // Act: fire once... + await _host.FinishWorkAsync("nudgeListener"); + + // Assert: the body ran, and exactly one listener is armed -- not the fresh one alongside the finished one. + Assert.Equal(1, _host.Log.Occurrences("executed:handleNudge")); + Assert.Equal(1, LiveListenerRecords()); + + // Act: ...and again, onto the slot the first fire vacated. + await _host.FinishWorkAsync("nudgeListener"); + + // Assert: a second, complete run of the body, and still exactly one armed listener. + Assert.Equal(2, _host.Log.Occurrences("executed:handleNudge")); + Assert.Equal(1, LiveListenerRecords()); + + var result = await _host.FinishWorkAsync("work"); + + // The scope retires that last listener and finishes, which a scope holding a stale second record could not do + // cleanly: the teardown would resolve the wrong record and leave the other bookmark behind. + Assert.Contains("cancelled:nudgeListener", _host.Log.Entries); + Assert.Empty(result.WorkflowState.Bookmarks); + Assert.Empty(result.WorkflowState.Incidents); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + } + + [Fact(DisplayName = "An event subprocess body is seeded at the start event named on the hint, and nothing else inherits it")] + public async Task EventSubprocessBody_IsSeededAtTheHintedStartEvent() + { + // Both directions of the hint, in one process, and each fails loudly rather than quietly. + // + // The body's only start event is event-defined, so a body that did not receive the hint has nothing to begin + // at: it faults with bpmn.start.none-available rather than starting anywhere plausible. The ordinary + // subprocess inside the body is the other direction -- its own invocation carries an ordinary scheduling + // cause, so inheriting the hint would seed it at an element it does not declare and fault it with + // bpmn.start.unresolved-hint. + + // Act + var result = await _host.RunAsync(BpmnTestProcesses.EventSubprocessBodyWithNestedSubprocess(_host.Log), typeof(FaultStrategy)); + + // Assert + Assert.Equal(["executed:risky", "cancelled:risky", "executed:handleError", "executed:innerOnly"], _host.Log.Entries); + Assert.Empty(result.WorkflowState.Incidents); + Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus); + + // The dictionary the hint arrives in belongs to the scope and is fixed for its lifetime. Read after the body + // has started and finished work of its own, it still says what the StartWork that created the scope said: a + // host that wrote a started or completing unit of work's correlation here would have overwritten it, and the + // damage would only surface the next time a body was seeded. + var correlation = _host.InvocationCorrelationOf("evtSub"); + + Assert.Equal("errStart", correlation[BpmnInterpreter.StartElementIdCorrelationKey]); + Assert.Equal(BpmnInterpreter.EventSubprocessBodySchedulingCause, correlation[BpmnInterpreter.SchedulingCauseCorrelationKey]); + } + + [Fact(DisplayName = "An event subprocess body declaring more than one start event is refused")] + public Task EventSubprocessBodyWithTwoStartEvents_IsRefused() => + AssertRefusedAsync(BpmnTestProcesses.EventSubprocessBodyWithTwoStartEvents(_host.Log), "body must declare exactly one start event"); + + [Fact(DisplayName = "A second error-triggered event subprocess in one scope is refused")] + public Task TwoErrorEventSubprocesses_AreRefused() => + AssertRefusedAsync(BpmnTestProcesses.TwoErrorEventSubprocesses(_host.Log), "more than one error event subprocess"); + + [Fact(DisplayName = "A second code-less catch-all escalation-triggered event subprocess in one scope is refused")] + public Task TwoCatchAllEscalationEventSubprocesses_AreRefused() => + AssertRefusedAsync( + BpmnTestProcesses.TwoCatchAllEscalationEventSubprocesses(_host.Log), + "more than one code-less catch-all escalation event subprocess"); + + [Fact(DisplayName = "A non-interrupting error-triggered event subprocess is refused")] + public Task NonInterruptingErrorEventSubprocess_IsRefused() => + AssertRefusedAsync(BpmnTestProcesses.NonInterruptingErrorEventSubprocess(_host.Log), "must be interrupting"); + + /// + /// Asserts a process the library refuses is refused, and refused before anything ran. + /// + /// + /// These are the library's rules about how an event subprocess may be declared, and it enforces them when the + /// scope builds its graph — before a single unit of work is started. Pinned here so a future reader meets them as + /// refusals rather than as gaps: the process does not half-run and then stop, it never starts. + /// + private async Task AssertRefusedAsync(IActivity process, string expectedMessageFragment) + { + var result = await _host.RunAsync(process, typeof(FaultStrategy)); + + Assert.Empty(_host.Log.Entries); + Assert.Equal(WorkflowSubStatus.Faulted, result.WorkflowState.SubStatus); + + var incident = Assert.Single(result.WorkflowState.Incidents); + + Assert.Contains(expectedMessageFragment, incident.Exception!.Message); + } + + private int LiveListenerRecords() => + _host.LiveWorkOf("scope").Count(record => record.BindingRef == BpmnTestProcesses.BindingRef("nudgeListener")); + + private static ActivityStatus? StatusOf(RunWorkflowResult result, string activityId) => + result.Journal.ActivityExecutionContexts.FirstOrDefault(x => x.Activity.Id == activityId)?.Status; +} diff --git a/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnHostInvariantTests.cs b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnHostInvariantTests.cs index fbcc1830c..fcda1c58b 100644 --- a/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnHostInvariantTests.cs +++ b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnHostInvariantTests.cs @@ -42,7 +42,9 @@ public class BpmnHostInvariantTests(ITestOutputHelper testOutputHelper) // ordering is not asserted here because it is not observable for these constructs: the interpreter reads // LiveWork only to resolve teardown handles by (binding ref, iteration id), and none of the processes in scope // tears down a slot a just-completed unit of work shares. It becomes observable with multi-instance work and - // re-armed scope listeners, which arrive with the issues that add them. + // re-armed scope listeners, which arrive with the issues that add them -- see + // BpmnEventSubprocessTests.NonInterruptingListener_ReArmsWithoutCollidingWithTheSlotItJustVacated, which is + // where the ordering is pinned. // Arrange await _host.RunAsync(BpmnTestProcesses.InterruptingTimerBoundary(_host.Log)); diff --git a/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestHost.cs b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestHost.cs index 615be8ff8..e8d38a0db 100644 --- a/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestHost.cs +++ b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestHost.cs @@ -85,6 +85,22 @@ public sealed class BpmnTestHost return ledger.Records.Select(record => (record.BindingRef, record.IterationId)).ToList(); } + /// + /// The invocation correlation the named BPMN scope carries — the dictionary the scope that started it wrote onto + /// its context, and the one an event subprocess body's start-element hint is read from. + /// + /// + /// It belongs to the scope and is fixed for its lifetime, so reading it after the scope has started and finished + /// work is what makes "nothing overwrote it" observable rather than merely documented. + /// + public IReadOnlyDictionary InvocationCorrelationOf(string scopeActivityId) + { + var scopeContext = _result!.Journal.ActivityExecutionContexts.First(x => x.Activity.Id == scopeActivityId); + + return BpmnScopeMemory.Read>(scopeContext, BpmnScopeHost.InvocationCorrelationPropertyKey) + ?? new Dictionary(StringComparer.Ordinal); + } + /// /// Replaces the current with what a round trip through Elsa's own /// hands back — what a real persistence store would return on load, diff --git a/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestProcesses.cs b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestProcesses.cs index 58f7c5d4a..024c32629 100644 --- a/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestProcesses.cs +++ b/test/integration/Elsa.Bpmn.IntegrationTests/Scenarios/HostPort/BpmnTestProcesses.cs @@ -472,6 +472,294 @@ internal static class BpmnTestProcesses return Scope("scope", definition, nested, Immediate("undoSub", log)); } + /// + /// A task that fails, with a dormant error-triggered event subprocess in the same scope to catch it. + /// + /// + /// An error event subprocess arms nothing: it rides the same FaultSignal seam an error boundary event does, + /// and the only thing that distinguishes it here is where the recovery work runs — inside a nested scope of its + /// own, seeded at the body's error start event, rather than on an outbound flow of the enclosing graph. + /// + public static BpmnProcess ErrorEventSubprocess(BpmnTestLog log) + { + var body = new BpmnProcessBuilder("error-event-subprocess-body") + .Element(EventSubprocessStart("errStart", Error())) + .Task("handleError", bindingRef: BindingRef("handleError")) + .EndEvent("errEnd") + .ConnectSequence("errStart", "handleError", "errEnd") + .Build(); + + var definition = new BpmnProcessBuilder("error-event-subprocess") + .StartEvent("start") + .Task("risky", bindingRef: BindingRef("risky")) + .Task("after", bindingRef: BindingRef("after")) + .EndEvent("end") + .Element(EventSubprocess("evtSub")) + .ConnectSequence("start", "risky", "after", "end") + .Build(); + + return Scope("scope", definition, Faulting("risky", log), Immediate("after", log), Scope("evtSub", body, Immediate("handleError", log))); + } + + /// + /// An escalation thrown out of an embedded subprocess, caught by a non-interrupting escalation-triggered event + /// subprocess on the enclosing scope rather than by a boundary event on the subprocess. + /// + /// + /// Non-interrupting, so the escalating subprocess keeps running and nothing in the scope is torn down. That is + /// also what makes "the scope-level catcher fired" distinguishable from "the subprocess was stopped": with an + /// interrupting catcher the two are the same observation. + /// + public static BpmnProcess EscalationEventSubprocessOutOfSubprocess(BpmnTestLog log) + { + var subBody = new BpmnProcessBuilder("escalating-subprocess-body") + .StartEvent("subStart") + .Task("subWork", bindingRef: BindingRef("subWork")) + .IntermediateThrowEvent("subEscalate", Escalation("REVIEW")) + .Task("subMore", bindingRef: BindingRef("subMore")) + .EndEvent("subEnd") + .ConnectSequence("subStart", "subWork", "subEscalate", "subMore", "subEnd") + .Build(); + + var handlerBody = new BpmnProcessBuilder("escalation-event-subprocess-body") + .Element(EventSubprocessStart("escStart", Escalation("REVIEW"), interrupting: false)) + .Task("handleEscalation", bindingRef: BindingRef("handleEscalation")) + .EndEvent("escEnd") + .ConnectSequence("escStart", "handleEscalation", "escEnd") + .Build(); + + var definition = new BpmnProcessBuilder("escalation-event-subprocess") + .StartEvent("start") + .SubProcess("sub", bindingRef: BindingRef("sub")) + .Task("after", bindingRef: BindingRef("after")) + .EndEvent("end") + .Element(EventSubprocess("evtSub")) + .ConnectSequence("start", "sub", "after", "end") + .Build(); + + var nested = Scope("sub", subBody, Blocking("subWork", log), Blocking("subMore", log)); + + return Scope("scope", definition, nested, Immediate("after", log), Scope("evtSub", handlerBody, Immediate("handleEscalation", log))); + } + + /// + /// A non-interrupting message-triggered event subprocess: a listener armed at scope start, and a body that runs + /// each time the listener fires while the scope's own long-running work is still going. + /// + /// + /// + /// The listener is the second binding channel — listenerBindingRef — and is bound in the same + /// WorkBindings map as everything else. It stands in for a real message wait: blocking work a test + /// finishes, which is exactly what "the trigger fired" means to the host. + /// + /// + /// work blocks so the scope stays open across the fires and so that when it finally completes, the armed + /// listener is a running activity rather than a scheduled-but-not-yet-invoked one — the second of which + /// this host cannot withdraw at all. + /// + /// + public static BpmnProcess MessageEventSubprocess(BpmnTestLog log) + { + var body = new BpmnProcessBuilder("message-event-subprocess-body") + .Element(EventSubprocessStart("msgStart", Message("nudge"), interrupting: false)) + .Task("handleNudge", bindingRef: BindingRef("handleNudge")) + .EndEvent("msgEnd") + .ConnectSequence("msgStart", "handleNudge", "msgEnd") + .Build(); + + var definition = new BpmnProcessBuilder("message-event-subprocess") + .StartEvent("start") + .Task("work", bindingRef: BindingRef("work")) + .EndEvent("end") + .Element(EventSubprocess("evtSub", listenerBindingRef: BindingRef("nudgeListener"))) + .ConnectSequence("start", "work", "end") + .Build(); + + return Scope( + "scope", + definition, + Blocking("work", log), + Blocking("nudgeListener", log), + Scope("evtSub", body, Immediate("handleNudge", log))); + } + + /// + /// The same message-triggered event subprocess, but inside an embedded subprocess that completes while the + /// enclosing scope carries on — so a listener that outlived the scope that armed it is distinguishable from one + /// that merely outlived the workflow. + /// + /// + /// At the root, "the armed work does not survive the scope" and "does not survive the workflow" are the same + /// observation, and Elsa tears a finished workflow's children down regardless. Here the workflow keeps running + /// after the scope that armed the listener has completed, which is the only shape in which a listener left behind + /// is a listener something could still resume into. + /// + public static BpmnProcess NestedMessageEventSubprocess(BpmnTestLog log) + { + var handlerBody = new BpmnProcessBuilder("nested-message-event-subprocess-body") + .Element(EventSubprocessStart("msgStart", Message("nudge"), interrupting: false)) + .Task("handleNudge", bindingRef: BindingRef("handleNudge")) + .EndEvent("msgEnd") + .ConnectSequence("msgStart", "handleNudge", "msgEnd") + .Build(); + + var subBody = new BpmnProcessBuilder("listening-subprocess-body") + .StartEvent("subStart") + .Task("subWork", bindingRef: BindingRef("subWork")) + .EndEvent("subEnd") + .Element(EventSubprocess("evtSub", listenerBindingRef: BindingRef("nudgeListener"))) + .ConnectSequence("subStart", "subWork", "subEnd") + .Build(); + + var definition = new BpmnProcessBuilder("nested-message-event-subprocess") + .StartEvent("start") + .SubProcess("sub", bindingRef: BindingRef("sub")) + .Task("after", bindingRef: BindingRef("after")) + .EndEvent("end") + .ConnectSequence("start", "sub", "after", "end") + .Build(); + + var nested = Scope( + "sub", + subBody, + Blocking("subWork", log), + Blocking("nudgeListener", log), + Scope("evtSub", handlerBody, Immediate("handleNudge", log))); + + return Scope("scope", definition, nested, Immediate("after", log)); + } + + /// + /// An error-triggered event subprocess whose body runs an ordinary embedded subprocess of its own, so the + /// start-element hint has both a place to arrive and a place it must not reach. + /// + /// + /// + /// The body's only start event is event-defined, which is what makes the hint's arrival observable rather than + /// merely asserted: seeded from the hint the body runs, and seeded as an ordinary direct invocation it faults + /// deterministically with bpmn.start.none-available, because there is no none start event to begin at. + /// + /// + /// The nested inner subprocess is the other direction. Its own invocation carries an ordinary scheduling + /// cause, so the hint must not be inherited: were it, the inner process would be seeded at an element it does not + /// declare and fault with bpmn.start.unresolved-hint instead of starting at its own none start event. + /// + /// + public static BpmnProcess EventSubprocessBodyWithNestedSubprocess(BpmnTestLog log) + { + var innerBody = new BpmnProcessBuilder("event-subprocess-inner-body") + .StartEvent("innerStart") + .Task("innerOnly", bindingRef: BindingRef("innerOnly")) + .EndEvent("innerEnd") + .ConnectSequence("innerStart", "innerOnly", "innerEnd") + .Build(); + + var body = new BpmnProcessBuilder("hinted-event-subprocess-body") + .Element(EventSubprocessStart("errStart", Error())) + .Task("handleError", bindingRef: BindingRef("handleError")) + .SubProcess("inner", bindingRef: BindingRef("inner")) + .EndEvent("errEnd") + .ConnectSequence("errStart", "handleError", "inner", "errEnd") + .Build(); + + var definition = new BpmnProcessBuilder("event-subprocess-start-hint") + .StartEvent("start") + .Task("risky", bindingRef: BindingRef("risky")) + .EndEvent("end") + .Element(EventSubprocess("evtSub")) + .ConnectSequence("start", "risky", "end") + .Build(); + + var handler = Scope("evtSub", body, Immediate("handleError", log), Scope("inner", innerBody, Immediate("innerOnly", log))); + + return Scope("scope", definition, Faulting("risky", log), handler); + } + + /// An event subprocess whose body declares two start events, which the library refuses. + public static BpmnProcess EventSubprocessBodyWithTwoStartEvents(BpmnTestLog log) + { + var body = new BpmnProcessBuilder("two-start-event-subprocess-body") + .Element(EventSubprocessStart("errStart", Error())) + .StartEvent("alsoStart") + .Task("handleError", bindingRef: BindingRef("handleError")) + .EndEvent("errEnd") + .ConnectSequence("errStart", "handleError", "errEnd") + .Connect("alsoStart", "handleError") + .Build(); + + return RefusedEventSubprocessScope("two-start-events", log, ("evtSub", body, "handleError")); + } + + /// Two error-triggered event subprocesses in one scope, which the library refuses. + public static BpmnProcess TwoErrorEventSubprocesses(BpmnTestLog log) + { + BpmnProcessDefinition Body(string prefix) => new BpmnProcessBuilder($"{prefix}-error-event-subprocess-body") + .Element(EventSubprocessStart($"{prefix}Start", Error())) + .Task($"{prefix}Handle", bindingRef: BindingRef($"{prefix}Handle")) + .EndEvent($"{prefix}End") + .ConnectSequence($"{prefix}Start", $"{prefix}Handle", $"{prefix}End") + .Build(); + + return RefusedEventSubprocessScope( + "two-error-event-subprocesses", + log, + ("evtSubA", Body("first"), "firstHandle"), + ("evtSubB", Body("second"), "secondHandle")); + } + + /// Two code-less catch-all escalation-triggered event subprocesses in one scope, which the library refuses. + public static BpmnProcess TwoCatchAllEscalationEventSubprocesses(BpmnTestLog log) + { + BpmnProcessDefinition Body(string prefix) => new BpmnProcessBuilder($"{prefix}-escalation-event-subprocess-body") + .Element(EventSubprocessStart($"{prefix}Start", Escalation(), interrupting: false)) + .Task($"{prefix}Handle", bindingRef: BindingRef($"{prefix}Handle")) + .EndEvent($"{prefix}End") + .ConnectSequence($"{prefix}Start", $"{prefix}Handle", $"{prefix}End") + .Build(); + + return RefusedEventSubprocessScope( + "two-catch-all-escalation-event-subprocesses", + log, + ("evtSubA", Body("first"), "firstHandle"), + ("evtSubB", Body("second"), "secondHandle")); + } + + /// A non-interrupting error-triggered event subprocess, which is not legal BPMN and which the library refuses. + public static BpmnProcess NonInterruptingErrorEventSubprocess(BpmnTestLog log) + { + var body = new BpmnProcessBuilder("non-interrupting-error-event-subprocess-body") + .Element(EventSubprocessStart("errStart", Error(), interrupting: false)) + .Task("handleError", bindingRef: BindingRef("handleError")) + .EndEvent("errEnd") + .ConnectSequence("errStart", "handleError", "errEnd") + .Build(); + + return RefusedEventSubprocessScope("non-interrupting-error-event-subprocess", log, ("evtSub", body, "handleError")); + } + + /// + /// The start/only/end graph the refusal processes share, carrying the event subprocesses whose declaration + /// the library refuses. Nothing in it ever runs: the refusal is raised when the scope builds its graph, which is + /// before any work is started. + /// + private static BpmnProcess RefusedEventSubprocessScope(string processId, BpmnTestLog log, params (string ElementId, BpmnProcessDefinition Body, string HandlerId)[] eventSubprocesses) + { + var builder = new BpmnProcessBuilder(processId) + .StartEvent("start") + .Task("only", bindingRef: BindingRef("only")) + .EndEvent("end") + .ConnectSequence("start", "only", "end"); + + foreach (var eventSubprocess in eventSubprocesses) + builder = builder.Element(EventSubprocess(eventSubprocess.ElementId)); + + var work = new List { Immediate("only", log) }; + + work.AddRange(eventSubprocesses.Select(eventSubprocess => Scope(eventSubprocess.ElementId, eventSubprocess.Body, Immediate(eventSubprocess.HandlerId, log)))); + + return Scope("scope", builder.Build(), work.ToArray()); + } + /// An embedded subprocess with one task in it, and one task after it in the enclosing scope. public static BpmnProcess NestedSubprocess(BpmnTestLog log) { @@ -576,8 +864,34 @@ internal static class BpmnTestProcesses private static BpmnElement CompensationHandler(string elementId) => new(elementId, BpmnElementTypes.Task, bindingRef: BindingRef(elementId), isForCompensation: true); - private static BpmnEventDefinition Escalation(string code) => - new(BpmnEventDefinitionTypes.Escalation, new Dictionary(StringComparer.Ordinal) { [BpmnEventDefinitionProperties.Code] = code }); + private static BpmnEventDefinition Escalation(string? code = null) => + code is null + ? new(BpmnEventDefinitionTypes.Escalation) + : new(BpmnEventDefinitionTypes.Escalation, new Dictionary(StringComparer.Ordinal) { [BpmnEventDefinitionProperties.Code] = code }); + + private static BpmnEventDefinition Error() => new(BpmnEventDefinitionTypes.Error); + + private static BpmnEventDefinition Message(string name) => + new(BpmnEventDefinitionTypes.Message, new Dictionary(StringComparer.Ordinal) { [BpmnEventDefinitionProperties.Name] = name }); + + /// + /// An event subprocess: a flow-less subprocess whose bound work is the body, activated by that body's single + /// start event rather than by a sequence flow. carries the + /// triggeredByEvent flag but not the listener binding, so this one is written out. + /// + private static BpmnElement EventSubprocess(string elementId, string? listenerBindingRef = null) => + new(elementId, + BpmnElementTypes.SubProcess, + bindingRef: BindingRef(elementId), + triggeredByEvent: true, + listenerBindingRef: listenerBindingRef); + + /// + /// An event subprocess body's single start event, carrying its trigger and its isInterrupting flag. + /// cannot carry the flag, so this one is written out. + /// + private static BpmnElement EventSubprocessStart(string elementId, BpmnEventDefinition trigger, bool interrupting = true) => + new(elementId, BpmnElementTypes.StartEvent, eventDefinitions: [trigger], cancelActivity: interrupting); private static BpmnProcess Scope(string id, BpmnProcessDefinition definition, params IActivity[] work) => Scope(id, definition, [], work); diff --git a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Assets/non-interrupting-error-event-subprocess.bpmn b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Assets/non-interrupting-error-event-subprocess.bpmn new file mode 100644 index 000000000..091d9e020 --- /dev/null +++ b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Assets/non-interrupting-error-event-subprocess.bpmn @@ -0,0 +1,43 @@ + + + + + + Flow_1 + + + Flow_1 + Flow_2 + + PT5M + + + + Flow_2 + + + + + + + Flow_E1 + + + Flow_E1 + Flow_E2 + + + Flow_E2 + + + + + + diff --git a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnInterchangeDocumentServiceTests.cs b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnInterchangeDocumentServiceTests.cs index c51a7d35f..48fb944f0 100644 --- a/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnInterchangeDocumentServiceTests.cs +++ b/test/integration/Elsa.Bpmn.Interchange.IntegrationTests/Scenarios/Interchange/BpmnInterchangeDocumentServiceTests.cs @@ -45,6 +45,35 @@ public class BpmnInterchangeDocumentServiceTests(ITestOutputHelper testOutputHel Assert.Contains(result.Analysis.Issues, issue => issue.ElementId == "NotifyWarehouse" && issue.Severity == BpmnImportIssueSeverity.Info); } + [Fact(DisplayName = "A non-interrupting error event subprocess is dropped at import, and the rest of the document still imports")] + public async Task Import_DropsANonInterruptingErrorEventSubprocess() + { + // A refusal, not a bug, and not a failed import: error events are always interrupting per BPMN, so the reader + // reports the whole as Dropped and reads the rest of the document as written. + // + // The import succeeding is what proves the drop was total. The dropped body declares an undeclared + // , and an unbound task the binder can see is refused outright -- so a drop that reported the + // element but left its bindings behind would surface here as a BpmnBindingException naming 'HandleError', + // not as a quietly half-imported process. + var xml = ReadAsset("non-interrupting-error-event-subprocess.bpmn"); + + var analysis = DocumentService.Analyze(xml); + + var dropped = Assert.Single(analysis.Issues, candidate => candidate.Severity == BpmnImportIssueSeverity.Dropped); + + Assert.Equal("OnError", dropped.ElementId); + Assert.Contains("non-interrupting error event subprocess", dropped.Message); + + // The reader's own findings about the dropped body's elements survive the drop, at Info: the body is read + // before the rule that drops the element around it is applied. Pinned so a future reader meets it as the + // library's behaviour rather than as evidence the drop was partial -- what proves it was total is the import. + Assert.Contains(analysis.Issues, candidate => candidate.ElementId == "HandleError" && candidate.Severity == BpmnImportIssueSeverity.Info); + + var imported = await DocumentService.ImportAsync(xml, definitionId: null, name: null, processId: null, CancellationToken.None); + + Assert.True(imported.ImportResult.Succeeded, string.Join("; ", imported.ImportResult.ValidationErrors.Select(error => error.Message))); + } + [Fact(DisplayName = "Exporting an imported definition retains foreign extension elements, foreign attributes and waypoints byte-identically")] public async Task Export_RetainsForeignContentAndWaypoints() {