From 6494ec563fba223c3f168019d9fd459600e511fc Mon Sep 17 00:00:00 2001 From: Emily Stark Date: Fri, 27 Sep 2013 18:50:39 -0700 Subject: [PATCH] Put .upsert() on LocalCollection and mongo driver. This way LocalCollection and Meteor.Collection have the same API. --- packages/minimongo/minimongo.js | 24 ++++++-- packages/minimongo/minimongo_tests.js | 4 +- packages/mongo-livedata/collection.js | 48 +++++----------- packages/mongo-livedata/mongo_driver.js | 55 +++++++++++++------ .../mongo-livedata/mongo_livedata_tests.js | 1 - .../remote_collection_driver.js | 2 +- 6 files changed, 75 insertions(+), 59 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 95941a7fdc..b78d873b3d 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -599,11 +599,17 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { updateCount = 1; } - var result = { - numberAffected: updateCount - }; - if (insertedId !== undefined) - result.insertedId = insertedId; + var result; + if (options.returnObject) { + result = { + numberAffected: updateCount + }; + if (insertedId !== undefined) + result.insertedId = insertedId; + } else { + result = updateCount; + } + if (callback) Meteor.defer(function () { callback(null, result); @@ -611,6 +617,14 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { return result; }; +LocalCollection.prototype.upsert = function (selector, mod, options, callback) { + var self = this; + return self.update(selector, mod, _.extend({}, options, { + upsert: true, + returnObject: true + }, callback)); +}; + LocalCollection.prototype._modifyAndNotify = function ( doc, mod, recomputeQids) { var self = this; diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 8df21830bf..343e7f198c 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -89,7 +89,7 @@ Tinytest.add("minimongo - basics", function (test) { test.length(c.find({type: "cryptographer"}).fetch(), 2); count = c.update({name: "snookums"}, {$set: {type: "cryptographer"}}); - test.equal(count, { numberAffected: 1 }); + 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); @@ -2016,7 +2016,7 @@ Tinytest.add("minimongo - saveOriginals", function (test) { c.update('bar', {$set: {k: 7}}); // update same doc twice // Verify returned count is correct - test.equal(count, { numberAffected: 2 }); + test.equal(count, 2); // Verify the originals. var originals = c.retrieveOriginals(); diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js index 32cbca3dd5..7f95b7c5df 100644 --- a/packages/mongo-livedata/collection.js +++ b/packages/mongo-livedata/collection.js @@ -343,23 +343,15 @@ _.each(["insert", "update", "remove", "upsert"], function (name) { var args = _.toArray(arguments); var callback; var ret; - var methodName = name; - // Calling `Collection.upsert()` is just like calling `Collection.update()` - // with upsert: true, except that we return the whole object with - // `numberAffected` and `idInserted` keys. So we do the same thing as an - // update, except that we save `isUpsert` to determine what to return when - // we're done. - var isUpsert = false; - if (methodName === "upsert") { - isUpsert = true; - methodName = "update"; - } + var isUpdateOrUpsert = (name === "update" || name === "upsert"); + var isUpsert = (name === "update" && options && options.upsert) || + (name === "upsert"); if (args.length && args[args.length - 1] instanceof Function) callback = args.pop(); - if (methodName === "insert") { + if (name === "insert") { if (!args.length) throw new Error("insert requires an argument"); // shallow-copy the document and generate an ID @@ -375,12 +367,10 @@ _.each(["insert", "update", "remove", "upsert"], function (name) { } else { args[0] = Meteor.Collection._rewriteSelector(args[0]); - if (methodName === "update") { + if (isUpdateOrUpsert) { // Mutate args but copy the original options object. var options = args[2] = _.clone(args[2]) || {}; - if (isUpsert) - options.upsert = true; - if (options && options.upsert) { + if (isUpsert) { // set `insertedId` if absent. `insertedId` is a Meteor extension. if (options.insertedId) { if (!(typeof options.insertedId === 'string' @@ -393,21 +383,13 @@ _.each(["insert", "update", "remove", "upsert"], function (name) { } } - // On inserts, always return the id that we generated. On updates and - // removes, return the number of documents we affected. On upsert(), return - // the whole object that the collection returns on update (with the number - // affected and insertedId). + // On inserts, always return the id that we generated; on all other + // operations, just return the result from the collection. var transformResultFromCollection = function (result) { - if (methodName === "insert") { + if (name === "insert") return ret; - } else if (methodName === "update" && ! isUpsert) { - if (result) - return result.numberAffected; - else - return undefined; - } else { + else return result; - } }; var wrappedCallback; @@ -429,20 +411,20 @@ _.each(["insert", "update", "remove", "upsert"], function (name) { // down. wrappedCallback = function (err) { if (err) - Meteor._debug(methodName + " failed: " + (err.reason || err.stack)); + Meteor._debug(name + " failed: " + (err.reason || err.stack)); }; } var enclosing = DDP._CurrentInvocation.get(); var alreadyInSimulation = enclosing && enclosing.isSimulation; - if (!alreadyInSimulation && methodName !== "insert") { + if (!alreadyInSimulation && name !== "insert") { // If we're about to actually send an RPC, we should throw an error if // this is a non-ID selector, because the mutation methods only allow // single-ID selectors. (If we don't throw here, we'll see flicker.) - throwIfSelectorIsNotId(args[0], methodName); + throwIfSelectorIsNotId(args[0], name); } - self._connection.apply(self._prefix + methodName, args, wrappedCallback); + self._connection.apply(self._prefix + name, args, wrappedCallback); } else { // it's my collection. descend into the collection object @@ -450,7 +432,7 @@ _.each(["insert", "update", "remove", "upsert"], function (name) { args.push(wrappedCallback); try { // If the user provided a callback, then we expect queryRet to be undefined. - var queryRet = self._collection[methodName].apply(self._collection, args); + var queryRet = self._collection[name].apply(self._collection, args); ret = transformResultFromCollection(queryRet); } catch (e) { if (callback) { diff --git a/packages/mongo-livedata/mongo_driver.js b/packages/mongo-livedata/mongo_driver.js index 149a0fc1bc..e680b1cd79 100644 --- a/packages/mongo-livedata/mongo_driver.js +++ b/packages/mongo-livedata/mongo_driver.js @@ -257,16 +257,6 @@ MongoConnection.prototype._refresh = function (collectionName, selector) { } }; -var numberAffectedCallback = function (callback) { - return Meteor.bindEnvironment(function (err, numberAffected) { - callback && callback(err, ! err && { - numberAffected: numberAffected - }); - }, function (err) { - Meteor._debug("Error in Mongo write:", err.stack); - }); -}; - MongoConnection.prototype._remove = function (collection_name, selector, callback) { var self = this; @@ -346,10 +336,20 @@ MongoConnection.prototype._update = function (collection_name, selector, mod, options.insertedId) { mongoOpts.insertedId = options.insertedId; simulateUpsertWithInsertedId(collection, mongoSelector, mongoMod, - isModify, mongoOpts, callback); + isModify, mongoOpts, function (err, result) { + // If we got here via a upsert() call, then + // we should return the whole + // object. Otherwise, we should just return + // the number of affected docs to match the + // mongo API. + if (result && ! options.returnObject) + callback(err, result.numberAffected); + else + callback(err, result); + }); } else { - collection.update(mongoSelector, mongoMod, mongoOpts, - numberAffectedCallback(callback)); + // For non-upserts, just return the number of affected documents. + collection.update(mongoSelector, mongoMod, mongoOpts, callback); } } catch (e) { write.committed(); @@ -364,13 +364,20 @@ var isModificationMod = function (mod) { return false; }; +// Assumes callback has already been wrapped with bindEnvironment. +var numberAffectedCallback = function (callback) { + return function (err, result) { + callback(err, ! err && { numberAffected: result }); + }; +}; + var NUM_OPTIMISTIC_TRIES = 3; var simulateUpsertWithInsertedId = function (collection, selector, mod, isModify, options, callback) { var insertedId = options.insertedId; // must exist - var mongoOptsForUpdate = _.extend({}, options); + var mongoOptsForUpdate = _.extend({}, options, { returnObject: true }); delete mongoOptsForUpdate.insertedId; delete mongoOptsForUpdate.upsert; @@ -426,7 +433,7 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod, newDoc = mod; } - var mongoOptsForInsert = _.extend({}, options); + var mongoOptsForInsert = _.extend({}, options, { returnObject: true }); delete mongoOptsForUpdate.insertedId; mongoOptsForInsert.upsert = true; delete mongoOptsForInsert.multi; @@ -444,8 +451,7 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod, Meteor._debug(err); callback(err); } else { - callback(null, _.extend(result, - { insertedId: insertedId })); + callback(null, _.extend(result, { insertedId: insertedId })); } })); }; @@ -460,6 +466,21 @@ _.each(["insert", "update", "remove"], function (method) { }; }); +MongoConnection.prototype.upsert = function (collectionName, selector, mod, + options, callback) { + var self = this; + if (typeof options === "function" && ! callback) { + callback = options; + options = {}; + } + + return self.update(collectionName, selector, mod, + _.extend({}, options, { + upsert: true, + returnObject: true + }, callback)); +}; + MongoConnection.prototype.find = function (collectionName, selector, options) { var self = this; diff --git a/packages/mongo-livedata/mongo_livedata_tests.js b/packages/mongo-livedata/mongo_livedata_tests.js index ae44683cd4..e473b1cc0c 100644 --- a/packages/mongo-livedata/mongo_livedata_tests.js +++ b/packages/mongo-livedata/mongo_livedata_tests.js @@ -50,7 +50,6 @@ EJSON.addType("dog", function (o) { return new Dog(o.name, o.color, o.actions);} // Parameterize tests. _.each( ['STRING', 'MONGO'], function(idGeneration) { - var collectionOptions = { idGeneration: idGeneration}; testAsyncMulti("mongo-livedata - database error reporting. " + idGeneration, [ diff --git a/packages/mongo-livedata/remote_collection_driver.js b/packages/mongo-livedata/remote_collection_driver.js index 9c155ca698..b32f82b0d3 100644 --- a/packages/mongo-livedata/remote_collection_driver.js +++ b/packages/mongo-livedata/remote_collection_driver.js @@ -9,7 +9,7 @@ _.extend(MongoInternals.RemoteCollectionDriver.prototype, { var ret = {}; _.each( ['find', 'findOne', 'insert', 'update', 'remove', '_ensureIndex', - '_dropIndex', '_createCappedCollection'], + '_dropIndex', '_createCappedCollection', 'upsert'], function (m) { ret[m] = _.bind(self.mongo[m], self.mongo, name); });