diff --git a/src/Test/TestCases.Workflows/Bpm/FlowchartTestExtensions.cs b/src/Test/TestCases.Workflows/Bpm/FlowchartTestExtensions.cs index f007626e..ed2a585d 100644 --- a/src/Test/TestCases.Workflows/Bpm/FlowchartTestExtensions.cs +++ b/src/Test/TestCases.Workflows/Bpm/FlowchartTestExtensions.cs @@ -1,6 +1,7 @@ using System.Activities; using System.Activities.Statements; using System; +using System.Linq; namespace TestCases.Activitiess.Bpm; @@ -8,21 +9,16 @@ public static class FlowchartTestExtensions { public static FlowSplit AddBranches(this FlowSplit split, params Activity[] nodes) { - foreach(var node in nodes) - { - var branch = FlowSplitBranch.New(split); - branch.StartNode = new FlowStep() { Action = node, Next = branch.StartNode }; - split.Branches.Add(branch); - } - return split; + return split.AddBranches(nodes.Select(a => new FlowStep { Action = a }).ToArray()); } public static FlowSplit AddBranches(this FlowSplit split, params FlowNode[] nodes) { foreach (var node in nodes) { - var branch = FlowSplitBranch.New(split); - node.FlowTo(split.MergeNode); - branch.StartNode = node; + var branch = new FlowSplitBranch() + { + StartNode = node, + }; split.Branches.Add(branch); } return split; @@ -31,6 +27,10 @@ public static FlowStep Step(this Activity activity) { return new FlowStep { Action = activity }; } + public static FlowMerge Merge(this Activity activity) + { + return new () { Next = new FlowStep() { Action = activity } }; + } public static FlowStep FlowTo(this Activity predeccessor, FlowNode successor) { @@ -40,6 +40,19 @@ public static FlowStep FlowTo(this Activity predeccessor, Activity successor) { return new FlowStep { Action = predeccessor }.FlowTo(successor); } + public static FlowMerge MergeTo(this FlowSplit split, FlowNode successor) + { + var merge = new FlowMerge() { Next = successor }; + foreach (var branch in split.Branches) + { + branch.StartNode.FlowTo(merge); + } + return merge; + } + public static FlowMerge MergeTo(this FlowSplit split, Activity successor) + { + return split.MergeTo(new FlowStep { Action = successor }); + } public static T FlowTo(this T predeccessor, Activity successor) where T : FlowNode { @@ -59,7 +72,10 @@ public static T FlowTo(this T predeccessor, FlowNode successor) (join.Next ??= successor).FlowTo(successor); break; case FlowSplit split: - (split.MergeNode.Next ??= successor).FlowTo(successor); + foreach (var branch in split.Branches) + { + branch.StartNode.FlowTo(successor); + } break; case FlowDecision decision: (decision.True ??= successor).FlowTo(successor); diff --git a/src/Test/TestCases.Workflows/Bpm/SplitAndMergeTests.cs b/src/Test/TestCases.Workflows/Bpm/SplitAndMergeTests.cs index 52c444ae..321c6cf7 100644 --- a/src/Test/TestCases.Workflows/Bpm/SplitAndMergeTests.cs +++ b/src/Test/TestCases.Workflows/Bpm/SplitAndMergeTests.cs @@ -9,7 +9,7 @@ using TestObjects.XamlTestDriver; using System.IO; using System; - +using WorkflowApplicationTestExtensions; namespace TestCases.Activitiess.Bpm; public class AddStringActivity : NativeActivity @@ -25,7 +25,7 @@ protected override void Execute(NativeActivityContext context) { using var _ = context.InheritVariables(); var stringsLocation = context.GetInheritedLocation>("strings"); - stringsLocation.Value ??= new(); + stringsLocation.Value ??= []; stringsLocation.Value.Add(Item); } } @@ -37,7 +37,7 @@ public class SplitAndMergeTests private AddStringActivity AddString(string stringToAdd) { - var act = new AddStringActivity() { Item = stringToAdd }; + var act = new AddStringActivity() { Item = stringToAdd, DisplayName = stringToAdd }; return act; } @@ -50,7 +50,8 @@ private List ExecuteFlowchart(FlowNode startNode) private List ExecuteFlowchart(Flowchart flowchart) { var root = new ActivityWithResult> { Body = flowchart, In = _stringsVariable }; - return Results = WorkflowInvoker.Invoke(root); + var app = new WorkflowApplication(root); + return Results = (List) app.RunUntilCompletion().Outputs["Result"]; } [Fact] @@ -101,7 +102,7 @@ public void Should_join_branches() AddString(branch1Str), AddString(branch2Str) ); - split.MergeNode.FlowTo(AddString(stopString)); + split.MergeTo(AddString(stopString)); ExecuteFlowchart(split); Results.ShouldBe(new() { branch1Str, branch2Str, stopString }); @@ -110,21 +111,27 @@ public void Should_join_branches() [Fact] public void Should_join_branches_with_inner_split() { + /// |--A--| + /// |---A---Split< Merge--| + /// Split< |--A--| Merge---Stop + /// |___A______________________| + + var outerMerge = AddString("stop").Merge(); + var innerMerge = AddString("innerMerged").Merge(); var innerSplit = new FlowSplit() .AddBranches( - AddString("branch1Inner"), - AddString("branch2Inner") + AddString("branch1Inner").FlowTo(innerMerge), + AddString("branch2Inner").FlowTo(innerMerge) ); - innerSplit.MergeNode.FlowTo(AddString("innerMerged")); + innerMerge.FlowTo(outerMerge); var outerSplit = new FlowSplit() .AddBranches( - innerSplit, - AddString("branch2Outer").Step() + AddString("branch1Outer").FlowTo(innerSplit), + AddString("branch2Outer").Step().FlowTo(outerMerge) ); - outerSplit.MergeNode.FlowTo(AddString("stop")); ExecuteFlowchart(outerSplit); - Results.ShouldBe(new() { "branch1Inner", "branch2Inner", "innerMerged", "branch2Outer", "stop"}); + Results.ShouldBe(["branch1Outer", "branch1Inner", "branch2Inner", "innerMerged", "branch2Outer", "stop"]); } @@ -141,7 +148,7 @@ public void Should_join_with_skiped_branches() AddString(branch2Str) ); split.Branches.First().Condition = new LambdaValue(c => false); - split.MergeNode.FlowTo(AddString(stopString)); + split.MergeTo(AddString(stopString)); ExecuteFlowchart(split); Results.ShouldBe(new() { branch2Str, stopString }); @@ -159,8 +166,8 @@ public void Should_join_branches_when_condition_is_met() AddString(branch1Str).FlowTo(AddString(branch1Str)), new BlockingActivity("whatever").FlowTo(AddString(branch2Str)) ); - split.MergeNode.FlowTo(AddString(stopString)); - split.MergeNode.Completion = new LambdaValue(c => true); + split.MergeTo(AddString(stopString)) + .Completion = new LambdaValue(c => true); ExecuteFlowchart(split); Results.ShouldBe(new() { branch1Str, branch1Str, stopString }); @@ -209,7 +216,7 @@ ActivityWithResult> ParallelActivities() AddString(branch1Str).FlowTo(new FlowStep()), AddString(branch2Str).FlowTo(new FlowStep()), blockingActivity.FlowTo(blockingContinuation).FlowTo(new FlowStep())); - split.MergeNode.FlowTo(AddString(stopString)); + split.MergeTo(AddString(stopString)); var flowchart = new Flowchart { StartNode = split }; return new() { In = _stringsVariable, Body = flowchart }; } diff --git a/src/Test/TestCases.Workflows/TestCases.Workflows.csproj b/src/Test/TestCases.Workflows/TestCases.Workflows.csproj index 541bf70a..2f85e124 100644 --- a/src/Test/TestCases.Workflows/TestCases.Workflows.csproj +++ b/src/Test/TestCases.Workflows/TestCases.Workflows.csproj @@ -4,6 +4,7 @@ + diff --git a/src/UiPath.Workflow.Runtime/Statements/FlowMerge.cs b/src/UiPath.Workflow.Runtime/Statements/FlowMerge.cs index 71d1b83e..de5f35dc 100644 --- a/src/UiPath.Workflow.Runtime/Statements/FlowMerge.cs +++ b/src/UiPath.Workflow.Runtime/Statements/FlowMerge.cs @@ -1,28 +1,16 @@ -using System.Linq; +using System.Activities.Validation; +using System.Linq; namespace System.Activities.Statements; public class FlowMerge : FlowNode { - private FlowSplit _split; [DefaultValue(null)] public Activity Completion { get; set; } [DefaultValue(null)] public FlowNode Next { get; set; } - public FlowSplit SplitNode - { - get => _split; - init - { - if (value?.MergeNode is { } merge && merge != this) - throw new InvalidOperationException("Split and merge must be linked both ways."); - _split = value; - } - } - internal override Activity ChildActivity => Completion; - - private List ConnectedBranches { get; set; } + private List ConnectedBranches { get; set; } private record MergeState { @@ -37,37 +25,24 @@ public FlowMerge() protected override void OnEndCacheMetadata() { var predecessors = Owner.GetPredecessors(this); - ConnectedBranches = predecessors.Select(p => Owner.GetBranch(p)).ToList(); + ConnectedBranches = predecessors.SelectMany(p => Owner.GetStaticBranches(p)).Distinct().ToList(); + var outgoingBranches = ConnectedBranches.SelectMany(b => Owner.GetStaticBranches(b.SplitNode)).Distinct().ToList(); + Owner.AddStaticBranches(this, outgoingBranches); - ValidateAllBranches(); + //ValidateAllBranches(); void ValidateAllBranches() { - HashSet visited = new() - { - this, - _split, - }; - List toVisit = new(1) { this }; - do + var splits = ConnectedBranches.Select(bl => bl.SplitNode).Distinct().ToList(); + if (splits.Count() > 1) { - var predecessors = toVisit.SelectMany(v => Owner.GetPredecessors(v)).ToList(); - toVisit = new List(predecessors.Count); - foreach (var predecessor in predecessors) - { - if (!visited.Add(predecessor)) - continue; - if (predecessor == null) - { - Metadata.AddValidationError("All join branches should start in the parent parallel node."); - continue; - } - toVisit.Add(predecessor); - } + Metadata.AddValidationError("All join branches should start in the same parallel node."); } - while (toVisit.Any()); - var allBranchesJoined = _split.Branches.All(b => visited.Contains(b.StartNode)); - if (!allBranchesJoined) + var split = splits.FirstOrDefault(); + if (split is null) + return; + var branches = ConnectedBranches.Select(b => b.RuntimeNode.Index).Distinct().ToList(); + if (branches.Count != split.Branches.Count) Metadata.AddValidationError("All parallel branches should end in same join node."); } } @@ -79,7 +54,7 @@ internal override void GetConnectedNodes(IList connections) } } - MergeState GetJoinState(Func add = null) + MergeState GetJoinState() { var key = $"{Index}"; var joinStates = Owner.GetPersistableState>("FlowMerge"); @@ -100,7 +75,11 @@ internal override void Execute(FlowNode predecessorNode) var branch = Owner.GetBranch(predecessorNode); if (!ConnectedBranches.Contains(branch)) return; - joinState.CompletedNodeIndeces.Add(branch.NodeIndex); + joinState.CompletedNodeIndeces.Add(branch.RuntimeNode.Index); + joinState.CompletedNodeIndeces + .AddRange(Owner + .GetCompletedBranches() + .Select(b => b.RuntimeNode.Index)); if (Completion is not null) { Owner.ScheduleWithCallback(Completion); @@ -114,7 +93,7 @@ protected override void OnCompletionCallback(bool result) { var joinState = GetJoinState(); var incompleteBranches = ConnectedBranches - .Select(b => b.NodeIndex) + .Select(b => b.RuntimeNode.Index) .Except(joinState.CompletedNodeIndeces).ToList(); if (result) { diff --git a/src/UiPath.Workflow.Runtime/Statements/FlowSplit.cs b/src/UiPath.Workflow.Runtime/Statements/FlowSplit.cs index 97df4842..449514d6 100644 --- a/src/UiPath.Workflow.Runtime/Statements/FlowSplit.cs +++ b/src/UiPath.Workflow.Runtime/Statements/FlowSplit.cs @@ -7,6 +7,8 @@ namespace System.Activities.Statements; public class FlowSplitBranch { + internal FlowNode RuntimeNode {get; set;} + internal FlowSplit SplitNode { get; set; } private string _displayName; public FlowNode StartNode { get; set; } @@ -25,66 +27,37 @@ public string DisplayName _displayName = value; } } - - public static FlowSplitBranch New(FlowSplit splitNode) - { - return new() - { - StartNode = splitNode.MergeNode - }; - } } public class FlowSplit : FlowNode { - private FlowMerge _merge; - public FlowMerge MergeNode - { - get => _merge; - init - { - if (value?.SplitNode is { } split && split != this) - throw new InvalidOperationException("Split and merge must be linked both ways."); - _merge = value; - } - } - [DefaultValue(null)] - public Collection Branches => _branches ??= ValidatingCollection.NullCheck(); - + public Collection Branches + => _branches ??= new ValidatingCollection + { + OnAddValidationCallback = item => + { + if (item == null) + { + throw FxTrace.Exception.ArgumentNull(nameof(item)); + } + if (item.SplitNode != null) + throw FxTrace.Exception.Argument(nameof(item), "Cannot add same branch to multiple Split nodes"); + item.SplitNode = this; + if (item.StartNode == null) + throw FxTrace.Exception.Argument(nameof(item.StartNode), "StartNode must not be null."); + } + }; private ValidatingCollection _branches; internal override Activity ChildActivity => null; - internal List RuntimeBranchesNodes { get; private set; } + private List RuntimeBranchesNodes { get; set; } - public FlowSplit() - { - MergeNode = new FlowMerge() { SplitNode = this }; - } internal override void GetConnectedNodes(IList connections) { - RuntimeBranchesNodes ??= GetRuntimeNodes(); + RuntimeBranchesNodes ??= Branches.Select(b => new StartBranch(b)).ToList(); connections.AddRange(RuntimeBranchesNodes); - List GetRuntimeNodes() - { - var result = new List(); - foreach (var splitBranch in Branches) - { - var node = (splitBranch.Condition is null) - ? splitBranch.StartNode - : new FlowDecision() - { - Condition = splitBranch.Condition, - DisplayName = splitBranch.DisplayName, - True = splitBranch.StartNode, - False = MergeNode - }; - result.Add(node); - Owner.AddBranch(node, splitBranch, this); - } - return result; - } } internal override void Execute(FlowNode predecessorNode) @@ -92,7 +65,39 @@ internal override void Execute(FlowNode predecessorNode) for (int i = RuntimeBranchesNodes.Count - 1; i >= 0; i--) { var branch = RuntimeBranchesNodes[i]; - Owner.EnqueueNodeExecution(node: branch, isNewBranch: true); + Owner.EnqueueNodeExecution(node: branch); + } + } + private class StartBranch : FlowNode + { + + public StartBranch(FlowSplitBranch flowSplitBranch) + { + FlowSplitBranch = flowSplitBranch; + } + + private FlowSplitBranch FlowSplitBranch { get; } + + internal override Activity ChildActivity => null; + + internal override void Execute(FlowNode predecessorNode) + { + Owner.StartBranch(FlowSplitBranch); + Owner.EnqueueNodeExecution(FlowSplitBranch.RuntimeNode, FlowSplitBranch); + } + + internal override void GetConnectedNodes(IList connections) + { + Owner.AddStaticBranches(this, new[] { FlowSplitBranch }); + FlowSplitBranch.RuntimeNode ??= (FlowSplitBranch.Condition is null) + ? FlowSplitBranch.StartNode + : new FlowDecision() + { + Condition = FlowSplitBranch.Condition, + DisplayName = FlowSplitBranch.DisplayName, + True = FlowSplitBranch.StartNode, + }; + connections.Add(FlowSplitBranch.RuntimeNode); } } } \ No newline at end of file diff --git a/src/UiPath.Workflow.Runtime/Statements/Flowchart.Execution.cs b/src/UiPath.Workflow.Runtime/Statements/Flowchart.Execution.cs index 1156f7ed..6b7f9c8e 100644 --- a/src/UiPath.Workflow.Runtime/Statements/Flowchart.Execution.cs +++ b/src/UiPath.Workflow.Runtime/Statements/Flowchart.Execution.cs @@ -2,6 +2,7 @@ // See LICENSE file in the project root for full license information. using System.Activities.Runtime; +using System.Activities.Validation; using System.Linq; #if DYNAMICUPDATE @@ -34,7 +35,7 @@ protected override void Execute(NativeActivityContext context) } } - private readonly Dictionary _splitBranchesByNode = new(); + private readonly Dictionary> _staticBranchesByNode = new(); private readonly Dictionary> _successors = new(); private readonly Dictionary> _predecessors = new(); private CompletionCallback _completionCallback; @@ -47,6 +48,8 @@ protected override void Execute(NativeActivityContext context) => GetPersistableState>("_nodeIndexByActivityId"); private Dictionary NodesStatesByIndex => GetPersistableState>("_nodesStatesByIndex"); + private Dictionary RuntimeBranchesState + => GetPersistableState>("_runtimeBranchesState"); private NativeActivityContext _activeContext; private bool _doNotCompleteNode; @@ -91,11 +94,11 @@ private void EndCacheMetadata(NativeActivityMetadata metadata) } void PropagateBranch(FlowNode predecessor, FlowNode successor) { - if (!_splitBranchesByNode.TryGetValue(predecessor, out var pre) - || _splitBranchesByNode.ContainsKey(successor)) + if (!_staticBranchesByNode.TryGetValue(predecessor, out var pre) + || _staticBranchesByNode.ContainsKey(successor)) return; - _splitBranchesByNode[successor] = pre; + _staticBranchesByNode[successor] = new(pre); } } private void SaveNodeActivityLink(ActivityInstance activityInstance) @@ -163,6 +166,14 @@ private NodeState GetNodeState(int index) return state; } + private NodeState GetBranchState(int index) + { + var nodesStatesByIndex = RuntimeBranchesState; + if (!nodesStatesByIndex.TryGetValue(index, out var state)) + nodesStatesByIndex[index] = state = new(); + + return state; + } internal bool Cancel(int branchNodeIndex) { @@ -188,19 +199,47 @@ internal List GetPredecessors(FlowNode node) return result?.ToList() ?? new(); } - internal void AddBranch(FlowNode node, FlowSplitBranch splitBranch, FlowSplit parentSplit) + internal void AddStaticBranches(FlowNode node, IEnumerable splitBranches) { - _splitBranchesByNode[node] = new BranchLinks + _staticBranchesByNode[node] = new(splitBranches); + } + + internal IEnumerable GetStaticBranches(FlowNode node) + { + if (_staticBranchesByNode.ContainsKey(node)) + return _staticBranchesByNode[node]; + else + return Array.Empty(); + + HashSet branches = new(); + Action act = ancestor => { - Split = parentSplit, - Branch = splitBranch, - RuntimeNode = node + if (_staticBranchesByNode.ContainsKey(ancestor)) + { + branches.AddRange(_staticBranchesByNode[ancestor]); + } + else if (ancestor is FlowMerge) + { + + } }; + + HashSet visited = new() { node }; + IEnumerable toVisit; + while ((toVisit = GetPredecessors(node).Except(visited)).Any()) + { + foreach (var ancestor in toVisit) + { + act(ancestor); + visited.Add(ancestor); + } + } + } - internal BranchLinks GetBranch(FlowNode predecessorNode) + internal FlowSplitBranch GetBranch(FlowNode predecessorNode) { - return _splitBranchesByNode[predecessorNode]; + return _staticBranchesByNode[predecessorNode].First(); } private void OnCompletionCallback(NativeActivityContext context, ActivityInstance completedInstance) { @@ -214,10 +253,11 @@ private void OnCompletionCallback(NativeActivityContext context, ActivityInst ExecuteQueue(); } - internal void EnqueueNodeExecution(FlowNode node, bool isNewBranch = false) + internal void EnqueueNodeExecution(FlowNode node, FlowSplitBranch startBranchBy = null) { if (node is null) return; + _executionQueue.Enqueue(node); } @@ -285,6 +325,15 @@ private void ExecuteNode(FlowNode node) node.Execute(previousNode); } + internal List GetCompletedBranches() + { + var completedBranches = RuntimeBranchesState + .Where(kv => kv.Value.IsCompleted) + .SelectMany(kv => _staticBranchesByNode[_reachableNodes[kv.Key]]) + .ToList(); + return completedBranches; + } + private List GetRunningMerges() { var runningNodes = _reachableNodes @@ -299,8 +348,7 @@ private List GetRunningMerges() }).ToList(); return runningNodes; } - - private void OnCurrentBranchCancelled() + private void OnCurrentBranchEnded() { var runningMerges = GetRunningMerges() .OfType(); @@ -310,6 +358,22 @@ private void OnCurrentBranchCancelled() } } + private void OnCurrentBranchCancelled() + => OnCurrentBranchEnded(); + + private class ExcecutingBranch + { + int StartedByRuntimeNodeIndex { get; set; } + string BranchId { get; set; } + } + + private class NodeInstance + { + int NodeIndex { get; set; } + string BranchId { get; set; } + int InstanceId { get; set; } + } + private class NodeState { public bool IsCancelRequested { get; set; } @@ -328,13 +392,6 @@ public void Dispose() public static IDisposable Create(Action onDispose) => new Disposable() { _onDispose = onDispose }; } - internal class BranchLinks - { - public FlowSplit Split { get; set; } - public FlowSplitBranch Branch { get; set; } - public FlowNode RuntimeNode { private get; set; } - public int NodeIndex => RuntimeNode.Index; - } internal T GetPersistableState(string key) where T: new() { var flowChartState = _flowchartState.Get(_activeContext); @@ -345,4 +402,19 @@ internal T GetPersistableState(string key) where T: new() } return (T)value; } + + internal void EndBranch(FlowSplitBranch flowSplitBranch) + { + var state = GetBranchState(flowSplitBranch.RuntimeNode.Index); + state.IsCompleted = true; + state.IsRunning = false; + + OnCurrentBranchEnded(); + } + + internal void StartBranch(FlowSplitBranch flowSplitBranch) + { + var state = GetBranchState(flowSplitBranch.RuntimeNode.Index); + state.IsRunning = true; + } }