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 <sipkeschoorstra@outlook.com> * 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 <sipkeschoorstra@outlook.com> * 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 <sipkeschoorstra@outlook.com> * test(workflows): assert stored array conversion is non-null Co-authored-by: Sipke Schoorstra <sipkeschoorstra@outlook.com> * 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 <sipkeschoorstra@outlook.com> * 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 <sipkeschoorstra@outlook.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This commit is contained in:
Sipke Schoorstra
GitHub
parent
5193bea262
commit
6503c0cb92
|
|
@ -27,4 +27,14 @@ public static class JsonSerializerOptionsExtensions
|
|||
{
|
||||
return new(options);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Clones the options and sets <see cref="ReferenceHandler.Preserve"/> for execution-time value conversion.
|
||||
/// </summary>
|
||||
public static JsonSerializerOptions CloneForValueConversion(this JsonSerializerOptions options)
|
||||
{
|
||||
var clone = options.Clone();
|
||||
clone.ReferenceHandler = ReferenceHandler.Preserve;
|
||||
return clone;
|
||||
}
|
||||
}
|
||||
|
|
@ -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<IJsonSerializer>().GetOptions().Clone();
|
||||
serializerOptions.ReferenceHandler = ReferenceHandler.Preserve;
|
||||
_serializerOptions = serializerOptions;
|
||||
return serializerOptions;
|
||||
return context.GetRequiredService<IJsonSerializer>().GetOptions().CloneForValueConversion();
|
||||
}
|
||||
|
||||
extension(ExpressionExecutionContext context)
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
|
|
|
|||
|
|
@ -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<int>("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<TaggedValue>("payload");
|
||||
|
||||
var secondContext = await CreateContextWithSerializerAsync("second");
|
||||
secondContext.WorkflowExecutionContext.Input["payload"] = """{"token":"x"}""";
|
||||
var second = secondContext.ExpressionExecutionContext.GetInput<TaggedValue>("payload");
|
||||
|
||||
// Assert
|
||||
Assert.Equal("first", first?.Tag);
|
||||
Assert.Equal("second", second?.Tag);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GetInput_ResolvesSerializerOptionsFromCurrentHostOnEachCall()
|
||||
{
|
||||
// Arrange
|
||||
var serializer = Substitute.For<IJsonSerializer>();
|
||||
serializer.GetOptions().Returns(CreateTaggedOptions("first"), CreateTaggedOptions("second"));
|
||||
|
||||
var context = await CreateContextAsync(serializer);
|
||||
context.WorkflowExecutionContext.Input["payload"] = """{"token":"x"}""";
|
||||
|
||||
// Act
|
||||
var first = context.ExpressionExecutionContext.GetInput<TaggedValue>("payload");
|
||||
var second = context.ExpressionExecutionContext.GetInput<TaggedValue>("payload");
|
||||
|
||||
// Assert
|
||||
Assert.Equal("first", first?.Tag);
|
||||
Assert.Equal("second", second?.Tag);
|
||||
}
|
||||
|
||||
private static Task<ActivityExecutionContext> CreateContextWithSerializerAsync(string converterTag) =>
|
||||
CreateContextAsync(new StubJsonSerializer(CreateTaggedOptions(converterTag)));
|
||||
|
||||
private static Task<ActivityExecutionContext> 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<TaggedValue>
|
||||
{
|
||||
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>(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<T>(string json) => throw new NotSupportedException();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<RefPayload>("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<RefPayload>(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<object>("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<string>());
|
||||
var restored = Assert.IsType<AliasedPerson>(read);
|
||||
Assert.Equal("Ada", restored.Name);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WriteAsync_WhenValueIsArray_StoresJsonArrayReadableWithDefaultConverter()
|
||||
{
|
||||
// Arrange
|
||||
var harness = CreateHarness(new Variable<string[]>("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<string[]>();
|
||||
|
||||
// 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<string[]>("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<string[]>();
|
||||
|
||||
// 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<string, object>();
|
||||
var executionContext = Substitute.For<IExecutionContext>();
|
||||
executionContext.Properties.Returns(properties);
|
||||
|
||||
var payloadSerializer = Substitute.For<IPayloadSerializer>();
|
||||
payloadSerializer.GetOptions().Returns(new JsonSerializerOptions());
|
||||
payloadSerializer.GetOptions().Returns(CreatePayloadSerializerOptions(typeRegistry));
|
||||
|
||||
var driver = new WorkflowInstanceStorageDriver(payloadSerializer, NullLogger<WorkflowInstanceStorageDriver>.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<string, object> 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)]
|
||||
|
|
|
|||
Loading…
Reference in a new issue