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

Replace specialized tracker code in formatters with general APIs #385

Merged
merged 1 commit into from
Nov 27, 2019
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
106 changes: 66 additions & 40 deletions src/StreamJsonRpc/JsonMessageFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,24 @@ public class JsonMessageFormatter : IJsonRpcAsyncMessageTextFormatter, IJsonRpcF
/// </summary>
private readonly SequenceTextReader sequenceTextReader = new SequenceTextReader();

/// <summary>
/// Backing field for the <see cref="MultiplexingStream"/> property.
/// </summary>
private MultiplexingStream? multiplexingStream;

/// <summary>
/// <see cref="MessageFormatterProgressTracker"/> instance containing useful methods to help on the implementation of message formatters.
/// </summary>
private readonly MessageFormatterProgressTracker formatterProgressTracker = new MessageFormatterProgressTracker();
private MessageFormatterProgressTracker? formatterProgressTracker;

/// <summary>
/// The helper for marshaling pipes as RPC method arguments.
/// </summary>
private readonly MessageFormatterDuplexPipeTracker duplexPipeTracker = new MessageFormatterDuplexPipeTracker();
private MessageFormatterDuplexPipeTracker? duplexPipeTracker;

/// <summary>
/// The helper for marshaling <see cref="IAsyncEnumerable{T}"/> in RPC method arguments or return values.
/// </summary>
private MessageFormatterEnumerableTracker? enumerableTracker;

/// <summary>
Expand Down Expand Up @@ -212,11 +220,11 @@ public Version ProtocolVersion
/// </summary>
public MultiplexingStream? MultiplexingStream
{
get => this.duplexPipeTracker.MultiplexingStream;
get => this.multiplexingStream;
set
{
Verify.Operation(this.rpc == null, Resources.FormatterConfigurationLockedAfterJsonRpcAssigned);
this.duplexPipeTracker.MultiplexingStream = value;
this.multiplexingStream = value;
}
}

Expand All @@ -230,7 +238,9 @@ JsonRpc IJsonRpcInstanceContainer.Rpc

if (value != null)
{
this.formatterProgressTracker = new MessageFormatterProgressTracker(value, this);
this.enumerableTracker = new MessageFormatterEnumerableTracker(value, this);
this.duplexPipeTracker = new MessageFormatterDuplexPipeTracker(value, this) { MultiplexingStream = this.multiplexingStream };
}
}
}
Expand All @@ -244,6 +254,42 @@ JsonRpc IJsonRpcInstanceContainer.Rpc
/// <inheritdoc/>
bool IJsonRpcFormatterState.SerializingRequest => this.serializingRequest;

/// <summary>
/// Gets the <see cref="MessageFormatterProgressTracker"/> instance containing useful methods to help on the implementation of message formatters.
/// </summary>
private MessageFormatterProgressTracker FormatterProgressTracker
{
get
{
Assumes.NotNull(this.formatterProgressTracker); // This should have been set in the Rpc property setter.
return this.formatterProgressTracker;
}
}

/// <summary>
/// Gets the helper for marshaling pipes as RPC method arguments.
/// </summary>
private MessageFormatterDuplexPipeTracker DuplexPipeTracker
{
get
{
Assumes.NotNull(this.duplexPipeTracker); // This should have been set in the Rpc property setter.
return this.duplexPipeTracker;
}
}

/// <summary>
/// Gets the helper for marshaling <see cref="IAsyncEnumerable{T}"/> in RPC method arguments or return values.
/// </summary>
private MessageFormatterEnumerableTracker EnumerableTracker
{
get
{
Assumes.NotNull(this.enumerableTracker); // This should have been set in the Rpc property setter.
return this.enumerableTracker;
}
}

/// <inheritdoc/>
public JsonRpcMessage Deserialize(ReadOnlySequence<byte> contentBuffer) => this.Deserialize(contentBuffer, this.Encoding);

Expand Down Expand Up @@ -332,11 +378,6 @@ public JsonRpcMessage Deserialize(JToken json)
/// <returns>The JSON of the message.</returns>
public JToken Serialize(JsonRpcMessage message)
{
if (message is IJsonRpcMessageWithId msgWithId && (message is JsonRpcResult || message is JsonRpcError))
{
this.duplexPipeTracker.OnResponseSent(msgWithId.RequestId, successful: msgWithId is JsonRpcResult);
}

this.observedTransmittedRequestWithStringId |= message is JsonRpcRequest request && request.RequestId.String != null;

// Pre-tokenize the user data so we can use their custom converters for just their data and not for the base message.
Expand Down Expand Up @@ -365,7 +406,7 @@ public JToken Serialize(JsonRpcMessage message)
/// <inheritdoc/>
public void Dispose()
{
this.duplexPipeTracker.Dispose();
this.duplexPipeTracker?.Dispose();
}

private static IReadOnlyDictionary<string, object> PartiallyParseNamedArguments(JObject args)
Expand Down Expand Up @@ -465,8 +506,6 @@ private void TokenizeUserData(JsonRpcMessage jsonRpcMessage)
if (jsonRpcMessage is Protocol.JsonRpcRequest request)
{
this.serializingRequest = true;
this.formatterProgressTracker.RequestIdBeingSerialized = request.RequestId;
this.duplexPipeTracker.RequestIdBeingSerialized = request.RequestId;

if (request.ArgumentsList != null)
{
Expand All @@ -493,8 +532,6 @@ private void TokenizeUserData(JsonRpcMessage jsonRpcMessage)
{
this.serializingMessageWithId = default;
this.serializingRequest = false;
this.formatterProgressTracker.RequestIdBeingSerialized = default;
this.duplexPipeTracker.RequestIdBeingSerialized = default;
}
}

Expand Down Expand Up @@ -535,7 +572,7 @@ private JsonRpcRequest ReadRequest(JToken json)
// If method is $/progress, get the progress instance from the dictionary and call Report
string method = json.Value<string>("method");

if (string.Equals(method, MessageFormatterProgressTracker.ProgressRequestSpecialMethod, StringComparison.Ordinal))
if (this.formatterProgressTracker != null && string.Equals(method, MessageFormatterProgressTracker.ProgressRequestSpecialMethod, StringComparison.Ordinal))
{
try
{
Expand Down Expand Up @@ -578,9 +615,6 @@ private JsonRpcResult ReadResult(JToken json)

JToken result = json["result"];

this.formatterProgressTracker.OnResponseReceived(id);
this.duplexPipeTracker.OnResponseReceived(id, successful: true);

return new JsonRpcResult(this.JsonSerializer)
{
RequestId = id,
Expand All @@ -595,9 +629,6 @@ private JsonRpcError ReadError(JToken json)
RequestId id = this.NormalizeId(json["id"].ToObject<RequestId>());
JToken error = json["error"];

this.formatterProgressTracker.OnResponseReceived(id);
this.duplexPipeTracker.OnResponseReceived(id, successful: false);

return new JsonRpcError
{
RequestId = id,
Expand Down Expand Up @@ -679,15 +710,13 @@ public override bool TryGetArgumentByNameOrIndex(string? name, int position, Typ
// Deserialization of messages should never occur concurrently for a single instance of a formatter.
Assumes.True(this.formatter.deserializingMessageWithId.IsEmpty);
this.formatter.deserializingMessageWithId = this.RequestId;
this.formatter.duplexPipeTracker.RequestIdBeingDeserialized = this.RequestId;
try
{
value = token?.ToObject(typeHint, this.formatter.JsonSerializer);
}
finally
{
this.formatter.deserializingMessageWithId = default;
this.formatter.duplexPipeTracker.RequestIdBeingDeserialized = default;
}

return true;
Expand Down Expand Up @@ -801,7 +830,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object? exis

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
{
long progressId = this.formatter.formatterProgressTracker.GetTokenForProgress(value);
long progressId = this.formatter.FormatterProgressTracker.GetTokenForProgress(value);
writer.WriteValue(progressId);
}
}
Expand Down Expand Up @@ -832,7 +861,7 @@ public override bool CanConvert(Type objectType)

Assumes.NotNull(this.formatter.rpc);
JToken token = JToken.Load(reader);
return this.formatter.formatterProgressTracker.CreateProgress(this.formatter.rpc, token, objectType);
return this.formatter.FormatterProgressTracker.CreateProgress(this.formatter.rpc, token, objectType);
}

public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
Expand All @@ -853,7 +882,7 @@ internal AsyncEnumerableConsumerConverter(JsonMessageFormatter jsonMessageFormat
this.formatter = jsonMessageFormatter;
}

public override bool CanConvert(Type objectType) => this.formatter.enumerableTracker != null && MessageFormatterEnumerableTracker.CanDeserialize(objectType);
public override bool CanConvert(Type objectType) => MessageFormatterEnumerableTracker.CanDeserialize(objectType);

public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
{
Expand All @@ -862,9 +891,8 @@ internal AsyncEnumerableConsumerConverter(JsonMessageFormatter jsonMessageFormat
return null;
}

Assumes.NotNull(this.formatter.enumerableTracker);
JToken token = JToken.Load(reader);
return this.formatter.enumerableTracker.CreateEnumerableProxy(objectType, token);
return this.formatter.EnumerableTracker.CreateEnumerableProxy(objectType, token);
}

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
Expand All @@ -885,7 +913,7 @@ internal AsyncEnumerableGeneratorConverter(JsonMessageFormatter jsonMessageForma
this.formatter = jsonMessageFormatter;
}

public override bool CanConvert(Type objectType) => this.formatter.enumerableTracker != null && MessageFormatterEnumerableTracker.CanSerialize(objectType);
public override bool CanConvert(Type objectType) => MessageFormatterEnumerableTracker.CanSerialize(objectType);

public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer)
{
Expand All @@ -894,9 +922,7 @@ internal AsyncEnumerableGeneratorConverter(JsonMessageFormatter jsonMessageForma

public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
{
Assumes.NotNull(this.formatter.enumerableTracker);

long token = this.formatter.enumerableTracker.GetToken(value);
long token = this.formatter.EnumerableTracker.GetToken(value);
writer.WriteValue(token);
}
}
Expand All @@ -913,12 +939,12 @@ public DuplexPipeConverter(JsonMessageFormatter jsonMessageFormatter)
public override IDuplexPipe? ReadJson(JsonReader reader, Type objectType, IDuplexPipe? existingValue, bool hasExistingValue, JsonSerializer serializer)
{
int? tokenId = JToken.Load(reader).Value<int?>();
return this.jsonMessageFormatter.duplexPipeTracker.GetPipe(tokenId);
return this.jsonMessageFormatter.duplexPipeTracker!.GetPipe(tokenId);
}

public override void WriteJson(JsonWriter writer, IDuplexPipe? value, JsonSerializer serializer)
{
var token = this.jsonMessageFormatter.duplexPipeTracker.GetToken(value);
var token = this.jsonMessageFormatter.duplexPipeTracker!.GetToken(value);
AArnott marked this conversation as resolved.
Show resolved Hide resolved
writer.WriteValue(token);
}
}
Expand All @@ -935,12 +961,12 @@ public PipeReaderConverter(JsonMessageFormatter jsonMessageFormatter)
public override PipeReader? ReadJson(JsonReader reader, Type objectType, PipeReader? existingValue, bool hasExistingValue, JsonSerializer serializer)
{
int? tokenId = JToken.Load(reader).Value<int?>();
return this.jsonMessageFormatter.duplexPipeTracker.GetPipeReader(tokenId);
return this.jsonMessageFormatter.DuplexPipeTracker.GetPipeReader(tokenId);
}

public override void WriteJson(JsonWriter writer, PipeReader? value, JsonSerializer serializer)
{
var token = this.jsonMessageFormatter.duplexPipeTracker.GetToken(value);
var token = this.jsonMessageFormatter.DuplexPipeTracker.GetToken(value);
writer.WriteValue(token);
}
}
Expand All @@ -957,12 +983,12 @@ public PipeWriterConverter(JsonMessageFormatter jsonMessageFormatter)
public override PipeWriter? ReadJson(JsonReader reader, Type objectType, PipeWriter? existingValue, bool hasExistingValue, JsonSerializer serializer)
{
int? tokenId = JToken.Load(reader).Value<int?>();
return this.jsonMessageFormatter.duplexPipeTracker.GetPipeWriter(tokenId);
return this.jsonMessageFormatter.DuplexPipeTracker.GetPipeWriter(tokenId);
}

public override void WriteJson(JsonWriter writer, PipeWriter? value, JsonSerializer serializer)
{
var token = this.jsonMessageFormatter.duplexPipeTracker.GetToken(value);
var token = this.jsonMessageFormatter.DuplexPipeTracker.GetToken(value);
writer.WriteValue(token);
}
}
Expand All @@ -979,12 +1005,12 @@ public StreamConverter(JsonMessageFormatter jsonMessageFormatter)
public override Stream? ReadJson(JsonReader reader, Type objectType, Stream? existingValue, bool hasExistingValue, JsonSerializer serializer)
{
int? tokenId = JToken.Load(reader).Value<int?>();
return this.jsonMessageFormatter.duplexPipeTracker.GetPipe(tokenId)?.AsStream();
return this.jsonMessageFormatter.DuplexPipeTracker.GetPipe(tokenId)?.AsStream();
}

public override void WriteJson(JsonWriter writer, Stream? value, JsonSerializer serializer)
{
var token = this.jsonMessageFormatter.duplexPipeTracker.GetToken(value?.UsePipe());
var token = this.jsonMessageFormatter.DuplexPipeTracker.GetToken(value?.UsePipe());
writer.WriteValue(token);
}
}
Expand Down
Loading