From 26389f0482e15efcfdec9569d3c3f0f002d9ed6e Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 29 May 2024 23:49:40 +0200 Subject: [PATCH 1/8] Add validation for empty cron expression This commit adds validation for cron expressions in the DefaultTriggerScheduler class. The system now checks if the cron expression provided is empty and issues a warning if that's the case. This prevents attempts to schedule triggers with an empty cron expression, which would fail. --- .../Elsa.Scheduling/Services/DefaultTriggerScheduler.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/modules/Elsa.Scheduling/Services/DefaultTriggerScheduler.cs b/src/modules/Elsa.Scheduling/Services/DefaultTriggerScheduler.cs index 0389e2f78..2426ddb31 100644 --- a/src/modules/Elsa.Scheduling/Services/DefaultTriggerScheduler.cs +++ b/src/modules/Elsa.Scheduling/Services/DefaultTriggerScheduler.cs @@ -71,6 +71,13 @@ public class DefaultTriggerScheduler : ITriggerScheduler { var payload = trigger.GetPayload(); var cronExpression = payload.CronExpression; + + if (string.IsNullOrWhiteSpace(cronExpression)) + { + _logger.LogWarning("Cron expression is empty. TriggerId: {TriggerId}. Skipping scheduling of this trigger", trigger.Id); + continue; + } + var input = new { CronExpression = cronExpression }.ToDictionary(); var request = new DispatchWorkflowDefinitionRequest { From 547ab4a1351ff4efd36b942d920a147b0ca88b2e Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 3 Jun 2024 15:05:39 +0200 Subject: [PATCH 2/8] Refactor Elsa.Workflows.Core for improved logging Logging functionality has been improved within the Elsa.Workflows.Core module. Logging has been introduced into the WorkflowRunner class, allowing for better tracking of workflow instance execution. Various debug log statements throughout the Flowchart activity have been removed or refactored to improve readability and efficiency of the code. --- .../Flowchart/Activities/Flowchart.cs | 60 ++++--------------- .../Services/WorkflowRunner.cs | 19 ++++-- 2 files changed, 25 insertions(+), 54 deletions(-) diff --git a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs index 6eaf5ba50..c839e96fd 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs @@ -20,7 +20,6 @@ namespace Elsa.Workflows.Activities.Flowchart.Activities; public class Flowchart : Container { internal const string ScopeProperty = "Scope"; - internal const string BranchMonitorsProperty = "BranchMonitoring"; /// public Flowchart([CallerFilePath] string? source = default, [CallerLineNumber] int? line = default) : base(source, line) @@ -45,75 +44,51 @@ public class Flowchart : Container /// protected override async ValueTask ScheduleChildrenAsync(ActivityExecutionContext context) { - var logger = context.GetRequiredService>(); - var startActivity = GetStartActivity(context); if (startActivity == null) { // Nothing else to execute. - logger.LogDebug("No start activity found. Completing flowchart"); await context.CompleteActivityAsync(); return; } // Schedule the start activity. - logger.LogDebug("Scheduling activity: {StartActivityId}", startActivity.Id); await context.ScheduleActivityAsync(startActivity, OnChildCompletedAsync); } private IActivity? GetStartActivity(ActivityExecutionContext context) { - var logger = context.GetRequiredService>(); - - logger.LogDebug("Looking for start activity..."); - // If there's a trigger that triggered this workflow, use that. var triggerActivityId = context.WorkflowExecutionContext.TriggerActivityId; var triggerActivity = triggerActivityId != null ? Activities.FirstOrDefault(x => x.Id == triggerActivityId) : default; if (triggerActivity != null) - { - logger.LogDebug("Found trigger activity: {TriggerActivityId}", triggerActivityId); return triggerActivity; - } // If an explicit Start activity was provided, use that. if (Start != null) - { - logger.LogDebug("An explicit start activity was provided: {StartActivityId}", Start.Id); return Start; - } // If there is a Start activity on the flowchart, use that. var startActivity = Activities.FirstOrDefault(x => x is Start); if (startActivity != null) - { - logger.LogDebug("A Start activity was found: {StartActivityId}", startActivity.Id); return startActivity; - } // If there's an activity marked as "Can Start Workflow", use that. var canStartWorkflowActivity = Activities.FirstOrDefault(x => x.GetCanStartWorkflow()); if (canStartWorkflowActivity != null) - { - logger.LogDebug("An activity marked as 'Can Start Workflow' was found: {CanStartWorkflowActivityId}", canStartWorkflowActivity.Id); return canStartWorkflowActivity; - } // If there is a single activity that has no inbound connections, use that. var root = GetRootActivity(); if (root != null) - { - logger.LogDebug("Found a single activity with no inbound connections: {ActivityId}", root.Id); return root; - } // If no start activity found, return the first activity. - logger.LogDebug("No start activity found. Using the first activity"); return Activities.FirstOrDefault(); } @@ -164,21 +139,24 @@ public class Flowchart : Container private async ValueTask OnChildCompletedAsync(ActivityCompletedContext context) { var logger = context.GetRequiredService>(); + var loggerScopeState = new Dictionary + { + ["ThreadId"] = Thread.CurrentThread.ManagedThreadId, + ["ActivityId"] = Id, + ["ActivityInstanceId"] = context.TargetContext.Id + }; + using var loggerScope = logger.BeginScope(loggerScopeState); + var flowchartContext = context.TargetContext; var completedActivityContext = context.ChildContext; var completedActivity = completedActivityContext.Activity; var result = context.Result; - logger.LogDebug("Child activity {ActivityId} completed with status {ActivityStatus}", completedActivity.Id, completedActivityContext.Status); - // If the complete activity's status is anything but "Completed", do not schedule its outbound activities. var scheduleChildren = completedActivityContext.Status == ActivityStatus.Completed; var outcomeNames = result is Outcomes outcomes ? outcomes.Names - : new[] - { - default(string), "Done" - }; + : [null!, "Done"]; // Only query the outbound connections if the completed activity wasn't already completed. var outboundConnections = Connections.Where(connection => connection.Source.Activity == completedActivity && outcomeNames.Contains(connection.Source.Port)).ToList(); @@ -190,18 +168,12 @@ public class Flowchart : Container // If the complete activity is a terminal node, complete the flowchart immediately. if (completedActivity is ITerminalNode) { - logger.LogDebug("Completed activity {ActivityId} is a terminal activity. Completing flowchart", completedActivity.Id); await flowchartContext.CompleteActivityAsync(); } else if (scheduleChildren) { if (children.Any()) { - if (children.Count == 1) - logger.LogDebug("Found 1 child for activity {ActivityId}: {ChildActivityId}", completedActivity.Id, children.First().Id); - else - logger.LogDebug("Found {Count} children for activity {ActivityId}: {ChildActivityIds}", children.Count, completedActivity.Id, children.Select(x => x.Id).ToList()); - scope.AddActivities(children); // Schedule each child, but only if all of its left inbound activities have already executed. @@ -212,7 +184,6 @@ public class Flowchart : Container // If the completed activity is not part of the left inbound path, always allow its children to be scheduled. if (!inboundActivities.Contains(completedActivity)) { - logger.LogDebug("Scheduling child activity {ChildActivityId}", activity.Id); await flowchartContext.ScheduleActivityAsync(activity, OnChildCompletedAsync); continue; } @@ -225,7 +196,6 @@ public class Flowchart : Container if (haveInboundActivitiesExecuted) { - logger.LogDebug("Scheduling child activity {ChildActivityId}", activity.Id); await flowchartContext.ScheduleActivityAsync(activity, OnChildCompletedAsync); } } @@ -241,11 +211,10 @@ public class Flowchart : Container }; if (joinContext != null) - logger.LogDebug("Next activity {ChildActivityId} is a join activity. Attaching to existing context {JoinContext}", activity.Id, joinContext.Id); + logger.LogDebug("Next activity {ChildActivityId} is a join activity. Attaching to existing join context {JoinContext}", activity.Id, joinContext.Id); else - logger.LogDebug("Next activity {ChildActivityId} is a join activity", activity.Id); - - logger.LogDebug("Scheduling child activity {ChildActivityId}", activity.Id); + logger.LogDebug("Next activity {ChildActivityId} is a join activity. Creating new join context", activity.Id); + await flowchartContext.ScheduleActivityAsync(activity, scheduleWorkOptions); } } @@ -253,7 +222,6 @@ public class Flowchart : Container if (!children.Any()) { - logger.LogDebug("No children found for activity {ActivityId}", completedActivity.Id); await CompleteIfNoPendingWorkAsync(flowchartContext); } } @@ -263,18 +231,14 @@ public class Flowchart : Container private async Task CompleteIfNoPendingWorkAsync(ActivityExecutionContext context) { - var logger = context.GetRequiredService>(); var hasPendingWork = HasPendingWork(context); if (!hasPendingWork) { - logger.LogDebug("No pending work found"); var hasFaultedActivities = context.GetActiveChildren().Any(x => x.Status == ActivityStatus.Faulted); if (!hasFaultedActivities) { - logger.LogDebug("No faulted activities found"); - logger.LogDebug("Completing flowchart"); await context.CompleteActivityAsync(); } } diff --git a/src/modules/Elsa.Workflows.Core/Services/WorkflowRunner.cs b/src/modules/Elsa.Workflows.Core/Services/WorkflowRunner.cs index bf74d0a31..266d2f8e6 100644 --- a/src/modules/Elsa.Workflows.Core/Services/WorkflowRunner.cs +++ b/src/modules/Elsa.Workflows.Core/Services/WorkflowRunner.cs @@ -6,6 +6,7 @@ using Elsa.Workflows.Models; using Elsa.Workflows.Notifications; using Elsa.Workflows.Options; using Elsa.Workflows.State; +using Microsoft.Extensions.Logging; namespace Elsa.Workflows.Services; @@ -17,7 +18,8 @@ public class WorkflowRunner( IWorkflowBuilderFactory workflowBuilderFactory, IWorkflowGraphBuilder workflowGraphBuilder, IIdentityGenerator identityGenerator, - INotificationSender notificationSender) + INotificationSender notificationSender, + ILogger logger) : IWorkflowRunner { /// @@ -54,9 +56,7 @@ public class WorkflowRunner( } /// - public async Task RunAsync( - RunWorkflowOptions? options = default, - CancellationToken cancellationToken = default) where T : WorkflowBase, new() + public async Task RunAsync(RunWorkflowOptions? options = default, CancellationToken cancellationToken = default) where T : WorkflowBase, new() { var builder = workflowBuilderFactory.CreateBuilder(); var workflow = await builder.BuildWorkflowAsync(cancellationToken); @@ -107,7 +107,7 @@ public class WorkflowRunner( var workflowGraph = await workflowGraphBuilder.BuildAsync(workflow, cancellationToken); return await RunAsync(workflowGraph, workflowState, options, cancellationToken); } - + /// public async Task RunAsync(WorkflowGraph workflowGraph, WorkflowState workflowState, RunWorkflowOptions? options = default, CancellationToken cancellationToken = default) { @@ -161,7 +161,8 @@ public class WorkflowRunner( } else if (activityInstanceId != null) { - var activityExecutionContext = workflowExecutionContext.ActivityExecutionContexts.FirstOrDefault(x => x.Id == activityInstanceId) ?? throw new Exception("No activity execution context found with the specified ID."); + var activityExecutionContext = workflowExecutionContext.ActivityExecutionContexts.FirstOrDefault(x => x.Id == activityInstanceId) ?? + throw new Exception("No activity execution context found with the specified ID."); workflowExecutionContext.ScheduleActivityExecutionContext(activityExecutionContext); } else if (workflowExecutionContext.Scheduler.HasAny) @@ -180,6 +181,12 @@ public class WorkflowRunner( /// public async Task RunAsync(WorkflowExecutionContext workflowExecutionContext) { + var workflowInstanceId = workflowExecutionContext.Id; + var logContext = new Dictionary + { + ["WorkflowInstanceId"] = workflowInstanceId + }; + using var loggingScope = logger.BeginScope(logContext); var workflow = workflowExecutionContext.Workflow; var applicationCancellationToken = workflowExecutionContext.CancellationTokens.ApplicationCancellationToken; var systemCancellationToken = workflowExecutionContext.CancellationTokens.SystemCancellationToken; From 92a60aa931a588d20d28b007ae276f97bddc7a44 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 3 Jun 2024 15:06:57 +0200 Subject: [PATCH 3/8] Disable SonarCloud analysis in packages workflow This commit disabled the setup of JDK 17, installation of SonarScanner for .NET and Coverlet, and the start and end of SonarCloud analysis in the .github/workflows/packages.yml workflow. These changes would allow the workflow to compile, test, and pack without performing the SonarCloud analysis. --- .github/workflows/packages.yml | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/.github/workflows/packages.yml b/.github/workflows/packages.yml index 009401d8e..fbcf533d1 100644 --- a/.github/workflows/packages.yml +++ b/.github/workflows/packages.yml @@ -64,25 +64,25 @@ jobs: else echo "VERSION=3.2.0-${PACKAGE_PREFIX}.${{github.run_number}}" >> $GITHUB_ENV fi - - name: Set up JDK 17 - uses: actions/setup-java@v2 - with: - java-version: '17' - distribution: 'adopt' - - name: Install SonarScanner for .NET - run: dotnet tool install --global dotnet-sonarscanner - - name: Install Coverlet for code coverage - run: dotnet tool install --global coverlet.console - - name: Begin SonarCloud analysis - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - run: dotnet sonarscanner begin /k:"elsa-workflows_elsa-core" /o:"elsa-workflows" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.token="${{ secrets.SONAR_TOKEN }}" /d:sonar.exclusions=**/obj/**,**/*.dll,build/**,samples/**,src/bundles/** /d:"sonar.verbose=true" /d:sonar.cs.opencover.reportsPaths=**/testresults/**/coverage.opencover.xml +# - name: Set up JDK 17 +# uses: actions/setup-java@v2 +# with: +# java-version: '17' +# distribution: 'adopt' +# - name: Install SonarScanner for .NET +# run: dotnet tool install --global dotnet-sonarscanner +# - name: Install Coverlet for code coverage +# run: dotnet tool install --global coverlet.console +# - name: Begin SonarCloud analysis +# env: +# SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} +# run: dotnet sonarscanner begin /k:"elsa-workflows_elsa-core" /o:"elsa-workflows" /d:sonar.host.url="https://sonarcloud.io" /d:sonar.token="${{ secrets.SONAR_TOKEN }}" /d:sonar.exclusions=**/obj/**,**/*.dll,build/**,samples/**,src/bundles/** /d:"sonar.verbose=true" /d:sonar.cs.opencover.reportsPaths=**/testresults/**/coverage.opencover.xml - name: Compile+Test+Pack run: ./build.sh Compile+Test+Pack --version ${VERSION} --analyseCode true - - name: End SonarCloud analysis - env: - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - run: dotnet sonarscanner end /d:sonar.token="${{ secrets.SONAR_TOKEN }}" +# - name: End SonarCloud analysis +# env: +# SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} +# run: dotnet sonarscanner end /d:sonar.token="${{ secrets.SONAR_TOKEN }}" - name: Upload artifact uses: actions/upload-artifact@v3 with: From 65604b65633ea5e20457d7c7a9e06207d7c59ed3 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Tue, 4 Jun 2024 09:54:27 +0200 Subject: [PATCH 4/8] Remove signal capturing phase (#5501) * Add logging to the WorkflowRunner service The WorkflowRunner service now uses the Microsoft.Extensions.Logging namespace to log the workflow execution context. These changes include passing the ILogger logger dependency through the constructor and implementing the context logging functionality in the RunAsync method. * Update error handling in FindActivityDescriptor method FindActivityDescriptor method's error handling has been updated. Now, instead of throwing exception, it returns null when an activity descriptor can't be found. Also, a logger warning has been added to indicate when this situation occurs. This change helps avoiding unexpected disruptions and improving debugging experiences. * Add ReSharper properties to .editorconfig This commit introduces specific ReSharper properties to the .editorconfig file. This update will maintain a consistent configuration of ReSharper across different development environments, intending to improve coding standard consistency. * Add thread and activity ID to Flowchart logging scope The Flowchart activity in Elsa Workflows Core module has been modified to include thread and activity ID in its logging scope. This change will provide more granular information when debugging workflow execution. Additionally, the definition for outcomeNames has been streamlined. * Add ActivityInstanceId to logger scope Added "ActivityInstanceId" as part of the logging scope dictionary in the Flowchart module. The new key records the context's target context's id for improved debugging capabilities. * Remove signal capturing functionality from workflow activities Removed the functionality related to signal capturing from the Elsa workflow activities. This refactor involves changes in core classes such as Activity, Behavior, and Flowchart and removes associated methods and handlers. This simplifies the signal handling process by only allowing activities to receive signals, eliminating the previous two-step process of capturing and receiving. * Update debugging messages in Flowchart.cs Clarified the debugging message when there's an existing join context. Removed unnecessary logging for "No pending work found", "No faulted activities found", and "Completing flowchart". This will make the debugging log less cluttered and more focused on relevant information. * Refactor logging messages in Flowchart activity The commit removes the verbose logging message indicating the completion of a terminal activity in the flowchart Context. This logging message was unnecessary and was generating excessive log messages. The log message for new join activities was also updated to accurately reflect the creation of a new join context. * Disable SonarCloud analysis from workflow The SonarCloud analysis steps, including scan setup, run and end steps have been commented out in the GitHub workflow. This is a temporary change to speed up build times while troubleshooting an issue. * Uncommented SonarCloud analysis related code in packages.yml In this commit, the parts of the code related to the set up of JDK 17, SonarScanner for .NET, Coverlet for code coverage, and SonarCloud analysis were uncommented in the GitHub Actions workflow packages.yml file. This will enable those tools and services during the execution of the workflow, improving code quality and test coverage. * Change position of root assignment comment in .editorconfig * Format method signatures in WorkflowRunner Changed the method signatures in the WorkflowRunner class to be in a single line for readability and to follow coding standards. The refactor involves three RunAsync method overloads, contributing to the overall cleanliness and readability of the source code. --- .editorconfig | 5 ++ .../Abstractions/Activity.cs | 57 +---------------- .../Abstractions/Behavior.cs | 64 +------------------ .../Contracts/ISignalHandler.cs | 5 -- .../ActivityExecutionContextExtensions.cs | 25 ++------ .../WorkflowDefinitionActivity.cs | 20 ++++-- 6 files changed, 25 insertions(+), 151 deletions(-) diff --git a/.editorconfig b/.editorconfig index 4f12c6f1c..e558c49f3 100644 --- a/.editorconfig +++ b/.editorconfig @@ -228,3 +228,8 @@ dotnet_naming_style.begins_with_i.required_prefix = I dotnet_naming_style.begins_with_i.required_suffix = dotnet_naming_style.begins_with_i.word_separator = dotnet_naming_style.begins_with_i.capitalization = pascal_case + +# ReSharper properties +resharper_max_array_initializer_elements_on_line = 50 +resharper_max_initializer_elements_on_line = 1 +resharper_wrap_array_initializer_style = chop_if_long diff --git a/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs b/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs index 7d9cebe97..8526ecb3b 100644 --- a/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs +++ b/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs @@ -17,7 +17,6 @@ namespace Elsa.Workflows; public abstract class Activity : IActivity, ISignalHandler { private readonly ICollection _signalReceivedHandlers = new List(); - private readonly ICollection _signalCapturedHandlers = new List(); /// /// Constructor. @@ -161,44 +160,6 @@ public abstract class Activity : IActivity, ISignalHandler return ValueTask.CompletedTask; }); } - - /// - /// Override this method to handle any signals sent from downstream activities. - /// - protected virtual ValueTask OnCaptureSignalAsync(object signal, SignalContext context) - { - OnSignalCaptured(signal, context); - return ValueTask.CompletedTask; - } - - /// - /// Override this method to handle any signals sent from downstream activities. - /// - protected virtual void OnSignalCaptured(object signal, SignalContext context) - { - } - - /// - /// Register a signal handler delegate. - /// - protected void OnSignalCaptured(Type signalType, Func handler) => _signalCapturedHandlers.Add(new SignalHandlerRegistration(signalType, handler)); - - /// - /// Register a signal handler delegate. - /// - protected void OnSignalCaptured(Func handler) => OnSignalCaptured(typeof(T), (signal, context) => handler((T)signal, context)); - - /// - /// Register a signal handler delegate. - /// - protected void OnSignalCaptured(Action handler) - { - OnSignalCaptured((signal, context) => - { - handler(signal, context); - return ValueTask.CompletedTask; - }); - } /// /// Notify the workflow that this activity completed. @@ -220,23 +181,7 @@ public abstract class Activity : IActivity, ISignalHandler // Invoke behaviors. foreach (var behavior in Behaviors) await behavior.ExecuteAsync(context); } - - async ValueTask ISignalHandler.CaptureSignalAsync(object signal, SignalContext context) - { - // Give derived activity a chance to do something with the signal. - await OnCaptureSignalAsync(signal, context); - - // Invoke registered signal delegates for this particular type of signal. - var signalType = signal.GetType(); - var handlers = _signalCapturedHandlers.Where(x => x.SignalType == signalType); - - foreach (var registration in handlers) - await registration.Handler(signal, context); - - // Invoke behaviors. - foreach (var behavior in Behaviors) await behavior.CaptureSignalAsync(signal, context); - } - + async ValueTask ISignalHandler.ReceiveSignalAsync(object signal, SignalContext context) { // Give derived activity a chance to do something with the signal. diff --git a/src/modules/Elsa.Workflows.Core/Abstractions/Behavior.cs b/src/modules/Elsa.Workflows.Core/Abstractions/Behavior.cs index 5d1653102..b7fc56ef9 100644 --- a/src/modules/Elsa.Workflows.Core/Abstractions/Behavior.cs +++ b/src/modules/Elsa.Workflows.Core/Abstractions/Behavior.cs @@ -7,7 +7,6 @@ namespace Elsa.Workflows; public abstract class Behavior : IBehavior { private readonly ICollection _signalReceivedHandlers = new List(); - private readonly ICollection _signalCapturedHandlers = new List(); /// /// Initializes a new instance of the class. @@ -71,54 +70,6 @@ public abstract class Behavior : IBehavior { } - /// - /// Registers a delegate to be invoked when a signal of the specified type is received. - /// - /// The type of signal to register a handler for. - /// The delegate to invoke when a signal of the specified type is received. - protected void OnSignalCaptured(Type signalType, Func handler) => _signalCapturedHandlers.Add(new SignalHandlerRegistration(signalType, handler)); - - /// - /// Registers a delegate to be invoked when a signal of the specified type is received. - /// - /// The delegate to invoke when a signal of the specified type is received. - /// The type of signal to register a handler for. - protected void OnSignalCaptured(Func handler) => OnSignalCaptured(typeof(T), (signal, context) => handler((T)signal, context)); - - /// - /// Registers a delegate to be invoked when a signal of the specified type is received. - /// - /// The delegate to invoke when a signal of the specified type is received. - /// The type of signal to register a handler for. - protected void OnSignalCaptured(Action handler) - { - OnSignalCaptured((signal, context) => - { - handler(signal, context); - return ValueTask.CompletedTask; - }); - } - - /// - /// Registers a delegate to be invoked when a signal of the specified type is received. - /// - /// The type of signal to register a handler for. - /// The signal context. - protected virtual ValueTask OnSignalCapturedAsync(object signal, SignalContext context) - { - OnSignalCaptured(signal, context); - return ValueTask.CompletedTask; - } - - /// - /// Registers a delegate to be invoked when a signal of the specified type is received. - /// - /// The signal to register a handler for. - /// The signal context. - protected virtual void OnSignalCaptured(object signal, SignalContext context) - { - } - /// /// /// @@ -137,20 +88,7 @@ public abstract class Behavior : IBehavior protected virtual void Execute(ActivityExecutionContext context) { } - - async ValueTask ISignalHandler.CaptureSignalAsync(object signal, SignalContext context) - { - // Give derived activity a chance to do something with the signal. - await OnSignalCapturedAsync(signal, context); - - // Invoke registered signal delegates for this particular type of signal. - var signalType = signal.GetType(); - var handlers = _signalCapturedHandlers.Where(x => x.SignalType == signalType); - - foreach (var registration in handlers) - await registration.Handler(signal, context); - } - + async ValueTask ISignalHandler.ReceiveSignalAsync(object signal, SignalContext context) { // Give derived activity a chance to do something with the signal. diff --git a/src/modules/Elsa.Workflows.Core/Contracts/ISignalHandler.cs b/src/modules/Elsa.Workflows.Core/Contracts/ISignalHandler.cs index e069ac35a..a1e061992 100644 --- a/src/modules/Elsa.Workflows.Core/Contracts/ISignalHandler.cs +++ b/src/modules/Elsa.Workflows.Core/Contracts/ISignalHandler.cs @@ -5,11 +5,6 @@ namespace Elsa.Workflows.Contracts; /// public interface ISignalHandler { - /// - /// Captures a signal. - /// - ValueTask CaptureSignalAsync(object signal, SignalContext context); - /// /// Receives a signal. /// diff --git a/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs b/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs index b97fa8a03..d69308d04 100644 --- a/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs +++ b/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs @@ -363,26 +363,9 @@ public static class ActivityExecutionContextExtensions public static async ValueTask SendSignalAsync(this ActivityExecutionContext context, object signal) { var receivingContexts = new[] { context }.Concat(context.GetAncestors()).ToList(); - var capturingContexts = receivingContexts.AsEnumerable().Reverse().ToList(); var logger = context.GetRequiredService>(); - - // Let all ancestors capture the signal. - foreach (var ancestorContext in capturingContexts) - { - var signalContext = new SignalContext(ancestorContext, context, context.CancellationToken); - - if (ancestorContext.Activity is not ISignalHandler handler) - continue; - - logger.LogDebug("Capturing signal {SignalType} on activity {ActivityId} of type {ActivityType}", signal.GetType().Name, ancestorContext.Activity.Id, ancestorContext.Activity.Type); - await handler.CaptureSignalAsync(signal, signalContext); - - if (signalContext.StopPropagationRequested) - { - logger.LogDebug("Propagation of signal {SignalType} on activity {ActivityId} of type {ActivityType} was stopped", signal.GetType().Name, ancestorContext.Activity.Id, ancestorContext.Activity.Type); - return; - } - } + var signalType = signal.GetType(); + var signalTypeName = signalType.Name; // Let all ancestors receive the signal. foreach (var ancestorContext in receivingContexts) @@ -392,12 +375,12 @@ public static class ActivityExecutionContextExtensions if (ancestorContext.Activity is not ISignalHandler handler) continue; - logger.LogDebug("Receiving signal {SignalType} on activity {ActivityId} of type {ActivityType}", signal.GetType().Name, ancestorContext.Activity.Id, ancestorContext.Activity.Type); + logger.LogDebug("Receiving signal {SignalType} on activity {ActivityId} of type {ActivityType}", signalTypeName, ancestorContext.Activity.Id, ancestorContext.Activity.Type); await handler.ReceiveSignalAsync(signal, signalContext); if (signalContext.StopPropagationRequested) { - logger.LogDebug("Propagation of signal {SignalType} on activity {ActivityId} of type {ActivityType} was stopped", signal.GetType().Name, ancestorContext.Activity.Id, ancestorContext.Activity.Type); + logger.LogDebug("Propagation of signal {SignalType} on activity {ActivityId} of type {ActivityType} was stopped", signalTypeName, ancestorContext.Activity.Id, ancestorContext.Activity.Type); return; } } diff --git a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs index e3572a7e5..950ce04fc 100644 --- a/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs +++ b/src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivity.cs @@ -165,10 +165,10 @@ public class WorkflowDefinitionActivity : Composite, IInitializable return workflowGraph; } - private ActivityDescriptor FindActivityDescriptor(IServiceProvider serviceProvider) + private ActivityDescriptor? FindActivityDescriptor(IServiceProvider serviceProvider) { var activityRegistry = serviceProvider.GetRequiredService(); - return activityRegistry.Find(Type, Version) ?? activityRegistry.Find(Type) ?? throw new Exception($"Could not find activity descriptor for {Type}."); + return activityRegistry.Find(Type, Version) ?? activityRegistry.Find(Type); } async ValueTask IInitializable.InitializeAsync(InitializationContext context) @@ -188,10 +188,18 @@ public class WorkflowDefinitionActivity : Composite, IInitializable var activityDescriptor = FindActivityDescriptor(serviceProvider); - // Declare input and output variables. - DeclareInputAsVariables(activityDescriptor, (_, variable) => Variables.Declare(variable)); - DeclareOutputAsVariables(activityDescriptor, (_, variable) => Variables.Declare(variable)); - + if (activityDescriptor == null) + { + var logger = serviceProvider.GetRequiredService>(); + logger.LogWarning("Could not find activity descriptor for activity type {ActivityType}", Type); + } + else + { + // Declare input and output variables. + DeclareInputAsVariables(activityDescriptor, (_, variable) => Variables.Declare(variable)); + DeclareOutputAsVariables(activityDescriptor, (_, variable) => Variables.Declare(variable)); + } + // Set the root activity. Root = workflowGraph.Workflow; } From dc2184fba413e9d18061a172d7f5cc71f61857d2 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Wed, 5 Jun 2024 18:07:48 +0200 Subject: [PATCH 5/8] Add warning for ID mismatch in workflow definitions Added a check and corresponding log warning in the 'DefaultWorkflowDefinitionStorePopulator' class for cases where an imported workflow definition has a different ID than the existing one in the store. This will help in identifying ID discrepancies that may impact workflow execution or management. Future updates may include storing these discrepancies for troubleshooting purposes. --- .../DefaultWorkflowDefinitionStorePopulator.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs index 03b89dc28..aa0208a7b 100644 --- a/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs +++ b/src/modules/Elsa.Workflows.Runtime/Services/DefaultWorkflowDefinitionStorePopulator.cs @@ -129,7 +129,17 @@ public class DefaultWorkflowDefinitionStorePopulator : IWorkflowDefinitionStoreP var workflowDefinitionsToSave = new HashSet(); if (existingDefinitionVersion != null) + { workflowDefinitionsToSave.Add(existingDefinitionVersion); + + if(existingDefinitionVersion.Id != workflow.Identity.Id) + { + // It's possible that the imported workflow definition has a different ID than the existing one in the store. + // In a future update, we might store this discrepancy in a "troubleshooting" table and provide tooling for managing these, and other, discrepancies. + // See https://github.com/elsa-workflows/elsa-core/issues/5540 + _logger.LogWarning("Workflow with ID {WorkflowId} already exists with a different ID {ExistingWorkflowId}", workflow.Identity.Id, existingDefinitionVersion.Id); + } + } await UpdateIsLatest(); await UpdateIsPublished(); From 07c889f4898ffa1498286974c3bf6ab3ffd79f81 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 6 Jun 2024 12:03:47 +0200 Subject: [PATCH 6/8] Update logger scope state in Flowchart activity The logger scope state within the Flowchart activity has been updated to use the current managed thread ID from the Environment class. Additionally, a new property 'TaskId' has been incorporated for providing the current task ID, defaulting to 'N/A' in case null. --- .../Activities/Flowchart/Activities/Flowchart.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs index c839e96fd..f927af9f0 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs @@ -141,7 +141,8 @@ public class Flowchart : Container var logger = context.GetRequiredService>(); var loggerScopeState = new Dictionary { - ["ThreadId"] = Thread.CurrentThread.ManagedThreadId, + ["ThreadId"] = Environment.CurrentManagedThreadId, + ["TaskId"] = Task.CurrentId?.ToString() ?? "N/A", ["ActivityId"] = Id, ["ActivityInstanceId"] = context.TargetContext.Id }; From b1adc28ba1d1439f7c5d307b392b5a1cc84257c5 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 6 Jun 2024 12:12:54 +0200 Subject: [PATCH 7/8] Increase default timeout in ISignalManager interface The timeouts for the WaitAsync methods in the ISignalManager interface have been increased. The new default value for these methods is now 2000 milliseconds, up from the previous 1000 milliseconds. This change will allow for more leniency in timing for workflow tests. --- .../Helpers/Contracts/ISignalManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/component/Elsa.Workflows.ComponentTests/Helpers/Contracts/ISignalManager.cs b/test/component/Elsa.Workflows.ComponentTests/Helpers/Contracts/ISignalManager.cs index f169b4423..f5b24e653 100644 --- a/test/component/Elsa.Workflows.ComponentTests/Helpers/Contracts/ISignalManager.cs +++ b/test/component/Elsa.Workflows.ComponentTests/Helpers/Contracts/ISignalManager.cs @@ -2,7 +2,7 @@ namespace Elsa.Workflows.ComponentTests; public interface ISignalManager { - Task WaitAsync(object signal, int millisecondsTimeout = 1000); - Task WaitAsync(object signal, int millisecondsTimeout = 1000); + Task WaitAsync(object signal, int millisecondsTimeout = 2000); + Task WaitAsync(object signal, int millisecondsTimeout = 2000); void Trigger(object signal, object? result = null); } \ No newline at end of file From 38cbd9662eed4df8fc11d4b79784f12c6b9bd823 Mon Sep 17 00:00:00 2001 From: Raymond den Haan Date: Thu, 6 Jun 2024 14:14:38 +0200 Subject: [PATCH 8/8] Add activity existence check in Flowchart This update prevents the creation of multiple flow activities by checking if the activity is already set to be created while the activity context has not yet been created. --- .../Activities/Flowchart/Activities/Flowchart.cs | 12 +++++++++--- .../Activities/Flowchart/Models/FlowScope.cs | 5 +++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs index f927af9f0..aa9b402de 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Activities/Flowchart.cs @@ -175,11 +175,12 @@ public class Flowchart : Container { if (children.Any()) { - scope.AddActivities(children); - // Schedule each child, but only if all of its left inbound activities have already executed. foreach (var activity in children) { + var existingActivity = scope.ContainsActivity(activity); + scope.AddActivity(activity); + var inboundActivities = Connections.LeftInboundActivities(activity).ToList(); // If the completed activity is not part of the left inbound path, always allow its children to be scheduled. @@ -213,8 +214,13 @@ public class Flowchart : Container if (joinContext != null) logger.LogDebug("Next activity {ChildActivityId} is a join activity. Attaching to existing join context {JoinContext}", activity.Id, joinContext.Id); - else + else if(!existingActivity) logger.LogDebug("Next activity {ChildActivityId} is a join activity. Creating new join context", activity.Id); + else + { + logger.LogDebug("Next activity {ChildActivityId} is a join activity. Join context was not found, but activity is already being created.", activity.Id); + continue; + } await flowchartContext.ScheduleActivityAsync(activity, scheduleWorkOptions); } diff --git a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Models/FlowScope.cs b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Models/FlowScope.cs index 0f5cb75ab..b1d0fd82d 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Models/FlowScope.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Models/FlowScope.cs @@ -45,6 +45,11 @@ internal class FlowScope return state; } + + public bool ContainsActivity(IActivity activity) + { + return Activities.ContainsKey(activity.Id); + } public void RegisterActivityExecution(IActivity activity) {