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 cfa0045ae..00d1b1e82 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/StateMachine/Activities/StateMachine.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/StateMachine/Activities/StateMachine.cs @@ -51,15 +51,13 @@ public class StateMachine : Activity /// [JsonIgnore] [Browsable(false)] - public IEnumerable Activities => - States.SelectMany(x => new[] { x.Entry, x.Exit }) - .Concat(Transitions.SelectMany(x => new[] { x.Trigger, x.Action })) - .Where(x => x != null) - .Cast(); + public IEnumerable Activities => GetActivities(); /// protected override async ValueTask ExecuteAsync(ActivityExecutionContext context) { + EnsureSupportedTriggerIdentities(); + var currentState = GetCurrentState(context); SetCurrentState(context, string.IsNullOrWhiteSpace(currentState) ? InitialState : currentState); @@ -281,6 +279,41 @@ public class StateMachine : Activity private string? GetCurrentState(ActivityExecutionContext context) => context.GetProperty(CurrentStateProperty) ?? CurrentState; + private IEnumerable GetActivities() + { + foreach (var stateActivity in States.SelectMany(x => new[] { x.Entry, x.Exit }).Where(x => x != null)) + yield return stateActivity!; + + var seenTriggerInstances = new HashSet(ReferenceEqualityComparer.Instance); + var seenTriggerIds = new HashSet(StringComparer.Ordinal); + + foreach (var transition in Transitions) + { + if (transition.Trigger != null && seenTriggerInstances.Add(transition.Trigger) && + (string.IsNullOrWhiteSpace(transition.Trigger.Id) || seenTriggerIds.Add(transition.Trigger.Id))) + yield return transition.Trigger; + + if (transition.Action != null) + yield return transition.Action; + } + } + + private void EnsureSupportedTriggerIdentities() + { + var triggers = Transitions.Where(x => x.Trigger != null).Select(x => x.Trigger!).ToList(); + var seenInstances = new HashSet(ReferenceEqualityComparer.Instance); + var seenIds = new HashSet(StringComparer.Ordinal); + + foreach (var trigger in triggers) + { + var sharesInstance = !seenInstances.Add(trigger); + var duplicatesId = !string.IsNullOrWhiteSpace(trigger.Id) && !seenIds.Add(trigger.Id); + + if (sharesInstance || duplicatesId) + throw new InvalidOperationException("StateMachine transitions cannot share a Trigger activity in Elsa 3.8. Give each transition its own trigger activity with a unique ID."); + } + } + private void SetCurrentState(ActivityExecutionContext context, string? state) { CurrentState = state; diff --git a/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs b/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs index 612bab136..640a329c1 100644 --- a/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs +++ b/test/unit/Elsa.Activities.UnitTests/StateMachine/StateMachineTests.cs @@ -201,6 +201,7 @@ public class StateMachineTests var context = await ExecuteAndEnterNewStateAsync(); _stateMachine.Transitions.Single(x => x.Name == "Pay").Condition = new(false); var cancelTriggerContext = await CreateScheduledActivityContextAsync(context, _cancelTrigger); + var cancelBookmark = cancelTriggerContext.CreateBookmark("cancel"); var scheduledPayTriggerCount = CountScheduledActivities(context, _payTrigger); await CompleteScheduledActivityAsync(context, _payTrigger); @@ -209,6 +210,7 @@ public class StateMachineTests Assert.False(context.HasScheduledActivity(_payAction)); Assert.Equal(scheduledPayTriggerCount + 1, CountScheduledActivities(context, _payTrigger)); Assert.NotEqual(ActivityStatus.Canceled, cancelTriggerContext.Status); + Assert.Contains(cancelBookmark, context.WorkflowExecutionContext.Bookmarks); } [Fact(DisplayName = "StateMachine cancels competing outbound triggers when a transition wins")] @@ -216,10 +218,32 @@ public class StateMachineTests { var context = await ExecuteAndEnterNewStateAsync(); var cancelTriggerContext = await CreateScheduledActivityContextAsync(context, _cancelTrigger); + var cancelBookmark = cancelTriggerContext.CreateBookmark("cancel"); await CompleteScheduledActivityAsync(context, _payTrigger); Assert.Equal(ActivityStatus.Canceled, cancelTriggerContext.Status); + Assert.DoesNotContain(cancelBookmark, context.WorkflowExecutionContext.Bookmarks); + } + + [Fact(DisplayName = "StateMachine rejects transitions that share a trigger instance")] + public async Task SharedTriggerInstanceIsRejected() + { + _stateMachine.Transitions.Single(x => x.Name == "Cancel").Trigger = _payTrigger; + + var exception = await Assert.ThrowsAsync(() => ExecuteAsync()); + + Assert.Contains("cannot share a Trigger activity", exception.Message); + } + + [Fact(DisplayName = "StateMachine rejects transition triggers with duplicate activity IDs")] + public async Task DuplicateTriggerIdIsRejected() + { + _cancelTrigger.Id = _payTrigger.Id; + + var exception = await Assert.ThrowsAsync(() => ExecuteAsync()); + + Assert.Contains("unique ID", exception.Message); } [Fact(DisplayName = "StateMachine removes re-armed competing triggers when a different transition wins")]