SemianiakaVY/elsa-core
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

Improves Flowchart activity robustness (#6938)

Browse source 
* Bump workflow base version and branch references to `3.5.2`.

* Refactor flowchart tests and `Flowchart` activity for improved readability and consistency, alongside minor code cleanup.

* Adds comment for clarity.

Adds a comment to explain the continue statement within the flowchart execution logic.
This improves code readability and maintainability.

* Refactor flowchart test to remove unused cases, update switch behavior, and adjust expected output for improved consistency and clarity.
This commit is contained in:
Sipke Schoorstra 2025-10-01 15:01:22 +02:00 committed by GitHub
parent d18809baa1
commit c6974a4e34
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 125 additions and 90 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs
View file

@ -151,7 +151,7 @@ public class Flowchart : Container
if (flowchartContext.Activity != this)
{
throw new Exception("Target context activity must be this flowchart");
throw new("Target context activity must be this flowchart");
}
// If the completed activity's status is anything but "Completed", do not schedule its outbound activities.
@ -167,16 +167,16 @@ public class Flowchart : Container
return;
}
// Determine the outcomes from the completed activity
// Determine the outcomes from the completed activity.
var outcomes = result is Outcomes o ? o : Outcomes.Default;
// Schedule the outbound activities
var flowGraph = GetFlowGraph(flowchartContext);
var flowScope = GetFlowScope(flowchartContext);
var completedActivityExcecutedByBackwardConnection = completedActivityContext.ActivityInput.GetValueOrDefault<bool>(BackwardConnectionActivityInput);
bool hasScheduledActivity = await ScheduleOutboundActivitiesAsync(flowGraph, flowScope, flowchartContext, completedActivity, outcomes, completedActivityExcecutedByBackwardConnection);
var hasScheduledActivity = await ScheduleOutboundActivitiesAsync(flowGraph, flowScope, flowchartContext, completedActivity, outcomes, completedActivityExcecutedByBackwardConnection);
// If there are not any outbound connections, complete the flowchart activity if there is no other pending work
// If there are not any outbound connections, complete the flowchart activity if there is no other pending work.
if (!hasScheduledActivity)
{
await CompleteIfNoPendingWorkAsync(flowchartContext);
@ -198,12 +198,12 @@ public class Flowchart : Container
/// <returns>True if at least one activity was scheduled; otherwise, false.</returns>
private async ValueTask<bool> ScheduleOutboundActivitiesAsync(FlowGraph flowGraph, FlowScope flowScope, ActivityExecutionContext flowchartContext, IActivity activity, Outcomes outcomes, bool completedActivityExecutedByBackwardConnection = false)
{
bool hasScheduledActivity = false;
var hasScheduledActivity = false;
// Check if the activity is dangling (i.e., it is not reachable from the flowchart graph)
if (flowGraph.IsDanglingActivity(activity))
{
throw new Exception($"Activity {activity.Id} is not reachable from the flowchart graph. Unable to schedule it's outbound activities.");
throw new($"Activity {activity.Id} is not reachable from the flowchart graph. Unable to schedule it's outbound activities.");
}
// Register the activity as visited unless it was executed due to a backward connection
@ -215,12 +215,16 @@ public class Flowchart : Container
// Process each outbound connection from the current activity
foreach (var outboundConnection in flowGraph.GetOutboundConnections(activity))
{
bool connectionFollowed = outcomes.Names.Contains(outboundConnection.Source.Port);
var connectionFollowed = outcomes.Names.Contains(outboundConnection.Source.Port);
flowScope.RegisterConnectionVisit(outboundConnection, connectionFollowed);
if(!connectionFollowed)
continue; // Skip if connection was not followed.
var outboundActivity = outboundConnection.Target.Activity;
// Determine scheduling strategy based on connection type
if (flowGraph.IsBackwardConnection(outboundConnection, out bool backwardConnectionIsValid))
// Determine the scheduling strategy based on connection-type.
if (flowGraph.IsBackwardConnection(outboundConnection, out var backwardConnectionIsValid))
{
hasScheduledActivity |= await ScheduleBackwardConnectionActivityAsync(flowGraph, flowchartContext, outboundConnection, outboundActivity, connectionFollowed, backwardConnectionIsValid);
}
@ -248,7 +252,7 @@ public class Flowchart : Container
if (!backwardConnectionIsValid)
{
throw new Exception($"Invalid backward connection: Every path from the source ('{outboundConnection.Source.Activity.Id}') must go through the target ('{outboundConnection.Target.Activity.Id}') when tracing back to the start.");
throw new($"Invalid backward connection: Every path from the source ('{outboundConnection.Source.Activity.Id}') must go through the target ('{outboundConnection.Target.Activity.Id}') when tracing back to the start.");
}
var scheduleWorkOptions = new ScheduleWorkOptions
@ -395,7 +399,7 @@ public class Flowchart : Container
if (activityExecutionContext != null)
{
await flowchartContext.ScheduleActivityAsync(activityExecutionContext.Activity, new ScheduleWorkOptions
await flowchartContext.ScheduleActivityAsync(activityExecutionContext.Activity, new()
{
ExistingActivityExecutionContext = activityExecutionContext,
CompletionCallback = OnChildCompletedAsync,
@ -404,7 +408,7 @@ public class Flowchart : Container
}
else
{
await flowchartContext.ScheduleActivityAsync(activity, new ScheduleWorkOptions
await flowchartContext.ScheduleActivityAsync(activity, new()
{
CompletionCallback = OnChildCompletedAsync,
Input = signal.Input

187
test/integration/Elsa.Workflows.IntegrationTests/Scenarios/FlowchartNextActivity/Tests.cs
View file

@ -32,7 +32,10 @@ public class FlowchartNextActivityTests
await _services.PopulateRegistriesAsync();
await _workflowRunner.RunAsync<FlowchartWorkflow>();
var lines = _capturingTextWriter.Lines.ToList();
Assert.Equal(new[] { "Line 1" }, lines);
Assert.Equal(new[]
{
"Line 1"
}, lines);
}
[Fact(DisplayName = "Flowchart with backward connections and a dangling activity")]
@ -46,15 +49,14 @@ public class FlowchartNextActivityTests
var dangling = new WriteLine("dangling");
var writeLineDecision = new FlowSwitch()
{
Cases = {
new FlowSwitchCase("LessThanThree", new Expression("JavaScript", "getVariable('LoopCount') < 3")),
new FlowSwitchCase("LessThanOne", new Expression("JavaScript", "getVariable('LoopCount') < 1")),
},
Cases =
{
new FlowSwitchCase("LessThanThree", new Expression("JavaScript", "getVariable('LoopCount') < 3"))
},
Mode = new(SwitchMode.MatchAny)
};
var a = new WriteLine("A");
var b = new WriteLine("B");
var c = new WriteLine("C");
var incrementLoop = new SetVariable()
{
Variable = loopVariable,
@ -62,56 +64,54 @@ public class FlowchartNextActivityTests
};
var loopbackDecision = new FlowSwitch()
{
Cases = {
Cases =
{
new FlowSwitchCase("EqualOne", new Expression("JavaScript", "getVariable('LoopCount') == 1")),
new FlowSwitchCase("LessThanFour", new Expression("JavaScript", "getVariable('LoopCount') < 4")),
},
Mode = new(SwitchMode.MatchFirst)
new FlowSwitchCase("EqualThree", new Expression("JavaScript", "getVariable('LoopCount') == 3")),
},
Mode = new(SwitchMode.MatchAny)
};
var d = new WriteLine("D");
var e = new WriteLine("E");
var f = new WriteLine("F");
var end = new End();
workflowBuilder.Root = new Flowchart
{
Variables =
{
loopVariable
},
{
loopVariable
},
Activities =
{
start,
dangling,
writeLineDecision,
a,
b,
c,
incrementLoop,
loopbackDecision,
d,
e,
f,
end
},
{
start,
dangling,
writeLineDecision,
a,
b,
incrementLoop,
loopbackDecision,
d,
e,
f,
end
},
Connections =
{
new(start, writeLineDecision),
new(dangling, writeLineDecision),
new(new Endpoint(writeLineDecision, "LessThanThree"), new Endpoint(a)),
new(new Endpoint(writeLineDecision, "LessThanThree"), new Endpoint(b)),
new(new Endpoint(writeLineDecision, "LessThanOne"), new Endpoint(c)),
new(new Endpoint(writeLineDecision, "Default"), new Endpoint(incrementLoop)),
new(a, incrementLoop),
new(b, incrementLoop),
new(c, incrementLoop),
new(incrementLoop, loopbackDecision),
new(new Endpoint(loopbackDecision, "EqualOne"), new Endpoint(d)),
new(d, incrementLoop),
new(new Endpoint(loopbackDecision, "LessThanFour"), new Endpoint(e)),
new(e, writeLineDecision),
new(new Endpoint(loopbackDecision, "Default"), new Endpoint(f)),
new(new Endpoint(loopbackDecision, "EqualThree"), new Endpoint(f)),
new(f, end),
}
};
@ -121,7 +121,10 @@ public class FlowchartNextActivityTests
var result = await _workflowRunner.RunAsync(workflow);
var lines = _capturingTextWriter.Lines.ToList();
Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus);
Assert.Equal(new[] { "A", "B", "C", "D", "E", "A", "B", "E", "F" }, lines);
Assert.Equal(new[]
{
"A", "B", "D", "E", "A", "B", "E", "E", "F"
}, lines);
}
[Fact(DisplayName = "Flowchart with an invalid backward connection")]
@ -129,24 +132,42 @@ public class FlowchartNextActivityTests
{
var workflow = new TestWorkflow(workflowBuilder =>
{
var start = new Start() { Id = "Start" };
var a = new WriteLine("A") { Id = "WriteLineA" };
var b = new WriteLine("B") { Id = "WriteLineB" };
var c = new WriteLine("C") { Id = "WriteLineC" };
var d = new WriteLine("D") { Id = "WriteLineD" };
var e = new WriteLine("E") { Id = "WriteLineE" };
var start = new Start()
{
Id = "Start"
};
var a = new WriteLine("A")
{
Id = "WriteLineA"
};
var b = new WriteLine("B")
{
Id = "WriteLineB"
};
var c = new WriteLine("C")
{
Id = "WriteLineC"
};
var d = new WriteLine("D")
{
Id = "WriteLineD"
};
var e = new WriteLine("E")
{
Id = "WriteLineE"
};
workflowBuilder.Root = new Flowchart
{
Activities =
{
start,
a,
b,
c,
d,
e,
},
{
start,
a,
b,
c,
d,
e,
},
Connections =
{
new(start, a),
@ -166,7 +187,10 @@ public class FlowchartNextActivityTests
Assert.Equal(WorkflowSubStatus.Faulted, result.WorkflowState.SubStatus);
Assert.Equal(1, result.WorkflowState.Incidents.Count());
Assert.Equal("Invalid backward connection: Every path from the source ('WriteLineE') must go through the target ('WriteLineC') when tracing back to the start.", result.WorkflowState.Incidents.First().Message);
Assert.Equal(new[] { "A", "B", "C", "D", "E" }, lines);
Assert.Equal(new[]
{
"A", "B", "C", "D", "E"
}, lines);
}
[Theory(DisplayName = "Flowchart with a Join activity executed multiple times")]
@ -194,7 +218,8 @@ public class FlowchartNextActivityTests
};
var loopbackDecision = new FlowSwitch()
{
Cases = {
Cases =
{
new FlowSwitchCase("LessThanThree", new Expression("JavaScript", "getVariable('LoopCount') < 3")),
},
Mode = new(SwitchMode.MatchFirst)
@ -205,21 +230,21 @@ public class FlowchartNextActivityTests
workflowBuilder.Root = new Flowchart
{
Variables =
{
loopVariable
},
{
loopVariable
},
Activities =
{
start,
a,
b,
c,
d,
join,
incrementLoop,
loopbackDecision,
end
},
{
start,
a,
b,
c,
d,
join,
incrementLoop,
loopbackDecision,
end
},
Connections =
{
new(start, a),
@ -230,7 +255,7 @@ public class FlowchartNextActivityTests
new(c, join),
new(d, join),
new(join, incrementLoop),
new(incrementLoop,loopbackDecision),
new(incrementLoop, loopbackDecision),
new(new Endpoint(loopbackDecision, "LessThanThree"), new Endpoint(a)),
new(new Endpoint(loopbackDecision, "Default"), new Endpoint(end)),
}
@ -241,8 +266,10 @@ public class FlowchartNextActivityTests
var result = await _workflowRunner.RunAsync(workflow);
var lines = _capturingTextWriter.Lines.ToList();
Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus);
Assert.Equal(new[] { "A", "B", "C", "D", "A", "B", "C", "D", "A", "B", "C", "D"}, lines);
Assert.Equal(new[]
{
"A", "B", "C", "D", "A", "B", "C", "D", "A", "B", "C", "D"
}, lines);
}
[Theory(DisplayName = "Flowchart with a Join activity executed multiple times, bug 6479")]
@ -257,7 +284,8 @@ public class FlowchartNextActivityTests
var start = new Start();
var loopbackSwitch = new FlowSwitch()
{
Cases = {
Cases =
{
new FlowSwitchCase("DoLoopback", new Expression("JavaScript", "getVariable('LoopCount') < 3")),
},
Mode = new(SwitchMode.MatchFirst)
@ -279,19 +307,19 @@ public class FlowchartNextActivityTests
workflowBuilder.Root = new Flowchart
{
Variables =
{
loopVariable
},
{
loopVariable
},
Activities =
{
start,
loopbackSwitch,
a,
incrementLoop,
join,
b,
end
},
{
start,
loopbackSwitch,
a,
incrementLoop,
join,
b,
end
},
Connections =
{
new(start, loopbackSwitch),
@ -310,6 +338,9 @@ public class FlowchartNextActivityTests
var result = await _workflowRunner.RunAsync(workflow);
var lines = _capturingTextWriter.Lines.ToList();
Assert.Equal(WorkflowSubStatus.Finished, result.WorkflowState.SubStatus);
Assert.Equal(new[] { "A", "A", "A", "B" }, lines);
Assert.Equal(new[]
{
"A", "A", "A", "B"
}, lines);
}
}