From 9e06e27effe6a0eb0576b9ab1923b5c18e7b2409 Mon Sep 17 00:00:00 2001 From: Joanna May Date: Wed, 24 Jul 2024 20:20:40 -0500 Subject: [PATCH] fix: serialize based on runtime type to support interfaces for serializable model property types --- .../test/src/CollectionsTest.cs | 2 + .../test/src/InterfaceTest.cs | 63 +++++++++++++++++++ .../test/src/MixAndMatchTest.cs | 2 + .../src/SerializableTypeConverter.cs | 38 ++++++++++- README.md | 4 -- 5 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 Chickensoft.Serialization.Tests/test/src/InterfaceTest.cs diff --git a/Chickensoft.Serialization.Tests/test/src/CollectionsTest.cs b/Chickensoft.Serialization.Tests/test/src/CollectionsTest.cs index 07ea54b..4f9fa30 100644 --- a/Chickensoft.Serialization.Tests/test/src/CollectionsTest.cs +++ b/Chickensoft.Serialization.Tests/test/src/CollectionsTest.cs @@ -134,6 +134,7 @@ public void SerializesCollections() { var bookAgain = JsonSerializer.Deserialize(json, options); + bookAgain.ShouldNotBeNull(); bookAgain.ShouldDeepEqual(book); } @@ -241,6 +242,7 @@ public void SerializesAList() { var bookcaseAgain = JsonSerializer.Deserialize(json, options); + bookcaseAgain.ShouldNotBeNull(); bookcaseAgain.ShouldDeepEqual(bookcase); } } diff --git a/Chickensoft.Serialization.Tests/test/src/InterfaceTest.cs b/Chickensoft.Serialization.Tests/test/src/InterfaceTest.cs new file mode 100644 index 0000000..2247659 --- /dev/null +++ b/Chickensoft.Serialization.Tests/test/src/InterfaceTest.cs @@ -0,0 +1,63 @@ +namespace Chickensoft.Serialization.Tests; + +using System.Text.Json; +using Chickensoft.Collections; +using Chickensoft.Introspection; +using Shouldly; +using Xunit; + +public partial class InterfaceTest { + public interface IAliasedModel { + int Value { get; } + } + + [Meta, Id("aliased_model")] + public partial class AliasedModel : IAliasedModel { + [Save("value")] + public int Value { get; set; } + } + + [Meta, Id("interface_test_model")] + public partial class TestModel { + [Save("aliased_model")] + public required IAliasedModel AliasedModel { get; set; } + } + + [Fact] + public void SerializesAsInterface() { + var model = new TestModel { + AliasedModel = new AliasedModel { Value = 10 } + }; + + var options = new JsonSerializerOptions { + WriteIndented = true, + TypeInfoResolver = new SerializableTypeResolver(), + Converters = { new SerializableTypeConverter(new Blackboard()) } + }; + + var serialized = JsonSerializer.Serialize(model, options); + + serialized.ShouldBe( + /*lang=json,strict*/ + """ + { + "$type": "interface_test_model", + "$v": 1, + "aliased_model": { + "$type": "aliased_model", + "$v": 1, + "value": 10 + } + } + """, + StringCompareShould.IgnoreLineEndings + ); + + var deserialized = JsonSerializer.Deserialize( + serialized, options + ); + + deserialized.ShouldNotBeNull(); + deserialized.AliasedModel.ShouldBeEquivalentTo(model.AliasedModel); + } +} diff --git a/Chickensoft.Serialization.Tests/test/src/MixAndMatchTest.cs b/Chickensoft.Serialization.Tests/test/src/MixAndMatchTest.cs index 2dfaf96..2042ac8 100644 --- a/Chickensoft.Serialization.Tests/test/src/MixAndMatchTest.cs +++ b/Chickensoft.Serialization.Tests/test/src/MixAndMatchTest.cs @@ -68,6 +68,7 @@ public void CanUseChickensoftModelsFromOutermostSTJModel() { var campCounselorAgain = JsonSerializer.Deserialize(json, options); + campCounselorAgain.ShouldNotBeNull(); campCounselorAgain.ShouldDeepEqual(campCounselor); } @@ -133,6 +134,7 @@ public void CanUseSTJModelsFromOutermostChickensoftModel() { var campInstructorAgain = JsonSerializer.Deserialize(json, options); + campInstructorAgain.ShouldNotBeNull(); campInstructorAgain.ShouldDeepEqual(campInstructor); } } diff --git a/Chickensoft.Serialization/src/SerializableTypeConverter.cs b/Chickensoft.Serialization/src/SerializableTypeConverter.cs index 256be1d..e8b6d5e 100644 --- a/Chickensoft.Serialization/src/SerializableTypeConverter.cs +++ b/Chickensoft.Serialization/src/SerializableTypeConverter.cs @@ -111,10 +111,34 @@ JsonSerializerOptions options object? propertyValue = null; + var propertyType = property.GenericType.ClosedType; + if (isPresentInJson) { + // Peek at the type of the property's value to see if it's more + // specific than the type in the metadata (i.e., a derived type). + // This allows us to support concrete implementations of declared types + // for properties that are interfaces or abstract classes. + + if ( + propertyValueJsonNode is JsonObject propertyJsonObj && + propertyValueJsonNode[TypeDiscriminator]?.ToString() + is { } propertyTypeId && + Graph.GetIdentifiableType(propertyTypeId) is { } idType + ) { + // Peeking a property value's type only works if the property value + // is a non-null object and it actually has a type discriminator. + // Types with System.Text.Json generated metadata won't necessarily + // have type discriminators or may have a different field name for the + // type discriminator, ensuring those will still be handled by STJ + // itself. + // + // Update known type to be the more specific type. + propertyType = idType; + } + propertyValue = JsonSerializer.Deserialize( propertyValueJsonNode, - property.GenericType.ClosedType, + propertyType, options ); } @@ -136,7 +160,7 @@ propertyValue is null && var typeInfo = options .TypeInfoResolver! - .GetTypeInfo(property.GenericType.ClosedType, options)!; + .GetTypeInfo(propertyType, options)!; // Our type resolver companion will have cached the closed type of // the collection type by using the callbacks provided in the generated @@ -223,8 +247,18 @@ metadata is not IConcreteIntrospectiveTypeMetadata concreteMetadata ); var propertyValue = property.Getter(value); + var valueType = propertyValue?.GetType(); var propertyType = property.GenericType.ClosedType; + if ( + valueType is { } && + options.TypeInfoResolver!.GetTypeInfo(valueType, options) is { } + ) { + // The actual instance type is a known serializable type, so we assume + // it is more specific than the declared property type. Use it instead. + propertyType = valueType; + } + json[propertyId] = JsonSerializer.SerializeToNode( value: propertyValue, inputType: propertyType, diff --git a/README.md b/README.md index 0fe7402..c86b43b 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,6 @@ Annoyingly, `System.Text.Json` requires you to tag derived types on the generati - ❌ Models must have parameterless constructors. - ❌ Serializable types must be partial. - ❌ Only collections supported are `HashSet`, `List`, and `Dictionary`. -- ❌ Referencing types by an interface is not supported. The Chickensoft serializer has strong opinions about how JSON serialization should be done. It's primarily intended to simplify the process of defining models for game save files, but you can use it any C# project which supports C# >= 11. @@ -194,9 +193,6 @@ public partial class Lawyer : Person { } ``` -> [!CAUTION] -> A serializable property cannot refer to a type by an interface. - ## ⏳ Versioning The serialization system provides support for versioning models when requirements inevitably change.