diff --git a/Elsa.sln b/Elsa.sln index 0b50488db..66d805dc4 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -311,6 +311,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Expressions.UnitTests" EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Scheduling.UnitTests", "test\unit\Elsa.Scheduling.UnitTests\Elsa.Scheduling.UnitTests.csproj", "{9AEE941A-3F23-4E4B-9B59-1A2F57FC762D}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Common.IntegrationTests", "test\integration\Elsa.Common.IntegrationTests\Elsa.Common.IntegrationTests.csproj", "{6C451CC5-280E-475E-B95E-23ADCF0CCBFD}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -619,6 +621,10 @@ Global {9AEE941A-3F23-4E4B-9B59-1A2F57FC762D}.Debug|Any CPU.Build.0 = Debug|Any CPU {9AEE941A-3F23-4E4B-9B59-1A2F57FC762D}.Release|Any CPU.ActiveCfg = Release|Any CPU {9AEE941A-3F23-4E4B-9B59-1A2F57FC762D}.Release|Any CPU.Build.0 = Release|Any CPU + {6C451CC5-280E-475E-B95E-23ADCF0CCBFD}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {6C451CC5-280E-475E-B95E-23ADCF0CCBFD}.Debug|Any CPU.Build.0 = Debug|Any CPU + {6C451CC5-280E-475E-B95E-23ADCF0CCBFD}.Release|Any CPU.ActiveCfg = Release|Any CPU + {6C451CC5-280E-475E-B95E-23ADCF0CCBFD}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -714,6 +720,7 @@ Global {20C75446-CAE0-48D1-85D9-459541C4D8B6} = {18453B51-25EB-4317-A4B3-B10518252E92} {67E1F0CC-C436-4D71-AD2C-AE42E3AA8A0B} = {18453B51-25EB-4317-A4B3-B10518252E92} {9AEE941A-3F23-4E4B-9B59-1A2F57FC762D} = {18453B51-25EB-4317-A4B3-B10518252E92} + {6C451CC5-280E-475E-B95E-23ADCF0CCBFD} = {1B8D5897-902E-4632-8698-E89CAF3DDF54} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {D4B5CEAA-7D70-4FCB-A68E-B03FBE5E0E5E} diff --git a/src/modules/Elsa.Common/Features/DefaultFormattersFeature.cs b/src/modules/Elsa.Common/Features/DefaultFormattersFeature.cs index 97ea1f3d6..05307f011 100644 --- a/src/modules/Elsa.Common/Features/DefaultFormattersFeature.cs +++ b/src/modules/Elsa.Common/Features/DefaultFormattersFeature.cs @@ -1,3 +1,4 @@ +using System.Collections; using System.ComponentModel; using Elsa.Common.Serialization; using Elsa.Common.Services; @@ -11,6 +12,7 @@ public class DefaultFormattersFeature(IModule module) : FeatureBase(module) { public override void Configure() { + TypeDescriptor.AddAttributes(typeof(IEnumerable), new TypeConverterAttribute(typeof(EnumerableTypeConverter))); TypeDescriptor.AddAttributes(typeof(Type), new TypeConverterAttribute(typeof(TypeTypeConverter))); Module.Services.AddSingleton(); } diff --git a/src/modules/Elsa.Common/Serialization/EnumerableTypeConverter.cs b/src/modules/Elsa.Common/Serialization/EnumerableTypeConverter.cs new file mode 100644 index 000000000..cb4327df4 --- /dev/null +++ b/src/modules/Elsa.Common/Serialization/EnumerableTypeConverter.cs @@ -0,0 +1,37 @@ +using System.Collections; +using System.ComponentModel; +using System.Globalization; +using System.Text.Json; + +namespace Elsa.Common.Serialization; + +/// +/// A type converter that converts types to and from strings using JSON serialization. +/// +public class EnumerableTypeConverter : TypeConverter +{ + public override bool CanConvertTo(ITypeDescriptorContext? context, Type? destinationType) + { + return destinationType == typeof(string) || base.CanConvertTo(context, destinationType); + } + + public override object? ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType) + { + if (destinationType == typeof(string) && value is IEnumerable enumerable) + { + // string, byte[], Memory, ReadOnlyMemory, and Span types implement IEnumerable but should not be serialized as JSON arrays + if (value is string or byte[]) return value; + + var valueType = value.GetType(); + if (valueType.IsGenericType) + { + var genericTypeDef = valueType.GetGenericTypeDefinition(); + if (genericTypeDef == typeof(Memory<>) || genericTypeDef == typeof(ReadOnlyMemory<>)) + return value; + } + + return JsonSerializer.Serialize(enumerable); + } + return base.ConvertTo(context, culture, value, destinationType); + } +} diff --git a/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledCronTask.cs b/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledCronTask.cs index 349e6b4a1..c0be4ddd8 100644 --- a/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledCronTask.cs +++ b/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledCronTask.cs @@ -23,6 +23,7 @@ public class ScheduledCronTask : IScheduledTask, IDisposable private readonly SemaphoreSlim _executionSemaphore = new(1, 1); private bool _executing; private bool _cancellationRequested; + private bool _disposed; /// /// Initializes a new instance of . @@ -107,6 +108,9 @@ public class ScheduledCronTask : IScheduledTask, IDisposable _timer?.Dispose(); _timer = null; + // Check if disposed before proceeding + if (_disposed) return; + using var scope = _scopeFactory.CreateScope(); var commandSender = scope.ServiceProvider.GetRequiredService(); @@ -136,18 +140,19 @@ public class ScheduledCronTask : IScheduledTask, IDisposable finally { _executing = false; - if (acquired) + if (acquired && !_disposed) _executionSemaphore.Release(); } } - if (!cancellationToken.IsCancellationRequested) + if (!cancellationToken.IsCancellationRequested && !_disposed) Schedule(); }; } void IDisposable.Dispose() { + _disposed = true; _timer?.Dispose(); _cancellationTokenSource.Dispose(); _executionSemaphore.Dispose(); diff --git a/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledRecurringTask.cs b/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledRecurringTask.cs index 3cfd8c464..e82e39979 100644 --- a/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledRecurringTask.cs +++ b/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledRecurringTask.cs @@ -23,6 +23,7 @@ public class ScheduledRecurringTask : IScheduledTask, IDisposable private Timer? _timer; private bool _executing; private bool _cancellationRequested; + private bool _disposed; /// /// Initializes a new instance of . @@ -77,7 +78,13 @@ public class ScheduledRecurringTask : IScheduledTask, IDisposable private void SetupTimer(TimeSpan delay) { - if (delay < TimeSpan.Zero) delay = TimeSpan.FromSeconds(1); + // Handle edge cases where delay is zero or negative (e.g., due to clock drift, fast execution, or time alignment) + // Instead of silently returning, use a minimum delay to ensure the timer fires and workflow continues scheduling + if (delay <= TimeSpan.Zero) + { + _logger.LogWarning("Calculated delay is {Delay} which is not positive. Using minimum delay of 1ms to ensure timer fires", delay); + delay = TimeSpan.FromMilliseconds(1); + } _timer = new(delay.TotalMilliseconds) { @@ -86,58 +93,59 @@ public class ScheduledRecurringTask : IScheduledTask, IDisposable _timer.Elapsed += async (_, _) => { - try + _timer?.Dispose(); + _timer = null; + + // Check if disposed before proceeding + if (_disposed) return; + + _startAt = _systemClock.UtcNow + _interval; + + using var scope = _scopeFactory.CreateScope(); + var commandSender = scope.ServiceProvider.GetRequiredService(); + + // Check disposed again before accessing CancellationTokenSource + if (_disposed) return; + + var cancellationToken = _cancellationTokenSource.Token; + if (!cancellationToken.IsCancellationRequested) { - - _timer?.Dispose(); - _timer = null; - _startAt = _systemClock.UtcNow + _interval; - - using var scope = _scopeFactory.CreateScope(); - var commandSender = scope.ServiceProvider.GetRequiredService(); - var cancellationToken = _cancellationTokenSource.Token; - if (!cancellationToken.IsCancellationRequested) + var acquired = false; + try { - var acquired = false; - try - { - acquired = await _executionSemaphore.WaitAsync(0, cancellationToken); - if (!acquired) return; - _executing = true; - await commandSender.SendAsync(new RunScheduledTask(_task), cancellationToken); + acquired = await _executionSemaphore.WaitAsync(0, cancellationToken); + if (!acquired) return; + _executing = true; + await commandSender.SendAsync(new RunScheduledTask(_task), cancellationToken); - if (_cancellationRequested) - { - _cancellationRequested = false; - _cancellationTokenSource.Cancel(); - } - } - catch (Exception e) + if (_cancellationRequested) { - _logger.LogError(e, "Error executing scheduled task"); - } - finally - { - _executing = false; - if (acquired) - _executionSemaphore.Release(); + _cancellationRequested = false; + _cancellationTokenSource.Cancel(); } } + catch (Exception e) + { + _logger.LogError(e, "Error executing scheduled task"); + } + finally + { + _executing = false; + if (acquired && !_disposed) + _executionSemaphore.Release(); + } + } - if (!cancellationToken.IsCancellationRequested) - Schedule(); - } - catch (ObjectDisposedException ex) - { - _logger.LogWarning(ex, "Service Provider was disposed."); - } + if (!cancellationToken.IsCancellationRequested && !_disposed) + Schedule(); }; } void IDisposable.Dispose() { - _cancellationTokenSource.Dispose(); + _disposed = true; _timer?.Dispose(); + _cancellationTokenSource.Dispose(); _executionSemaphore.Dispose(); } } \ No newline at end of file diff --git a/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledSpecificInstantTask.cs b/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledSpecificInstantTask.cs index 8d10f4e1a..71fa87535 100644 --- a/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledSpecificInstantTask.cs +++ b/src/modules/Elsa.Scheduling/ScheduledTasks/ScheduledSpecificInstantTask.cs @@ -22,6 +22,7 @@ public class ScheduledSpecificInstantTask : IScheduledTask, IDisposable private Timer? _timer; private bool _executing; private bool _cancellationRequested; + private bool _disposed; /// /// Initializes a new instance of . @@ -57,8 +58,13 @@ public class ScheduledSpecificInstantTask : IScheduledTask, IDisposable var now = _systemClock.UtcNow; var delay = _startAt - now; + // Handle edge cases where delay is zero or negative (e.g., due to clock drift, fast execution, or time alignment) + // Instead of silently returning, use a minimum delay to ensure the timer fires and workflow continues scheduling if (delay <= TimeSpan.Zero) + { + _logger.LogWarning("Calculated delay is {Delay} which is not positive. Using minimum delay of 1ms to ensure timer fires", delay); delay = TimeSpan.FromMilliseconds(1); + } _timer = new(delay.TotalMilliseconds) { @@ -70,8 +76,15 @@ public class ScheduledSpecificInstantTask : IScheduledTask, IDisposable _timer?.Dispose(); _timer = null; + // Check if disposed before proceeding + if (_disposed) return; + using var scope = _scopeFactory.CreateScope(); var commandSender = scope.ServiceProvider.GetRequiredService(); + + // Check disposed again before accessing CancellationTokenSource + if (_disposed) return; + var cancellationToken = _cancellationTokenSource.Token; if (!cancellationToken.IsCancellationRequested) { @@ -96,7 +109,7 @@ public class ScheduledSpecificInstantTask : IScheduledTask, IDisposable finally { _executing = false; - if (acquired) + if (acquired && !_disposed) _executionSemaphore.Release(); } } @@ -105,8 +118,9 @@ public class ScheduledSpecificInstantTask : IScheduledTask, IDisposable void IDisposable.Dispose() { - _cancellationTokenSource.Dispose(); + _disposed = true; _timer?.Dispose(); + _cancellationTokenSource.Dispose(); _executionSemaphore.Dispose(); } } \ No newline at end of file diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/Activities/RemoveTopElementStep.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/Activities/RemoveTopElementStep.cs new file mode 100644 index 000000000..7f6a507fe --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/Activities/RemoveTopElementStep.cs @@ -0,0 +1,13 @@ +using Elsa.Extensions; + +namespace Elsa.Workflows.ComponentTests.Scenarios.VariablesArray.Activities; + +public class RemoveTopElementStep : Activity +{ + protected override void Execute(ActivityExecutionContext context) + { + var elements = context.GetVariable("Elements"); + context.SetVariable("Elements", elements!.Skip(1).ToArray()); + context.CreateBookmark(); + } +} \ No newline at end of file diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/VariablesArrayWorkflowTests.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/VariablesArrayWorkflowTests.cs new file mode 100644 index 000000000..a54ca4b5f --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/VariablesArrayWorkflowTests.cs @@ -0,0 +1,79 @@ +using Elsa.Common.Models; +using Elsa.Expressions.Helpers; +using Elsa.Extensions; +using Elsa.Workflows.ComponentTests.Abstractions; +using Elsa.Workflows.ComponentTests.Fixtures; +using Elsa.Workflows.ComponentTests.Scenarios.VariablesArray.Workflows; +using Elsa.Workflows.Management; +using Elsa.Workflows.Models; +using Elsa.Workflows.Runtime; +using Elsa.Workflows.Runtime.Entities; +using Elsa.Workflows.Runtime.Messages; +using Elsa.Workflows.State; +using Microsoft.Extensions.DependencyInjection; + +namespace Elsa.Workflows.ComponentTests.Scenarios.VariablesArray; + +public class VariablesArrayWorkflowTests(App app) : AppComponentTest(app) +{ + [Fact(DisplayName = "Array variable is persisted across workflow runs")] + public async Task VariableIsPersistedAcrossWorkflowRuns() + { + var workflowRuntime = Scope.ServiceProvider.GetRequiredService(); + var workflowClient = await workflowRuntime.CreateClientAsync(); + var workflowInstanceStore = Scope.ServiceProvider.GetRequiredService(); + var workflowDefinitionStore = Scope.ServiceProvider.GetRequiredService(); + var bookmarkStore = Scope.ServiceProvider.GetRequiredService(); + var runAndCreateRequest = new CreateAndRunWorkflowInstanceRequest + { + WorkflowDefinitionHandle = WorkflowDefinitionHandle.ByDefinitionId(VariableArrayWorkflow.DefinitionId, VersionOptions.Latest), + }; + var runResponse = await workflowClient.CreateAndRunInstanceAsync(runAndCreateRequest); + var workflowInstanceId = runResponse.WorkflowInstanceId; + var createdBookmarks = await bookmarkStore.FindManyAsync(new() + { + WorkflowInstanceId = workflowInstanceId + }); + var bookmarks = new Stack(createdBookmarks); + var expectedElementLength = 3; + + var result = await workflowDefinitionStore.FindLastVersionAsync(new Management.Filters.WorkflowDefinitionFilter() + { + DefinitionId = VariableArrayWorkflow.DefinitionId + }, default); + + Assert.Equal(["Element 1", "Element 2", "Element 3"], result?.Variables.FirstOrDefault(v => v.Id == "elementsVariable")?.Value as IEnumerable); + + while (bookmarks.Any()) + { + var workflowInstance = await workflowInstanceStore.FindAsync(workflowInstanceId); + var workflowState = workflowInstance!.WorkflowState; + var rootWorkflowActivityExecutionContext = workflowState.ActivityExecutionContexts.Single(x => x.ParentContextId == null); + var variables = GetVariablesDictionary(rootWorkflowActivityExecutionContext); + var actualElements = variables["elementsVariable"].ConvertTo(); + Assert.Equal(--expectedElementLength, actualElements?.Length); + + var bookmark = bookmarks.Pop(); + var runRequest = new RunWorkflowInstanceRequest + { + BookmarkId = bookmark.Id, + }; + + await workflowClient.RunInstanceAsync(runRequest); + + createdBookmarks = await bookmarkStore.FindManyAsync(new() + { + WorkflowInstanceId = workflowInstanceId + }); + + foreach (var newBookmark in createdBookmarks) + bookmarks.Push(newBookmark); + } + Assert.Equal(0, expectedElementLength); + } + + private VariablesDictionary GetVariablesDictionary(ActivityExecutionContextState context) + { + return context.Properties.GetOrAdd(WorkflowInstanceStorageDriver.VariablesDictionaryStateKey, () => new VariablesDictionary()); + } +} \ No newline at end of file diff --git a/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/Workflows/VariableArrayWorkflow.cs b/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/Workflows/VariableArrayWorkflow.cs new file mode 100644 index 000000000..e82046ae4 --- /dev/null +++ b/test/component/Elsa.Workflows.ComponentTests/Scenarios/VariablesArray/Workflows/VariableArrayWorkflow.cs @@ -0,0 +1,34 @@ +using Elsa.Extensions; +using Elsa.Workflows.Activities; +using Elsa.Workflows.ComponentTests.Scenarios.VariablesArray.Activities; + +namespace Elsa.Workflows.ComponentTests.Scenarios.VariablesArray.Workflows; + +public class VariableArrayWorkflow : WorkflowBase +{ + public static readonly string DefinitionId = Guid.NewGuid().ToString(); + + protected override void Build(IWorkflowBuilder builder) + { + builder.WithDefinitionId(DefinitionId); + var elements = builder.WithVariable("Elements", ["Element 1", "Element 2", "Element 3"]).WithWorkflowStorage(); + + builder.Root = new Sequence + { + Activities = + { + new While(context => elements.Get(context)!.Length > 0) + { + Body = new Sequence + { + Activities = + { + new WriteLine(context => $"Top Element: {elements.Get(context)![0]}"), + new RemoveTopElementStep() + } + } + } + } + }; + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Common.IntegrationTests/Elsa.Common.IntegrationTests.csproj b/test/integration/Elsa.Common.IntegrationTests/Elsa.Common.IntegrationTests.csproj new file mode 100644 index 000000000..a3f092e77 --- /dev/null +++ b/test/integration/Elsa.Common.IntegrationTests/Elsa.Common.IntegrationTests.csproj @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/test/integration/Elsa.Common.IntegrationTests/Serialization/EnumerableTypeConverterTests.cs b/test/integration/Elsa.Common.IntegrationTests/Serialization/EnumerableTypeConverterTests.cs new file mode 100644 index 000000000..0cc3efe7a --- /dev/null +++ b/test/integration/Elsa.Common.IntegrationTests/Serialization/EnumerableTypeConverterTests.cs @@ -0,0 +1,52 @@ +using Elsa.Common.Serialization; + +namespace Elsa.Common.IntegrationTests.Serialization; + +public class EnumerableTypeConverterTests +{ + [Fact(DisplayName = "String variable is not serialized as JSON array")] + public void StringVariableIsNotSerializedAsJsonArray() + { + var testString = "Hello World"; + AssertTypeConverterPreservesValue(testString); + } + + [Fact(DisplayName = "Byte array variable is not serialized as JSON array")] + public void ByteArrayVariableIsNotSerializedAsJsonArray() + { + var testByteArray = new byte[] { 0x01, 0x02, 0x03, 0x04, 0xFF }; + AssertTypeConverterPreservesValue(testByteArray); + } + + [Fact(DisplayName = "String array is serialized as JSON array")] + public void StringArrayIsSerializedAsJsonArray() + { + var testArray = new[] { "Element 1", "Element 2", "Element 3" }; + AssertTypeConverterSerializesToJson(testArray, "[\"Element 1\",\"Element 2\",\"Element 3\"]"); + } + + [Fact(DisplayName = "List is serialized as JSON array")] + public void ListIsSerializedAsJsonArray() + { + var testList = new List { 1, 2, 3 }; + AssertTypeConverterSerializesToJson(testList, "[1,2,3]"); + } + + private void AssertTypeConverterPreservesValue(T expectedValue) + { + var converter = new EnumerableTypeConverter(); + var result = converter.ConvertTo(null, null, expectedValue, typeof(string)); + + Assert.Equal(expectedValue, result); + Assert.IsType(result); + } + + private void AssertTypeConverterSerializesToJson(T value, string expectedJson) + { + var converter = new EnumerableTypeConverter(); + var result = converter.ConvertTo(null, null, value, typeof(string)); + + Assert.IsType(result); + Assert.Equal(expectedJson, result); + } +} diff --git a/test/integration/Elsa.Common.IntegrationTests/Usings.cs b/test/integration/Elsa.Common.IntegrationTests/Usings.cs new file mode 100644 index 000000000..c802f4480 --- /dev/null +++ b/test/integration/Elsa.Common.IntegrationTests/Usings.cs @@ -0,0 +1 @@ +global using Xunit; diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/SetGetVariablesArray/Tests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/SetGetVariablesArray/Tests.cs new file mode 100644 index 000000000..0720b0cbd --- /dev/null +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/SetGetVariablesArray/Tests.cs @@ -0,0 +1,28 @@ +using Elsa.Testing.Shared; +using Microsoft.Extensions.DependencyInjection; +using Xunit.Abstractions; + +namespace Elsa.Workflows.IntegrationTests.Scenarios.SetGetVariablesArray; + +public class Tests +{ + private readonly IWorkflowRunner _workflowRunner; + private readonly CapturingTextWriter _capturingTextWriter = new(); + private readonly IServiceProvider _services; + + public Tests(ITestOutputHelper testOutputHelper) + { + _services = new TestApplicationBuilder(testOutputHelper).WithCapturingTextWriter(_capturingTextWriter).Build(); + _services.GetRequiredService(); + _workflowRunner = _services.GetRequiredService(); + } + + [Fact(DisplayName = "Workflow can set variable")] + public async Task Test1() + { + await _services.PopulateRegistriesAsync(); + await _workflowRunner.RunAsync(); + var lines = _capturingTextWriter.Lines.ToList(); + Assert.Equal(new[] { "Line 1", "Line 2" }, lines); + } +} \ No newline at end of file diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/SetGetVariablesArray/Workflows.cs b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/SetGetVariablesArray/Workflows.cs new file mode 100644 index 000000000..2caa4d735 --- /dev/null +++ b/test/integration/Elsa.Workflows.IntegrationTests/Scenarios/SetGetVariablesArray/Workflows.cs @@ -0,0 +1,33 @@ +using Elsa.Extensions; +using Elsa.Workflows.Activities; +using Elsa.Workflows.Memory; +using Elsa.Workflows.Models; + +namespace Elsa.Workflows.IntegrationTests.Scenarios.SetGetVariablesArray; + +class SetGetVariableArrayWorkflow : WorkflowBase +{ + protected override void Build(IWorkflowBuilder workflow) + { + var variable1 = new Variable("Variable1", []); + var currentValueVariable = new Variable("CurrentValue", null!); + + workflow.Root = new Sequence + { + Variables = + { + variable1 + }, + + Activities = + { + new SetVariable(variable1, ["Line 1", "Line 2"]), + new ForEach(new Input>(variable1)) + { + CurrentValue = new Output(currentValueVariable), + Body = new WriteLine(currentValueVariable) + } + } + }; + } +} diff --git a/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledCronTaskTests.cs b/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledCronTaskTests.cs index 9bf59db61..5752968c3 100644 --- a/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledCronTaskTests.cs +++ b/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledCronTaskTests.cs @@ -1,8 +1,6 @@ using Elsa.Common; using Elsa.Mediator.Contracts; -using Elsa.Scheduling; using Elsa.Scheduling.ScheduledTasks; -using Elsa.Scheduling.Tasks; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using NSubstitute; @@ -16,8 +14,7 @@ public class ScheduledCronTaskTests : IDisposable { private const string DefaultCronExpression = "0 */5 * * * *"; private static readonly DateTimeOffset DefaultNow = new(2025, 11, 06, 22, 50, 00, 0, TimeSpan.Zero); - - private readonly ServiceCollection _services; + private readonly ServiceProvider _serviceProvider; private readonly ISystemClock _systemClock; private readonly ICronParser _cronParser; @@ -26,13 +23,13 @@ public class ScheduledCronTaskTests : IDisposable public ScheduledCronTaskTests() { - _services = new ServiceCollection(); + var services = new ServiceCollection(); _systemClock = Substitute.For(); _cronParser = Substitute.For(); _logger = Substitute.For>(); - _services.AddSingleton(Substitute.For()); - _serviceProvider = _services.BuildServiceProvider(); + services.AddSingleton(Substitute.For()); + _serviceProvider = services.BuildServiceProvider(); } private ScheduledCronTask CreateScheduledTask( @@ -137,7 +134,7 @@ public class ScheduledCronTaskTests : IDisposable // Act - This should not crash and should set up a timer with minimum delay CreateScheduledTask(); - // Assert - Should call GetNextOccurrence twice (initial + retry) and log warning + // Assert - Should call GetNextOccurrence twice (initial + retry) and log warning once (on final attempt) _cronParser.Received(2).GetNextOccurrence(DefaultCronExpression); AssertWarningLogged(); } @@ -180,10 +177,13 @@ public class ScheduledCronTaskTests : IDisposable public void Dispose() { + // Dispose tasks first to stop timers before disposing ServiceProvider foreach (var task in _tasksToDispose) { ((IDisposable)task).Dispose(); } + // Small delay to ensure any timer callbacks have completed + Thread.Sleep(10); _serviceProvider.Dispose(); } } diff --git a/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledRecurringTaskTests.cs b/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledRecurringTaskTests.cs new file mode 100644 index 000000000..5fc93d315 --- /dev/null +++ b/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledRecurringTaskTests.cs @@ -0,0 +1,177 @@ +using Elsa.Common; +using Elsa.Mediator.Contracts; +using Elsa.Scheduling.ScheduledTasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NSubstitute; + +namespace Elsa.Scheduling.UnitTests.ScheduledTasks; + +/// +/// Tests for ScheduledRecurringTask to ensure recurring tasks handle edge cases correctly. +/// +public class ScheduledRecurringTaskTests : IDisposable +{ + private static readonly DateTimeOffset DefaultNow = new(2025, 11, 06, 22, 50, 00, 0, TimeSpan.Zero); + private static readonly TimeSpan DefaultInterval = TimeSpan.FromMinutes(5); + + private readonly ServiceProvider _serviceProvider; + private readonly ISystemClock _systemClock; + private readonly ILogger _logger; + private readonly List _tasksToDispose = new(); + + public ScheduledRecurringTaskTests() + { + var services = new ServiceCollection(); + _systemClock = Substitute.For(); + _logger = Substitute.For>(); + + services.AddSingleton(Substitute.For()); + _serviceProvider = services.BuildServiceProvider(); + } + + private ScheduledRecurringTask CreateScheduledTask( + DateTimeOffset? startAt = null, + TimeSpan? interval = null, + ISystemClock? systemClock = null) + { + var task = Substitute.For(); + var scheduledTask = new ScheduledRecurringTask( + task, + startAt ?? DefaultNow.AddMinutes(5), + interval ?? DefaultInterval, + systemClock ?? _systemClock, + _serviceProvider.CreateScope().ServiceProvider.GetRequiredService(), + _logger + ); + _tasksToDispose.Add(scheduledTask); + return scheduledTask; + } + + private void SetupSystemClock(params DateTimeOffset[] times) + { + _systemClock.UtcNow.Returns(times[0], times.Skip(1).ToArray()); + } + + private void AssertNoErrorLogged() + { + _logger.DidNotReceive().Log( + LogLevel.Error, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + private void AssertWarningLogged(int expectedCount = 1) + { + _logger.Received(expectedCount).Log( + LogLevel.Warning, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public void Schedule_WithVerySmallDelay_ShouldStillSetupTimer() + { + // Arrange - simulate a case where the delay is very small (1 tick = 100ns) + SetupSystemClock(DefaultNow); + var startAt = DefaultNow.AddTicks(1); // Only 1 tick in the future (100 nanoseconds) + + // Act + CreateScheduledTask(startAt: startAt); + + // Assert - Verify that no error was logged (timer should be set up successfully) + AssertNoErrorLogged(); + } + + [Fact] + public void Schedule_WithZeroDelay_ShouldRetryAndSetupTimer() + { + // Arrange - simulate a case where the first call returns exactly now + // but the second call returns a proper future time + SetupSystemClock(DefaultNow, DefaultNow); + var startAt = DefaultNow; // First: delay=0 + + // Act + CreateScheduledTask(startAt: startAt); + + // Assert - System clock should be called twice (once for initial delay=0, once for retry) + _ = _systemClock.Received(2).UtcNow; + } + + [Fact] + public void Schedule_WithNegativeDelay_ShouldRetryAndSetupTimer() + { + // Arrange - simulate a case where the first call returns a time in the past + SetupSystemClock(DefaultNow, DefaultNow); + var startAt = DefaultNow.AddMinutes(-1); // Past time + + // Act + CreateScheduledTask(startAt: startAt); + + // Assert - System clock should be called twice + _ = _systemClock.Received(2).UtcNow; + } + + [Fact] + public void Schedule_WithPersistentZeroDelay_ShouldLogWarningAndUseMinimumDelay() + { + // Arrange - simulate the bug scenario: both attempts return zero/negative delay + // This can happen if the system clock doesn't advance or if there's clock drift + SetupSystemClock(DefaultNow); + var startAt = DefaultNow; // Both calls return exactly now (delay = 0) + + // Act - This should not crash and should set up a timer with minimum delay + CreateScheduledTask(startAt: startAt); + + // Assert - Should call UtcNow twice (initial + retry) and log warning + _ = _systemClock.Received(2).UtcNow; + AssertWarningLogged(); + } + + [Fact] + public void Schedule_WithNegativeDelayAfterRetry_ShouldLogWarningAndUseMinimumDelay() + { + // Arrange - simulate a case where even after retry, delay is negative + // This could happen due to system clock adjustments + SetupSystemClock(DefaultNow, DefaultNow); + var startAt = DefaultNow.AddMilliseconds(-100); // Negative delay + + // Act - Should handle negative delay gracefully + CreateScheduledTask(startAt: startAt); + + // Assert - Should log a warning and still set up timer + AssertWarningLogged(); + } + + [Fact] + public void DisposeDuringTimerCallback_ShouldNotCrash() + { + // Arrange - set up a very short delay so timer fires quickly + SetupSystemClock(DefaultNow); + var startAt = DefaultNow; // Will use 1ms minimum delay + + // Act - Create task and immediately dispose it (simulating race condition) + var task = CreateScheduledTask(startAt: startAt); + Thread.Sleep(5); // Give timer a chance to start firing + ((IDisposable)task).Dispose(); + Thread.Sleep(10); // Give any in-flight callbacks time to complete + + // Assert - Should not crash (implicit - test passes if no exception thrown) + } + + public void Dispose() + { + // Dispose tasks first to stop timers before disposing ServiceProvider + foreach (var task in _tasksToDispose) + { + ((IDisposable)task).Dispose(); + } + // Small delay to ensure any timer callbacks have completed + Thread.Sleep(10); + _serviceProvider.Dispose(); + } +} diff --git a/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledSpecificInstantTaskTests.cs b/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledSpecificInstantTaskTests.cs new file mode 100644 index 000000000..8321d796b --- /dev/null +++ b/test/unit/Elsa.Scheduling.UnitTests/ScheduledTasks/ScheduledSpecificInstantTaskTests.cs @@ -0,0 +1,142 @@ +using Elsa.Common; +using Elsa.Mediator.Contracts; +using Elsa.Scheduling.ScheduledTasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NSubstitute; + +namespace Elsa.Scheduling.UnitTests.ScheduledTasks; + +/// +/// Tests for ScheduledSpecificInstantTask to ensure specific instant tasks handle edge cases correctly. +/// +public class ScheduledSpecificInstantTaskTests : IDisposable +{ + private static readonly DateTimeOffset DefaultNow = new(2025, 11, 06, 22, 50, 00, 0, TimeSpan.Zero); + + private readonly ServiceProvider _serviceProvider; + private readonly ISystemClock _systemClock; + private readonly ILogger _logger; + private readonly List _tasksToDispose = new(); + + public ScheduledSpecificInstantTaskTests() + { + var services = new ServiceCollection(); + _systemClock = Substitute.For(); + _logger = Substitute.For>(); + + services.AddSingleton(Substitute.For()); + _serviceProvider = services.BuildServiceProvider(); + } + + private ScheduledSpecificInstantTask CreateScheduledTask( + DateTimeOffset? startAt = null, + ISystemClock? systemClock = null) + { + var task = Substitute.For(); + var scheduledTask = new ScheduledSpecificInstantTask( + task, + startAt ?? DefaultNow.AddMinutes(5), + systemClock ?? _systemClock, + _serviceProvider.CreateScope().ServiceProvider.GetRequiredService(), + _logger + ); + _tasksToDispose.Add(scheduledTask); + return scheduledTask; + } + + private void SetupSystemClock(params DateTimeOffset[] times) + { + _systemClock.UtcNow.Returns(times[0], times.Skip(1).ToArray()); + } + + private void AssertNoErrorLogged() + { + _logger.DidNotReceive().Log( + LogLevel.Error, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + private void AssertWarningLogged(int expectedCount = 1) + { + _logger.Received(expectedCount).Log( + LogLevel.Warning, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + [Fact] + public void Schedule_WithVerySmallDelay_ShouldStillSetupTimer() + { + // Arrange - simulate a case where the delay is very small (1 tick = 100ns) + SetupSystemClock(DefaultNow); + var startAt = DefaultNow.AddTicks(1); // Only 1 tick in the future (100 nanoseconds) + + // Act + CreateScheduledTask(startAt: startAt); + + // Assert - Verify that no error was logged (timer should be set up successfully) + AssertNoErrorLogged(); + } + + [Fact] + public void Schedule_WithZeroDelay_ShouldUseMinimumDelay() + { + // Arrange - simulate a case where startAt is exactly now + SetupSystemClock(DefaultNow); + var startAt = DefaultNow; // delay = 0 + + // Act - Should adjust to 1ms minimum delay + CreateScheduledTask(startAt: startAt); + + // Assert - Should not crash and should log warning + AssertWarningLogged(); + } + + [Fact] + public void Schedule_WithNegativeDelay_ShouldUseMinimumDelay() + { + // Arrange - simulate a case where startAt is in the past + SetupSystemClock(DefaultNow); + var startAt = DefaultNow.AddMinutes(-1); // Past time + + // Act - Should adjust to 1ms minimum delay + CreateScheduledTask(startAt: startAt); + + // Assert - Should log a warning + AssertWarningLogged(); + } + + [Fact] + public void DisposeDuringTimerCallback_ShouldNotCrash() + { + // Arrange - set up a very short delay so timer fires quickly + SetupSystemClock(DefaultNow); + var startAt = DefaultNow; // Will use 1ms minimum delay + + // Act - Create task and immediately dispose it (simulating race condition) + var task = CreateScheduledTask(startAt: startAt); + Thread.Sleep(5); // Give timer a chance to start firing + ((IDisposable)task).Dispose(); + Thread.Sleep(10); // Give any in-flight callbacks time to complete + + // Assert - Should not crash (implicit - test passes if no exception thrown) + } + + public void Dispose() + { + // Dispose tasks first to stop timers before disposing ServiceProvider + foreach (var task in _tasksToDispose) + { + ((IDisposable)task).Dispose(); + } + // Small delay to ensure any timer callbacks have completed + Thread.Sleep(10); + _serviceProvider.Dispose(); + } +}