From 3a622fdc15963877eec50c887c35cae5c015f3dc Mon Sep 17 00:00:00 2001 From: Herman Schoenfeld Date: Tue, 7 May 2024 23:56:14 +1000 Subject: [PATCH] Serialization: avoid use of PolymorphicSerializer for sealed members --- .../Builder/CompositeSerializer.cs | 2 ++ .../Serialization/SerializerHelper.cs | 19 ++++++++++-- .../Serialization/ReferenceSerializerTests.cs | 31 +++++++++++++++++++ .../Serialization/SerializerBuilderTests.cs | 17 ++++++++++ 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/Hydrogen/Serialization/Builder/CompositeSerializer.cs b/src/Hydrogen/Serialization/Builder/CompositeSerializer.cs index 01f1cc31..d7b60c8c 100644 --- a/src/Hydrogen/Serialization/Builder/CompositeSerializer.cs +++ b/src/Hydrogen/Serialization/Builder/CompositeSerializer.cs @@ -39,6 +39,8 @@ internal void Configure(Func activator, MemberSerializationBinding[] memb public override long ConstantSize => _constantSize; + public MemberSerializationBinding[] MemberBindings => _memberBindings; + public override long CalculateTotalSize(SerializationContext context, IEnumerable items, bool calculateIndividualItems, out long[] itemSizes) { var itemsArr = items as TItem[] ?? items.ToArray(); itemSizes = null; diff --git a/src/Hydrogen/Serialization/SerializerHelper.cs b/src/Hydrogen/Serialization/SerializerHelper.cs index c7a3930a..af3e2bd1 100644 --- a/src/Hydrogen/Serialization/SerializerHelper.cs +++ b/src/Hydrogen/Serialization/SerializerHelper.cs @@ -69,13 +69,26 @@ IItemSerializer AssembleRecursively(SerializerFactory factory, Type itemType) { Guard.Ensure(factory.HasSerializer(member.PropertyType), "Failed to assemble serializer for member type"); - // We don't use the member type serializer but instead use a PolymorphicSerializer to ensure polymorphic references are handled correctly + // Get any specified reference annotation for this member var referenceMode = member.MemberInfo.TryGetCustomAttributeOfType(false, out var attr) ? attr.Mode : ReferenceSerializerMode.Default; - // TODO: avoid PolymorphicSerializer usage for SEALED types (will never be polymorphic) - var memberSerializer = CreatePolymorphicSerializer(factory, member.PropertyType).AsReferenceSerializer(referenceMode); + + // Ensure value types have no such annotations (since they can never be references) + Guard.Against(attr is not null && member.PropertyType.IsValueType, $"Member {member.DeclaringType.ToStringCS()}.{member.Name} has incorrectly specified a {nameof(ReferenceModeAttribute)} for a value type {member.PropertyType.ToStringCS()}"); + + // If member type is sealed, we can use a serializer for that type, otherwise we need a polymorphic serializer for that type. + // NOTE: these may recursively call AssembleRecursively to ensure all types are registered + var memberSerializer = + member.PropertyType.IsSealed ? + factory.GetSerializer(member.PropertyType).AsDereferencedSerializer() : + CreatePolymorphicSerializer(factory, member.PropertyType); + + // if member type is a reference type, we need to wrap the serializer as a reference serializer + if (!member.PropertyType.IsValueType) + memberSerializer = memberSerializer.AsReferenceSerializer(referenceMode); + memberBindings.Add(new(member, memberSerializer)); } ConfigureCompositeSerializer(compositeSerializer, itemType, memberBindings); diff --git a/tests/Hydrogen.Tests/Serialization/ReferenceSerializerTests.cs b/tests/Hydrogen.Tests/Serialization/ReferenceSerializerTests.cs index bf6e6901..44296def 100644 --- a/tests/Hydrogen.Tests/Serialization/ReferenceSerializerTests.cs +++ b/tests/Hydrogen.Tests/Serialization/ReferenceSerializerTests.cs @@ -23,6 +23,12 @@ public class TestObject { public string Property3 { get; set; } } + + public class SealedMembersObject { + public string MemberOfSealedType { get; set; } + public SealedMembersObject MemberOfUnsealedType { get; set; } + } + [Test] public void NullableDoesNotReuseContextReference() { // test object @@ -93,5 +99,30 @@ public void BugCase_1() { Assert.That(() => serializer.CalculateSize(obj), Throws.Nothing); } + + + [Test] + public void SealedMembersDoNotUsePolymorphicSerializers() { + + //var unsealedSerializer = SerializerBuilder.AutoBuild(); + var sealedSerializer = SerializerBuilder.AutoBuild(); + Assert.That(sealedSerializer, Is.InstanceOf>()); + + var asReferenceSerializer = (ReferenceSerializer)sealedSerializer; + Assert.That(asReferenceSerializer.Internal, Is.InstanceOf>()); + + var asCompositeSerializer = (CompositeSerializer)asReferenceSerializer.Internal; + Assert.That(asCompositeSerializer.MemberBindings.Length, Is.EqualTo(2)); + Assert.That(asCompositeSerializer.MemberBindings[0].Serializer, Is.InstanceOf>()); + Assert.That(asCompositeSerializer.MemberBindings[1].Serializer, Is.InstanceOf>()); + + // check sealed member does not have polymorphic serializer + var sealedMemberSerializer = (ReferenceSerializer)asCompositeSerializer.MemberBindings[0].Serializer; + Assert.That(sealedMemberSerializer.Internal, Is.InstanceOf()); + + // check unsealed member has polymorphic serializer + var unsealedMemberSerializer = (ReferenceSerializer)asCompositeSerializer.MemberBindings[1].Serializer; + Assert.That(unsealedMemberSerializer.Internal, Is.InstanceOf>()); + } } diff --git a/tests/Hydrogen.Tests/Serialization/SerializerBuilderTests.cs b/tests/Hydrogen.Tests/Serialization/SerializerBuilderTests.cs index 0a95ae70..cee7744d 100644 --- a/tests/Hydrogen.Tests/Serialization/SerializerBuilderTests.cs +++ b/tests/Hydrogen.Tests/Serialization/SerializerBuilderTests.cs @@ -32,6 +32,19 @@ public class TwoPropertyObject { public object Prop2 { get; set; } } + public struct TestStruct { + public string Property1 { get; set; } + public string Property2 { get; set; } + public string Property3 { get; set; } + + } + + public class BadAnnotationObject { + + [ReferenceMode(Nullable = true)] // can't specify this for value type members + public TestStruct Property1 { get; set; } + } + [Test] public void TestObject_1() { // test object @@ -540,4 +553,8 @@ public void SupportsNull_True() { } + [Test] + public void BadAnnotation_1() { + Assert.That(SerializerBuilder.AutoBuild, Throws.InvalidOperationException); + } }