Merge pull request #53 from tintoy/feature/handler-payload-deserialization

 Move payload deserialisation out of handlers

Author: tintoy

src/Client/Dispatcher/LspDispatcher.cs

@@ -4,6 +4,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Newtonsoft.Json.Linq;
+using OmniSharp.Extensions.JsonRpc;
 using OmniSharp.Extensions.LanguageServer.Client.Handlers;
 
 namespace OmniSharp.Extensions.LanguageServer.Client.Dispatcher
@@ -21,10 +22,22 @@ public class LspDispatcher
         /// <summary>
         ///     Create a new <see cref="LspDispatcher"/>.
         /// </summary>
-        public LspDispatcher()
+        /// <param name="serializer">
+        ///     The JSON serialiser for notification / request / response payloads.
+        /// </param>
+        public LspDispatcher(ISerializer serializer)
         {
+            if (serializer == null)
+                throw new ArgumentNullException(nameof(serializer));
+
+            Serializer = serializer;
         }
 
+        /// <summary>
+        ///     The JSON serialiser to use for notification / request / response payloads.
+        /// </summary>
+        public ISerializer Serializer { get; set; }
+
         /// <summary>
         ///     Register a handler invoker.
         /// </summary>
@@ -92,7 +105,9 @@ public async Task<bool> TryHandleNotification(string method, JObject notificatio
 
             if (_handlers.TryGetValue(method, out IHandler handler) && handler is IInvokeNotificationHandler notificationHandler)
             {
-                await notificationHandler.Invoke(notification);
+                object notificationPayload = DeserializePayload(notificationHandler.PayloadType, notification);
+
+                await notificationHandler.Invoke(notificationPayload);
 
                 return true;
             }
@@ -121,9 +136,36 @@ public Task<object> TryHandleRequest(string method, JObject request, Cancellatio
                 throw new ArgumentException($"Argument cannot be null, empty, or entirely composed of whitespace: {nameof(method)}.", nameof(method));
 
             if (_handlers.TryGetValue(method, out IHandler handler) && handler is IInvokeRequestHandler requestHandler)
-                return requestHandler.Invoke(request, cancellationToken);
+            {
+                object requestPayload = DeserializePayload(requestHandler.PayloadType, request);
+
+                return requestHandler.Invoke(requestPayload, cancellationToken);
+            }
 
             return null;
         }
+
+        /// <summary>
+        ///     Deserialise a notification / request payload from JSON.
+        /// </summary>
+        /// <param name="payloadType">
+        ///     The payload's CLR type.
+        /// </param>
+        /// <param name="payload">
+        ///     JSON representing the payload.
+        /// </param>
+        /// <returns>
+        ///     The deserialised payload (if one is present and expected).
+        /// </returns>
+        object DeserializePayload(Type payloadType, JObject payload)
+        {
+            if (payloadType == null)
+                throw new ArgumentNullException(nameof(payloadType));
+
+            if (payloadType == null || payload == null)
+                return null;
+
+            return payload.ToObject(payloadType, Serializer.JsonSerializer);
+        }
     }
 }

src/Client/Handlers/DelegateEmptyNotificationHandler.cs

@@ -32,6 +32,11 @@ public DelegateEmptyNotificationHandler(string method, NotificationHandler handl
         /// </summary>
         public NotificationHandler Handler { get; }
 
+        /// <summary>
+        ///     The expected CLR type of the notification payload (<c>null</c>, since the handler does not use the request payload).
+        /// </summary>
+        public override Type PayloadType => null;
+
         /// <summary>
         ///     Invoke the handler.
         /// </summary>

src/Client/Handlers/DelegateHandler.cs

@@ -26,5 +26,10 @@ protected DelegateHandler(string method)
         ///     The name of the method handled by the handler.
         /// </summary>
         public string Method { get; }
+
+        /// <summary>
+        ///     The expected CLR type of the request / notification payload (if any; <c>null</c> if the handler does not use the request payload).
+        /// </summary>
+        public abstract Type PayloadType { get; }
     }
 }

src/Client/Handlers/DelegateNotificationHandler.cs

@@ -38,6 +38,11 @@ public DelegateNotificationHandler(string method, NotificationHandler<TNotificat
         /// </summary>
         public NotificationHandler<TNotification> Handler { get; }
 
+        /// <summary>
+        ///     The expected CLR type of the notification payload.
+        /// </summary>
+        public override Type PayloadType => typeof(TNotification);
+
         /// <summary>
         ///     Invoke the handler.
         /// </summary>
@@ -47,12 +52,12 @@ public DelegateNotificationHandler(string method, NotificationHandler<TNotificat
         /// <returns>
         ///     A <see cref="Task"/> representing the operation.
         /// </returns>
-        public async Task Invoke(JObject notification)
+        public async Task Invoke(object notification)
         {
             await Task.Yield();
 
             Handler(
-                notification != null ? notification.ToObject<TNotification>(Serializer.Instance.JsonSerializer /* Fix me: this is ugly */) : default(TNotification)
+                (TNotification)notification
             );
         }
     }

src/Client/Handlers/DelegateRequestHandler.cs

@@ -39,6 +39,11 @@ public DelegateRequestHandler(string method, RequestHandler<TRequest> handler)
         /// </summary>
         public RequestHandler<TRequest> Handler { get; }
 
+        /// <summary>
+        ///     The expected CLR type of the request payload.
+        /// </summary>
+        public override Type PayloadType => typeof(TRequest);
+
         /// <summary>
         ///     Invoke the handler.
         /// </summary>
@@ -51,10 +56,10 @@ public DelegateRequestHandler(string method, RequestHandler<TRequest> handler)
         /// <returns>
         ///     A <see cref="Task{TResult}"/> representing the operation.
         /// </returns>
-        public async Task<object> Invoke(JObject request, CancellationToken cancellationToken)
+        public async Task<object> Invoke(object request, CancellationToken cancellationToken)
         {
             await Handler(
-                request != null ? request.ToObject<TRequest>(Serializer.Instance.JsonSerializer /* Fix me: this is ugly */) : default(TRequest),
+                (TRequest)request,
                 cancellationToken
             );
 

src/Client/Handlers/DelegateRequestResponseHandler.cs

@@ -42,6 +42,11 @@ public DelegateRequestResponseHandler(string method, RequestHandler<TRequest, TR
         /// </summary>
         public RequestHandler<TRequest, TResponse> Handler { get; }
 
+        /// <summary>
+        ///     The expected CLR type of the request payload.
+        /// </summary>
+        public override Type PayloadType => typeof(TRequest);
+
         /// <summary>
         ///     Invoke the handler.
         /// </summary>
@@ -54,10 +59,10 @@ public DelegateRequestResponseHandler(string method, RequestHandler<TRequest, TR
         /// <returns>
         ///     A <see cref="Task{TResult}"/> representing the operation.
         /// </returns>
-        public async Task<object> Invoke(JObject request, CancellationToken cancellationToken)
+        public async Task<object> Invoke(object request, CancellationToken cancellationToken)
         {
             return await Handler(
-                request != null ? request.ToObject<TRequest>(Serializer.Instance.JsonSerializer /* Fix me: this is ugly */) : default(TRequest),
+                (TRequest)request,
                 cancellationToken
             );
         }

src/Client/Handlers/DynamicRegistrationHandler.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Threading;
 using System.Threading.Tasks;
 using Newtonsoft.Json.Linq;
@@ -31,6 +32,11 @@ public DynamicRegistrationHandler()
         /// </summary>
         public string Method => "client/registerCapability";
 
+        /// <summary>
+        ///     The expected CLR type of the request / notification payload (if any; <c>null</c> if the handler does not use the request payload).
+        /// </summary>
+        public Type PayloadType => null;
+
         /// <summary>
         ///     Invoke the handler.
         /// </summary>
@@ -43,7 +49,7 @@ public DynamicRegistrationHandler()
         /// <returns>
         ///     A <see cref="Task{TResult}"/> representing the operation.
         /// </returns>
-        public Task<object> Invoke(JObject request, CancellationToken cancellationToken)
+        public Task<object> Invoke(object request, CancellationToken cancellationToken)
         {
             // For now, we don't really support dynamic registration but OmniSharp's implementation sends a request even when dynamic registrations are not supported.
 

src/Client/Handlers/IHandler.cs

@@ -1,4 +1,6 @@
-namespace OmniSharp.Extensions.LanguageServer.Client.Handlers
+using System;
+
+namespace OmniSharp.Extensions.LanguageServer.Client.Handlers
 {
     /// <summary>
     ///     Represents a client-side message handler.
@@ -9,5 +11,10 @@ public interface IHandler
         ///     The name of the method handled by the handler.
         /// </summary>
         string Method { get; }
+
+        /// <summary>
+        ///     The expected CLR type of the request / notification payload (if any; <c>null</c> if the handler does not use the request body).
+        /// </summary>
+        Type PayloadType { get; }
     }
 }

src/Client/Handlers/IInvokeNotificationHandler.cs

@@ -18,6 +18,6 @@ public interface IInvokeNotificationHandler
         /// <returns>
         ///     A <see cref="Task"/> representing the operation.
         /// </returns>
-        Task Invoke(JObject notification);
+        Task Invoke(object notification);
     }
 }

src/Client/Handlers/IInvokeRequestHandler.cs

@@ -22,6 +22,6 @@ public interface IInvokeRequestHandler
         /// <returns>
         ///     A <see cref="Task{TResult}"/> representing the operation.
         /// </returns>
-        Task<object> Invoke(JObject request, CancellationToken cancellationToken);
+        Task<object> Invoke(object request, CancellationToken cancellationToken);
     }
 }