Integrate headers into the NatsSerializer API

[NicholasTNG]

Proposed change

Adapt the INatsDeserialize and INatsSerialize interfaces in a way that makes it possible to access the headers during deserialization and set them during serialization. E.g. like this

public interface INatsSerialize<in T>
{
    void Serialize(IBufferWriter<byte> bufferWriter, T value);
    void Serialize(IBufferWriter<byte> bufferWriter, T value, out NatsHeaders headers);
}

public interface INatsDeserialize<out T>
{
    T? Deserialize(in ReadOnlySequence<byte> buffer);
    T? Deserialize(in ReadOnlySequence<byte> buffer, NatsHeaders headers);
}

One could add a default implementation for the new method to call the old method without forwarding the headers to ensure backwards compatibility. I'm not sure if there is a way to do this and still have the option to only implement the new method and not both.

Alternatively, one could also provide the whole message object to be able to access everything necessary (maybe the subject is also interesting?). However, that open the object to be changed in non-obvious ways.

Use case

With this change the headers can be used to change the behavior of the deserialization (e.g. which data format was used, which encoding was used and so on). Of course, this would need the headers to also be correctly set during serialization. An example where this would be helpful is the proposal for usage of CloudEvents with NATS in the binary mode: https://github.com/cloudevents/spec/blob/main/cloudevents/bindings/nats-protocol-binding.md#31-binary-content-mode

With the current state, it is not possible for the Headers to be accessed during (de-)serialization. If one wants to implement cloudevents protocol bindings, this needs to happen in a separate step to the deserialization even though it is deserialization itself. To use them as conveniently as normal serialization, one needs to add wrappers for consumers and extension methods for subscribe, publish and so forth.

Contribution

I'd probably be able to implement it, but I'd want some tips on how this can best be done backwardscompatibly.

[mtmk]

This is a good idea. A few notes: needless to say this would be a breaking change. just an idea about passing the headers in: maybe we should create a wrapper class e.g. NatsSerializationInfo or something which would have the headers and potentially the subject? also headers are readonly at that stage so we probably should spit out an object that contains the changes required rather than altering the headers.

EDIT: I think the proposal above is great. (just thinking out loud) here is what I would add for compatibility i.e. not breaking the implementations 'too much' with an easy upgrade path + leave room for future expansion:

public interface INatsSerializerInfo
{
    NatsHeader Headers { get }
    string Subject { get; }
}

public interface INatsSerializerUpdater
{
    IEnumerable<INatsSerializerCommand> Commands { get; }
}

public class NatsSerializerSetHeaderCommand : INatsSerializerCommand
{
}

public interface INatsSerialize<in T>
{
    void Serialize(IBufferWriter<byte> bufferWriter, T value);

    bool SupportsInfo => false
    INatsSerializerUpdater Serialize(IBufferWriter<byte> bufferWriter, T value, in INatsSerializerInfo info) => throw new NotImplementedException();
}

public interface INatsDeserialize<out T>
{
    T? Deserialize(in ReadOnlySequence<byte> buffer);

    bool SupportsInfo => false;
    T? Deserialize(in ReadOnlySequence<byte> buffer, INatsSerializerInfo info) => throw new NotImplementedException();
}

For increased build compatibility (binary compatibility isn't possible) we can use default implementations. Unfortunately netstandard2.0 (which is mostly for .NET Framework Targets) builds will break. netstandard2.1 and net6.0+ targets would build without breaking existing code.

#if NETSTANDARD2_0
    bool SupportsInfo { get; }
    INatsSerializerUpdater Serialize(IBufferWriter<byte> bufferWriter, T value, in INatsSerializerInfo info);
#else
    bool SupportsInfo => false
    INatsSerializerUpdater Serialize(IBufferWriter<byte> bufferWriter, T value, in INatsSerializerInfo info) => throw new NotImplementedException();
#endif

[NicholasTNG]

Thanks for your input! I agree that adding the abstraction of INatsSerializerInfo is probably a good idea. And there is probably not a way around anyone who implements the new version of the methods to also need to implement the old ones if we want to keep backwards compatibility as much as possible.

Do you think there is any problem with using default interface implementations in this way:

public interface INatsSerialize<in T>
{
    void Serialize(IBufferWriter<byte> bufferWriter, T value);

    INatsSerializerUpdater Serialize(IBufferWriter<byte> bufferWriter, T value, in INatsSerializerInfo info) {
        Serialize(bufferWriter, value);
        return /* Empty updater */;
    }
}

public interface INatsDeserialize<out T>
{
    T? Deserialize(in ReadOnlySequence<byte> buffer);

    T? Deserialize(in ReadOnlySequence<byte> buffer, INatsSerializerInfo info) => Deserialize(buffer);
}

That way, there is no need for the SupportsInfo field, making it all a bit simpler.

[mtmk]

Thanks for your input! I agree that adding the abstraction of INatsSerializerInfo is probably a good idea. And there is probably not a way around anyone who implements the new version of the methods to also need to implement the old ones if we want to keep backwards compatibility as much as possible.

Do you think there is any problem with using default interface implementations in this way:

public interface INatsSerialize
{
void Serialize(IBufferWriter bufferWriter, T value);

INatsSerializerUpdater Serialize(IBufferWriter<byte> bufferWriter, T value, in INatsSerializerInfo info) {
    Serialize(bufferWriter, value);
    return /* Empty updater */;
}

}

public interface INatsDeserialize
{
T? Deserialize(in ReadOnlySequence buffer);

T? Deserialize(in ReadOnlySequence<byte> buffer, INatsSerializerInfo info) => Deserialize(buffer);

}
That way, there is no need for the SupportsInfo field, making it all a bit simpler.

that is a good idea but how do we decide what to call? are you suggesting we essentially deprecate the old one? Would there be cases where applications might want to avoid additional allocations e.g. INatsSerializerInfo?