From df2820ffd92bb39d71ed55d884e149fa52b1a376 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@meteor.com>
Date: Thu, 22 May 2014 21:04:16 -0700
Subject: [PATCH] Only do one query for forEach/count in Deps

They used to run the query twice: once for the actual result and once to
set up the observe. Now it shares the work between the observe and the
actual query.

This required me to inline the _depend helper, but I actually think this
made what's going on more direct and clear.

Drop the _allow_unordered hack. I'm not convinced that it was ever truly
valid; the observe code really doesn't support unordered observes with
skip and limit, and I could not remember what it was about count's use
that made it hypothetically safe.  Easier to just remove the hack (until
we maybe eventually actually fix #1643)

Stop using Deps.Dependency in an unidiomatic way; just use
Deps.currentComputation directly.
---
 packages/minimongo/minimongo.js | 103 +++++++++++++++++++-------------
 1 file changed, 62 insertions(+), 41 deletions(-)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 25fc10f8b6..3e688fe352 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -146,22 +146,45 @@ LocalCollection.prototype.findOne = function (selector, options) {
 LocalCollection.Cursor.prototype.forEach = function (callback, thisArg) {
   var self = this;
 
-  var objects = self._getRawObjects({ordered: true});
-
-  if (self.reactive) {
-    self._depend({
-      addedBefore: true,
-      removed: true,
-      changed: true,
-      movedBefore: true});
+  var docs;
+  var needsClone = true;
+  if (self.reactive && Deps.active) {
+    // Ensure that we invalidate the current computation if the result of this
+    // query changes. We also piggy-back on top of the query done by
+    // observeChanges so we don't need to do another query.
+    var computation = Deps.currentComputation;
+    var invalidate = function () {
+      computation.invalidate();
+    };
+    var initial = true;
+    docs = [];
+    // observeChanges will stop() when this computation is invalidated
+    self.observeChanges({
+      added: function (id, fields) {
+        if (initial) {
+          fields._id = id;
+          docs.push(fields);
+        } else {
+          invalidate();
+        }
+      },
+      changed: invalidate,
+      removed: invalidate,
+      movedBefore: invalidate
+    });
+    initial = false;
+    needsClone = false;  // observeChanges gives us cloned docs
+  } else {
+    docs = self._getRawObjects({ordered: true});
   }
 
-  _.each(objects, function (elt, i) {
+  _.each(docs, function (elt, i) {
     if (self.projectionFn) {
       elt = self.projectionFn(elt);
-    } else {
-      // projection functions always clone the pieces they use, but if not we
-      // have to do it here.
+    } else if (needsClone) {
+      // projection functions always clone the pieces they use, and
+      // observeChanges callbacks got a cloned document, but otherwise we have
+      // to do it here.
       elt = EJSON.clone(elt);
     }
 
@@ -196,9 +219,32 @@ LocalCollection.Cursor.prototype.fetch = function () {
 LocalCollection.Cursor.prototype.count = function () {
   var self = this;
 
-  if (self.reactive)
-    self._depend({added: true, removed: true},
-                 true /* allow the observe to be unordered */);
+  if (self.reactive && Deps.active) {
+    // Ensure that we invalidate the current computation if the result of this
+    // query changes. We also piggy-back on top of the query done by
+    // observeChanges so we don't need to do another query.
+    var computation = Deps.currentComputation;
+    var invalidate = function () {
+      computation.invalidate();
+    };
+    var initial = true;
+    var count = 0;
+    // observeChanges will stop() when this computation is invalidated
+    self.observeChanges({
+      // we have to use addedBefore rather than added, because observeChanges in
+      // unordered (added) mode doesn't support skip/limit
+      addedBefore: function () {
+        if (initial) {
+          count++;
+        } else {
+          invalidate();
+        }
+      },
+      removed: invalidate
+    });
+    initial = false;
+    return count;
+  }
 
   return self._getRawObjects({ordered: true}).length;
 };
@@ -274,7 +320,7 @@ _.extend(LocalCollection.Cursor.prototype, {
     // unordered observe.  eg, update's EJSON.clone, and the "there are several"
     // comment in _modifyAndNotify
     // XXX allow skip/limit with unordered observe
-    if (!options._allow_unordered && !ordered && (self.skip || self.limit))
+    if (!ordered && (self.skip || self.limit))
       throw new Error("must use ordered observe with skip or limit");
 
     if (self.fields && (self.fields._id === 0 || self.fields._id === false))
@@ -469,31 +515,6 @@ LocalCollection.Cursor.prototype._getRawObjects = function (options) {
   return results.slice(idx_start, idx_end);
 };
 
-// XXX Maybe we need a version of observe that just calls a callback if
-// anything changed.
-LocalCollection.Cursor.prototype._depend = function (changers, _allow_unordered) {
-  var self = this;
-
-  if (Deps.active) {
-    var v = new Deps.Dependency;
-    v.depend();
-    var notifyChange = _.bind(v.changed, v);
-
-    var options = {
-      _suppress_initial: true,
-      _allow_unordered: _allow_unordered
-    };
-    _.each(['added', 'changed', 'removed', 'addedBefore', 'movedBefore'],
-           function (fnName) {
-             if (changers[fnName])
-               options[fnName] = notifyChange;
-           });
-
-    // observeChanges will stop() when this computation is invalidated
-    self.observeChanges(options);
-  }
-};
-
 // XXX enforce rule that field names can't start with '$' or contain '.'
 // (real mongodb does in fact enforce this)
 // XXX possibly enforce that 'undefined' does not appear (we assume