From c4a6ce2796ff2b2695dfb36ba2afdcf903cf8508 Mon Sep 17 00:00:00 2001 From: Robin Sue Date: Thu, 28 Nov 2024 02:48:11 +0100 Subject: [PATCH] Fix Primitive Collection serialization This was broken by both #5871 and #5682: 1. When a List was serialized, it was recognized as a primitive collection and thus plainly written to the JSON without any type information: ["d4d8404c-4357-47ff-a343-649a116539f5"] 2. When this JSON was deserialized, due to lack of type info, it was deserialized as List, containing strings. This is already not good. 3. When this List gets serialized again (e.g. due to multiple workflow suspends causing WorkflowState serialization), this time it fails the primitive collection recognition, because object is not a primitive type. It now gets serialized as {"_items": ["d4d8404c-4357-47ff-a343-649a116539f5"], "_type": "Object[]"} 4. When that JSON gets deserialized, it tries to ReadType() but ReadType() fails to parse Object[] since it lacks the logic from TypeJsonConverter to throw away the [] before looking up Object in the WellKnownTypeRegistry, so it returns null as a type. Without a type but being faced with a json object { ... } it now deserializes into an ExpandoObject 5. Any further serialization / deserializations will now cause the expando object to get nested deeper and deeper every time. --- .../Converters/PolymorphicObjectConverter.cs | 55 +++++++++---------- .../Serialization/JsonSerialization/Tests.cs | 17 ++++++ 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverter.cs b/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverter.cs index 1f3ccc9cb..fe9111295 100644 --- a/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverter.cs +++ b/src/modules/Elsa.Workflows.Core/Serialization/Converters/PolymorphicObjectConverter.cs @@ -31,7 +31,7 @@ public class PolymorphicObjectConverter(IWellKnownTypeRegistry wellKnownTypeRegi if (reader.TokenType != JsonTokenType.StartObject && reader.TokenType != JsonTokenType.StartArray) return ReadPrimitive(ref reader, newOptions); - var targetType = ReadType(reader); + var targetType = ReadType(reader, options); if (targetType == null) return ReadObject(ref reader, newOptions); @@ -183,15 +183,7 @@ public class PolymorphicObjectConverter(IWellKnownTypeRegistry wellKnownTypeRegi || valueType.IsEnum; } - bool IsListOfPrimitives(Type valueType) - { - var isEnumerable = typeof(IEnumerable).IsAssignableFrom(valueType) && valueType.IsGenericType && valueType.GetGenericArguments().Length == 1; - if (!isEnumerable) return false; - var elementType = valueType.GetGenericArguments()[0]; - return IsPrimitive(elementType); - } - - if (IsPrimitive(type) || IsListOfPrimitives(type)) + if (IsPrimitive(type)) { // Remove the converter so that we don't end up in an infinite loop. newOptions.Converters.RemoveWhere(x => x is PolymorphicObjectConverterFactory); @@ -207,8 +199,8 @@ public class PolymorphicObjectConverter(IWellKnownTypeRegistry wellKnownTypeRegi if (type == typeof(JObject) || type == typeof(JArray) || type == typeof(JsonObject) || type == typeof(JsonArray)) { writer.WriteStartObject(); - writer.WriteString(IslandPropertyName, value.ToString()); writer.WriteString(TypePropertyName, type.GetSimpleAssemblyQualifiedName()); + writer.WriteString(IslandPropertyName, value.ToString()); writer.WriteEndObject(); return; } @@ -253,20 +245,6 @@ public class PolymorphicObjectConverter(IWellKnownTypeRegistry wellKnownTypeRegi writer.WriteStartObject(); - if (jsonElement.ValueKind == JsonValueKind.Array) - { - writer.WritePropertyName(ItemsPropertyName); - jsonElement.WriteTo(writer); - } - else - { - foreach (var property in jsonElement.EnumerateObject().Where(property => !property.NameEquals(TypePropertyName))) - { - writer.WritePropertyName(property.Name); - property.Value.WriteTo(writer); - } - } - if (type != typeof(ExpandoObject)) { if (shouldWriteTypeField) @@ -283,10 +261,24 @@ public class PolymorphicObjectConverter(IWellKnownTypeRegistry wellKnownTypeRegi } } + if (jsonElement.ValueKind == JsonValueKind.Array) + { + writer.WritePropertyName(ItemsPropertyName); + jsonElement.WriteTo(writer); + } + else + { + foreach (var property in jsonElement.EnumerateObject().Where(property => !property.NameEquals(TypePropertyName))) + { + writer.WritePropertyName(property.Name); + property.Value.WriteTo(writer); + } + } + writer.WriteEndObject(); } - private Type? ReadType(Utf8JsonReader reader) + private Type? ReadType(Utf8JsonReader reader, JsonSerializerOptions options) { if (reader.TokenType != JsonTokenType.StartObject) return null; @@ -301,7 +293,14 @@ public class PolymorphicObjectConverter(IWellKnownTypeRegistry wellKnownTypeRegi if (reader.TokenType == JsonTokenType.PropertyName && reader.ValueTextEquals(TypePropertyName)) { reader.Read(); // Move to the value of the _type property - typeName = reader.GetString(); + if (options.Converters.OfType().FirstOrDefault() is { } typeJsonConverter) + { + return typeJsonConverter.Read(ref reader, typeof(Type), options); + } + else + { + typeName = reader.GetString(); + } break; } @@ -331,7 +330,7 @@ public class PolymorphicObjectConverter(IWellKnownTypeRegistry wellKnownTypeRegi } // If we found the _type property, attempt to resolve the type. - var targetType = typeName != null ? wellKnownTypeRegistry.TryGetType(typeName, out var type) ? type : Type.GetType(typeName) : default; + var targetType = typeName != null ? Type.GetType(typeName) : default; return targetType; } diff --git a/test/integration/Elsa.Workflows.IntegrationTests/Serialization/JsonSerialization/Tests.cs b/test/integration/Elsa.Workflows.IntegrationTests/Serialization/JsonSerialization/Tests.cs index 95329ed2c..1a6ff38d5 100644 --- a/test/integration/Elsa.Workflows.IntegrationTests/Serialization/JsonSerialization/Tests.cs +++ b/test/integration/Elsa.Workflows.IntegrationTests/Serialization/JsonSerialization/Tests.cs @@ -89,6 +89,23 @@ public class SerializationTests(ITestOutputHelper testOutputHelper) Assert.Equal(typeof(List), result.GetType()); } + [Fact] + public void RoundtripPrimitiveCollections() + { + var dict = new Dictionary + { + { "Content", new List + { + Guid.NewGuid() + } + } + }; + var jsonSerialized = SerializeUsingPayloadSerializer(dict); + var transformationModel = DeSerializeDictionaryUsingPayloadSerializer(jsonSerialized); + var result = transformationModel["Content"]; + Assert.Equal(typeof(List), result.GetType()); + } + private string SerializeUsingPayloadSerializer(object obj) { var payloadSerializer = _services.GetRequiredService();