elsa-workflows / elsa-workflows/elsa-core
Include Transient Variables in Merged Variables for JavaScriptEngine Access
- Dominant language
- C#
- Stars
- 7.9k
- Forks
- 1.5k
- Avg merge
- 15h 22m
- Merged PRs (30d)
- 114
Description
In reviewing the current implementation in `WorkflowExecutionContext.cs`, I've noticed an interesting point regarding the handling of transient variables. Currently, it seems these variables are not included in the merged variables collection. This omission could limit the JavaScriptEngine's access to these variables. I'm curious if a change in the way we merge these variables might improve their accessibility and functionality within the JavaScriptEngine and other parts of the ecosystem.
## Proposed Changes for Discussion
Would the following adjustment to the `WorkflowExecutionContext.cs` file be beneficial in ensuring transient variables are also included in the mergedvariables collection.
Here's the diff for the proposed changes:
```diff
diff --git a/src/core/Elsa.Abstractions/Services/Models/WorkflowExecutionContext.cs b/src/core/Elsa.Abstractions/Services/Models/WorkflowExecutionContext.cs
index 76aec2e82..ff54ba2d4 100644
--- a/src/core/Elsa.Abstractions/Services/Models/WorkflowExecutionContext.cs
+++ b/src/core/Elsa.Abstractions/Services/Models/WorkflowExecutionContext.cs
@@ -166,13 +166,17 @@ public Variables GetMergedVariables()
{
var scopes = WorkflowInstance.Scopes.ToList();
- var mergedVariables = scopes.Select(x => x.Variables).Aggregate(WorkflowInstance.Variables, (current, next) =>
- {
- var combined = current.Data.MergedWith(next.Data);
- return new Variables(combined);
- });
-
- return mergedVariables;
+ // Start the aggregation with the TransientState.
+ var mergedVariables = scopes.Select(x => x.Variables)
+ .Aggregate(TransientState, (current, next) =>
+ {
+ var combined = current.Data.MergedWith(next.Data);
+ return new Variables(combined);
+ });
+
+ // Merge the WorkflowInstance.Variables at the end
+ var finalCombined = mergedVariables.Data.MergedWith(WorkflowInstance.Variables.Data);
+ return new Variables(finalCombined);
}
```
```diff
diff --git a/test/unit/Elsa.UnitTests/Services/Models/WorkflowExecutionContextTests.cs b/test/unit/Elsa.UnitTests/Services/Models/WorkflowExecutionContextTests.cs
index 0ccd7424c..01b5b8c65 100644
--- a/test/unit/Elsa.UnitTests/Services/Models/WorkflowExecutionContextTests.cs
+++ b/test/unit/Elsa.UnitTests/Services/Models/WorkflowExecutionContextTests.cs
@@ -24,7 +24,17 @@ public class WorkflowExecutionContextTests
Assert.Empty(sut.WorkflowInstance.Variables.Data);
}
+ [Theory(DisplayName = "Transient variables should be retrievable in merged variable collection"), AutoMoqData]
+ public void TransientVariableSetInExecutionContext_ShouldBeRetrievable([WithAutofixtureResolution, Frozen] IServiceProvider serviceProvider,
+ [OmitOnRecursion] WorkflowExecutionContext workflowExecutionContext,
+ IActivityBlueprint activityBlueprint,
+ CancellationToken cancellationToken)
+ {
+ var sut = new ActivityExecutionContext(serviceProvider, workflowExecutionContext, activityBlueprint, null, false, cancellationToken);
+ sut.SetTransientVariable("foo", "bar");
+
+ Assert.Equal("bar", sut.GetVariable("foo"));
+ }
}
}
Contributor guide
Assessment
This issue has not been assessed yet.