Review of PR #716 (multi-cursor publish).

- History.md update

- tweak docs

- refactor the "one per collection" check

- make errors into internal errors.  Programming errors like returning the wrong
  type from a function on the server don't need to be reported to the client.

History.md

@@ -1,6 +1,9 @@
 
 ## vNEXT
 
+* Publish functions may now return an array of cursors to publish. Currently,
+  the cursors must all be from different collections.
+
 * User documents have id's when onCreateUser and validateNewUser hooks run.
 
 * Removed all restrictions on EJSON types in MongoDB, even user-defined ones.

docs/client/api.html

@@ -43,10 +43,17 @@ To publish records to clients, call `Meteor.publish` on the server with
 two parameters: the name of the record set, and a *publish function*
 that Meteor will call each time a client subscribes to the name.
 
-Publish functions can return an instance or array of
+Publish functions can return a
 [`Collection.Cursor`](#meteor_collection_cursor), in which case Meteor
-will publish that cursor's documents. You can return only one cursor
-for each collection when returning an array of cursors.
+will publish that cursor's documents. You can also return an array of
+`Collection.Cursor`s, in which case Meteor will publish all of the
+cursors.
+
+{{#warning}}
+If you return multiple cursors in an array, they currently must all be from
+different collections. We hope to lift this restriction in a future release.
+cursors.
+{{/warning}}
 
     // server: publish the rooms collection, minus secret info.
     Meteor.publish("rooms", function () {
@@ -61,7 +68,7 @@ for each collection when returning an array of cursors.
     });
 
     // publish dependent documents and simulate joins
-    Meteor.publish("room", function (roomId) {
+    Meteor.publish("roomAndMessages", function (roomId) {
       return [
         Rooms.find({_id: roomId}, {fields: {secretInfo: 0}}),
         Messages.find({roomId: roomId})

packages/livedata/livedata_server.js

@@ -819,10 +819,11 @@ _.extend(Meteor._LivedataSubscription.prototype, {
       return;
     }
 
-   // SPECIAL CASE: Instead of writing their own callbacks that invoke
+    // SPECIAL CASE: Instead of writing their own callbacks that invoke
     // this.added/changed/ready/etc, the user can just return a collection
-    // cursor or array of cursors from the publish function; we call its _publishCursor method which
-    // starts observing the cursor and publishes the results.
+    // cursor or array of cursors from the publish function; we call their
+    // _publishCursor method which starts observing the cursor and publishes the
+    // results. Note that _publishCursor does NOT call ready().
     //
     // XXX This uses an undocumented interface which only the Mongo cursor
     // interface publishes. Should we make this interface public and encourage
@@ -834,33 +835,39 @@ _.extend(Meteor._LivedataSubscription.prototype, {
     //       reactiveThingy.publishMe();
     //     });
     //   };
-    if (res) {
-      if (res._publishCursor) {
-        res._publishCursor(self);
-        // _publishCursor only returns after the initial added callbacks have run.
-        // mark subscription as ready.
-        self.ready();
-      } else if (_.isArray(res)) {
-        // check all the elements are cursors
-        if (! _.every(res, function (cur) { return cur && cur._publishCursor; })) {
-          self.error(new Meteor.Error(500, "Pulish function returned an array of non-Cursors"));
+    var isCursor = function (c) {
+      return c && c._publishCursor;
+    };
+    if (isCursor(res)) {
+      res._publishCursor(self);
+      // _publishCursor only returns after the initial added callbacks have run.
+      // mark subscription as ready.
+      self.ready();
+    } else if (_.isArray(res)) {
+      // check all the elements are cursors
+      if (! _.all(res, isCursor)) {
+        self.error(new Error("Publish function returned an array of non-Cursors"));
+        return;
+      }
+      // find duplicate collection names
+      // XXX we should support overlapping cursors, but that would require the
+      // merge box to allow overlap within a subscription
+      var collectionNames = {};
+      for (var i = 0; i < res.length; ++i) {
+        var collectionName = res[i]._getCollectionName();
+        if (_.has(collectionNames, collectionName)) {
+          self.error(new Error(
+            "Publish function returned multiple cursors for collection " +
+              collectionName));
           return;
         }
-        // find duplicate collection names
-        var dups = [];
-        _.each(_.countBy(res, function (cur) { return cur._getCollectionName(); }),
-          function (count, col) {
-            count > 1 && dups.push(col);
-          });
+        collectionNames[collectionName] = true;
+      };
 
-        if (dups.length === 0) { // no duplicates
-          _.each(res, function (cur) {
-            cur._publishCursor(self);
-          });
-          self.ready();
-        } else
-          self.error(new Meteor.Error(500, "Publish function returned multiple cursors for one collection", dups));
-      }
+      _.each(res, function (cur) {
+        cur._publishCursor(self);
+      });
+      self.ready();
     }
   },
 

packages/livedata/livedata_test_service.js

@@ -294,24 +294,28 @@ if (Meteor.isServer) {
   Two.insert({name: "value5"});
 
   Meteor.publish("multiPublish", function (options) {
-    if (options.normal)
+    if (options.normal) {
       return [
         One.find(),
         Two.find()
       ];
-    else if (options.dup)
+    } else if (options.dup) {
+      // Suppress the log of the expected internal error.
+      Meteor._suppress_log(1);
       return [
         One.find(),
         One.find({name: "value2"}), // multiple cursors for one collection - error
         Two.find()
       ];
-    else if (options.notCursor)
+    } else if (options.notCursor) {
+      // Suppress the log of the expected internal error.
+      Meteor._suppress_log(1);
       return [
         One.find(),
         "not a cursor",
         Two.find()
       ];
-    else
+    } else
       throw "unexpected options";
   });
 }

packages/livedata/livedata_tests.js

@@ -521,15 +521,15 @@ if (Meteor.isClient) {
       function (test, expect) {
         Meteor.subscribe("multiPublish", {dup: 1}, {
           onReady: failure(),
-          onError: expect(failure(test, 500, "Publish function returned multiple cursors for one collection"))
+          onError: expect(failure(test, 500, "Internal server error"))
         });
       },
       function (test, expect) {
         Meteor.subscribe("multiPublish", {notCursor: 1}, {
           onReady: failure(),
-          onError: expect(failure(test, 500, "Pulish function returned an array of non-Cursors"))
+          onError: expect(failure(test, 500, "Internal server error"))
         });
-      },
+      }
     ]);
 }
 

packages/mongo-livedata/mongo_driver.js

@@ -398,13 +398,16 @@ Cursor.prototype._publishCursor = function (sub) {
     }
   });
 
+  // We don't call sub.ready() here: it gets called in livedata_server, after
+  // possibly calling _publishCursor on multiple returned cursors.
+
   // register stop callback (expects lambda w/ no args).
   sub.onStop(function () {observeHandle.stop();});
 };
 
-// When you call Meteor.publish() with a function that returns an array of Cursor, we need
-// to check uniqueness of Cursor for each collection in that array. We can accomplish it through
-// Cursor collection name comparison.
+// Used to guarantee that publish functions return at most one cursor per
+// collection. Private, because we might later have cursors that include
+// documents from multiple collections somehow.
 Cursor.prototype._getCollectionName = function () {
   var self = this;
   return self._cursorDescription.collectionName;