Skip to content

Commit

Permalink
Merge pull request #733 from AArnott/namedArgs
Browse files Browse the repository at this point in the history 
Add `ClientRequiresNamedArguments` option for `IProgress<T>` notifications
  • Loading branch information
@AArnott
AArnott authored Nov 4, 2021
2 parents b5c78a1 + cb2ab8d commit 29ab31b
Show file tree
Hide file tree
Showing 10 changed files with 258 additions and 70 deletions.
63 changes: 41 additions & 22 deletions src/StreamJsonRpc/JsonMessageFormatter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ public class JsonMessageFormatter : IJsonRpcAsyncMessageTextFormatter, IJsonRpcF
/// </remarks>
private JsonRpc? rpc;

/// <summary>
/// The message whose arguments are being deserialized.
/// </summary>
private JsonRpcMessage? deserializingMessage;

/// <summary>
/// Initializes a new instance of the <see cref="JsonMessageFormatter"/> class
/// that uses JsonProgress (without the preamble) for its text encoding.
Expand Down Expand Up @@ -905,42 +910,53 @@ internal JsonRpcRequest(JsonMessageFormatter formatter)
[IgnoreDataMember]
public TopLevelPropertyBag? TopLevelPropertyBag { get; set; }

internal JsonRpcMethodAttribute? ApplicableMethodAttribute { get; private set; }

public override ArgumentMatchResult TryGetTypedArguments(ReadOnlySpan<ParameterInfo> parameters, Span<object?> typedArguments)
{
if (parameters.Length == 1 && this.NamedArguments is not null)
// Consider the attribute applied to the particular overload that we're considering right now.
this.ApplicableMethodAttribute = this.Method is not null ? this.formatter.rpc?.GetJsonRpcMethodAttribute(this.Method, parameters) : null;
try
{
// Special support for accepting a single JToken instead of all parameters individually.
if (parameters[0].ParameterType == typeof(JToken))
{
var obj = new JObject();
foreach (KeyValuePair<string, object?> property in this.NamedArguments)
{
obj.Add(new JProperty(property.Key, property.Value));
}

typedArguments[0] = obj;
return ArgumentMatchResult.Success;
}

// Support for opt-in to deserializing all named arguments into a single parameter.
if (this.Method is not null)
if (parameters.Length == 1 && this.NamedArguments is not null)
{
JsonRpcMethodAttribute? attribute = this.formatter.rpc?.GetJsonRpcMethodAttribute(this.Method, parameters);
if (attribute?.UseSingleObjectParameterDeserialization ?? false)
// Special support for accepting a single JToken instead of all parameters individually.
if (parameters[0].ParameterType == typeof(JToken))
{
var obj = new JObject();
foreach (KeyValuePair<string, object?> property in this.NamedArguments)
{
obj.Add(new JProperty(property.Key, property.Value));
}

typedArguments[0] = obj.ToObject(parameters[0].ParameterType, this.formatter.JsonSerializer);
typedArguments[0] = obj;
return ArgumentMatchResult.Success;
}

// Support for opt-in to deserializing all named arguments into a single parameter.
if (this.Method is not null)
{
if (this.ApplicableMethodAttribute?.UseSingleObjectParameterDeserialization ?? false)
{
var obj = new JObject();
foreach (KeyValuePair<string, object?> property in this.NamedArguments)
{
obj.Add(new JProperty(property.Key, property.Value));
}

typedArguments[0] = obj.ToObject(parameters[0].ParameterType, this.formatter.JsonSerializer);
return ArgumentMatchResult.Success;
}
}
}
}

return base.TryGetTypedArguments(parameters, typedArguments);
return base.TryGetTypedArguments(parameters, typedArguments);
}
finally
{
// Clear this, because we might choose another overload with a different attribute, and we don't want to 'leak' an attribute that isn't on the overload that is ultimately picked.
this.ApplicableMethodAttribute = null;
}
}

public override bool TryGetArgumentByNameOrIndex(string? name, int position, Type? typeHint, out object? value)
Expand All @@ -953,13 +969,15 @@ 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.deserializingMessage = this;
try
{
value = token?.ToObject(typeHint!, this.formatter.JsonSerializer); // Null typeHint is allowed (https://github.com/JamesNK/Newtonsoft.Json/pull/2562)
}
finally
{
this.formatter.deserializingMessageWithId = default;
this.formatter.deserializingMessage = null;
}

return true;
Expand Down Expand Up @@ -1197,7 +1215,8 @@ 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);
bool clientRequiresNamedArgs = this.formatter.deserializingMessage is JsonRpcRequest { ApplicableMethodAttribute: { ClientRequiresNamedArguments: true } };
return this.formatter.FormatterProgressTracker.CreateProgress(this.formatter.rpc, token, objectType, clientRequiresNamedArgs);
}

public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer)
Expand Down
14 changes: 11 additions & 3 deletions src/StreamJsonRpc/JsonRpcTargetOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public JsonRpcTargetOptions(JsonRpcTargetOptions copyFrom)
this.NotifyClientOfEvents = copyFrom.NotifyClientOfEvents;
this.AllowNonPublicInvocation = copyFrom.AllowNonPublicInvocation;
this.UseSingleObjectParameterDeserialization = copyFrom.UseSingleObjectParameterDeserialization;
this.ClientRequiresNamedArguments = copyFrom.ClientRequiresNamedArguments;
this.DisposeOnDisconnect = copyFrom.DisposeOnDisconnect;
}

Expand Down Expand Up @@ -63,11 +64,18 @@ public JsonRpcTargetOptions(JsonRpcTargetOptions copyFrom)
/// </remarks>
public bool AllowNonPublicInvocation { get; set; }

/// <summary>
/// Gets or sets a value indicating whether JSON-RPC named arguments should all be deserialized into the RPC method's first parameter.
/// </summary>
/// <inheritdoc cref="JsonRpcMethodAttribute.UseSingleObjectParameterDeserialization" />
/// <remarks>
/// This value serves as a default for <see cref="JsonRpcMethodAttribute.UseSingleObjectParameterDeserialization"/> for members that have no <see cref="JsonRpcMethodAttribute"/> applied.
/// </remarks>
public bool UseSingleObjectParameterDeserialization { get; set; }

/// <inheritdoc cref="JsonRpcMethodAttribute.ClientRequiresNamedArguments" />
/// <remarks>
/// This value serves as a default for <see cref="JsonRpcMethodAttribute.ClientRequiresNamedArguments"/> for members that have no <see cref="JsonRpcMethodAttribute"/> applied.
/// </remarks>
public bool ClientRequiresNamedArguments { get; set; }

/// <summary>
/// Gets or sets a value indicating whether to dispose of the target object
/// when the connection with the remote party is lost.
Expand Down
52 changes: 35 additions & 17 deletions src/StreamJsonRpc/MessagePackFormatter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ public class MessagePackFormatter : IJsonRpcMessageFormatter, IJsonRpcInstanceCo
/// </summary>
private RequestId deserializingMessageWithId;

/// <summary>
/// The message whose arguments are being deserialized.
/// </summary>
private JsonRpcMessage? deserializingMessage;

/// <summary>
/// Backing field for the <see cref="IJsonRpcFormatterState.SerializingRequest"/> property.
/// </summary>
Expand Down Expand Up @@ -1027,7 +1032,8 @@ public TClass Deserialize(ref MessagePackReader reader, MessagePackSerializerOpt

Assumes.NotNull(this.formatter.rpc);
RawMessagePack token = RawMessagePack.ReadRaw(ref reader, copy: true);
return (TClass)this.formatter.FormatterProgressTracker.CreateProgress(this.formatter.rpc, token, typeof(TClass));
bool clientRequiresNamedArgs = this.formatter.deserializingMessage is JsonRpcRequest { ApplicableMethodAttribute: { ClientRequiresNamedArguments: true } };
return (TClass)this.formatter.FormatterProgressTracker.CreateProgress(this.formatter.rpc, token, typeof(TClass), clientRequiresNamedArgs);
}

public void Serialize(ref MessagePackWriter writer, TClass value, MessagePackSerializerOptions options)
Expand Down Expand Up @@ -2337,6 +2343,8 @@ internal JsonRpcRequest(MessagePackFormatter formatter)

internal IReadOnlyList<ReadOnlySequence<byte>>? MsgPackPositionalArguments { get; set; }

internal JsonRpcMethodAttribute? ApplicableMethodAttribute { get; private set; }

void IJsonRpcMessageBufferManager.DeserializationComplete(JsonRpcMessage message)
{
Assumes.True(message == this);
Expand All @@ -2351,27 +2359,36 @@ void IJsonRpcMessageBufferManager.DeserializationComplete(JsonRpcMessage message

public override ArgumentMatchResult TryGetTypedArguments(ReadOnlySpan<ParameterInfo> parameters, Span<object?> typedArguments)
{
if (parameters.Length == 1 && this.MsgPackNamedArguments is not null)
// Consider the attribute applied to the particular overload that we're considering right now.
this.ApplicableMethodAttribute = this.Method is not null ? this.formatter.rpc?.GetJsonRpcMethodAttribute(this.Method, parameters) : null;
try
{
Assumes.NotNull(this.Method);

JsonRpcMethodAttribute? attribute = this.formatter.rpc?.GetJsonRpcMethodAttribute(this.Method, parameters);
if (attribute?.UseSingleObjectParameterDeserialization ?? false)
if (parameters.Length == 1 && this.MsgPackNamedArguments is not null)
{
var reader = new MessagePackReader(this.MsgPackArguments);
try
{
typedArguments[0] = MessagePackSerializer.Deserialize(parameters[0].ParameterType, ref reader, this.formatter.userDataSerializationOptions);
return ArgumentMatchResult.Success;
}
catch (MessagePackSerializationException)
Assumes.NotNull(this.Method);

if (this.ApplicableMethodAttribute?.UseSingleObjectParameterDeserialization ?? false)
{
return ArgumentMatchResult.ParameterArgumentTypeMismatch;
var reader = new MessagePackReader(this.MsgPackArguments);
try
{
typedArguments[0] = MessagePackSerializer.Deserialize(parameters[0].ParameterType, ref reader, this.formatter.userDataSerializationOptions);
return ArgumentMatchResult.Success;
}
catch (MessagePackSerializationException)
{
return ArgumentMatchResult.ParameterArgumentTypeMismatch;
}
}
}
}

return base.TryGetTypedArguments(parameters, typedArguments);
return base.TryGetTypedArguments(parameters, typedArguments);
}
finally
{
// Clear this, because we might choose another overload with a different attribute, and we don't want to 'leak' an attribute that isn't on the overload that is ultimately picked.
this.ApplicableMethodAttribute = null;
}
}

public override bool TryGetArgumentByNameOrIndex(string? name, int position, Type? typeHint, out object? value)
Expand Down Expand Up @@ -2401,7 +2418,7 @@ 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.deserializingMessage = this;
value = MessagePackSerializer.Deserialize(typeHint ?? typeof(object), ref reader, this.formatter.userDataSerializationOptions);
return true;
}
Expand All @@ -2417,6 +2434,7 @@ public override bool TryGetArgumentByNameOrIndex(string? name, int position, Typ
finally
{
this.formatter.deserializingMessageWithId = default;
this.formatter.deserializingMessage = null;
}
}

Expand Down
16 changes: 14 additions & 2 deletions src/StreamJsonRpc/Reflection/JsonRpcMethodAttribute.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,20 @@ public JsonRpcMethodAttribute()
public string? Name { get; }

/// <summary>
/// Gets or sets a value indicating whether JSON-RPC named arguments should all be deserialized into this method's first parameter.
/// Gets or sets a value indicating whether JSON-RPC named arguments should all be deserialized into the RPC method's first parameter.
/// </summary>
public bool UseSingleObjectParameterDeserialization { get; set; }
public bool UseSingleObjectParameterDeserialization { get; set; }

/// <summary>
/// Gets or sets a value indicating whether JSON-RPC named arguments should be used in callbacks sent back to the client.
/// </summary>
/// <value>The default value is <see langword="false"/>.</value>
/// <remarks>
/// An example of impact of this setting is when the client sends an <see cref="IProgress{T}"/> argument and this server
/// will call <see cref="IProgress{T}.Report(T)"/> on that argument.
/// The notification that the server then sends back to the client may use positional or named arguments in that notification.
/// Named arguments are used if and only if this property is set to <see langword="true" />.
/// </remarks>
public bool ClientRequiresNamedArguments { get; set; }
}
}
Loading

0 comments on commit 29ab31b

Please sign in to comment.