From d29df110d21e18b087774ac3cea89b6eac0eae35 Mon Sep 17 00:00:00 2001 From: gurkanguran Date: Mon, 31 Jul 2023 18:53:50 +0200 Subject: [PATCH] Fixed getting and setting variables in context (#4267) * Fixed getting and setting variables in context * Added unit tests * Add test for named variable * Search variable in parent contexts --------- Co-authored-by: Sipke Schoorstra --- Elsa.sln | 7 ++ .../ActivityExecutionContextExtensions.cs | 21 +++- .../ExpressionExecutionContextExtensions.cs | 38 ++++++- .../Scenarios/SetGetVariables/Tests.cs | 14 ++- .../Scenarios/SetGetVariables/Workflows.cs | 32 +++++- .../Elsa.Workflows.Core.UnitTests.csproj | 30 +++++ ...pressionExecutionContextExtensionsTests.cs | 103 ++++++++++++++++++ .../Elsa.Workflows.Core.UnitTests/Usings.cs | 1 + 8 files changed, 228 insertions(+), 18 deletions(-) create mode 100644 test/unit/Elsa.Workflows.Core.UnitTests/Elsa.Workflows.Core.UnitTests.csproj create mode 100644 test/unit/Elsa.Workflows.Core.UnitTests/ExpressionExecutionContextExtensionsTests.cs create mode 100644 test/unit/Elsa.Workflows.Core.UnitTests/Usings.cs diff --git a/Elsa.sln b/Elsa.sln index 9858e64e8..636ddb4d3 100644 --- a/Elsa.sln +++ b/Elsa.sln @@ -232,6 +232,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Samples.AspNet.Dynamic EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Mediator", "src\common\Elsa.Mediator\Elsa.Mediator.csproj", "{28818676-F6AF-4203-8B65-BD33A50CB9A2}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Elsa.Workflows.Core.UnitTests", "test\unit\Elsa.Workflows.Core.UnitTests\Elsa.Workflows.Core.UnitTests.csproj", "{DC9CCAD0-7363-4691-B964-FF5B3AEA3F95}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -582,6 +584,10 @@ Global {28818676-F6AF-4203-8B65-BD33A50CB9A2}.Debug|Any CPU.Build.0 = Debug|Any CPU {28818676-F6AF-4203-8B65-BD33A50CB9A2}.Release|Any CPU.ActiveCfg = Release|Any CPU {28818676-F6AF-4203-8B65-BD33A50CB9A2}.Release|Any CPU.Build.0 = Release|Any CPU + {DC9CCAD0-7363-4691-B964-FF5B3AEA3F95}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {DC9CCAD0-7363-4691-B964-FF5B3AEA3F95}.Debug|Any CPU.Build.0 = Debug|Any CPU + {DC9CCAD0-7363-4691-B964-FF5B3AEA3F95}.Release|Any CPU.ActiveCfg = Release|Any CPU + {DC9CCAD0-7363-4691-B964-FF5B3AEA3F95}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(NestedProjects) = preSolution {155227F0-A33B-40AA-A4B4-06F813EB921B} = {61017E64-6D00-49CB-9E81-5002DC8F7D5F} @@ -687,5 +693,6 @@ Global {2430CB5F-7D07-4A9E-BD40-EC1B111B85FF} = {08B41FFA-CEE3-46A7-B5C0-3EB65D37A16C} {85E13383-7C39-4719-AAC0-0B357C3A97C7} = {56C2FFB8-EA54-45B5-A095-4A78142EB4B5} {28818676-F6AF-4203-8B65-BD33A50CB9A2} = {C6658DE0-2B2F-47F0-BB61-2CA66D435C09} + {DC9CCAD0-7363-4691-B964-FF5B3AEA3F95} = {18453B51-25EB-4317-A4B3-B10518252E92} EndGlobalSection EndGlobal diff --git a/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs b/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs index 8da4fd4c4..0e60411ed 100644 --- a/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs +++ b/src/modules/Elsa.Workflows.Core/Extensions/ActivityExecutionContextExtensions.cs @@ -114,25 +114,36 @@ public static class ActivityExecutionContextExtensions } /// - /// Sets a workflow variable by name. + /// Creates a workflow variable by name and optionally sets the value. /// /// The being extended. /// The name of the variable. /// The value of the variable. - /// The type of the storage driver to use. + /// The type of storage driver to use for the variable. /// A callback to configure the memory block. /// The created . - public static Variable SetVariable(this ActivityExecutionContext context, string name, object? value, Type? storageDriverType = default, Action? configure = default) => - context.ExpressionExecutionContext.SetVariable(name, value, storageDriverType, configure); + public static Variable CreateVariable(this ActivityExecutionContext context, string name, object? value, Type? storageDriverType = default, Action? configure = default) => + context.ExpressionExecutionContext.CreateVariable(name, value, storageDriverType, configure); /// /// Sets a workflow variable by name. /// /// The being extended. /// The name of the variable. + /// The value of the variable. + /// A callback to configure the memory block. + /// The created . + public static Variable SetVariable(this ActivityExecutionContext context, string name, object? value, Action? configure = default) => + context.ExpressionExecutionContext.SetVariable(name, value, configure); + + /// + /// Gets a workflow variable by name. + /// + /// The being extended. + /// The name of the variable. /// The type of the variable. /// The variable if found, otherwise null. - public static T? GetVariable(this ActivityExecutionContext context, string name) => context.ExpressionExecutionContext.GetVariable(name); + public static T? GetVariableByName(this ActivityExecutionContext context, string name) => context.ExpressionExecutionContext.GetVariableByName(name); /// /// Returns a dictionary of variable keys and their values across scopes. diff --git a/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs b/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs index 4f7c63134..9a94a8714 100644 --- a/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs +++ b/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs @@ -4,6 +4,7 @@ using Elsa.Workflows.Core; using Elsa.Workflows.Core.Activities; using Elsa.Workflows.Core.Memory; using Elsa.Workflows.Core.Models; +using Elsa.Workflows.Core.Services; // ReSharper disable once CheckNamespace namespace Elsa.Extensions; @@ -44,21 +45,46 @@ public static class ExpressionExecutionContextExtensions public static T? Get(this ExpressionExecutionContext context, Input? input) => input != null ? context.GetBlock(input.MemoryBlockReference).Value.ConvertTo() : default; public static T? Get(this ExpressionExecutionContext context, Output output) => context.GetBlock(output.MemoryBlockReference).Value.ConvertTo(); public static object? Get(this ExpressionExecutionContext context, Output output) => context.GetBlock(output.MemoryBlockReference).Value; - public static T? GetVariable(this ExpressionExecutionContext context, string name) => (T?)context.GetVariable(name); - public static object? GetVariable(this ExpressionExecutionContext context, string name) => new Variable(name).Get(context); - public static Variable SetVariable(this ExpressionExecutionContext context, string name, T? value, Type? storageDriverType = default) => context.SetVariable(name, (object?)value, storageDriverType, default); + public static T? GetVariableByName(this ExpressionExecutionContext context, string name) => (T?)context.GetVariableByName(name)?.Value; - public static Variable SetVariable(this ExpressionExecutionContext context, string name, object? value, Type? storageDriverType, Action? configure = default) + private static Variable? GetVariableByName(this ExpressionExecutionContext context, string name) { + foreach (var block in context.Memory.Blocks.Where(b => b.Value.Metadata is VariableBlockMetadata)) + { + var metadata = block.Value.Metadata as VariableBlockMetadata; + if (metadata!.Variable.Name == name) + return metadata.Variable; + } + + return context.ParentContext?.GetVariableByName(name); + } + + public static Variable CreateVariable(this ExpressionExecutionContext context, string name, T? value, Type? storageDriverType = null, Action? configure = default) + { + var existingVariable = context.GetVariableByName(name); + if(existingVariable != null) + throw new Exception($"Variable {name} already exists in the context."); + var variable = new Variable(name, value) { - StorageDriverType = storageDriverType + StorageDriverType = storageDriverType ?? typeof(WorkflowStorageDriver) }; - + context.Set(variable, value, configure); return variable; } + public static Variable SetVariable(this ExpressionExecutionContext context, string name, T? value, Action? configure = default) + { + var variable = context.GetVariableByName(name); + if(variable is null) + throw new Exception($"Variable {name} not found in the context."); + + variable.Value = value; + variable.Set(context, value, configure); + return variable; + } + public static void Set(this ExpressionExecutionContext context, Output? output, object? value, Action? configure = default) { if (output != null) context.Set(output.MemoryBlockReference(), value, configure); diff --git a/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Tests.cs b/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Tests.cs index b440f89a6..bc41ff057 100644 --- a/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Tests.cs +++ b/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Tests.cs @@ -28,7 +28,7 @@ public class Tests var lines = _capturingTextWriter.Lines.ToList(); Assert.Equal(new[] { "Line 5" }, lines); } - + [Fact(DisplayName = "Workflow can reference variables set in previous activities")] public async Task Test2() { @@ -36,4 +36,16 @@ public class Tests var lines = _capturingTextWriter.Lines.ToList(); Assert.Equal(new[] { "Variable 2: The value of variable 1" }, lines); } + + [Fact(DisplayName = "Workflow can set and get named variables")] + public async Task Test3() + { + await _workflowRunner.RunAsync(); + var lines = _capturingTextWriter.Lines.ToList(); + Assert.Equal(new[] + { + "Foo = Bar", + "Foo = Baz" + }, lines); + } } \ No newline at end of file diff --git a/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Workflows.cs b/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Workflows.cs index 1248473cf..ad2fe45dd 100644 --- a/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Workflows.cs +++ b/test/integration/Elsa.IntegrationTests/Scenarios/SetGetVariables/Workflows.cs @@ -1,3 +1,4 @@ +using Elsa.Extensions; using Elsa.Workflows.Core.Abstractions; using Elsa.Workflows.Core.Activities; using Elsa.Workflows.Core.Contracts; @@ -14,13 +15,14 @@ class SetGetVariableWorkflow : WorkflowBase workflow.Root = new Sequence { - Variables = { + Variables = + { variable1 }, Activities = { - new SetVariable(variable1,"Line 5"), + new SetVariable(variable1, "Line 5"), new WriteLine(variable1) } }; @@ -33,17 +35,17 @@ class SetGetVariablesWorkflow : WorkflowBase { var variable1 = new Variable(); var variable2 = new Variable(); - + workflow.Root = new Sequence - { + { Variables = { variable1, variable2 }, - + Activities = { new SetVariable { Variable = variable1, - Value = new ("The value of variable 1") + Value = new("The value of variable 1") }, new SetVariable() { @@ -55,3 +57,21 @@ class SetGetVariablesWorkflow : WorkflowBase }; } } + +class SetGetNamedVariableWorkflow : WorkflowBase +{ + protected override void Build(IWorkflowBuilder workflow) + { + workflow.Root = new Sequence + { + Variables = { new Variable("Foo", "Bar") }, + + Activities = + { + new WriteLine(context => $"Foo = {context.GetVariableByName("Foo")}"), + Inline.From(context => context.SetVariable("Foo", "Baz")), + new WriteLine(context => $"Foo = {context.GetVariableByName("Foo")}"), + } + }; + } +} \ No newline at end of file diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Elsa.Workflows.Core.UnitTests.csproj b/test/unit/Elsa.Workflows.Core.UnitTests/Elsa.Workflows.Core.UnitTests.csproj new file mode 100644 index 000000000..f1412712f --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Elsa.Workflows.Core.UnitTests.csproj @@ -0,0 +1,30 @@ + + + + net7.0 + enable + enable + + false + true + + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + + diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/ExpressionExecutionContextExtensionsTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/ExpressionExecutionContextExtensionsTests.cs new file mode 100644 index 000000000..960389be2 --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/ExpressionExecutionContextExtensionsTests.cs @@ -0,0 +1,103 @@ +using Elsa.Expressions.Models; +using Elsa.Extensions; +using Elsa.Workflows.Core.Memory; + +namespace Elsa.Workflows.Core.UnitTests; + +public class ExpressionExecutionContextExtensionsTests +{ + [Fact] + public void GetVariableByName_ReturnsVariable_WhenVariableExists() + { + // Arrange + var variable = new Variable("test", 5); + var memoryRegister = new MemoryRegister(new Dictionary + { + { variable.Id, new MemoryBlock(variable.Value, new VariableBlockMetadata(variable, typeof(object), true)) } + }); + + var context = new ExpressionExecutionContext(null!, memoryRegister); + + // Act + var result = context.GetVariableByName("test"); + + // Assert + Assert.Equal(5, result); + } + + [Fact] + public void GetVariableByName_ReturnsNull_WhenVariableDoesNotExist() + { + // Arrange + var memoryRegister = new MemoryRegister(new Dictionary()); + var context = new ExpressionExecutionContext(null!, memoryRegister); + + // Act + var result = context.GetVariableByName("nonexistent"); + + // Assert + Assert.Null(result); + } + + [Fact] + public void CreateVariable_ThrowsException_WhenVariableExists() + { + // Arrange + var variable = new Variable("test", 5); + var memoryRegister = new MemoryRegister(new Dictionary + { + { variable.Id, new MemoryBlock(variable.Value, new VariableBlockMetadata(variable, typeof(object), true)) } + }); + + var context = new ExpressionExecutionContext(null!, memoryRegister); + + // Act & Assert + Assert.Throws(() => context.CreateVariable("test", 10)); + } + + [Fact] + public void CreateVariable_CreatesVariable_WhenVariableDoesNotExist() + { + // Arrange + var memoryRegister = new MemoryRegister(new Dictionary()); + var context = new ExpressionExecutionContext(null!, memoryRegister); + + // Act + context.CreateVariable("newVariable", 10); + + // Assert + var variable = context.GetVariableByName("newVariable"); + Assert.Equal(10, variable); + } + + [Fact] + public void SetVariable_ThrowsException_WhenVariableDoesNotExist() + { + // Arrange + var memoryRegister = new MemoryRegister(new Dictionary()); + var context = new ExpressionExecutionContext(null!, memoryRegister); + + // Act & Assert + Assert.Throws(() => context.SetVariable("nonexistent", 10)); + } + + [Fact] + public void SetVariable_SetsValue_WhenVariableExists() + { + // Arrange + var variable = new Variable("test", 5); + var memoryRegister = new MemoryRegister(new Dictionary + { + { variable.Id, new MemoryBlock(variable.Value, new VariableBlockMetadata(variable, typeof(object), true)) } + }); + + var context = new ExpressionExecutionContext(null!, memoryRegister); + + // Act + context.SetVariable("test", 10); + + // Assert + var updatedVariable = context.GetVariableByName("test"); + Assert.Equal(10, updatedVariable); + } +} diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Usings.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Usings.cs new file mode 100644 index 000000000..8c927eb74 --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Usings.cs @@ -0,0 +1 @@ +global using Xunit; \ No newline at end of file