From add4f6e0155faa31e1e2a67454a423c387fab709 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 6 Jan 2014 17:54:16 -0800 Subject: [PATCH] Disallow {fields:{_id:0}} in observeChanges This implies it is not allowed in `observe` either, or in cursors returned from publish functions, or in cursors used in {{#each}} Why? observeChanges and DDP publication use the ID as part of the callback/message, and eliding it completely breaks them. Meteor UI uses the ID with {{#each}} to properly move nodes around instead of re-rendering. We could try to allow it for `observe` outside of {{#each}}, but it would feel somewhat inconsistent. --- History.md | 4 +++ docs/client/api.html | 25 +++++++++++++------ packages/minimongo/minimongo.js | 3 +++ packages/minimongo/minimongo_tests.js | 8 ++++++ packages/mongo-livedata/mongo_driver.js | 8 ++++++ .../mongo-livedata/observe_changes_tests.js | 7 ++++++ 6 files changed, 48 insertions(+), 7 deletions(-) diff --git a/History.md b/History.md index 05d090bf21..1cbdd9428f 100644 --- a/History.md +++ b/History.md @@ -2,6 +2,10 @@ * Hash login tokens before storing them in the database. +* Cursors with a field specifier containing `{_id: 0}` can no longer be used + with `observeChanges` or `observe`. This includes the implicit calls to these + functions that are done when returning a cursor from a publish function or + using `{{#each}}`. ## v0.7.0.1 diff --git a/docs/client/api.html b/docs/client/api.html index 8a63b56bc9..8435a3952e 100644 --- a/docs/client/api.html +++ b/docs/client/api.html @@ -1319,21 +1319,32 @@ it's up to you to be sure. Queries can specify a particular set of fields to include or exclude from the result object. -To exclude certain fields from the result objects, the field specifier -is a dictionary whose keys are field names and whose values are `0`. +To exclude specific fields from the result objects, the field specifier is a +dictionary whose keys are field names and whose values are `0`. All unspecified +fields are included. // Users.find({}, {fields: {password: 0, hash: 0}}) -To return an object that only includes the specified field, use `1` as +To include only specific fields in the result documents, use `1` as the value. The `_id` field is still included in the result. // Users.find({}, {fields: {firstname: 1, lastname: 1}}) -It is not possible to mix inclusion and exclusion styles (except for the cases -when `_id` is included by default or explicitly excluded). Field operators such -as `$` and `$elemMatch` are not available on the client side yet. +With one exception, it is not possible to mix inclusion and exclusion styles: +the keys must either be all 1 or all 0. The exception is that you may specify +`_id: 0` in an inclusion specifier, which will leave `_id` out of the result +object as well. However, such field specifiers can not be used with +[`observeChanges`](#observe_changes), [`observe`](#observe), cursors returned +from a [publish function](#meteor_publish), or cursors used in +`{{dstache}}#each}}` in a template. They may be used with [`fetch`](#fetch), +[`findOne`](#findone), [`forEach`](#foreach), and [`map`](#map). -More advanced example: + +Field +operators such as `$` and `$elemMatch` are not available on the client side +yet. + +A more advanced example: Users.insert({ alterEgos: [{ name: "Kira", alliance: "murderer" }, { name: "L", alliance: "police" }], diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 460ef06a9d..aa5a8b60cd 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -270,6 +270,9 @@ _.extend(LocalCollection.Cursor.prototype, { if (!options._allow_unordered && !ordered && (self.skip || self.limit)) throw new Error("must use ordered observe with skip or limit"); + if (self.fields && (self.fields._id === 0 || self.fields._id === false)) + throw Error("You may not observe a cursor with {fields: {_id: 0}}"); + var query = { matcher: self.matcher, // not fast pathed sorter: ordered && self.sorter, diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 666362ffcc..cd726a5f52 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -1432,6 +1432,14 @@ Tinytest.add("minimongo - observe ordered with projection", function (test) { c.insert({_id: idA2, a:2}); test.equal(operations.shift(), undefined); + var cursor = c.find({}, {fields: {a: 1, _id: 0}}); + test.throws(function () { + cursor.observeChanges({added: function () {}}); + }); + test.throws(function () { + cursor.observe({added: function () {}}); + }); + // test initial inserts (and backwards sort) handle = c.find({}, {sort: {a: -1}, fields: { a: 1 } }).observe(cbs); test.equal(operations.shift(), ['added', {a:2}, 0, null]); diff --git a/packages/mongo-livedata/mongo_driver.js b/packages/mongo-livedata/mongo_driver.js index bdb9207571..2036f5cd16 100644 --- a/packages/mongo-livedata/mongo_driver.js +++ b/packages/mongo-livedata/mongo_driver.js @@ -957,6 +957,14 @@ MongoConnection.prototype._observeChanges = function ( return self._observeChangesTailable(cursorDescription, ordered, callbacks); } + // You may not filter out _id when observing changes, because the id is a core + // part of the observeChanges API. + if (cursorDescription.options.fields && + (cursorDescription.options.fields._id === 0 || + cursorDescription.options.fields._id === false)) { + throw Error("You may not observe a cursor with {fields: {_id: 0}}"); + } + var observeKey = JSON.stringify( _.extend({ordered: ordered}, cursorDescription)); diff --git a/packages/mongo-livedata/observe_changes_tests.js b/packages/mongo-livedata/observe_changes_tests.js index 1831718afc..db6d54d4c2 100644 --- a/packages/mongo-livedata/observe_changes_tests.js +++ b/packages/mongo-livedata/observe_changes_tests.js @@ -25,6 +25,7 @@ _.each ([{added:'added', forceOrdered: true}, function (logger) { var barid = c.insert({thing: "stuff"}); var fooid = c.insert({noodles: "good", bacon: "bad", apples: "ok"}); + var handle = c.find(fooid).observeChanges(logger); if (added === 'added') logger.expectResult(added, [fooid, {noodles: "good", bacon: "bad",apples: "ok"}]); @@ -43,6 +44,12 @@ _.each ([{added:'added', forceOrdered: true}, c.insert({noodles: "good", bacon: "bad", apples: "ok"}); logger.expectNoResult(); handle.stop(); + + var badCursor = c.find({}, {fields: {noodles: 1, _id: false}}); + test.throws(function () { + badCursor.observeChanges(logger); + }); + onComplete(); }); });