Deserialize empty payloads

[mtmk]

Remove the payload empty check when deserializing so that empty payloads are also passed to the deserializer for it to make the decision. This way, deserializers will have the option of what to do with the empty payload rather than just being skipped and a default value being returned. We also altered the existing default deserializers to still return a default value when the payload is empty to maintain backward compatibility. However, any custom deserializer implementations might break with this update. Applications can place a check for empty payloads or test their deserialization behavior for empty payloads:

public class MyDeserialize<T> : INatsDeserialize<T>
    public T? Deserialize(in ReadOnlySequence<byte> buffer)
    {
        if (buffer.Length == 0)
        {
            return default;
        }
        // deserializer implementation
    }
}

• resolves Deserializer is skipped for 0 length payloads #449

[caleblloyd]

Should we handle this at the sterilizer level instead? Make a chain-able ignoreEmptyPayload serailizer and add that as a default to the serailizer chains we use?

I feel like the setting is tied to the serailizer, not the subscription

[mtmk]

Should we handle this at the sterilizer level instead? Make a chain-able ignoreEmptyPayload serailizer and add that as a default

good idea but would still be a breaking change. it may break custom serializers application is using.

[thinkbeforecoding]

Is there a way to keep existing serializers as is, and be able to define new deserializers that parse empty as non null ? Like using an interface that inherit from IDeserialize...
That would not be a breaking change

[mtmk]

Is there a way to keep existing serializers as is, and be able to define new deserializers that parse empty as non null ? Like using an interface that inherit from IDeserialize...
That would not be a breaking change

genius! 🥇 ... created INatsDeserializeWithEmpty marker interface

[thinkbeforecoding]

That seems perfect.

[caleblloyd]

For what it's worth, the Interface Design Guidelines say:

❌ AVOID using marker interfaces (interfaces with no members).
If you need to mark a class as having a specific characteristic (marker), in general, use a custom attribute rather than an interface.

Should we use a custom attribute here instead?

[caleblloyd]

Another issue I see is the way that we have our serializes chain-able would mean that this marker interface (or attribute, if changed to an attribute) would mean that the marker interface or attribute would have to be on the first serailizer in the chain. What if my chain is

NatsRawSerializer (does not serialize empty) -> CustomSerailizer (does serialize empty)

Even though my CustomSerailizer is marked, it still does not work

If we want to preserve the chain-ability, we should probably re-think the approach to either

1. Add another property to INatsDeserialize such as HandlesEmptyPayload
2. Always try to Deserailize, but catch certain kinds of exceptions like System.NotSupportedException and return null when the payload is empty instead of throwing. Biggest issue with this approach is different serailizers may use different exception types for this

[mtmk]

AVOID using marker interfaces

didn't know this. I see it is a workaround and somewhat misusing a feature of the language etc. at the same time I don't like to be dogmatic. if it works, it's OK! but never mind...

1. Add another property to INatsDeserialize such as HandlesEmptyPayload
2. Always try to Deserailize, but catch certain kinds of exceptions like System.NotSupportedException

If we're going to introduce a breaking change, maybe we should fix the issue and just remove the 'payload > 0' check and be done with it. We already are going for a breaking release, maybe just fix the issue instead of working around it. as I think @caleblloyd already suggested this, we can fix the default serializers to handle empty payloads.

WDYT?

edit: updated the implementation accordingly

[caleblloyd]

Needs a format and a few tests failing then LGTM