From 051e12f864e54dd355395a7f8e0f2ebd230c0316 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 31 Aug 2026 02:47:29 +0200 Subject: [PATCH] Fix StateMachine transition lifecycle ordering --- .../StateMachine/Activities/StateMachine.cs | 54 +++++++++--- .../StateMachine/StateMachineTests.cs | 86 +++++++++++++++++-- 2 files changed, 121 insertions(+), 19 deletions(-) diff --git a/src/modules/Elsa.Workflows.Core/Activities/StateMachine/Activities/StateMachine.cs b/src/modules/Elsa.Workflows.Core/Activities/StateMachine/Activities/StateMachine.cs index 0a3c7895c..cfa0045ae 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/StateMachine/Activities/StateMachine.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/StateMachine/Activities/StateMachine.cs @@ -98,7 +98,7 @@ public class StateMachine : Activity private async ValueTask ScheduleOutboundTriggersAsync(ActivityExecutionContext context, ActivityExecutionContext? schedulingContext = null) { - var outboundTransitions = GetOutboundTransitions(GetCurrentState(context)).Where(x => x.Trigger != null && FindState(x.To) != null).ToList(); + var outboundTransitions = GetOutboundTransitions(GetCurrentState(context)).Where(x => FindState(x.To) != null).ToList(); if (!outboundTransitions.Any()) { @@ -106,7 +106,18 @@ public class StateMachine : Activity return; } - foreach (var transition in outboundTransitions) + foreach (var transition in outboundTransitions.Where(x => x.Trigger == null)) + { + if (await TryTakeTransitionAsync(context, transition, schedulingContext)) + return; + } + + var triggeredTransitions = outboundTransitions.Where(x => x.Trigger != null).ToList(); + + if (!triggeredTransitions.Any()) + return; + + foreach (var transition in triggeredTransitions) await ScheduleAsync(context, transition.Trigger!, OnTriggerCompletedAsync, GetTransitionKey(transition), schedulingContext); } @@ -118,25 +129,40 @@ public class StateMachine : Activity if (transition == null || !IsCurrentSource(targetContext, transition) || FindState(transition.To) == null) return; - var canTransition = transition.Condition == null || await EvaluateConditionAsync(targetContext, transition.Condition); - - if (!canTransition) + if (!await TryTakeTransitionAsync(targetContext, transition, context.ChildContext)) { if (transition.Trigger != null) await ScheduleAsync(targetContext, transition.Trigger, OnTriggerCompletedAsync, GetTransitionKey(transition), context.ChildContext); - - return; } + } - await CancelCompetingTriggersAsync(targetContext, transition, context.ChildContext); + private async ValueTask TryTakeTransitionAsync( + ActivityExecutionContext context, + Transition transition, + ActivityExecutionContext? schedulingContext) + { + var canTransition = transition.Condition == null || await EvaluateConditionAsync(context, transition.Condition); + + if (!canTransition) + return false; + + await CancelCompetingTriggersAsync(context, transition, schedulingContext); + await ExitStateAsync(context, transition, schedulingContext); + return true; + } + + private async ValueTask RunTransitionActionAsync(ActivityExecutionContext context, Transition transition, ActivityExecutionContext? schedulingContext = null) + { + if (!IsCurrentSource(context, transition)) + return; if (transition.Action != null) { - await ScheduleTransitionActivityAsync(targetContext, transition.Action, transition, OnTransitionActionCompletedAsync, context.ChildContext); + await ScheduleTransitionActivityAsync(context, transition.Action, transition, OnTransitionActionCompletedAsync, schedulingContext); return; } - await ExitStateAsync(targetContext, transition, context.ChildContext); + await CompleteTransitionAsync(context, transition, schedulingContext); } private async ValueTask OnTransitionActionCompletedAsync(ActivityCompletedContext context) @@ -147,7 +173,7 @@ public class StateMachine : Activity if (transition == null || !IsCurrentSource(targetContext, transition)) return; - await ExitStateAsync(targetContext, transition, context.ChildContext); + await CompleteTransitionAsync(targetContext, transition, context.ChildContext); } private async ValueTask ExitStateAsync(ActivityExecutionContext context, Transition transition, ActivityExecutionContext? schedulingContext = null) @@ -160,7 +186,7 @@ public class StateMachine : Activity return; } - await CompleteTransitionAsync(context, transition, schedulingContext); + await RunTransitionActionAsync(context, transition, schedulingContext); } private async ValueTask OnStateExitCompletedAsync(ActivityCompletedContext context) @@ -171,7 +197,7 @@ public class StateMachine : Activity if (transition == null || !IsCurrentSource(targetContext, transition)) return; - await CompleteTransitionAsync(targetContext, transition, context.ChildContext); + await RunTransitionActionAsync(targetContext, transition, context.ChildContext); } private async ValueTask CompleteTransitionAsync(ActivityExecutionContext context, Transition transition, ActivityExecutionContext? schedulingContext = null) @@ -207,7 +233,7 @@ public class StateMachine : Activity await context.ScheduleActivityAsync(activity, options); } - private async Task CancelCompetingTriggersAsync(ActivityExecutionContext context, Transition winningTransition, ActivityExecutionContext winningTriggerContext) + private async Task CancelCompetingTriggersAsync(ActivityExecutionContext context, Transition winningTransition, ActivityExecutionContext? winningTriggerContext) { var competingTriggerIds = GetOutboundTransitions(winningTransition.From) .Where(x => !ReferenceEquals(x, winningTransition)) diff --git a/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs b/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs index 9fd647e9e..612bab136 100644 --- a/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs +++ b/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs @@ -83,18 +83,19 @@ public class StateMachineTests Assert.Equal(ActivityStatus.Running, context.Status); } - [Fact(DisplayName = "StateMachine executes accepted transition action, source exit, target entry and target triggers")] + [Fact(DisplayName = "StateMachine executes source exit, accepted transition action, target entry and target triggers")] public async Task ExecutesAcceptedTransitionPath() { var context = await ExecuteAndEnterNewStateAsync(); await CompleteScheduledActivityAsync(context, _payTrigger); + Assert.True(context.HasScheduledActivity(_newExit)); + Assert.False(context.HasScheduledActivity(_payAction)); + + await CompleteScheduledActivityAsync(context, _newExit); Assert.True(context.HasScheduledActivity(_payAction)); await CompleteScheduledActivityAsync(context, _payAction); - Assert.True(context.HasScheduledActivity(_newExit)); - - await CompleteScheduledActivityAsync(context, _newExit); Assert.Equal("Paid", _stateMachine.CurrentState); Assert.Equal("Paid", context.GetProperty(CurrentStateProperty)); Assert.True(context.HasScheduledActivity(_paidEntry)); @@ -120,6 +121,80 @@ public class StateMachineTests Assert.True(context.HasScheduledActivity(_paidTrigger)); } + [Fact(DisplayName = "StateMachine evaluates a triggerless transition immediately after state entry")] + public async Task TriggerlessTransitionIsEvaluatedImmediately() + { + var payTransition = _stateMachine.Transitions.Single(x => x.Name == "Pay"); + payTransition.Trigger = null; + payTransition.Condition = null; + var context = await ExecuteAsync(); + + await CompleteScheduledActivityAsync(context, _newEntry); + + Assert.True(context.HasScheduledActivity(_newExit)); + Assert.False(context.HasScheduledActivity(_cancelTrigger)); + + await CompleteScheduledActivityAsync(context, _newExit); + await CompleteScheduledActivityAsync(context, _payAction); + + Assert.Equal("Paid", context.GetProperty(CurrentStateProperty)); + Assert.True(context.HasScheduledActivity(_paidEntry)); + } + + [Fact(DisplayName = "StateMachine schedules event triggers when triggerless transition conditions are false")] + public async Task FalseTriggerlessConditionAllowsTriggeredTransitions() + { + var payTransition = _stateMachine.Transitions.Single(x => x.Name == "Pay"); + payTransition.Trigger = null; + payTransition.Condition = new(false); + var context = await ExecuteAsync(); + + await CompleteScheduledActivityAsync(context, _newEntry); + + Assert.Equal("New", _stateMachine.CurrentState); + Assert.False(context.HasScheduledActivity(_newExit)); + Assert.True(context.HasScheduledActivity(_cancelTrigger)); + Assert.Equal(ActivityStatus.Running, context.Status); + } + + [Fact(DisplayName = "StateMachine leaves an all-false triggerless state active without rescheduling")] + public async Task AllFalseTriggerlessTransitionsDoNotCompleteOrSpin() + { + _stateMachine.Transitions.Remove(_stateMachine.Transitions.Single(x => x.Name == "Cancel")); + var payTransition = _stateMachine.Transitions.Single(x => x.Name == "Pay"); + payTransition.Trigger = null; + payTransition.Condition = new(false); + var context = await ExecuteAsync(); + + await CompleteScheduledActivityAsync(context, _newEntry); + + Assert.Equal("New", _stateMachine.CurrentState); + Assert.Equal(ActivityStatus.Running, context.Status); + Assert.False(context.HasScheduledActivity(_newExit)); + Assert.False(context.HasScheduledActivity(_payAction)); + Assert.False(context.HasScheduledActivity(_paidEntry)); + Assert.DoesNotContain(context.WorkflowExecutionContext.CompletionCallbacks, x => x.Owner == context); + } + + [Fact(DisplayName = "StateMachine self-transition executes exit, action and entry in order")] + public async Task SelfTransitionExecutesExitActionAndEntryInOrder() + { + var transition = _stateMachine.Transitions.Single(x => x.Name == "Pay"); + transition.To = "New"; + var context = await ExecuteAndEnterNewStateAsync(); + + await CompleteScheduledActivityAsync(context, _payTrigger); + Assert.True(context.HasScheduledActivity(_newExit)); + Assert.False(context.HasScheduledActivity(_payAction)); + + await CompleteScheduledActivityAsync(context, _newExit); + Assert.True(context.HasScheduledActivity(_payAction)); + + await CompleteScheduledActivityAsync(context, _payAction); + Assert.Equal("New", _stateMachine.CurrentState); + Assert.True(context.HasScheduledActivity(_newEntry)); + } + [Fact(DisplayName = "StateMachine false transition condition leaves competing triggers active")] public async Task FalseConditionLeavesCompetingTriggersActive() { @@ -242,10 +317,11 @@ public class StateMachineTests var context = await ExecuteAndEnterNewStateAsync(); await CompleteScheduledActivityAsync(context, _payTrigger); + await CompleteScheduledActivityAsync(context, _newExit); context.SetProperty(CurrentStateProperty, "Paid"); await CompleteScheduledActivityAsync(context, _payAction); - Assert.False(context.HasScheduledActivity(_newExit)); + Assert.False(context.HasScheduledActivity(_paidEntry)); } private async Task ExecuteAndEnterNewStateAsync()