Reduce unnecessary Mongo polling in more cases.

Specifically, we can now detect if a selector matches at most a fixed set of
IDs (instead of just "at most one ID"), both for queries and for writes. This
includes every write made from the client on a restricted collection!

packages/minimongo/minimongo.js

@@ -410,13 +410,15 @@ LocalCollection.prototype.remove = function (selector) {
   var selector_f = LocalCollection._compileSelector(selector);
 
   // Avoid O(n) for "remove a single doc by ID".
-  var onlyMatchingId = LocalCollection._idMatchedBySelector(selector);
-  if (onlyMatchingId !== undefined) {
-    var strId = LocalCollection._idStringify(onlyMatchingId);
-    // We still have to run selector_f, in case it's something like
-    //   {_id: "X", a: 42}
-    if (_.has(self.docs, strId) && selector_f(self.docs[strId]))
-      remove.push(strId);
+  var specificIds = LocalCollection._idsMatchedBySelector(selector);
+  if (specificIds) {
+    _.each(specificIds, function (id) {
+      var strId = LocalCollection._idStringify(id);
+      // We still have to run selector_f, in case it's something like
+      //   {_id: "X", a: 42}
+      if (_.has(self.docs, strId) && selector_f(self.docs[strId]))
+        remove.push(strId);
+    });
   } else {
     for (var id in self.docs) {
       var doc = self.docs[id];

packages/minimongo/minimongo_tests.js

@@ -1696,3 +1696,24 @@ Tinytest.add("minimongo - pause", function (test) {
 
   h.stop();
 });
+
+Tinytest.add("minimongo - ids matched by selector", function (test) {
+  var check = function (selector, ids) {
+    var idsFromSelector = LocalCollection._idsMatchedBySelector(selector);
+    // XXX normalize order, in a way that also works for ObjectIDs?
+    test.equal(idsFromSelector, ids);
+  };
+  check("foo", ["foo"]);
+  check({_id: "foo"}, ["foo"]);
+  var oid1 = new LocalCollection._ObjectID();
+  check(oid1, [oid1]);
+  check({_id: oid1}, [oid1]);
+  check({_id: "foo", x: 42}, ["foo"]);
+  check({}, null);
+  check({_id: {$in: ["foo", oid1]}}, ["foo", oid1]);
+  check({_id: {$ne: "foo"}}, null);
+  // not actually valid, but works for now...
+  check({$and: ["foo"]}, ["foo"]);
+  check({$and: [{x: 42}, {_id: oid1}]}, [oid1]);
+  check({$and: [{x: 42}, {_id: {$in: [oid1]}}]}, [oid1]);
+});

packages/minimongo/objectid.js

@@ -59,43 +59,45 @@ LocalCollection._selectorIsId = function (selector) {
     selector instanceof LocalCollection._ObjectID;
 };
 
-// If this is a selector that matches at most one document, return that
-// id. Otherwise returns undefined. Note that the selector may have other
-// restrictions so it may not even match that document!
-// We care about $in and $and since those are generated access-controlled
-// update and remove.
-LocalCollection._idMatchedBySelector = function (selector) {
+// If this is a selector which explicitly constrains the match by ID to a finite
+// number of documents, returns a list of their IDs.  Otherwise returns
+// null. Note that the selector may have other restrictions so it may not even
+// match those document!  We care about $in and $and since those are generated
+// access-controlled update and remove.
+LocalCollection._idsMatchedBySelector = function (selector) {
   // Is the selector just an ID?
   if (LocalCollection._selectorIsId(selector))
-    return selector;
+    return [selector];
   if (!selector)
-    return undefined;
+    return null;
 
   // Do we have an _id clause?
   if (_.has(selector, '_id')) {
     // Is the _id clause just an ID?
     if (LocalCollection._selectorIsId(selector._id))
-      return selector._id;
-    // Is the _id clause {_id: {$in: [oneId]}}?
+      return [selector._id];
+    // Is the _id clause {_id: {$in: ["x", "y", "z"]}}?
     if (selector._id && selector._id.$in
-        && _.isArray(selector._id.$in) && selector._id.$in.length === 1
-        && LocalCollection._selectorIsId(selector._id.$in[0])) {
-      return selector._id.$in[0];
+        && _.isArray(selector._id.$in)
+        && !_.isEmpty(selector._id.$in)
+        && _.all(selector._id.$in, LocalCollection._selectorIsId)) {
+      return selector._id.$in;
     }
-    return undefined;
+    return null;
   }
 
-  // If this is a top-level $and, and any of the clauses can match at most one
-  // document, then the whole selector can match at most that document.
+  // If this is a top-level $and, and any of the clauses constrain their
+  // documents, then the whole selector is constrained by any one clause's
+  // constraint. (Well, by their intersection, but that seems unlikely.)
   if (selector.$and && _.isArray(selector.$and)) {
     for (var i = 0; i < selector.$and.length; ++i) {
-      var subId = LocalCollection._idMatchedBySelector(selector.$and[i]);
-      if (subId !== undefined)
-        return subId;
+      var subIds = LocalCollection._idsMatchedBySelector(selector.$and[i]);
+      if (subIds)
+        return subIds;
     }
   }
 
-  return undefined;
+  return null;
 };
 
 EJSON.addType("oid",  function (str) {

packages/mongo-livedata/mongo_driver.js

@@ -166,6 +166,25 @@ _Mongo.prototype.insert = function (collection_name, document) {
     throw err;
 };
 
+// Cause queries that may be affected by the selector to poll in this write
+// fence.
+_Mongo.prototype._refresh = function (collectionName, selector) {
+  var self = this;
+  var refreshKey = {collection: collectionName};
+  // If we know which documents we're removing, don't poll queries that are
+  // specific to other documents. (Note that multiple notifications here should
+  // not cause multiple polls, since all our listener is doing is enqueueing a
+  // poll.)
+  var specificIds = LocalCollection._idsMatchedBySelector(selector);
+  if (specificIds) {
+    _.each(specificIds, function (id) {
+      Meteor.refresh(_.extend({id: id}, refreshKey));
+    });
+  } else {
+    Meteor.refresh(refreshKey);
+  }
+};
+
 _Mongo.prototype.remove = function (collection_name, selector) {
   var self = this;
 
@@ -192,13 +211,7 @@ _Mongo.prototype.remove = function (collection_name, selector) {
   });
 
   var err = future.wait();
-  var refreshKey = {collection: collection_name};
-  // If we know which document we're removing, don't poll queries that are
-  // specific to other documents.
-  var onlyMatchingId = LocalCollection._idMatchedBySelector(selector);
-  if (onlyMatchingId !== undefined)
-    refreshKey.id = onlyMatchingId;
-  Meteor.refresh(refreshKey);
+  self._refresh(collection_name, selector);
   write.committed();
   if (err)
     throw err;
@@ -238,13 +251,7 @@ _Mongo.prototype.update = function (collection_name, selector, mod, options) {
   });
 
   var err = future.wait();
-  var refreshKey = {collection: collection_name};
-  // If we know which document we're removing, don't poll queries that are
-  // specific to other documents.
-  var onlyMatchingId = LocalCollection._idMatchedBySelector(selector);
-  if (onlyMatchingId !== undefined)
-    refreshKey.id = onlyMatchingId;
-  Meteor.refresh(refreshKey);
+  self._refresh(collection_name, selector);
   write.committed();
   if (err)
     throw err;
@@ -603,25 +610,36 @@ var LiveResultsSet = function (cursorDescription, mongoHandle, ordered,
   self._taskQueue = new Meteor._SynchronousQueue();
 
   // Listen for the invalidation messages that will trigger us to poll the
-  // database for changes. If this selector is just a single ID, specify it
-  // here, so that updates that are also a single ID don't require a poll.
+  // database for changes. If this selector specifies specific IDs, specify them
+  // here, so that updates to different specific IDs don't cause us to poll.
+  var listenOnTrigger = function (trigger) {
+    var listener = Meteor._InvalidationCrossbar.listen(
+      trigger, function (notification, complete) {
+        // When someone does a transaction that might affect us, schedule a poll
+        // of the database. If that transaction happens inside of a write fence,
+        // block the fence until we've polled and notified observers.
+        var fence = Meteor._CurrentWriteFence.get();
+        if (fence)
+          self._pendingWrites.push(fence.beginWrite());
+        // Ensure a poll is scheduled... but if we already know that one is,
+        // don't hit the throttled _ensurePollIsScheduled function (which might
+        // lead to us calling it unnecessarily in 50ms).
+        if (self._pollsScheduledButNotStarted === 0)
+          self._ensurePollIsScheduled();
+        complete();
+      });
+    self._stopCallbacks.push(function () { listener.stop(); });
+  };
   var key = {collection: cursorDescription.collectionName};
-  var onlyMatchingId = LocalCollection._idMatchedBySelector(
+  var specificIds = LocalCollection._idsMatchedBySelector(
     cursorDescription.selector);
-  if (onlyMatchingId !== undefined)
-    key.id = onlyMatchingId;
-  var listener = Meteor._InvalidationCrossbar.listen(
-    key, function (notification, complete) {
-      // When someone does a transaction that might affect us, schedule a poll
-      // of the database. If that transaction happens inside of a write fence,
-      // block the fence until we've polled and notified observers.
-      var fence = Meteor._CurrentWriteFence.get();
-      if (fence)
-        self._pendingWrites.push(fence.beginWrite());
-      self._ensurePollIsScheduled();
-      complete();
+  if (specificIds) {
+    _.each(specificIds, function (id) {
+      listenOnTrigger(_.extend({id: id}, key));
     });
-  self._stopCallbacks.push(function () { listener.stop(); });
+  } else {
+    listenOnTrigger(key);
+  }
 
   // Map from handle ID to ObserveHandle.
   self._observeHandles = {};