Serialize & Deserialize should have access to headers

[adminnz]

Proposed change

Provide the NatsHeaders to the Deserialize method.

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

Provide the ability for the Serialize to provide the headers for the serialized payload.

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

Use case

• If an endpoint wants to handle $SYS.REQ.USER.AUTH request messages that have encrypted contents, you need access to the headers to correctly deserialize to a NatsAuthorizationRequestClaims.
• A consumer might need access to a header to understand the version of a message, encoding, etc.

The current INatsDeserialize interface only gives you the payload bytes, and so there is no way (that i can see) to implement a clean message handler when you need header information for deserializing.

This would improve the code readability and maintainability, and allow it to be implemented like other "simple" messages.

Because currently the code for $SYS.REQ.USER.AUTH would have to do:

await _svcServer.AddEndpointAsync<byte[]>(HandleMessage, subject: "$SYS.REQ.USER.AUTH");

async ValueTask HandleMessage(NatsSvcMsg<byte[]> message)
{
var request = DecodeMessage(message);
/// handle request
}

NatsAuthorizationRequest DecodeMessage(NatsSvcMsg<byte[]> message)
{
    string jwt;
    if (message.Headers.TryGetValue("Nats-Server-Xkey", out var serverKey))
    {
        var encryptionKeyPair = FindEncryptionKey(serverKey);
        var decrypted = encryptionKeyPair.Open(message.Data);
        jwt = Encoding.ASCII.GetString(decrypted);
    }
    else
    {
        jwt = Encoding.ASCII.GetString(message.Data);
    }
     var arc = NatsJwt.DecodeClaims<NatsAuthorizationRequestClaims>(jwt);
    return arc.AuthorizationRequest;

}

If the headers were available, it would allow for Serialization/Deserialization to be handled the same way other "simple" messages are.

await _svcServer.AddEndpointAsync<NatsAuthorizationRequest>(HandleMessage, subject: "$SYS.REQ.USER.AUTH", serializer: new NatsAuthorizationRequestDeserializer());

async ValueTask HandleMessage(NatsSvcMsg<NatsAuthorizationRequest> message)
{
    var request = message.Data;
    //handle request
}

class NatsAuthorizationRequestDeserializer: INatsDeserialize<NatsAuthorizationRequest>
{
    public T? Deserialize(in ReadOnlySequence<byte> buffer, NatsHeaders? headers)
    {
        string jwt;
        if (headers.TryGetValue("Nats-Server-Xkey", out var serverKey))
        {
            var encryptionKeyPair = FindEncryptionKey(serverKey);
            var decrypted = encryptionKeyPair.Open(buffer);
            jwt = Encoding.ASCII.GetString(decrypted);
        }
        else
        {
            jwt = Encoding.ASCII.GetString(buffer);
        }
         var arc = NatsJwt.DecodeClaims<NatsAuthorizationRequestClaims>(jwt);
        return arc.AuthorizationRequest;
    }
}

Contribution

No response

[mtmk]

I think this is a good idea. i always thought passing the headers to serializers would be beneficial but never heard anyone coming up with a use case. This would also be a good candidate for using default interface methods to make sure the changes are backward compatible (see also #1061).

[adminnz]

If default interface methods were to be used like this:

    T? Deserialize(in ReadOnlySequence<byte> buffer);

#if NET8_0_OR_GREATER
    T? Deserialize(in ReadOnlySequence<byte> buffer, in INatsHeaders? headers) => Deserialize(in buffer);
#endif

Because default interface methods are only supported .NET 8+, the source code is going to be littered with

#if NET8_0_OR_GREATER
    deserializer.Deserialize(payload, headers);
#else
    deserializer.Deserialize(payload);
#endif

Also looking at the project code, there are multiple places where a NatsMsg is being constructed "differently".
For example NatsKVStore.cs has a code path that works with the jetstream protocol and deals with a StoredMessage.
But as a user of the library I would expect to have the same "API" feel as the non-jetstream APIs.
So that means the internal implementation (non-direct) path would be something like this.

if (response.Message.Data.Length > 0)
{
	var buffer = new ReadOnlySequence<byte>(response.Message.Data);
	try
	{
		NatsHeaders? headers = null;
		if (response.Message.Hdrs is not null)
		{
			headers = new NatsHeaders();
			var bytes = Convert.FromBase64String(response.Message.Hdrs);
			var sequence = new ReadOnlySequence<byte>(bytes);
			if (!JetStreamContext.Connection.HeaderParser.ParseHeaders(new SequenceReader<byte>(sequence), headers))
			{
				throw new NatsException("Error parsing headers");
			}
		}

		data = serializer.Deserialize(buffer, headers);
	}
	catch (Exception e)
	{
		deserializeException = new NatsDeserializeException(buffer.ToArray(), e);
		data = default;
	}
}
else
{
	data = default;
}

This gets rather messy the further through the code base you see that there are multiple implementations of constructing the parts of a NatsMsg.
Is there maybe the potential to consolidate the message handling into clear & concise factory Api ?

[mtmk]

@adminnz

Because default interface methods are only supported .NET 8+, the source code is going to be littered with

yes we would have to do that if we want to maintain BC at least for net8+

Is there maybe the potential to consolidate the message handling into clear & concise factory Api ?

sure. we do have MsgBuilder but not used internally. Happy to accept contributions ;-) or feel free to create a separate issue for it we can prioritize it.

nats.net/src/NATS.Client.Core/NatsMsg.cs

Lines 443 to 452 in 4856762

	/// <summary>
	/// Builder class for creating <see cref="NatsMsg{T}" />
	/// </summary>
	public class NatsMsgBuilder<T>
	{
	/// <summary>
	/// The destination subject to publish to.
	/// </summary>
	public string Subject { get; set; } = null!;

[mtmk]

• I have a proposal here: Add message context to serialization interfaces #1082
• add it to 3.0 release
• breaking change but cleaner
• no TFM ifdef