Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SystemTextJsonFormatter in NewLineDelimitedMessageHandler #962

Merged
merged 1 commit into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions src/StreamJsonRpc/NewLineDelimitedMessageHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Globalization;
using System.IO.Pipelines;
using System.Text;
using Nerdbank.Streams;
using StreamJsonRpc.Protocol;
using StreamJsonRpc.Reflection;

Expand All @@ -19,6 +20,11 @@ namespace StreamJsonRpc;
/// </remarks>
public class NewLineDelimitedMessageHandler : PipeMessageHandler
{
/// <summary>
/// The <see cref="IBufferWriter{T}"/> that buffers an outgoing message.
/// </summary>
private readonly Sequence<byte> contentSequenceBuilder = new Sequence<byte>(ArrayPool<byte>.Shared);

/// <summary>
/// Backing field for the <see cref="NewLine"/> property.
/// </summary>
Expand Down Expand Up @@ -105,7 +111,22 @@ protected override void Write(JsonRpcMessage content, CancellationToken cancella
Assumes.NotNull(this.Writer);

cancellationToken.ThrowIfCancellationRequested();
this.Formatter.Serialize(this.Writer, content);

// Some formatters (e.g. MessagePackFormatter) needs the encoded form in order to produce JSON for tracing.
// Other formatters (e.g. JsonMessageFormatter) would prefer to do its own tracing while it still has a JToken.
// We only help the formatters that need the byte-encoded form here. The rest can do it themselves.
if (this.Formatter is IJsonRpcFormatterTracingCallbacks tracer)
{
this.Formatter.Serialize(this.contentSequenceBuilder, content);
tracer.OnSerializationComplete(content, this.contentSequenceBuilder);
this.Writer.Write(this.contentSequenceBuilder);
this.contentSequenceBuilder.Reset();
}
else
{
this.Formatter.Serialize(this.Writer, content);
}

this.Writer.Write(this.newLineBytes.Span);
}

Expand Down Expand Up @@ -180,13 +201,6 @@ private static ReadOnlyMemory<byte> GetLineFeedSequence(Encoding encoding, NewLi
/// </summary>
private void CommonConstructor()
{
if (this.Formatter is IJsonRpcFormatterTracingCallbacks)
{
// Such a formatter requires that we make their own written bytes available back to them for logging purposes.
// We haven't implemented support for such, and the JsonMessageFormatter doesn't need it, so no need to.
throw new NotSupportedException("Formatters that implement " + nameof(IJsonRpcFormatterTracingCallbacks) + " are not supported. Try using " + nameof(JsonMessageFormatter) + ".");
}

if (this.Formatter.Encoding.WebName != Encoding.UTF8.WebName)
{
throw new NotSupportedException("Only UTF-8 formatters are supported.");
Expand Down
13 changes: 7 additions & 6 deletions test/StreamJsonRpc.Tests/NewLineDelimitedMessageHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ public void NewLine()
Assert.Equal(NewLineDelimitedMessageHandler.NewLineStyle.Lf, handler.NewLine);
}

[Fact]
public async Task Reading_MixedLineEndings()
[Theory, PairwiseData]
public async Task Reading_MixedLineEndings(bool useSystemTextJson)
{
var pipe = new Pipe();
var handler = new NewLineDelimitedMessageHandler(null, pipe.Reader, new JsonMessageFormatter());
var handler = new NewLineDelimitedMessageHandler(null, pipe.Reader, useSystemTextJson ? new SystemTextJsonFormatter() : new JsonMessageFormatter());

// Send messages with mixed line endings to the handler..
var testFormatter = new JsonMessageFormatter();
Expand Down Expand Up @@ -105,11 +105,11 @@ public async Task Reading_IncompleteLine()
Assert.True(msg is JsonRpcRequest);
}

[Fact]
public async Task Writing_MixedLineEndings()
[Theory, PairwiseData]
public async Task Writing_MixedLineEndings(bool useSystemTextJson)
{
var pipe = new Pipe();
var handler = new NewLineDelimitedMessageHandler(pipe.Writer, null, new JsonMessageFormatter());
var handler = new NewLineDelimitedMessageHandler(pipe.Writer, null, useSystemTextJson ? new SystemTextJsonFormatter() : new JsonMessageFormatter());

// Use the handler to write out a couple messages with mixed line endings.
await handler.WriteAsync(this.mockMessages[0], this.TimeoutToken); // CRLF
Expand All @@ -119,6 +119,7 @@ public async Task Writing_MixedLineEndings()

using var streamReader = new StreamReader(pipe.Reader.AsStream(), handler.Formatter.Encoding);
string allMessages = await streamReader.ReadToEndAsync();
this.Logger.WriteLine(allMessages);

// Use CR and LF counts to quickly figure whether our new line styles were honored.
Assert.Equal(3, allMessages.Split('\n').Length);
Expand Down