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.

Sign up for GitHub

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

Add ClientRequiresNamedArguments option for IProgress<T> notifications #733

Merged
merged 1 commit into from
Nov 4, 2021
Merged

Add ClientRequiresNamedArguments option for IProgress<T> notifications #733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
63 changes: 41 additions & 22 deletions src/StreamJsonRpc/JsonMessageFormatter.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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