diff --git a/packages/livedata/livedata_client.js b/packages/livedata/livedata_client.js index 152f1a7581..73492cb941 100644 --- a/packages/livedata/livedata_client.js +++ b/packages/livedata/livedata_client.js @@ -100,12 +100,17 @@ if (typeof Meteor === "undefined") Meteor = {}; _name: name, _collection: new Collection(), - find: function (selector, options) { - return this._collection.find(selector, options); + find: function (/* selector, options */) { + // Collection.find() (return all docs) behaves differently + // from Collection.find(undefined) (return 0 docs). so be + // careful about preserving the length of arguments when + // descending into minimongo. + return this._collection.find.apply(this._collection, Array.prototype.slice.call(arguments)); }, - findOne: function (selector) { - return this._collection.findOne(selector); + findOne: function (/* selector, options */) { + // as above + return this._collection.findOne.apply(this._collection, Array.prototype.slice.call(arguments)); }, insert: function (obj) { @@ -125,9 +130,6 @@ if (typeof Meteor === "undefined") Meteor = {}; }, update: function (selector, mutator, options) { - if (typeof(selector) === "string") - selector = {_id: selector}; - if (this._name) Meteor._stream.emit('handle', { collection: this._name, type: 'update', @@ -136,9 +138,6 @@ if (typeof Meteor === "undefined") Meteor = {}; }, remove: function (selector) { - if (typeof(selector) === "string") - selector = {_id: selector}; - if (this._name) Meteor._stream.emit('handle', { collection: this._name, type: 'remove', selector: selector}); diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js index 23b324081d..0b42d9a3e8 100644 --- a/packages/livedata/livedata_server.js +++ b/packages/livedata/livedata_server.js @@ -256,11 +256,16 @@ if (typeof Meteor === "undefined") Meteor = {}; // kill it. find: function (selector, options) { - var cursor = new Meteor._mongo_driver.Cursor(this._name, selector, options); - return cursor; + if (arguments.length === 0) + selector = {}; + + return new Meteor._mongo_driver.Cursor(this._name, selector, options); }, findOne: function (selector, options) { + if (arguments.length === 0) + selector = {}; + options = options || {}; options.limit = 1; return this.find(selector, options).fetch()[0]; @@ -310,11 +315,4 @@ if (typeof Meteor === "undefined") Meteor = {}; return ret; }; - - Meteor.Collection.Query = function (collection, selector, options) { - if (!options) options = {}; - - this.cursor = Meteor._mongo_driver.find(this._name, selector, options); - }; - })(); diff --git a/packages/livedata/mongo_driver.js b/packages/livedata/mongo_driver.js index 3d566d18c3..15286800e7 100644 --- a/packages/livedata/mongo_driver.js +++ b/packages/livedata/mongo_driver.js @@ -46,6 +46,21 @@ } } + // protect against dangerous selectors. falsey and {_id: falsey} + // are both likely programmer error, and not what you want, + // particularly for destructive operations. + function rewrite_selector (selector) { + // shorthand -- scalars match _id + if ((typeof selector === 'string') || (typeof selector === 'number')) + selector = {_id: selector}; + + if (!selector || (('_id' in selector) && !selector._id)) + // can't match anything + return {_id: Meteor.uuid()}; + else + return selector; + } + //////////// Public API ////////// function Cursor (collection_name, selector, options) { @@ -53,15 +68,7 @@ withCollection(collection_name, function(err, collection) { // XXX err handling - var single_result = false; - // if single id is passed - // XXX deal with both string and objectid - if (typeof selector === 'string') { - selector = {_id: selector}; - single_result = true; - } else if (!selector) { - selector = {}; - } + selector = rewrite_selector(selector); var cursor = collection.find(selector); // XXX is there a way to do this as for x in ['sort', 'limit', 'skip']? @@ -150,10 +157,8 @@ // needed, really. // XXX does not allow options. matches the client. - // XXX consider allowing string -> {_id: id} shortcut. - if (typeof(selector) === "string") - selector = {_id: selector}; + selector = rewrite_selector(selector); withCollection(collection_name, function(err, collection) { // XXX err handling @@ -172,8 +177,7 @@ // I couldn't convince myself it was safe not to. Not sure if it is // needed, really. - if (typeof(selector) === "string") - selector = {_id: selector}; + selector = rewrite_selector(selector); if (!options) options = {}; // Default to multi. This is the oppposite of mongo. We'll see how it goes. diff --git a/packages/liveui/liveui.js b/packages/liveui/liveui.js index de2eb7c6d4..0f8aa87d3e 100644 --- a/packages/liveui/liveui.js +++ b/packages/liveui/liveui.js @@ -169,7 +169,7 @@ Meteor.ui.renderList = function (query, options) { // protect against old invocations passing in a Collection or a // LiveResultsSet. // XXX remove in a few releases - if (!(query instanceof Collection.Query)) + if (!(query instanceof Collection.Cursor)) throw new Error("insert_before: at least one entry must exist"); // create the top-level document fragment/range that will be diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index c60e774a72..3445a23c58 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -1,14 +1,11 @@ // XXX indexes // XXX type checking on selectors (graceful error if malformed) -// XXX merge ad-hoc live query object and Query - -// XXX keep observe()s running after they're killed, because usually -// the dependency tracker is going to spin them up again right away. +// XXX merge ad-hoc live query object and Cursor // Collection: a set of documents that supports queries and modifiers. -// Query: a specification for a particular subset of documents, w/ -// a defined order, limit, and offset. creating a Query with Collection.find(), +// Cursor: a specification for a particular subset of documents, w/ +// a defined order, limit, and offset. creating a Cursor with Collection.find(), // LiveResultsSet: the return value of a live query. @@ -40,19 +37,25 @@ Collection = function () { // XXX add one more sort form: "key" // XXX tests Collection.prototype.find = function (selector, options) { - return new Collection.Query(this, selector, options); + // default syntax for everything is to omit the selector argument. + // but if selector is explicitly passed in as false or undefined, we + // want a selector that matches nothing. + if (arguments.length === 0) + selector = {}; + + return new Collection.Cursor(this, selector, options); }; // don't call this ctor directly. use Collection.find(). -Collection.Query = function (collection, selector, options) { +Collection.Cursor = function (collection, selector, options) { if (!options) options = {}; this.collection = collection; - if (typeof selector === "string") { + if ((typeof selector === "string") || (typeof selector === "number")) { // stash for fast path this.selector_id = selector; - this.selector_f = Collection._compileSelector({_id: selector}); + this.selector_f = Collection._compileSelector(selector); } else { this.selector_f = Collection._compileSelector(selector); this.sort_f = options.sort ? Collection._compileSort(options.sort) : null; @@ -68,19 +71,22 @@ Collection.Query = function (collection, selector, options) { this.reactive = (options.reactive === undefined) ? true : options.reactive; }; -Collection.Query.prototype.rewind = function () { +Collection.Cursor.prototype.rewind = function () { var self = this; self.db_objects = null; self.cursor_pos = 0; }; Collection.prototype.findOne = function (selector, options) { + if (arguments.length === 0) + selector = {}; + options = options || {}; options.limit = 1; return this.find(selector, options).fetch()[0]; }; -Collection.Query.prototype.forEach = function (callback) { +Collection.Cursor.prototype.forEach = function (callback) { var self = this; var doc; @@ -97,7 +103,7 @@ Collection.Query.prototype.forEach = function (callback) { callback(Collection._deepcopy(self.db_objects[self.cursor_pos++])); }; -Collection.Query.prototype.map = function (callback) { +Collection.Cursor.prototype.map = function (callback) { var self = this; var res = []; self.forEach(function (doc) { @@ -106,7 +112,7 @@ Collection.Query.prototype.map = function (callback) { return res; }; -Collection.Query.prototype.fetch = function () { +Collection.Cursor.prototype.fetch = function () { var self = this; var res = []; self.forEach(function (doc) { @@ -115,7 +121,7 @@ Collection.Query.prototype.fetch = function () { return res; }; -Collection.Query.prototype.count = function () { +Collection.Cursor.prototype.count = function () { var self = this; if (self.reactive) @@ -130,7 +136,31 @@ Collection.Query.prototype.count = function () { // the handle that comes back from observe. Collection.LiveResultsSet = function () {}; -Collection.Query.prototype.observe = function (options) { +// options to contain: +// * callbacks: +// - added (object, before_index) +// - changed (new_object, at_index) +// - moved (object, old_index, new_index) - can only fire with changed() +// - removed (id, at_index) +// * sort: sort descriptor +// +// attributes available on returned query handle: +// * stop(): end updates +// * indexOf(id): return current index of object in result set, or -1 +// * collection: the collection this query is querying +// +// iff x is a returned query handle, (x instanceof +// Collection.LiveResultsSet) is true +// +// 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 +// XXX it'd be helpful if removed got the object that just left the +// query, not just its id +// XXX document that initial results will definitely be delivered before we return [do, add to asana] + +Collection.Cursor.prototype.observe = function (options) { var self = this; if (self.skip || self.limit) @@ -138,7 +168,7 @@ Collection.Query.prototype.observe = function (options) { var qid = self.collection.next_qid++; - // XXX merge this object w/ "this" Query. they're the same. + // XXX merge this object w/ "this" Cursor. they're the same. var query = self.collection.queries[qid] = { selector_f: self.selector_f, // not fast pathed sort_f: self.sort_f, @@ -167,11 +197,11 @@ Collection.Query.prototype.observe = function (options) { // constructs sorted array of matching objects, but doesn't copy them. // respects sort, skip, and limit properties of the query. // if sort_f is falsey, no sort -- you get the natural order -Collection.Query.prototype._getRawObjects = function () { +Collection.Cursor.prototype._getRawObjects = function () { var self = this; // fast path for single ID value - if (self.selector_id) + if (self.selector_id && (self.selector_id in self.collection.docs)) return [self.collection.docs[self.selector_id]]; // slow path for arbitrary selector, sort, skip, limit @@ -190,7 +220,7 @@ Collection.Query.prototype._getRawObjects = function () { return results.slice(idx_start, idx_end); }; -Collection.Query.prototype._markAsReactive = function (options) { +Collection.Cursor.prototype._markAsReactive = function (options) { var self = this; var context = Meteor.deps.Context.current; @@ -233,36 +263,11 @@ Collection.prototype.insert = function (doc) { } }; -// options to contain: -// * callbacks: -// - added (object, before_index) -// - changed (new_object, at_index) -// - moved (object, old_index, new_index) - can only fire with changed() -// - removed (id, at_index) -// * sort: sort descriptor -// -// attributes available on returned query handle: -// * stop(): end updates -// * indexOf(id): return current index of object in result set, or -1 -// * collection: the collection this query is querying -// -// iff x is a returned query handle, (x instanceof -// Collection.LiveResultsSet) is true -// -// 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 -// XXX it'd be helpful if removed got the object that just left the -// query, not just its id -// XXX document that initial results will definitely be delivered before we return [do, add to asana] - Collection.prototype.remove = function (selector) { var self = this; var remove = []; var query_remove = []; - if (typeof(selector) === "string") - selector = {_id: selector}; + var selector_f = Collection._compileSelector(selector); for (var id in self.docs) { var doc = self.docs[id]; @@ -288,13 +293,10 @@ Collection.prototype.remove = function (selector) { // XXX atomicity: if multi is true, and one modification fails, do // we rollback the whole operation, or what? Collection.prototype.update = function (selector, mod, options) { - if (typeof(selector) === "string") - selector = {_id: selector}; - if (!options) options = {}; // Default to multi. This is the oppposite of mongo. We'll see how it goes. if (typeof(options.multi) === "undefined") - options.multi = true + options.multi = true; var self = this; var any = false; diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 94ec518e85..fb28eeab84 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -65,7 +65,17 @@ test("minimongo - basics", function () { assert.length(c.find({type: "kitten"}).fetch(), 1); assert.length(c.find({type: "cryptographer"}).fetch(), 3); - c.remove({}); + c.remove(null); + c.remove(false); + c.remove(undefined); + assert.equal(c.find().count(), 4); + + c.remove({_id: null}); + c.remove({_id: false}); + c.remove({_id: undefined}); + assert.equal(c.find().count(), 4); + + c.remove(); assert.equal(0, c.find().count()); c.insert({_id: 1, name: "strawberry", tags: ["fruit", "red", "squishy"]}); @@ -79,6 +89,19 @@ test("minimongo - basics", function () { assert.length(c.find({tags: "fruit"}).fetch(), 2); assert.length(c.find({tags: "red"}).fetch(), 3); + assert.equal(c.findOne(1).name, "strawberry"); + assert.equal(c.findOne(2).name, "apple"); + assert.equal(c.findOne(3).name, "rose"); + assert.equal(c.findOne(4), undefined); + assert.equal(c.findOne("abc"), undefined); + assert.equal(c.findOne(undefined), undefined); + + assert.equal(c.find(1).count(), 1); + assert.equal(c.find(4).count(), 0); + assert.equal(c.find("abc").count(), 0); + assert.equal(c.find(undefined).count(), 0); + assert.equal(c.find().count(), 3); + var ev = ""; var makecb = function (tag) { return { @@ -196,6 +219,21 @@ test("minimongo - selector_compiler", function () { match({}, {}); match({}, {a: 12}); + // scalars + match(1, {_id: 1, a: 'foo'}); + nomatch(1, {_id: 2, a: 'foo'}); + match('a', {_id: 'a', a: 'foo'}); + nomatch('a', {_id: 'b', a: 'foo'}); + + // safety + nomatch(undefined, {}); + nomatch(undefined, {_id: 'foo'}); + nomatch(false, {_id: 'foo'}); + nomatch(null, {_id: 'foo'}); + nomatch({_id: undefined}, {_id: 'foo'}); + nomatch({_id: false}, {_id: 'foo'}); + nomatch({_id: null}, {_id: 'foo'}); + // matching one or more keys nomatch({a: 12}, {}); match({a: 12}, {a: 12}); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index f759024a80..47d421c74e 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -259,7 +259,17 @@ Collection._compileSelector = function (selector) { var literals = []; // you can pass a literal function instead of a selector if (selector instanceof Function) - return function (doc) {return selector.call(doc);} + return function (doc) {return selector.call(doc);}; + + // shorthand -- scalars match _id + if ((typeof(selector) === "string") || (typeof(selector) === "number")) + selector = {_id: selector}; + + // protect against dangerous selectors. falsey and {_id: falsey} + // are both likely programmer error, and not what you want, + // particularly for destructive operations. + if (!selector || (('_id' in selector) && !selector._id)) + return function (doc) {return false;}; // eval() does not return a value in IE8, nor does the spec say it // should. Assign to a local to get the value, instead. diff --git a/packages/templating/deftemplate.js b/packages/templating/deftemplate.js index 8e4b40414a..aa971d0fe2 100644 --- a/packages/templating/deftemplate.js +++ b/packages/templating/deftemplate.js @@ -25,7 +25,7 @@ Meteor._hook_handlebars_each = function () { var orig = Handlebars._default_helpers.each; Handlebars._default_helpers.each = function (context, options) { - if (!(context instanceof Collection.Query)) + if (!(context instanceof Collection.Cursor)) return orig(context, options); var id = Meteor._pending_partials_idx_nonce++;