From f89fc9e979104eb6c8f252c17da5fbd06f7d6109 Mon Sep 17 00:00:00 2001 From: Geoff Jacobsen Date: Wed, 15 May 2013 16:42:16 +1200 Subject: [PATCH] Return counts for collection update and remove commands In order to support optimistic locking the counts from the MongoDB update and remove commands need to be returned to the application code so that the update/remove can be verified they took place and action taken if they did not (like throwing a 409 Error). For consistency make LocalCollection return counts also. --- packages/minimongo/minimongo.js | 5 +++++ packages/minimongo/minimongo_tests.js | 20 +++++++++++++------ packages/mongo-livedata/collection.js | 4 +++- packages/mongo-livedata/mongo_driver.js | 6 ++++-- .../mongo-livedata/mongo_livedata_tests.js | 17 ++++++++++++++-- 5 files changed, 41 insertions(+), 11 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index ceac91e251..256e450005 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -498,6 +498,7 @@ LocalCollection.prototype.remove = function (selector) { LocalCollection._recomputeResults(query); }); self._observeQueue.drain(); + return remove.length; }; // XXX atomicity: if multi is true, and one modification fails, do @@ -523,12 +524,15 @@ LocalCollection.prototype.update = function (selector, mod, options) { }); var recomputeQids = {}; + var updateCount = 0; + for (var id in self.docs) { var doc = self.docs[id]; if (selector_f(doc)) { // XXX Should we save the original even if mod ends up being a no-op? self._saveOriginal(id, doc); self._modifyAndNotify(doc, mod, recomputeQids); + ++updateCount; if (!options.multi) break; } @@ -541,6 +545,7 @@ LocalCollection.prototype.update = function (selector, mod, options) { qidToOriginalResults[qid]); }); self._observeQueue.drain(); + return updateCount; }; LocalCollection.prototype._modifyAndNotify = function ( diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 23b938dea6..e30784e46a 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -66,7 +66,8 @@ var log_callbacks = function (operations) { // XXX test shared structure in all MM entrypoints Tinytest.add("minimongo - basics", function (test) { var c = new LocalCollection(), - fluffyKitten_id; + fluffyKitten_id, + count; fluffyKitten_id = c.insert({type: "kitten", name: "fluffy"}); c.insert({type: "kitten", name: "snookums"}); @@ -87,7 +88,8 @@ Tinytest.add("minimongo - basics", function (test) { test.length(c.find({type: "kitten"}).fetch(), 2); test.length(c.find({type: "cryptographer"}).fetch(), 2); - c.update({name: "snookums"}, {$set: {type: "cryptographer"}}); + count = c.update({name: "snookums"}, {$set: {type: "cryptographer"}}); + test.equal(count, 1); test.equal(c.find().count(), 4); test.equal(c.find({type: "kitten"}).count(), 1); test.equal(c.find({type: "cryptographer"}).count(), 3); @@ -102,10 +104,12 @@ Tinytest.add("minimongo - basics", function (test) { c.remove({_id: null}); c.remove({_id: false}); c.remove({_id: undefined}); - c.remove(); + count = c.remove(); + test.equal(count, 0); test.equal(c.find().count(), 4); - c.remove({}); + count = c.remove({}); + test.equal(count, 4); test.equal(c.find().count(), 0); c.insert({_id: 1, name: "strawberry", tags: ["fruit", "red", "squishy"]}); @@ -1647,7 +1651,8 @@ Tinytest.add("minimongo - diff", function (test) { Tinytest.add("minimongo - saveOriginals", function (test) { // set up some data - var c = new LocalCollection(); + var c = new LocalCollection(), + count; c.insert({_id: 'foo', x: 'untouched'}); c.insert({_id: 'bar', x: 'updateme'}); c.insert({_id: 'baz', x: 'updateme'}); @@ -1658,9 +1663,12 @@ Tinytest.add("minimongo - saveOriginals", function (test) { c.saveOriginals(); c.insert({_id: "hooray", z: 'insertme'}); c.remove({y: 'removeme'}); - c.update({x: 'updateme'}, {$set: {z: 5}}, {multi: true}); + count = c.update({x: 'updateme'}, {$set: {z: 5}}, {multi: true}); c.update('bar', {$set: {k: 7}}); // update same doc twice + // Verify returned count is correct + test.equal(count, 2); + // Verify the originals. var originals = c.retrieveOriginals(); var affected = ['bar', 'baz', 'quux', 'whoa', 'hooray']; diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js index d67ea71158..b0bfd0f899 100644 --- a/packages/mongo-livedata/collection.js +++ b/packages/mongo-livedata/collection.js @@ -357,7 +357,9 @@ _.each(["insert", "update", "remove"], function (name) { // it's my collection. descend into the collection object // and propagate any exception. try { - self._collection[name].apply(self._collection, args); + var result = self._collection[name].apply(self._collection, args); + if (ret === undefined) + ret = result; } catch (e) { if (callback) { callback(e); diff --git a/packages/mongo-livedata/mongo_driver.js b/packages/mongo-livedata/mongo_driver.js index 1922570c4e..f1aa92a59b 100644 --- a/packages/mongo-livedata/mongo_driver.js +++ b/packages/mongo-livedata/mongo_driver.js @@ -228,9 +228,10 @@ _Mongo.prototype.remove = function (collection_name, selector) { var future = new Future; collection.remove(replaceTypes(selector, replaceMeteorAtomWithMongo), {safe: true}, future.resolver()); - future.wait(); + var result = future.wait(); // XXX We don't have to run this on error, right? self._refresh(collection_name, selector); + return result; } finally { write.committed(); } @@ -266,8 +267,9 @@ _Mongo.prototype.update = function (collection_name, selector, mod, options) { collection.update(replaceTypes(selector, replaceMeteorAtomWithMongo), replaceTypes(mod, replaceMeteorAtomWithMongo), mongoOpts, future.resolver()); - future.wait(); + var result = future.wait(); self._refresh(collection_name, selector); + return result; } finally { write.committed(); } diff --git a/packages/mongo-livedata/mongo_livedata_tests.js b/packages/mongo-livedata/mongo_livedata_tests.js index 948a7962bd..f7875fff5e 100644 --- a/packages/mongo-livedata/mongo_livedata_tests.js +++ b/packages/mongo-livedata/mongo_livedata_tests.js @@ -191,8 +191,14 @@ Tinytest.addAsync("mongo-livedata - basics, " + idGeneration, function (test, on test.equal(_.pluck(coll.find({run: run}, {sort: {x: -1}}).fetch(), "x"), [4, 1]); + expectObserve('', function () { + var count = coll.update({run: run, x: -1}, {$inc: {x: 2}}, {multi: true}); + test.equal(count, 0); + }); + expectObserve('c(3,0,1)c(6,1,4)', function () { - coll.update({run: run}, {$inc: {x: 2}}, {multi: true}); + var count = coll.update({run: run}, {$inc: {x: 2}}, {multi: true}); + test.equal(count, 2); test.equal(_.pluck(coll.find({run: run}, {sort: {x: -1}}).fetch(), "x"), [6, 3]); }); @@ -205,7 +211,8 @@ Tinytest.addAsync("mongo-livedata - basics, " + idGeneration, function (test, on }); expectObserve('r(13,1)', function () { - coll.remove({run: run, x: {$gt: 10}}); + var count = coll.remove({run: run, x: {$gt: 10}}); + test.equal(count, 1); test.equal(coll.find({run: run}).count(), 1); }); @@ -214,6 +221,12 @@ Tinytest.addAsync("mongo-livedata - basics, " + idGeneration, function (test, on test.equal(coll.find({run: run}).count(), 0); }); + expectObserve('', function () { + var count = coll.remove({run: run}); + test.equal(count, 0); + test.equal(coll.find({run: run}).count(), 0); + }); + obs.stop(); onComplete(); });