Skip to content

Commit

Permalink
Merge branch 'master' into fix/server/invalid-params
Browse files Browse the repository at this point in the history
  • Loading branch information
david-driscoll authored Jan 12, 2018
2 parents 86226b7 + 82ce908 commit b8a544f
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 43 deletions.
6 changes: 3 additions & 3 deletions src/Client/Dispatcher/LspDispatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public async Task<bool> TryHandleEmptyNotification(string method)
/// <returns>
/// <c>true</c>, if a notification handler was registered for specified method; otherwise, <c>false</c>.
/// </returns>
public async Task<bool> TryHandleNotification(string method, JObject notification)
public async Task<bool> TryHandleNotification(string method, JToken notification)
{
if (string.IsNullOrWhiteSpace(method))
throw new ArgumentException($"Argument cannot be null, empty, or entirely composed of whitespace: {nameof(method)}.", nameof(method));
Expand Down Expand Up @@ -130,7 +130,7 @@ public async Task<bool> TryHandleNotification(string method, JObject notificatio
/// <returns>
/// If a registered handler was found, a <see cref="Task"/> representing the operation; otherwise, <c>null</c>.
/// </returns>
public Task<object> TryHandleRequest(string method, JObject request, CancellationToken cancellationToken)
public Task<object> TryHandleRequest(string method, JToken request, CancellationToken cancellationToken)
{
if (string.IsNullOrWhiteSpace(method))
throw new ArgumentException($"Argument cannot be null, empty, or entirely composed of whitespace: {nameof(method)}.", nameof(method));
Expand All @@ -157,7 +157,7 @@ public Task<object> TryHandleRequest(string method, JObject request, Cancellatio
/// <returns>
/// The deserialised payload (if one is present and expected).
/// </returns>
object DeserializePayload(Type payloadType, JObject payload)
object DeserializePayload(Type payloadType, JToken payload)
{
if (payloadType == null)
throw new ArgumentNullException(nameof(payloadType));
Expand Down
4 changes: 2 additions & 2 deletions src/Client/Protocol/ServerMessage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ public class ServerMessage
/// The request / notification message, if the message represents a request or a notification.
/// </summary>
[Optional]
public JObject Params { get; set; }
public JToken Params { get; set; }

/// <summary>
/// The response message, if the message represents a response.
/// </summary>
[Optional]
public JObject Result { get; set; }
public JToken Result { get; set; }

/// <summary>
/// The response error (if any).
Expand Down
2 changes: 1 addition & 1 deletion src/JsonRpc/Reciever.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ protected virtual Renor GetRenor(JToken @object)
// that we don't fall over and throw an error.
if (@params?.Type == JTokenType.Null)
{
@params = null;
@params = new JObject();
}

// id == request
Expand Down
12 changes: 2 additions & 10 deletions src/JsonRpc/ReflectionRequestHandlers.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Reflection;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -22,14 +22,6 @@ public static Task HandleNotification(IHandlerDescriptor instance, object @param
return (Task)method.Invoke(instance.Handler, new[] { @params });
}

public static Task HandleRequest(IHandlerDescriptor instance, CancellationToken token)
{
var method = instance.HandlerType.GetTypeInfo()
.GetMethod(nameof(IRequestHandler<object>.Handle), BindingFlags.Public | BindingFlags.Instance);

return (Task)method.Invoke(instance.Handler, new object[] { token });
}

public static Task HandleRequest(IHandlerDescriptor instance, object @params, CancellationToken token)
{
var method = instance.HandlerType.GetTypeInfo()
Expand All @@ -38,4 +30,4 @@ public static Task HandleRequest(IHandlerDescriptor instance, object @params, Ca
return (Task)method.Invoke(instance.Handler, new[] { @params, token });
}
}
}
}
10 changes: 1 addition & 9 deletions src/JsonRpc/RequestRouter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,6 @@ protected virtual async Task<ErrorResponse> RouteRequest(IHandlerDescriptor hand
return new MethodNotFound(request.Id, request.Method);
}

Task result;
if (handler.Params is null)
{
result = ReflectionRequestHandlers.HandleRequest(handler, token);
}
else
{
object @params;
try
{
Expand All @@ -73,8 +66,7 @@ protected virtual async Task<ErrorResponse> RouteRequest(IHandlerDescriptor hand
return new InvalidParams(request.Id);
}

result = ReflectionRequestHandlers.HandleRequest(handler, @params, token);
}
var result = ReflectionRequestHandlers.HandleRequest(handler, @params, token);

await result.ConfigureAwait(false);

Expand Down
25 changes: 9 additions & 16 deletions src/Server/LspRequestRouter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,27 +111,20 @@ public async Task<ErrorResponse> RouteRequest(IHandlerDescriptor descriptor, Req
return new MethodNotFound(request.Id, request.Method);
}

Task result;
if (descriptor.Params is null)
object @params;
try
{
result = ReflectionRequestHandlers.HandleRequest(descriptor, cts.Token);
@params = request.Params?.ToObject(descriptor.Params, _serializer.JsonSerializer);
}
else
catch (Exception cannotDeserializeRequestParams)
{
object @params;
try
{
@params = request.Params?.ToObject(descriptor.Params, _serializer.JsonSerializer);
}
catch
{
return new InvalidParams(request.Id);
}

result = ReflectionRequestHandlers.HandleRequest(descriptor, @params, cts.Token);
_logger.LogError(new EventId(-32602), cannotDeserializeRequestParams, "Failed to deserialise request parameters.");

return new InvalidParams(request.Id);
}

await result.ConfigureAwait(false);
var result = ReflectionRequestHandlers.HandleRequest(descriptor, @params, cts.Token).ConfigureAwait(false);
await result;

object responseValue = null;
if (result.GetType().GetTypeInfo().IsGenericType)
Expand Down
4 changes: 2 additions & 2 deletions test/JsonRpc.Tests/Server/SpecifictionRecieverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public override IEnumerable<ValueTuple<string, Renor[]>> GetValues()
@"{""jsonrpc"": ""2.0"", ""method"": ""subtract"", ""params"": null, ""id"": 4}",
new Renor[]
{
new Request(4, "subtract", null)
new Request(4, "subtract", new JObject())
});

yield return (
Expand All @@ -96,7 +96,7 @@ public override IEnumerable<ValueTuple<string, Renor[]>> GetValues()
@"{""jsonrpc"": ""2.0"", ""method"": ""foobar"", ""params"": null}",
new Renor[]
{
new Notification("foobar", null)
new Notification("foobar", new JObject())
});

yield return (
Expand Down
19 changes: 19 additions & 0 deletions test/Lsp.Tests/LspRequestRouterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,25 @@ public async Task ShouldRouteToCorrect_Request_WithManyHandlers()
await codeActionHandler2.Received(0).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
}

[Fact]
public async Task ShouldRouteTo_CorrectRequestWhenGivenNullParams()
{
var handler = Substitute.For<IShutdownHandler>();
handler
.Handle(Arg.Any<object>(), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);

var collection = new HandlerCollection { handler };
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());

var id = Guid.NewGuid().ToString();
var request = new Request(id, GeneralNames.Shutdown, new JObject());

await mediator.RouteRequest(mediator.GetDescriptor(request), request);

await handler.Received(1).Handle(Arg.Any<object>(), Arg.Any<CancellationToken>());
}

[Fact]
public async Task ShouldHandle_Request_WithNullParameters()
{
Expand Down

0 comments on commit b8a544f

Please sign in to comment.