From 6503c0cb927de7dd0ccfc713bc9f3a212f910d43 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 14 Sep 2026 19:07:24 +0200 Subject: [PATCH] fix(workflows): resolve GetInput serializer options per host (#8158) * fix(workflows): resolve GetInput serializer options per host Remove the process-wide JsonSerializerOptions cache from ExpressionExecutionContextExtensions so DI converters registered on a later host remain visible after the first GetInput. Share CloneForValueConversion with WorkflowInstanceStorageDriver reads. Closes #8131 Co-authored-by: Sipke Schoorstra * fix(workflows): match storage write/read JSON reference semantics Use the same cloned payload-serializer options for WorkflowInstanceStorageDriver Write and Read. Forcing ReferenceHandler.Preserve on read only treated ordinary $ref properties as metadata and broke restore. Closes the P1 on #8158. Co-authored-by: Sipke Schoorstra * fix(workflows): serialize storage values by runtime type SerializeToNode(object, options) used the compile-time object converter, so PolymorphicObjectConverter wrapped arrays as _items metadata. Write now uses the runtime type with the same cloned host options as Read. Co-authored-by: Sipke Schoorstra * test(workflows): assert stored array conversion is non-null Co-authored-by: Sipke Schoorstra * fix(workflows): restore _type for object-typed storage values Serialize object-typed variables through the polymorphic converter so aliased CLR types keep a _type discriminator. Typed arrays still serialize by runtime type so they stay JSON arrays. Co-authored-by: Sipke Schoorstra * fix(workflows): deserialize object variables through polymorphic options ConvertTo(object) returns the stored JsonObject because it is assignable to object. Read object-typed variables with the payload serializer so _type restores the concrete CLR type. Co-authored-by: Sipke Schoorstra --------- Co-authored-by: Cursor Agent --- .../JsonSerializerOptionsExtensions.cs | 10 ++ .../ExpressionExecutionContextExtensions.cs | 10 +- .../WorkflowInstanceStorageDriver.cs | 20 +++- ...pressionExecutionContextExtensionsTests.cs | 89 +++++++++++++++ .../WorkflowInstanceStorageDriverTests.cs | 106 +++++++++++++++++- 5 files changed, 222 insertions(+), 13 deletions(-) diff --git a/src/modules/Elsa.Common/Extensions/JsonSerializerOptionsExtensions.cs b/src/modules/Elsa.Common/Extensions/JsonSerializerOptionsExtensions.cs index 0bf9a9910..e573f3d4f 100644 --- a/src/modules/Elsa.Common/Extensions/JsonSerializerOptionsExtensions.cs +++ b/src/modules/Elsa.Common/Extensions/JsonSerializerOptionsExtensions.cs @@ -27,4 +27,14 @@ public static class JsonSerializerOptionsExtensions { return new(options); } + + /// + /// Clones the options and sets for execution-time value conversion. + /// + public static JsonSerializerOptions CloneForValueConversion(this JsonSerializerOptions options) + { + var clone = options.Clone(); + clone.ReferenceHandler = ReferenceHandler.Preserve; + return clone; + } } \ No newline at end of file diff --git a/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs b/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs index e3c6860cb..744a3275a 100644 --- a/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs +++ b/src/modules/Elsa.Workflows.Core/Extensions/ExpressionExecutionContextExtensions.cs @@ -394,17 +394,9 @@ public static class ExpressionExecutionContextExtensions } } - private static JsonSerializerOptions? _serializerOptions; - private static JsonSerializerOptions GetSerializerOptions(ExpressionExecutionContext context) { - if (_serializerOptions != null) - return _serializerOptions; - - var serializerOptions = context.GetRequiredService().GetOptions().Clone(); - serializerOptions.ReferenceHandler = ReferenceHandler.Preserve; - _serializerOptions = serializerOptions; - return serializerOptions; + return context.GetRequiredService().GetOptions().CloneForValueConversion(); } extension(ExpressionExecutionContext context) diff --git a/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs b/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs index 7c1d4e963..af2bcf605 100644 --- a/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs +++ b/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs @@ -32,7 +32,7 @@ public class WorkflowInstanceStorageDriver(IPayloadSerializer payloadSerializer, { try { - var node = JsonSerializer.SerializeToNode(value); + var node = JsonSerializer.SerializeToNode(value, GetSerializationType(value, context), GetSerializerOptions()); dictionary[id] = node!; } catch (Exception ex) when (ex is JsonException or NotSupportedException or ObjectDisposedException) @@ -51,10 +51,15 @@ public class WorkflowInstanceStorageDriver(IPayloadSerializer payloadSerializer, var node = dictionary.GetValueOrDefault(id); var variable = context.Variable; var variableType = variable.GetVariableType(); + var serializerOptions = GetSerializerOptions(); + + if (variableType == typeof(object)) + return new(node?.Deserialize(typeof(object), serializerOptions)); + var options = new ObjectConverterOptions { DeserializeJsonObjectToObject = true, - SerializerOptions = payloadSerializer.GetOptions() + SerializerOptions = serializerOptions }; var result = node.TryConvertTo(variableType, options); if (result.IsSuccess) @@ -76,6 +81,17 @@ public class WorkflowInstanceStorageDriver(IPayloadSerializer payloadSerializer, return ValueTask.CompletedTask; } + private JsonSerializerOptions GetSerializerOptions() => payloadSerializer.GetOptions().Clone(); + + private static Type GetSerializationType(object? value, StorageDriverContext context) + { + var declaredType = context.Variable.GetVariableType(); + if (declaredType == typeof(object)) + return typeof(object); + + return value?.GetType() ?? declaredType; + } + private VariablesDictionary GetVariablesDictionary(StorageDriverContext context) => context.ExecutionContext.Properties.GetOrAdd(VariablesDictionaryStateKey, () => new VariablesDictionary()); private void SetVariablesDictionary(StorageDriverContext context, VariablesDictionary dictionary) => context.ExecutionContext.Properties[VariablesDictionaryStateKey] = dictionary; diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ExpressionExecutionContextExtensions/ExpressionExecutionContextExtensionsTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ExpressionExecutionContextExtensions/ExpressionExecutionContextExtensionsTests.cs index 4afd76c17..62adca947 100644 --- a/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ExpressionExecutionContextExtensions/ExpressionExecutionContextExtensionsTests.cs +++ b/test/unit/Elsa.Workflows.Core.UnitTests/Extensions/ExpressionExecutionContextExtensions/ExpressionExecutionContextExtensionsTests.cs @@ -1,9 +1,14 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using Elsa.Common; using Elsa.Expressions.JavaScript.Activities; using Elsa.Expressions.Models; using Elsa.Extensions; using Elsa.Testing.Shared; using Elsa.Workflows.Activities; using Elsa.Workflows.Memory; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; namespace Elsa.Workflows.Core.UnitTests; @@ -107,4 +112,88 @@ public class ExpressionExecutionContextExtensionsTests var updatedVariable = context.GetVariable("test"); Assert.Equal(10, updatedVariable); } + + [Fact] + public async Task GetInput_UsesSecondHostConverters_AfterFirstHostAlreadyConverted() + { + // Arrange + var firstContext = await CreateContextWithSerializerAsync("first"); + firstContext.WorkflowExecutionContext.Input["payload"] = """{"token":"x"}"""; + + // Act + var first = firstContext.ExpressionExecutionContext.GetInput("payload"); + + var secondContext = await CreateContextWithSerializerAsync("second"); + secondContext.WorkflowExecutionContext.Input["payload"] = """{"token":"x"}"""; + var second = secondContext.ExpressionExecutionContext.GetInput("payload"); + + // Assert + Assert.Equal("first", first?.Tag); + Assert.Equal("second", second?.Tag); + } + + [Fact] + public async Task GetInput_ResolvesSerializerOptionsFromCurrentHostOnEachCall() + { + // Arrange + var serializer = Substitute.For(); + serializer.GetOptions().Returns(CreateTaggedOptions("first"), CreateTaggedOptions("second")); + + var context = await CreateContextAsync(serializer); + context.WorkflowExecutionContext.Input["payload"] = """{"token":"x"}"""; + + // Act + var first = context.ExpressionExecutionContext.GetInput("payload"); + var second = context.ExpressionExecutionContext.GetInput("payload"); + + // Assert + Assert.Equal("first", first?.Tag); + Assert.Equal("second", second?.Tag); + } + + private static Task CreateContextWithSerializerAsync(string converterTag) => + CreateContextAsync(new StubJsonSerializer(CreateTaggedOptions(converterTag))); + + private static Task CreateContextAsync(IJsonSerializer serializer) + { + var fixture = new ActivityTestFixture(new WriteLine("test")) + .ConfigureServices(services => services.AddSingleton(serializer)); + return fixture.BuildAsync(); + } + + private static JsonSerializerOptions CreateTaggedOptions(string tag) + { + var options = new JsonSerializerOptions(); + options.Converters.Add(new TaggedValueConverter(tag)); + return options; + } + + private sealed class TaggedValue + { + public string Tag { get; set; } = ""; + } + + private sealed class TaggedValueConverter(string tag) : JsonConverter + { + public override TaggedValue Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + using var _ = JsonDocument.ParseValue(ref reader); + return new TaggedValue { Tag = tag }; + } + + public override void Write(Utf8JsonWriter writer, TaggedValue value, JsonSerializerOptions options) => + writer.WriteStringValue(value.Tag); + } + + private sealed class StubJsonSerializer(JsonSerializerOptions options) : IJsonSerializer + { + public JsonSerializerOptions GetOptions() => options; + public void ApplyOptions(JsonSerializerOptions _) { } + public string Serialize(object value) => throw new NotSupportedException(); + public string Serialize(object value, Type type) => throw new NotSupportedException(); + public string Serialize(T value) => throw new NotSupportedException(); + public object Deserialize(string json) => throw new NotSupportedException(); + public object Deserialize(string json, Type type) => throw new NotSupportedException(); + public T Deserialize(string json) => throw new NotSupportedException(); + } } diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs index b3e767fa0..cf52c95e9 100644 --- a/test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs +++ b/test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs @@ -1,7 +1,10 @@ using System.Text.Json; using System.Text.Json.Nodes; +using System.Text.Json.Serialization; +using Elsa.Common.Serialization; using Elsa.Expressions.Helpers; using Elsa.Workflows.Memory; +using Elsa.Workflows.Serialization.Converters; using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; @@ -53,6 +56,81 @@ public class WorkflowInstanceStorageDriverTests Assert.False(GetVariables(harness.Properties).ContainsKey(id)); } + [Fact] + public async Task WriteThenRead_WhenValueHasOrdinaryDollarRefProperty_RoundTrips() + { + // Arrange + var harness = CreateHarness(new Variable("payload", new())); + const string id = "payloadVariable"; + var value = new RefPayload { Ref = "ordinary" }; + + // Act + await harness.Driver.WriteAsync(id, value, harness.Context); + var read = await harness.Driver.ReadAsync(id, harness.Context); + + // Assert + var restored = Assert.IsType(read); + Assert.Equal("ordinary", restored.Ref); + } + + [Fact] + public async Task WriteThenRead_WhenObjectVariableHoldsAliasedType_RestoresConcreteClrType() + { + // Arrange + var registry = SerializationTypeRegistry.CreateDefault(); + registry.RegisterType(typeof(AliasedPerson), nameof(AliasedPerson)); + var harness = CreateHarness(new Variable("payload", new()), registry); + const string id = "payloadVariable"; + var value = new AliasedPerson { Name = "Ada" }; + + // Act + await harness.Driver.WriteAsync(id, value, harness.Context); + var stored = GetVariables(harness.Properties)[id].AsObject(); + var read = await harness.Driver.ReadAsync(id, harness.Context); + + // Assert + Assert.Equal(nameof(AliasedPerson), stored["_type"]?.GetValue()); + var restored = Assert.IsType(read); + Assert.Equal("Ada", restored.Name); + } + + [Fact] + public async Task WriteAsync_WhenValueIsArray_StoresJsonArrayReadableWithDefaultConverter() + { + // Arrange + var harness = CreateHarness(new Variable("elements", [])); + const string id = "elementsVariable"; + + // Act + await harness.Driver.WriteAsync(id, new[] { "Element 1", "Element 2" }, harness.Context); + var node = GetVariables(harness.Properties)[id]; + var actual = node.ConvertTo(); + + // Assert + Assert.Equal(JsonValueKind.Array, node.GetValueKind()); + Assert.NotNull(actual); + Assert.Equal(["Element 1", "Element 2"], actual); + } + + [Fact] + public async Task WriteAsync_WhenValueIsProjectedEnumerable_StoresJsonArrayReadableWithDefaultConverter() + { + // Arrange + var harness = CreateHarness(new Variable("messages", [])); + const string id = "messagesVariable"; + var value = new[] { "a", "b", "c" }.Select(x => x.ToUpperInvariant()); + + // Act + await harness.Driver.WriteAsync(id, value, harness.Context); + var node = GetVariables(harness.Properties)[id]; + var actual = node.ConvertTo(); + + // Assert + Assert.Equal(JsonValueKind.Array, node.GetValueKind()); + Assert.NotNull(actual); + Assert.Equal(["A", "B", "C"], actual); + } + [Fact] public async Task ReadAsync_WhenConvertFails_DoesNotReturnUntypedJsonNode() { @@ -89,14 +167,14 @@ public class WorkflowInstanceStorageDriverTests } } - private static Harness CreateHarness(Variable variable) + private static Harness CreateHarness(Variable variable, ISerializationTypeRegistry? typeRegistry = null) { var properties = new Dictionary(); var executionContext = Substitute.For(); executionContext.Properties.Returns(properties); var payloadSerializer = Substitute.For(); - payloadSerializer.GetOptions().Returns(new JsonSerializerOptions()); + payloadSerializer.GetOptions().Returns(CreatePayloadSerializerOptions(typeRegistry)); var driver = new WorkflowInstanceStorageDriver(payloadSerializer, NullLogger.Instance); var context = new StorageDriverContext(executionContext, variable, CancellationToken.None); @@ -104,6 +182,19 @@ public class WorkflowInstanceStorageDriverTests return new(driver, context, properties); } + private static JsonSerializerOptions CreatePayloadSerializerOptions(ISerializationTypeRegistry? typeRegistry = null) + { + var options = new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + PropertyNameCaseInsensitive = true + }; + options.Converters.Add(typeRegistry is null + ? new PolymorphicObjectConverterFactory() + : new PolymorphicObjectConverterFactory(typeRegistry)); + return options; + } + private static VariablesDictionary GetVariables(IDictionary properties) => (VariablesDictionary)properties[WorkflowInstanceStorageDriver.VariablesDictionaryStateKey]; @@ -131,6 +222,17 @@ public class WorkflowInstanceStorageDriverTests { public CyclicValue Self { get; set; } = null!; } + + private sealed class RefPayload + { + [JsonPropertyName("$ref")] + public string Ref { get; set; } = ""; + } + + private sealed class AliasedPerson + { + public string Name { get; set; } = ""; + } } [CollectionDefinition(nameof(WorkflowInstanceStorageDriverTestsCollection), DisableParallelization = true)]