Add async serialization support#1574
Conversation
| get => _scheduleDelay; | ||
| } | ||
|
|
||
| public async ValueTask<byte[]?> GetDataAsync() |
There was a problem hiding this comment.
This is additive change GetDataAsync can be executed and it fallback to Data
| } | ||
| AssertMessage(); | ||
|
|
||
| if(Serializer is IAsyncMessageSerializer asyncMessaeSerializer) |
There was a problem hiding this comment.
We use only when serializer support async
dominikjeske
force-pushed
the
features/asyncSerializer
branch
from
a0867fd to
464a822
Compare
| /// <summary> | ||
| /// Async version of <seealso cref="IMessageSerializer"/> | ||
| /// </summary> | ||
| public interface IAsyncMessageSerializer : IMessageSerializer |
There was a problem hiding this comment.
We extend standard serializer so nothing is changing in code - envelope serializer type is still IMessageSerializer
| } | ||
|
|
||
| public bool TryDeserializeEnvelope(Envelope envelope, out IContinuation continuation) | ||
| public async ValueTask<IContinuation> TryDeserializeEnvelope(Envelope envelope) |
There was a problem hiding this comment.
Method signature is changing - for me it is even better because it is more function like - one result instead two (bool and IContinuation)
| if (_graph.TryFindMessageType(envelope.MessageType, out var messageType)) | ||
| { | ||
| envelope.Message = serializer.ReadFromData(messageType, envelope); | ||
| if (serializer is IAsyncMessageSerializer asyncMessageSerializer) |
There was a problem hiding this comment.
We use async if it implement this
| Task InvokeAsync(Envelope envelope, IChannelCallback channel); | ||
| Task InvokeAsync(Envelope envelope, IChannelCallback channel, Activity activity); | ||
| bool TryDeserializeEnvelope(Envelope envelope, out IContinuation continuation); | ||
| ValueTask<IContinuation> TryDeserializeEnvelope(Envelope envelope); |
There was a problem hiding this comment.
This is only one potential breaking change but I'm wonder if this interface should be public? Maybe it should be internal or nobody is executing this so event public we don't break anything
There was a problem hiding this comment.
This should not be part of the public interface. It's just a detail of the HandlerPipeline details
There was a problem hiding this comment.
@jeremydmiller So I think it should be no problem with change :) As a note in our company we use MS https://www.nuget.org/packages/Microsoft.CodeAnalysis.PublicApiAnalyzers/ to keep public API stable. It reminds to keep internal things (when things are internal your life is easier because analyser is not screaming in your face :) ). You have to use to this analyser especially when adding new API but tooling is quite good (especially "use this for whole project/solution" so you can add new API's in one click.
dominikjeske
force-pushed
the
features/asyncSerializer
branch
2 times, most recently
from
3171f71 to
a12297c
Compare
dominikjeske
force-pushed
the
features/asyncSerializer
branch
from
a12297c to
32a5c2b
Compare
2 participants
No description provided.