diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 91477e54df..0e328253c0 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -70,9 +70,9 @@ LocalCollection.Cursor = function (collection, selector, options) { } else { this.selector_f = LocalCollection._compileSelector(selector); this.sort_f = options.sort ? LocalCollection._compileSort(options.sort) : null; - this.skip = options.skip; - this.limit = options.limit; } + this.skip = options.skip; + this.limit = options.limit; // db_objects is a list of the objects that match the cursor. (It's always a // list, never an object: LocalCollection.Cursor is always ordered.) @@ -94,10 +94,17 @@ LocalCollection.prototype.findOne = function (selector, options) { if (arguments.length === 0) selector = {}; - // XXX disable limit here so that we can observe findOne() cursor, - // as required by markAsReactive. - // options = options || {}; - // options.limit = 1; + // NOTE: by setting limit 1 here, we end up using very inefficient + // code that recomputes the whole query on each update. The upside is + // that when you reactively depend on a findOne you only get + // invalidated when the found object changes, not any object in the + // collection. Most findOne will be by id, which has a fast path, so + // this might not be a big deal. In most cases, invalidation causes + // the called to re-query anyway, so this should be a net performance + // improvement. + options = options || {}; + options.limit = 1; + return this.find(selector, options).fetch()[0]; }; @@ -173,7 +180,6 @@ LocalCollection.LiveResultsSet = function () {}; // initial results delivered through added callback // XXX maybe callbacks should take a list of objects, to expose transactions? // XXX maybe support field limiting (to limit what you're notified on) -// XXX maybe support limit/skip _.extend(LocalCollection.Cursor.prototype, { observe: function (options) { @@ -187,8 +193,8 @@ _.extend(LocalCollection.Cursor.prototype, { _observeInternal: function (ordered, options) { var self = this; - if (self.skip || self.limit) - throw new Error("cannot observe queries with skip or limit"); + if (!ordered && (self.skip || self.limit)) + throw new Error("must use ordered observe with skip or limit"); var qid = self.collection.next_qid++; @@ -198,7 +204,8 @@ _.extend(LocalCollection.Cursor.prototype, { sort_f: ordered && self.sort_f, results_snapshot: null, ordered: ordered, - cursor: this + cursor: this, + needsRecompute: false }; query.results = self._getRawObjects(ordered); if (self.collection.paused) @@ -254,6 +261,12 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered) { // fast path for single ID value if (self.selector_id) { + // If you have non-zero skip and ask for a single id, you get + // nothing. This is so it matches the behavior of the '{_id: foo}' + // path. + if (self.skip) + return results; + if (_.has(self.collection.docs, self.selector_id)) { var selectedDoc = self.collection.docs[self.selector_id]; if (ordered) @@ -273,6 +286,10 @@ LocalCollection.Cursor.prototype._getRawObjects = function (ordered) { else results[id] = doc; } + // Fast path for limited unsorted queries. + if (self.limit && !self.skip && !self.sort_f && + results.length === self.limit) + return results; } if (!ordered) @@ -337,9 +354,15 @@ LocalCollection.prototype.insert = function (doc) { // trigger live queries that match for (var qid in self.queries) { var query = self.queries[qid]; - if (query.selector_f(doc)) - LocalCollection._insertInResults(query, doc); + if (query.selector_f(doc)) { + if (query.cursor.skip || query.cursor.limit) + query.needsRecompute = true; + else + LocalCollection._insertInResults(query, doc); + } } + + self._recomputeNecessaryQueries(); }; LocalCollection.prototype.remove = function (selector) { @@ -365,8 +388,12 @@ LocalCollection.prototype.remove = function (selector) { var removeId = remove[i]; var removeDoc = self.docs[removeId]; _.each(self.queries, function (query) { - if (query.selector_f(removeDoc)) - queryRemove.push([query, removeDoc]); + if (query.selector_f(removeDoc)) { + if (query.cursor.skip || query.cursor.limit) + query.needsRecompute = true; + else + queryRemove.push([query, removeDoc]); + } }); self._saveOriginal(removeId, removeDoc); delete self.docs[removeId]; @@ -376,6 +403,8 @@ LocalCollection.prototype.remove = function (selector) { for (var i = 0; i < queryRemove.length; i++) { LocalCollection._removeFromResults(queryRemove[i][0], queryRemove[i][1]); } + + self._recomputeNecessaryQueries(); }; // XXX atomicity: if multi is true, and one modification fails, do @@ -393,7 +422,7 @@ LocalCollection.prototype.update = function (selector, mod, options) { self._saveOriginal(id, doc); self._modifyAndNotify(doc, mod); if (!options.multi) - return; + break; any = true; } } @@ -408,6 +437,8 @@ LocalCollection.prototype.update = function (selector, mod, options) { self.insert(insert); } } + + self._recomputeNecessaryQueries(); }; LocalCollection.prototype._modifyAndNotify = function (doc, mod) { @@ -431,12 +462,24 @@ LocalCollection.prototype._modifyAndNotify = function (doc, mod) { query = self.queries[qid]; var before = matched_before[qid]; var after = query.selector_f(doc); - if (before && !after) + + if (query.cursor.skip || query.cursor.limit) { + // We need to recompute any query where the doc may have been in the + // cursor's window either before or after the update. (Note that if skip + // or limit is set, "before" and "after" being true do not necessarily + // mean that the document is in the cursor's output after skip/limit is + // applied... but if they are false, then the document definitely is NOT + // in the output. So it's safe to skip recompute if neither before or + // after are true.) + if (before || after) + query.needsRecompute = true; + } else if (before && !after) { LocalCollection._removeFromResults(query, doc); - else if (!before && after) + } else if (!before && after) { LocalCollection._insertInResults(query, doc); - else if (before && after) + } else if (before && after) { LocalCollection._updateInResults(query, doc, old_doc); + } } }; @@ -463,6 +506,9 @@ LocalCollection._deepcopy = function (v) { // XXX the sorted-query logic below is laughably inefficient. we'll // need to come up with a better datastructure for this. +// +// XXX the logic for observing with a skip or a limit is even more +// laughably inefficient. we recompute the whole results every time! LocalCollection._insertInResults = function (query, doc) { if (query.ordered) { @@ -517,6 +563,26 @@ LocalCollection._updateInResults = function (query, doc, old_doc) { query.moved(LocalCollection._deepcopy(doc), orig_idx, new_idx); }; +LocalCollection.prototype._recomputeNecessaryQueries = function () { + var self = this; + _.each(self.queries, function (query) { + if (query.needsRecompute) { + LocalCollection._recomputeResults(query); + query.needsRecompute = false; + } + }); +}; + +LocalCollection._recomputeResults = function (query) { + var old_results = query.results; + query.results = query.cursor._getRawObjects(query.ordered); + + if (!query.paused) + LocalCollection._diffQuery( + query.ordered, old_results, query.results, query, true); +}; + + LocalCollection._findInOrderedResults = function (query, doc) { if (!query.ordered) throw new Error("Can't call _findInOrderedResults on unordered query"); diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index fdfe5b3c54..b3713eb25c 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -59,123 +59,83 @@ var log_callbacks = function (operations) { }; // XXX test shared structure in all MM entrypoints +Tinytest.add("minimongo - basics", function (test) { + var c = new LocalCollection(); -_.each(['observe', '_observeUnordered'], function (observeMethod) { - Tinytest.add("minimongo - basics (" + observeMethod + ")", function (test) { - var c = new LocalCollection(); + c.insert({type: "kitten", name: "fluffy"}); + c.insert({type: "kitten", name: "snookums"}); + c.insert({type: "cryptographer", name: "alice"}); + c.insert({type: "cryptographer", name: "bob"}); + c.insert({type: "cryptographer", name: "cara"}); + test.equal(c.find().count(), 5); + test.equal(c.find({type: "kitten"}).count(), 2); + test.equal(c.find({type: "cryptographer"}).count(), 3); + test.length(c.find({type: "kitten"}).fetch(), 2); + test.length(c.find({type: "cryptographer"}).fetch(), 3); - c.insert({type: "kitten", name: "fluffy"}); - c.insert({type: "kitten", name: "snookums"}); - c.insert({type: "cryptographer", name: "alice"}); - c.insert({type: "cryptographer", name: "bob"}); - c.insert({type: "cryptographer", name: "cara"}); - test.equal(c.find().count(), 5); - test.equal(c.find({type: "kitten"}).count(), 2); - test.equal(c.find({type: "cryptographer"}).count(), 3); - test.length(c.find({type: "kitten"}).fetch(), 2); - test.length(c.find({type: "cryptographer"}).fetch(), 3); + c.remove({name: "cara"}); + test.equal(c.find().count(), 4); + test.equal(c.find({type: "kitten"}).count(), 2); + test.equal(c.find({type: "cryptographer"}).count(), 2); + test.length(c.find({type: "kitten"}).fetch(), 2); + test.length(c.find({type: "cryptographer"}).fetch(), 2); - c.remove({name: "cara"}); - test.equal(c.find().count(), 4); - test.equal(c.find({type: "kitten"}).count(), 2); - test.equal(c.find({type: "cryptographer"}).count(), 2); - test.length(c.find({type: "kitten"}).fetch(), 2); - test.length(c.find({type: "cryptographer"}).fetch(), 2); + c.update({name: "snookums"}, {$set: {type: "cryptographer"}}); + test.equal(c.find().count(), 4); + test.equal(c.find({type: "kitten"}).count(), 1); + test.equal(c.find({type: "cryptographer"}).count(), 3); + test.length(c.find({type: "kitten"}).fetch(), 1); + test.length(c.find({type: "cryptographer"}).fetch(), 3); - c.update({name: "snookums"}, {$set: {type: "cryptographer"}}); - test.equal(c.find().count(), 4); - test.equal(c.find({type: "kitten"}).count(), 1); - test.equal(c.find({type: "cryptographer"}).count(), 3); - test.length(c.find({type: "kitten"}).fetch(), 1); - test.length(c.find({type: "cryptographer"}).fetch(), 3); + c.remove(null); + c.remove(false); + c.remove(undefined); + test.equal(c.find().count(), 4); - c.remove(null); - c.remove(false); - c.remove(undefined); - test.equal(c.find().count(), 4); + c.remove({_id: null}); + c.remove({_id: false}); + c.remove({_id: undefined}); + c.remove(); + test.equal(c.find().count(), 4); - c.remove({_id: null}); - c.remove({_id: false}); - c.remove({_id: undefined}); - c.remove(); - test.equal(c.find().count(), 4); + c.remove({}); + test.equal(c.find().count(), 0); - c.remove({}); - test.equal(c.find().count(), 0); + c.insert({_id: 1, name: "strawberry", tags: ["fruit", "red", "squishy"]}); + c.insert({_id: 2, name: "apple", tags: ["fruit", "red", "hard"]}); + c.insert({_id: 3, name: "rose", tags: ["flower", "red", "squishy"]}); - c.insert({_id: 1, name: "strawberry", tags: ["fruit", "red", "squishy"]}); - c.insert({_id: 2, name: "apple", tags: ["fruit", "red", "hard"]}); - c.insert({_id: 3, name: "rose", tags: ["flower", "red", "squishy"]}); + test.equal(c.find({tags: "flower"}).count(), 1); + test.equal(c.find({tags: "fruit"}).count(), 2); + test.equal(c.find({tags: "red"}).count(), 3); + test.length(c.find({tags: "flower"}).fetch(), 1); + test.length(c.find({tags: "fruit"}).fetch(), 2); + test.length(c.find({tags: "red"}).fetch(), 3); - test.equal(c.find({tags: "flower"}).count(), 1); - test.equal(c.find({tags: "fruit"}).count(), 2); - test.equal(c.find({tags: "red"}).count(), 3); - test.length(c.find({tags: "flower"}).fetch(), 1); - test.length(c.find({tags: "fruit"}).fetch(), 2); - test.length(c.find({tags: "red"}).fetch(), 3); + test.equal(c.findOne(1).name, "strawberry"); + test.equal(c.findOne(2).name, "apple"); + test.equal(c.findOne(3).name, "rose"); + test.equal(c.findOne(4), undefined); + test.equal(c.findOne("abc"), undefined); + test.equal(c.findOne(undefined), undefined); - test.equal(c.findOne(1).name, "strawberry"); - test.equal(c.findOne(2).name, "apple"); - test.equal(c.findOne(3).name, "rose"); - test.equal(c.findOne(4), undefined); - test.equal(c.findOne("abc"), undefined); - test.equal(c.findOne(undefined), undefined); + test.equal(c.find(1).count(), 1); + test.equal(c.find(4).count(), 0); + test.equal(c.find("abc").count(), 0); + test.equal(c.find(undefined).count(), 0); + test.equal(c.find().count(), 3); + test.equal(c.find(1, {skip: 1}).count(), 0); + test.equal(c.find({_id: 1}, {skip: 1}).count(), 0); - test.equal(c.find(1).count(), 1); - test.equal(c.find(4).count(), 0); - test.equal(c.find("abc").count(), 0); - test.equal(c.find(undefined).count(), 0); - test.equal(c.find().count(), 3); + // Regression test for #455. + c.insert({foo: {bar: 'baz'}}); + test.equal(c.find({foo: {bam: 'baz'}}).count(), 0); + test.equal(c.find({foo: {bar: 'baz'}}).count(), 1); - // Regression test for #455. - c.insert({foo: {bar: 'baz'}}); - test.equal(c.find({foo: {bam: 'baz'}}).count(), 0); - test.equal(c.find({foo: {bar: 'baz'}}).count(), 1); - - // Duplicate ID. - test.throws(function () { c.insert({_id: 1, name: "bla"}); }); - test.equal(c.find({_id: 1}).count(), 1); - test.equal(c.findOne(1).name, "strawberry"); - - var ev = ""; - var makecb = function (tag) { - return { - added: function (doc) { ev += "a" + tag + doc._id + "_"; }, - changed: function (doc) { ev += "c" + tag + doc._id + "_"; }, - removed: function (doc) { ev += "r" + tag + doc._id + "_"; } - }; - }; - var expect = function (x) { - test.equal(ev, x); - ev = ""; - }; - // This should work equally well for ordered and unordered observations - // (because the callbacks don't look at indices and there's no 'moved' - // callback). - var handle = c.find({tags: "flower"})[observeMethod](makecb('a')); - expect("aa3_"); - c.update({name: "rose"}, {$set: {tags: ["bloom", "red", "squishy"]}}); - expect("ra3_"); - c.update({name: "rose"}, {$set: {tags: ["flower", "red", "squishy"]}}); - expect("aa3_"); - c.update({name: "rose"}, {$set: {food: false}}); - expect("ca3_"); - c.remove({}); - expect("ra3_"); - c.insert({_id: 4, name: "daisy", tags: ["flower"]}); - expect("aa4_"); - handle.stop(); - // After calling stop, no more callbacks are called. - c.insert({_id: 5, name: "iris", tags: ["flower"]}); - expect(""); - - // Test that observing a lookup by ID works. - handle = c.find(4)[observeMethod](makecb('b')); - expect('ab4_'); - c.update(4, {$set: {eek: 5}}); - expect('cb4_'); - handle.stop(); - }); + // Duplicate ID. + test.throws(function () { c.insert({_id: 1, name: "bla"}); }); + test.equal(c.find({_id: 1}).count(), 1); + test.equal(c.findOne(1).name, "strawberry"); }); Tinytest.add("minimongo - cursors", function (test) { @@ -1159,7 +1119,7 @@ Tinytest.add("minimongo - modify", function (test) { // XXX test update() (selecting docs, multi, upsert..) -Tinytest.add("minimongo - observe", function (test) { +Tinytest.add("minimongo - observe ordered", function (test) { var operations = []; var cbs = log_callbacks(operations); var handle; @@ -1203,8 +1163,81 @@ Tinytest.add("minimongo - observe", function (test) { c.insert({a:100}); test.equal(operations.shift(), ['added', {a:100}, 0]); handle.stop(); + + // test skip and limit. + c.remove({}); + handle = c.find({}, {sort: {a: 1}, skip: 1, limit: 2}).observe(cbs); + test.equal(operations.shift(), undefined); + c.insert({a:1}); + test.equal(operations.shift(), undefined); + c.insert({a:2}); + test.equal(operations.shift(), ['added', {a:2}, 0]); + c.insert({a:3}); + test.equal(operations.shift(), ['added', {a:3}, 1]); + c.insert({a:4}); + test.equal(operations.shift(), undefined); + id = c.findOne({a:2})._id; + c.update({a:1}, {a:0}); + test.equal(operations.shift(), undefined); + c.update({a:0}, {a:5}); + test.equal(operations.shift(), ['removed', id, 0, {a:2}]); + test.equal(operations.shift(), ['added', {a:4}, 1]); + + handle.stop(); }); +_.each(['observe', '_observeUnordered'], function (observeMethod) { + Tinytest.add("minimongo - observe (" + observeMethod + ")", function (test) { + var c = new LocalCollection(); + + var ev = ""; + var makecb = function (tag) { + return { + added: function (doc) { ev += "a" + tag + doc._id + "_"; }, + changed: function (doc) { ev += "c" + tag + doc._id + "_"; }, + removed: function (doc) { ev += "r" + tag + doc._id + "_"; } + }; + }; + var expect = function (x) { + test.equal(ev, x); + ev = ""; + }; + + c.insert({_id: 1, name: "strawberry", tags: ["fruit", "red", "squishy"]}); + c.insert({_id: 2, name: "apple", tags: ["fruit", "red", "hard"]}); + c.insert({_id: 3, name: "rose", tags: ["flower", "red", "squishy"]}); + + // This should work equally well for ordered and unordered observations + // (because the callbacks don't look at indices and there's no 'moved' + // callback). + var handle = c.find({tags: "flower"})[observeMethod](makecb('a')); + expect("aa3_"); + c.update({name: "rose"}, {$set: {tags: ["bloom", "red", "squishy"]}}); + expect("ra3_"); + c.update({name: "rose"}, {$set: {tags: ["flower", "red", "squishy"]}}); + expect("aa3_"); + c.update({name: "rose"}, {$set: {food: false}}); + expect("ca3_"); + c.remove({}); + expect("ra3_"); + c.insert({_id: 4, name: "daisy", tags: ["flower"]}); + expect("aa4_"); + handle.stop(); + // After calling stop, no more callbacks are called. + c.insert({_id: 5, name: "iris", tags: ["flower"]}); + expect(""); + + // Test that observing a lookup by ID works. + handle = c.find(4)[observeMethod](makecb('b')); + expect('ab4_'); + c.update(4, {$set: {eek: 5}}); + expect('cb4_'); + handle.stop(); + }); +}); + + + Tinytest.add("minimongo - diff", function (test) { // test correctness