Permalink
Browse files

Revert part of "Test that reverting df2820 fixed #2275."

This reverts commit c05ae24 EXCEPT for
the "fetch in observe" test, which I'm leaving in because we still want
a test for #2275.

This commit made it an error to start an observeChanges from within an
observeChanges callback for the same collection, but it turns out that
this is actually a somewhat common thing to do (for example, nested
'each'). Instead, we'll leave things the way they were pre-0.8.2: you
can start an observeChanges from within an observeChanges callback, but
it'll be subtly buggy in that you won't get synchronous 'added'
callbacks. This issue is described in #2315, along with the fact that
insert/update/remove/resumeObservers won't run their affected observe
callbacks if they are called from within a task on the collection's
queue. Eventually we'll implement the full fix (which relaxes the
requirement that insert/update/remove run all their callbacks before
returning) described in #2315.
  • Loading branch information...
1 parent d3eae5a commit 78d08b5537235f63cb40e538952568dd3e892ab6 Emily Stark committed Jul 21, 2014
Showing with 18 additions and 49 deletions.
  1. +18 −24 packages/minimongo/minimongo.js
  2. +0 −25 packages/minimongo/minimongo_tests.js
@@ -339,6 +339,21 @@ _.extend(LocalCollection.Cursor.prototype, {
query.movedBefore = wrapCallback(options.movedBefore);
}
+ if (!options._suppress_initial && !self.collection.paused) {
+ // XXX unify ordered and unordered interface
+ var each = ordered
+ ? _.bind(_.each, null, query.results)
+ : _.bind(query.results.forEach, query.results);
+ each(function (doc) {
+ var fields = EJSON.clone(doc);
+
+ delete fields._id;
+ if (ordered)
+ query.addedBefore(doc._id, fields, null);
+ query.added(doc._id, fields);
+ });
+ }
+
var handle = new LocalCollection.ObserveHandle;
_.extend(handle, {
collection: self.collection,
@@ -358,30 +373,9 @@ _.extend(LocalCollection.Cursor.prototype, {
handle.stop();
});
}
-
- if (!options._suppress_initial && !self.collection.paused) {
- // XXX unify ordered and unordered interface
- var each = ordered
- ? _.bind(_.each, null, query.results)
- : _.bind(query.results.forEach, query.results);
- each(function (doc) {
- var fields = EJSON.clone(doc);
-
- delete fields._id;
- if (ordered)
- query.addedBefore(doc._id, fields, null);
- query.added(doc._id, fields);
- });
-
- // run the observe callbacks resulting from the initial contents
- // before we leave the observe.
- if (self.collection._observeQueue.safeToRunTask()) {
- self.collection._observeQueue.drain();
- } else if (options.added || options.addedBefore) {
- // See #2315.
- throw Error("observeChanges called from an observe callback on the same collection cannot differentiate between initial and later adds");
- }
- }
+ // run the observe callbacks resulting from the initial contents
+ // before we leave the observe.
+ self.collection._observeQueue.drain();
return handle;
}
@@ -3109,28 +3109,3 @@ Tinytest.add("minimongo - fetch in observe", function (test) {
observe.stop();
computation.stop();
});
-
-Tinytest.add("minimongo - observe in observe", function (test) {
- var coll = new LocalCollection;
- coll.insert({foo: 2});
-
- var observe1AddedCalled = false;
- var observe1 = coll.find({foo: 1}).observeChanges({
- added: function (id, fields) {
- observe1AddedCalled = true;
- test.equal(fields, {foo: 1});
-
- // It would be even better if this didn't throw; see #2315.
- test.throws(function () {
- coll.find({foo: 2}).observeChanges({
- added: function () {
- }
- });
- });
- }
- });
- test.isFalse(observe1AddedCalled);
- coll.insert({foo: 1});
- test.isTrue(observe1AddedCalled);
- observe1.stop();
-});

0 comments on commit 78d08b5

Please sign in to comment.