From ee733e3229971e731d22ee903c420e1c46d00640 Mon Sep 17 00:00:00 2001 From: bobhauser Date: Sat, 15 Jun 2024 12:39:05 -0400 Subject: [PATCH] Flowchart variables are not serialized by FlowchartJsonConverter (#5533) * Flowchart variables are not serialized by FlowchartJsonConverter * Added unit tests to ensure that serialized/deserialized container is equivalent to original, and fixed issues found --------- Co-authored-by: Bob Hauser Co-authored-by: Sipke Schoorstra --- .../Abstractions/Activity.cs | 3 + .../Serialization/FlowchartJsonConverter.cs | 66 ++++-- .../PolymorphicObjectConverterFactory.cs | 13 +- .../ContainerSerialization/Tests.cs | 199 ++++++++++++++++++ 4 files changed, 254 insertions(+), 27 deletions(-) create mode 100644 test/integration/Elsa.Workflows.IntegrationTests/Serialization/ContainerSerialization/Tests.cs diff --git a/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs b/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs index 8526ecb3bd..ba297b501d 100644 --- a/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs +++ b/src/modules/Elsa.Workflows.Core/Abstractions/Activity.cs @@ -5,6 +5,7 @@ using Elsa.Workflows.Contracts; using Elsa.Workflows.Helpers; using Elsa.Workflows.Models; +using Elsa.Workflows.Serialization.Converters; using JetBrains.Annotations; namespace Elsa.Workflows; @@ -75,6 +76,7 @@ public bool RunAsynchronously } /// + [JsonConverter(typeof(PolymorphicObjectConverterFactory))] public IDictionary CustomProperties { get; set; } = new Dictionary(); /// @@ -82,6 +84,7 @@ public bool RunAsynchronously public IDictionary SyntheticProperties { get; set; } = new Dictionary(); /// + [JsonConverter(typeof(PolymorphicObjectConverterFactory))] public IDictionary Metadata { get; set; } = new Dictionary(); /// diff --git a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Serialization/FlowchartJsonConverter.cs b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Serialization/FlowchartJsonConverter.cs index 2175977587..48fe143c3f 100644 --- a/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Serialization/FlowchartJsonConverter.cs +++ b/src/modules/Elsa.Workflows.Core/Activities/Flowchart/Serialization/FlowchartJsonConverter.cs @@ -1,7 +1,10 @@ using System.Text.Json; using System.Text.Json.Serialization; +using Elsa.Extensions; using Elsa.Workflows.Activities.Flowchart.Models; -using Elsa.Workflows.Contracts; +using Elsa.Workflows.Contracts; +using Elsa.Workflows.Memory; +using Elsa.Workflows.Serialization.Converters; namespace Elsa.Workflows.Activities.Flowchart.Serialization; @@ -27,35 +30,49 @@ public override Activities.Flowchart Read(ref Utf8JsonReader reader, Type typeTo if (!JsonDocument.TryParseValue(ref reader, out var doc)) throw new JsonException("Failed to parse JsonDocument"); - var connectionsElement = doc.RootElement.TryGetProperty("connections", out var connectionsEl) ? connectionsEl : default; - var activitiesElement = doc.RootElement.TryGetProperty("activities", out var activitiesEl) ? activitiesEl : default; var id = doc.RootElement.TryGetProperty("id", out var idAttribute) ? idAttribute.GetString()! : _identityGenerator.GenerateId(); var nodeId = doc.RootElement.TryGetProperty("nodeId", out var nodeIdAttribute) ? nodeIdAttribute.GetString() : default; var name = doc.RootElement.TryGetProperty("name", out var nameElement) ? nameElement.GetString() : default; + var type = doc.RootElement.TryGetProperty("type", out var typeElement) ? typeElement.GetString() : default; + var version = doc.RootElement.TryGetProperty("version", out var versionElement) ? versionElement.GetInt32() : 1; + + var connectionsElement = doc.RootElement.TryGetProperty("connections", out var connectionsEl) ? connectionsEl : default; + var activitiesElement = doc.RootElement.TryGetProperty("activities", out var activitiesEl) ? activitiesEl : default; var activities = activitiesElement.ValueKind != JsonValueKind.Undefined ? activitiesElement.Deserialize>(options) ?? new List() : new List(); - var metadataElement = doc.RootElement.TryGetProperty("metadata", out var metadataEl) ? metadataEl : default; - var metadata = metadataElement.ValueKind != JsonValueKind.Undefined ? metadataElement.Deserialize>(options) ?? new Dictionary() : new Dictionary(); var activityDictionary = activities.ToDictionary(x => x.Id); var connections = DeserializeConnections(connectionsElement, activityDictionary, options); var notFoundConnections = GetNotFoundConnections(doc.RootElement, activityDictionary, connections, options); var connectionsToRestore = FindConnectionsThatCanBeRestored(notFoundConnections, activities); var connectionComparer = new ConnectionComparer(); var connectionsWithRestoredOnes = connections.Except(notFoundConnections, connectionComparer).Union(connectionsToRestore, connectionComparer).ToList(); + + var variablesElement = doc.RootElement.TryGetProperty("variables", out var variablesEl) ? variablesEl : default; + var variables = variablesElement.ValueKind != JsonValueKind.Undefined ? variablesElement.Deserialize>(options) ?? new List() : new List(); + + JsonSerializerOptions polymorphicOptions = options.Clone(); + polymorphicOptions.Converters.Add(new PolymorphicDictionaryConverter(options)); + + var metadataElement = doc.RootElement.TryGetProperty("metadata", out var metadataEl) ? metadataEl : default; + var metadata = metadataElement.ValueKind != JsonValueKind.Undefined ? metadataElement.Deserialize>(polymorphicOptions) ?? new Dictionary() : new Dictionary(); + + var customPropertiesElement = doc.RootElement.TryGetProperty("customProperties", out var customPropertiesEl) ? customPropertiesEl : default; + var customProperties = customPropertiesEl.ValueKind != JsonValueKind.Undefined ? customPropertiesElement.Deserialize>(polymorphicOptions) ?? new Dictionary() : new Dictionary(); + customProperties[AllActivitiesKey] = activities.ToList(); + customProperties[AllConnectionsKey] = connectionsWithRestoredOnes; + customProperties[NotFoundConnectionsKey] = notFoundConnections.Except(connectionsToRestore, connectionComparer).ToList(); var flowChart = new Activities.Flowchart - { + { Id = id, NodeId = nodeId!, Name = name, + Type = type!, + Version = version, + CustomProperties = customProperties, Metadata = metadata, Activities = activities, - Connections = connectionsWithRestoredOnes, - CustomProperties = - { - [AllActivitiesKey] = activities.ToList(), - [AllConnectionsKey] = connectionsWithRestoredOnes, - [NotFoundConnectionsKey] = notFoundConnections.Except(connectionsToRestore, connectionComparer).ToList() - } + Variables = variables, + Connections = connectionsWithRestoredOnes, }; return flowChart; @@ -65,11 +82,8 @@ public override Activities.Flowchart Read(ref Utf8JsonReader reader, Type typeTo public override void Write(Utf8JsonWriter writer, Activities.Flowchart value, JsonSerializerOptions options) { var activities = value.Activities; - var connectionSerializerOptions = new JsonSerializerOptions(options); var activityDictionary = activities.ToDictionary(x => x.Id); - connectionSerializerOptions.Converters.Add(new ConnectionJsonConverter(activityDictionary)); - var customProperties = new Dictionary(value.CustomProperties); var allActivities = customProperties.GetValueOrDefault(AllActivitiesKey, activities); var allConnections = (ICollection)(customProperties.TryGetValue(AllConnectionsKey, out var c) ? c : value.Connections); @@ -79,17 +93,23 @@ public override void Write(Utf8JsonWriter writer, Activities.Flowchart value, Js var model = new { - value.Type, - value.Version, value.Id, value.NodeId, - value.Metadata, + value.Name, + value.Type, + value.Version, CustomProperties = customProperties, + value.Metadata, Activities = allActivities, - Connections = allConnections - }; - - JsonSerializer.Serialize(writer, model, connectionSerializerOptions); + value.Variables, + Connections = allConnections, + }; + + var flowchartSerializerOptions = new JsonSerializerOptions(options); + flowchartSerializerOptions.Converters.Add(new ConnectionJsonConverter(activityDictionary)); + flowchartSerializerOptions.Converters.Add(new PolymorphicDictionaryConverter(options)); + + JsonSerializer.Serialize(writer, model, flowchartSerializerOptions); } private static ICollection GetNotFoundConnections(JsonElement rootElement, IDictionary activities, IEnumerable connections, JsonSerializerOptions connectionSerializerOptions) diff --git a/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverterFactory.cs b/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverterFactory.cs index f304829c69..319bdab8e7 100644 --- a/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverterFactory.cs +++ b/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverterFactory.cs @@ -12,12 +12,17 @@ public class PolymorphicObjectConverterFactory : JsonConverterFactory /// public override bool CanConvert(Type typeToConvert) { - var canConvert = typeToConvert.IsClass + if (typeToConvert.IsClass && typeToConvert == typeof(object) || typeToConvert == typeof(ExpandoObject) - || typeToConvert == typeof(Dictionary); - - return canConvert; + || typeToConvert == typeof(Dictionary)) + return true; + + if (typeToConvert.IsInterface + && typeToConvert == typeof(IDictionary)) + return true; + + return false; } /// diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Serialization/ContainerSerialization/Tests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Serialization/ContainerSerialization/Tests.cs new file mode 100644 index 0000000000..08c787da9c --- /dev/null +++ b/test/integration/Elsa.Workflows.IntegrationTests/Serialization/ContainerSerialization/Tests.cs @@ -0,0 +1,199 @@ +using Elsa.Expressions.Models; +using Elsa.Testing.Shared; +using Elsa.Workflows.Activities; +using Elsa.Workflows.Activities.Flowchart.Activities; +using Elsa.Workflows.Activities.Flowchart.Models; +using Elsa.Workflows.Contracts; +using Elsa.Workflows.Memory; +using Elsa.Workflows.Models; +using Microsoft.Extensions.DependencyInjection; +using Xunit; +using Xunit.Abstractions; + +namespace Elsa.Workflows.IntegrationTests.Serialization.ContainerSerialization; + +public class Tests +{ + private readonly IServiceProvider _services; + private readonly IActivitySerializer _activitySerializer; + + public Tests(ITestOutputHelper testOutputHelper) + { + _services = new TestApplicationBuilder(testOutputHelper).Build(); + _activitySerializer = _services.GetService()!; + } + + [Fact] + public async void SerializeFlowchartContainerTest() + { + await _services.PopulateRegistriesAsync(); + + // Arrange + + var start = new Start() + { + Id = "start", + Name = "Start", + }; + var writeLine = new WriteLine(new Input(new Expression("JavaScript", "getVariable('TextVar')"))) + { + Id = "writeLine", + Name = "WriteLine", + Version = 3, + }; + var end = new End() + { + Id = "end", + Name = "end", + }; + var container = new Flowchart() + { + Id = "flowchart", + Name = "Flowchart", + Type = "Elsa.Flowchart", + Version = 42, + CustomProperties = new Dictionary() + { + { "purpose", "somePurpose" } + }, + Metadata = new Dictionary() + { + { "int", 10 }, + { "bool", false }, + { "string", "str" }, + }, + Activities = new List() { + start, + writeLine, + end + }, + Variables = new List() { + new Variable("TextVar", "This is the text to write") + }, + Connections = new List() + { + new Connection(start, writeLine), + new Connection(writeLine, end), + }, + }; + + // Act + + var serialized = _activitySerializer.Serialize(container); + var deserializedContainer = _activitySerializer.Deserialize(serialized) as Container; + + // Assert + + ValidateContainer(container, deserializedContainer); + } + + [Fact] + public async void SerializeSequenceContainerTest() + { + await _services.PopulateRegistriesAsync(); + + // Arrange + + var container = new Sequence() + { + Id = "sequence", + Name = "Sequence", + Type = "Elsa.Sequence", + Version = 42, + Variables = new List() { + new Variable("TextVar", "This is the text to write") + }, + Activities = new List() { + new WriteLine(new Input(new Expression("JavaScript", "getVariable('TextVar')"))) + { + Id = "writeLine", + Name = "WriteLine", + CanStartWorkflow = true, + }, + }, + CustomProperties = new Dictionary() + { + { "purpose", "somePurpose" } + }, + Metadata = new Dictionary() + { + { "int", 10 }, + { "bool", false }, + { "string", "str"}, + } + }; + + // Act + + var serialized = _activitySerializer.Serialize(container); + var deserializedContainer = _activitySerializer.Deserialize(serialized) as Container; + + // Assert + + ValidateContainer(container, deserializedContainer); + } + + [Fact] + public async void SerializeParallelContainerTest() + { + await _services.PopulateRegistriesAsync(); + + // Arrange + + var container = new Workflows.Activities.Parallel() + { + Id = "parallel", + Name = "Parallel", + Type = "Elsa.Parallel", + Version = 42, + Variables = new List() { + new Variable("TextVar", "This is the text to write") + }, + Activities = new List() { + new WriteLine(new Input(new Expression("JavaScript", "getVariable('TextVar')"))) + { + Id = "writeLine", + Name = "WriteLine", + CanStartWorkflow = true, + }, + }, + CustomProperties = new Dictionary() + { + { "purpose", "somePurpose" } + }, + Metadata = new Dictionary() + { + { "int", 10 }, + { "bool", false }, + { "string", "str"}, + } + }; + + // Act + + var serialized = _activitySerializer.Serialize(container); + var deserializedContainer = _activitySerializer.Deserialize(serialized) as Container; + + // Assert + + ValidateContainer(container, deserializedContainer); + } + + private static void ValidateContainer(Container container, Container? deserializedContainer) + { + if (deserializedContainer == null) + throw new ArgumentNullException(nameof(deserializedContainer)); + + // Assert.Equivalent has trouble with the Behavior.Owner reference - since these aren't serialzied anyway, ignore them + deserializedContainer.Behaviors.Clear(); + container.Behaviors.Clear(); + foreach (Activity activity in deserializedContainer.Activities) + activity.Behaviors.Clear(); + foreach (Activity activity in container.Activities) + activity.Behaviors.Clear(); + + // strict:false here allows "actual" to have extra public members that aren't part of "expected", and collection + // comparison allows "actual" to have more data in it than is present in "expected". + Assert.Equivalent(container, deserializedContainer, strict: false); + } +}