Skip to content

Commit

Permalink
Serialization: avoid use of PolymorphicSerializer for sealed members
Browse files Browse the repository at this point in the history
  • Loading branch information
HermanSchoenfeld committed May 7, 2024
1 parent 4ead258 commit 3a622fd
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 3 deletions.
2 changes: 2 additions & 0 deletions src/Hydrogen/Serialization/Builder/CompositeSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ internal void Configure(Func<TItem> activator, MemberSerializationBinding[] memb

public override long ConstantSize => _constantSize;

public MemberSerializationBinding[] MemberBindings => _memberBindings;

public override long CalculateTotalSize(SerializationContext context, IEnumerable<TItem> items, bool calculateIndividualItems, out long[] itemSizes) {
var itemsArr = items as TItem[] ?? items.ToArray();
itemSizes = null;
Expand Down
19 changes: 16 additions & 3 deletions src/Hydrogen/Serialization/SerializerHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReferenceModeAttribute>(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);
Expand Down
31 changes: 31 additions & 0 deletions tests/Hydrogen.Tests/Serialization/ReferenceSerializerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -93,5 +99,30 @@ public void BugCase_1() {

Assert.That(() => serializer.CalculateSize(obj), Throws.Nothing);
}


[Test]
public void SealedMembersDoNotUsePolymorphicSerializers() {

//var unsealedSerializer = SerializerBuilder.AutoBuild<UnsealedMembersObject>();
var sealedSerializer = SerializerBuilder.AutoBuild<SealedMembersObject>();
Assert.That(sealedSerializer, Is.InstanceOf<ReferenceSerializer<SealedMembersObject>>());

var asReferenceSerializer = (ReferenceSerializer<SealedMembersObject>)sealedSerializer;
Assert.That(asReferenceSerializer.Internal, Is.InstanceOf<CompositeSerializer<SealedMembersObject>>());

var asCompositeSerializer = (CompositeSerializer<SealedMembersObject>)asReferenceSerializer.Internal;
Assert.That(asCompositeSerializer.MemberBindings.Length, Is.EqualTo(2));
Assert.That(asCompositeSerializer.MemberBindings[0].Serializer, Is.InstanceOf<ReferenceSerializer<string>>());
Assert.That(asCompositeSerializer.MemberBindings[1].Serializer, Is.InstanceOf<ReferenceSerializer<SealedMembersObject>>());

// check sealed member does not have polymorphic serializer
var sealedMemberSerializer = (ReferenceSerializer<string>)asCompositeSerializer.MemberBindings[0].Serializer;
Assert.That(sealedMemberSerializer.Internal, Is.InstanceOf<StringSerializer>());

// check unsealed member has polymorphic serializer
var unsealedMemberSerializer = (ReferenceSerializer<SealedMembersObject>)asCompositeSerializer.MemberBindings[1].Serializer;
Assert.That(unsealedMemberSerializer.Internal, Is.InstanceOf<PolymorphicSerializer<SealedMembersObject>>());
}

}
17 changes: 17 additions & 0 deletions tests/Hydrogen.Tests/Serialization/SerializerBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -540,4 +553,8 @@ public void SupportsNull_True() {

}

[Test]
public void BadAnnotation_1() {
Assert.That(SerializerBuilder.AutoBuild<BadAnnotationObject>, Throws.InvalidOperationException);
}
}

0 comments on commit 3a622fd

Please sign in to comment.