Web Socket API for talking to Editor.

sdk/lib/_internal/pub/lib/src/barback/server.dart

Index: sdk/lib/_internal/pub/lib/src/barback/server.dart
diff --git a/sdk/lib/_internal/pub/lib/src/barback/server.dart b/sdk/lib/_internal/pub/lib/src/barback/server.dart
index 36faf4a8369e591b5c61fea1be022d7247a804d6..22b45e9269321f36ddc84df7b4addb4c5cd2fc50 100644
--- a/sdk/lib/_internal/pub/lib/src/barback/server.dart
+++ b/sdk/lib/_internal/pub/lib/src/barback/server.dart
@@ -5,6 +5,7 @@
 library pub.barback.server;
 
 import 'dart:async';
+import 'dart:convert';
 import 'dart:io';
 
 import 'package:barback/barback.dart';
@@ -70,6 +71,11 @@ class BarbackServer {
 
   /// Handles an HTTP request.
   void _handleRequest(HttpRequest request) {
+    if (WebSocketTransformer.isUpgradeRequest(request)) {
+      _handleWebSocket(request);
+      return;
+    }
+
     if (request.method != "GET" && request.method != "HEAD") {
       _methodNotAllowed(request);
       return;
@@ -77,7 +83,7 @@ class BarbackServer {
 
     var id;
     try {
-      id = _uriToId(_rootPackage, request.uri);
+      id = _uriToId(request.uri);
     } on FormatException catch (ex) {
       // If we got here, we had a path like "/packages" which is a special
       // directory, but not a valid path since it lacks a following package name.
@@ -135,9 +141,65 @@ class BarbackServer {
     });
   }
 
+  /// Creates a web socket for [request] which is should be an upgrade request.

[nweiz, 2013/10/09 19:16:37]

"which is should be" -> "which should be"

[Bob Nystrom, 2013/10/11 20:50:10]

Done.

+  void _handleWebSocket(HttpRequest request) {
+    WebSocketTransformer.upgrade(request).then((socket) {
+      socket.listen((data) {
+        var command;
+        try {
+          command = JSON.decode(data);
+        } on FormatException catch (ex) {
+          socket.add(JSON.encode({"error": '"$data" is not valid JSON.'}));

[nweiz, 2013/10/09 19:16:37]

I think we should include a stack trace in the error messages. In this one in
particular, we should also include the original exception's message, in case it
provides more detailed information about the syntax error.

It would also be nice to remove some of the duplication of constructing these
error responses with a utility method. Something like "_sendError(String
message, [StackTrace stack])".

[Bob Nystrom, 2013/10/11 20:50:10]

I really don't. In these cases, these are meaningful errors that pub is
intentionally reporting back to the client because they used the API wrongly. A
callstack within pub isn't relevant.
Good call. Done.
Done.

+          return;
+        }
+
+        if (command is! Map) {
+          socket.add(JSON.encode({"error": "Command must be a JSON map."}));

[nweiz, 2013/10/09 19:16:37]

Include the actual value. In fact, every error message should include the actual
value.

[Bob Nystrom, 2013/10/11 20:50:10]

Done.

[Bob Nystrom, 2013/10/11 20:50:10]

Added to some of them. In general, I don't think the error messages need to be
too user-friendly here. This is an API that will be called programmatically, not
by a human user. Once the Editor code is doing the right thing, we shouldn't see
any errors.

+          return;
+        }
+
+        if (!command.containsKey("command")) {
+          socket.add(JSON.encode({"error": "Missing command name."}));
+          return;
+        }
+
+        switch (command["command"]) {
+          case "uriToAsset":

[nweiz, 2013/10/09 19:16:37]

Using "URI" in the command names seems weird, since it's always actually going
to be a URL.

[Bob Nystrom, 2013/10/11 20:50:10]

I've never learned the difference between the two. :( Changed to URL.

[nweiz, 2013/10/15 23:04:49]

My rule of thumb is that anything a browser can understand is a URL, and URIs
are a broader class that includes a bunch of things that are rarely used in
practice.

[Bob Nystrom, 2013/10/16 00:01:28]

Done.

+            var uri = new Uri(path: command["uri"]);

[nweiz, 2013/10/09 19:16:37]

Send an error response if command["uri"] is not a String.

Also, I don't think "uri" is a good name for this, since only a path is valid.
"path" would probably be better.

[Bob Nystrom, 2013/10/11 20:50:10]

Done.

+            var asset = _uriToId(uri);

[nweiz, 2013/10/09 19:16:37]

"asset" -> "id" or "assetId"

[Bob Nystrom, 2013/10/11 20:50:10]

Done.

+            socket.add(JSON.encode({
+              "package": asset.package,
+              "path": asset.path
+            }));
+            break;
+
+          case "assetToUri":
+            var id = new AssetId(command["package"], command["path"]);

[nweiz, 2013/10/09 19:16:37]

Send an error response if command["package"] or command["path"] is not a String.

[Bob Nystrom, 2013/10/11 20:50:10]

Done.

+            try {
+              var uri = _idtoUri(id);
+              socket.add(JSON.encode({"uri": uri}));
+            } on FormatException catch (ex) {
+              socket.add(JSON.encode({"error": ex.message}));
+            }
+            break;
+
+          default:
+            socket.add(JSON.encode({
+              "error": 'Unknown command "${command["command"]}".'
+            }));
+            break;
+        }
+      }, onError: (error) {
+        _resultsController.addError(error);
+      });

[nweiz, 2013/10/09 19:16:37]

This should probably have cancelOnError set to true.

[Bob Nystrom, 2013/10/11 20:50:10]

Done.

+    }).catchError((error) {
+      _resultsController.addError(error);
+    });
+  }
+
   /// Converts a [url] served by pub serve into an [AssetId] that can be
   /// requested from barback.
-  AssetId _uriToId(String rootPackage, Uri url) {
+  AssetId _uriToId(Uri url) {
     var id = specialUrlToId(url);
     if (id != null) return id;
 
@@ -145,10 +207,44 @@ class BarbackServer {
     var parts = path.url.split(url.path);
 
     // Strip the leading "/" from the URL.
-    parts = parts.skip(1);
+    if (parts.isNotEmpty && parts.first == "/") parts = parts.skip(1);
 
     var relativePath = path.url.join("web", path.url.joinAll(parts));
-    return new AssetId(rootPackage, relativePath);
+    return new AssetId(_rootPackage, relativePath);
+  }
+
+  /// Converts [id] to at URI path that would access an asset with that ID.

[nweiz, 2013/10/09 19:16:37]

"URI path" -> "root-relative URL"

[Bob Nystrom, 2013/10/11 20:50:10]

This function is now in barback.dart (the pub build patch uses it too). The docs
there explain the path semantics a bit better.

+  ///
+  /// Throws a [FormatException] if [id] is not a valid servable asset.
+  String _idtoUri(AssetId id) {
+    var parts = path.url.split(id.path);
+
+    if (parts.length < 2) {
+      throw new FormatException(
+          "Can not serve asset from top-level directory.");

[nweiz, 2013/10/09 19:16:37]

"Can not serve asset" -> "Cannot serve assets"

[Bob Nystrom, 2013/10/11 20:50:10]

Done.

+    }
+
+    // Each top-level directory gets handled differently.
+    var dir = parts[0];
+    parts = parts.skip(1);
+
+    switch (dir) {
+      case "asset":
+        return path.join("assets", id.package, path.joinAll(parts));

[nweiz, 2013/10/09 19:16:37]

All of these return values should be root-relative -- that is, they should start
with "/". Right now they're relative, but it's unclear what they're relative to.

[Bob Nystrom, 2013/10/11 20:50:10]

Added TODO.

[nweiz, 2013/10/15 23:04:49]

I don't think this warrants a TODO; just add an additional "/" parameter to the
path.join call.

Also, these should be path.url.join.

[Bob Nystrom, 2013/10/16 00:01:28]

Done.

+
+      case "lib":
+        return path.join("packages", id.package, path.joinAll(parts));
+
+      case "web":
+        if (id.package != _rootPackage) {
+          throw new FormatException(
+              'Can only access "web" directory of root package.');

[nweiz, 2013/10/09 19:16:37]

This is ambiguous; it could mean "of all the assets in the root package, only
those in the 'web' directory are accessible."

[Bob Nystrom, 2013/10/11 20:50:10]

Reworded.

+        }
+        return path.join(path.joinAll(parts));
+
+      default:
+        throw new FormatException('Cannot serve assets from "$dir".');
+    }
   }
 
   /// Responds to [request] with a 405 response and closes it.