API enhancements for reflection-based object binary serialization and deserialization

[pleonex]

Goal

Reduce the responsibility of DataReader and DataWriter by extracting the code that (de)serialize objects into a separate class.

Description

The classes DataReader and DataWriter are big and contain a lot of logic. I propose to clarify their responsibility to only handle primitive types and string. This includes byte[] and helper methods like writing padding as it's still integers / bytes.

The logic to serialize / deserialize models / objects (WriteUsingReflection, ReadUsingReflection) based on attributes on properties should be moved into a separate class. That code is similar to what a JsonSerializer or BinaryFormatter would do, so it should be different to the responsibility of DataWriter.

Additionally, we can take this opportunity to improve the API so that:

• Fix the issue where the order to process the properties is not guaranteed with .NET reflection. For instance by introducing a new attribute that sets the order / ID.

  • In .NET 6 and earlier versions, the GetProperties method does not return properties in a particular order, such as alphabetical or declaration order. Your code must not depend on the order in which properties are returned, because that order varies. However, starting with .NET 7, the ordering is deterministic based upon the metadata ordering in the assembly.

• Force users to specify the underlying type for boolean and enums, so it's clear reading the model.

Proposed solution

Some ideas to improve the design of the API, reduces the complexity, clarify implementation and fixes some of its issues:

• Move reflection-based into separate classes: BinarySerializer and BinaryDeserializer
  • API method will be Serialize<T>(object), Serialize(Type, object) and T Deserialize<T>(), object Deserialize(Type)
  • Create static methods as well
  • DataWriter / DataReader will be only for primitive types + string
  • Internally it uses a DataWriter / DataReader
• Remove need of top-level attribute Serializable as it is not needed
• Fix bug where the order of type.GetProperties is not guaranteed to match the C# code file
  • In .NET due to this, reflection-based API requires to define their own attribute like Order to define the order to iterate properties.
  • Proposal: create the attribute BinaryField(Order = x)
    • This attribute will guarantee the order to read / write properties
    • It will also specify that the property is to be used - replaces need of BinaryIgnore attribute
• Improve BinaryBoolean attribute:
  • Merge properties ReadAs and WriteAs into a new UnderlyingType
    • Do we have a use case to write and read with different types? Not so far.
  • Make property UnderlyingType mandatory so it's clear what type will be read/write.
  • Make TrueValue and FalseValue properties of type long instead of object Required object to read/write as string.
• Improve BinaryEnum attribute
  • Merge properties ReadAs and WriteAs into a new UnderlyingType
  • Make property UnderlyingType mandatory so it's clear what type will be read/write. By default use the underlying type defined in the enum.
• Rename BinaryForceEndianness to just BinaryEndianness
  • By default we will use the endianness from the underlying DataWriter / DataReader
  • The class will have an Endianness property that will forward the values to the underlying reader/writer

The concept is similar to the IConverter<,> concept. We could also consider being an implementation of IConverter<IBinary, object> and IConverter<object, IBinary> but we would need the actual types as a parameter.

Example

Deserializer:

Stream input = ...
MyHeader header = BinaryDeserializer.Deserialize<MyHeader>(input);
// or
var deserializer = new BinaryDeserializer(input);
MyHeader header = deserializer.Deserialize<MyHeader>();
Section section = deserializer.Deserialize<Section>();

Serializer:

Stream output = ...
MyHeader header = ...
BinarySerializer.Serialize(header);
// or
var serializer = new BinarySerializer(output);
serializer.Serialize(header);
serializer.Serialize(section);

Acceptance criteria

• Existing features still work after changes
• Breaking changes are documented
• There are classes with the unique responsibility of (de)serializing models
• Known issues with reflection are fixed
• There are documentation pages explaining how to use it

Status

• Design and write proposal
• Approve proposal after devs review
• Extract current serialization into separate class
• Extract current deserialization into separate class
• Remove Serializable attribute
• Improve BinaryBooleanAttribute
• Improve BinaryEnumAttribute
• Improve BinaryForceEndianness
• Implement order attribute
• Consider IConverter<T, IBinary> it won't work for most formats, useful for format sections / headers so no need of converters. It would be confusing for users.

[pleonex]

Starting .NET 7.0 (so .NET 8.0) the order of the properties is deterministic (see this PR). However we still support .NET 6.0, so the issue would be there. However, the declared order could be problematic when using base types. Base type property always are returned last, which could not be desirable.

We could keep the BinaryIgnore attribute and create an attribute like BinaryFieldOrder to optionally define the order of the properties. If one property in a type specify it, every property should as well, so the order is clear. The order doesn't need to be incremental, for instance there could be a difference of 10 between on property and another, allowing for base types to define the property in the middle. For .NET 6.0 this attribute would be mandatory.