split client and server channels

pkg/analysis_server/lib/src/protocol.dart

Index: pkg/analysis_server/lib/src/protocol.dart
diff --git a/pkg/analysis_server/lib/src/protocol.dart b/pkg/analysis_server/lib/src/protocol.dart
index 7541aed2dd5f0a109b22122483a3f859e9c1ecb0..0d39c312dbf0a6811b826d9d7ca1661bf18ae985 100644
--- a/pkg/analysis_server/lib/src/protocol.dart
+++ b/pkg/analysis_server/lib/src/protocol.dart
@@ -75,9 +75,9 @@ class Request {
       }
       String id = result[Request.ID];
       String method = result[Request.METHOD];
-      Map<String, Object> params = result[Request.PARAMS];
+      var params = result[Request.PARAMS];

[Brian Wilkerson, 2014/02/27 19:06:02]

Why did you remove the type? What other values do we expect this to have?

[danrubel, 2014/02/27 20:21:14]

Given valid but inappropriate JSON from the client, this could be a String. I
changed it to a var here then strengthened the if statement below to only allow
Map when run in unchecked mode.

[Brian Wilkerson, 2014/02/27 21:12:40]

Ah. Then we should probably be checking the type of everything else as well.
That is, the code should read

  String id = getString(result, Request.ID);

where getString will get the property and throw an exception if it's not a
string. The exception should then be caught and an invalid request response
should be returned to the client.

       Request request = new Request(id, method);
-      if (params != null) {
+      if (params is Map) {
         params.forEach((String key, Object value) {
           request.setParameter(key, value);
         });
@@ -153,12 +153,12 @@ class Request {
    * sent to the client to represent this response.
    */
   Map<String, Object> toJson() {
-    Map jsonObject = new Map();
+    Map<String, Object> jsonObject = new Map<String, Object>();
     jsonObject[ID] = id;
     jsonObject[METHOD] = method;
-    params.forEach((String key, Object value) {
-      jsonObject[key] = value;
-    });
+    if (params.isNotEmpty) {
+      jsonObject[PARAMS] = params;
+    }
     return jsonObject;
   }
 }
@@ -261,6 +261,32 @@ class Response {
     : this(request.id, new RequestError(-7, 'Unknown request'));
 
   /**
+   * Initialize a newly created instance based upon the given JSON data
+   */
+  factory Response.fromJson(Map<String, Object> json) {
+    try {
+      // TODO process result
+      String id = json[Response.ID];
+      var error = json[Response.ERROR];
+      var result = json[Response.RESULT];
+      Response response;
+      if (error is Map) {

[Brian Wilkerson, 2014/02/27 19:06:02]

What other non-null value are we guarding against (both with 'error' and with
'result')?

[danrubel, 2014/02/27 20:21:14]

We expect this to be a Map, but given valid JSON, it could be a String.

+        response = new Response(id, new RequestError.fromJson(error));
+      } else {
+        response = new Response(id);
+      }
+      if (result is Map) {
+        result.forEach((String key, Object value) {
+          response.setResult(key, value);
+        });
+      }
+      return response;
+    } catch (exception) {
+      return null;
+    }
+  }
+
+  /**
    * Return the value of the result field with the given [name].
    */
   Object getResult(String name) {
@@ -279,7 +305,7 @@ class Response {
    * sent to the client to represent this response.
    */
   Map<String, Object> toJson() {
-    Map jsonObject = new Map();
+    Map<String, Object> jsonObject = new Map<String, Object>();
     jsonObject[ID] = id;
     if (error == null) {
       jsonObject[ERROR] = null;
@@ -399,6 +425,26 @@ class RequestError {
   RequestError.internalError() : this(CODE_INTERNAL_ERROR, "Internal error");
 
   /**
+   * Initialize a newly created [Error] from the given JSON.
+   */
+  factory RequestError.fromJson(Map<String, Object> json) {
+    try {
+      int code = json[RequestError.CODE];
+      String message = json[RequestError.MESSAGE];
+      Map<String, Object> data = json[RequestError.DATA];
+      RequestError requestError = new RequestError(code, message);
+      if (data != null) {

[Brian Wilkerson, 2014/02/27 21:12:40]

We're not checking here to ensure that data is a Map.

+        data.forEach((String key, Object value) {
+          requestError.setData(key, value);
+        });
+      }
+      return requestError;
+    } catch (exception) {
+      return null;
+    }
+  }
+
+  /**
    * Return the value of the data with the given [name], or `null` if there is
    * no such data associated with this error.
    */
@@ -416,7 +462,7 @@ class RequestError {
    * sent to the client to represent this response.
    */
   Map<String, Object> toJson() {
-    Map jsonObject = new Map();
+    Map<String, Object> jsonObject = new Map<String, Object>();
     jsonObject[CODE] = code;
     jsonObject[MESSAGE] = message;
     if (!data.isEmpty) {
@@ -458,6 +504,25 @@ class Notification {
   Notification(this.event);
 
   /**
+   * Initialize a newly created instance based upon the given JSON data
+   */
+  factory Notification.fromJson(Map<String, Object> json) {
+    try {
+      String event = json[Notification.EVENT];
+      var params = json[Notification.PARAMS];
+      Notification notification = new Notification(event);
+      if (params is Map) {

[Brian Wilkerson, 2014/02/27 19:06:02]

What other possible values are we expecting?

[danrubel, 2014/02/27 20:21:14]

Given valid JSON, it could be a String.

+        params.forEach((String key, Object value) {
+          notification.setParameter(key, value);
+        });
+      }
+      return notification;
+    } catch (exception) {
+      return null;
+    }
+  }
+
+  /**
    * Return the value of the parameter with the given [name], or `null` if there
    * is no such parameter associated with this notification.
    */
@@ -475,7 +540,7 @@ class Notification {
    * sent to the client to represent this response.
    */
   Map<String, Object> toJson() {
-    Map jsonObject = new Map();
+    Map<String, Object> jsonObject = new Map<String, Object>();
     jsonObject[EVENT] = event;
     if (!params.isEmpty) {
       jsonObject[PARAMS] = params;