diff --git a/packages/mongo-livedata/allow_tests.js b/packages/mongo-livedata/allow_tests.js index bc1bd941f39..6d297388588 100644 --- a/packages/mongo-livedata/allow_tests.js +++ b/packages/mongo-livedata/allow_tests.js @@ -335,7 +335,7 @@ if (Meteor.isClient) { }, function (test, expect) { test.equal( - restrictedCollectionWithTransform.findOne({"a.bar": "bar"}), + _.omit(restrictedCollectionWithTransform.findOne({"a.bar": "bar"}), '_id'), {foo: "foo", bar: "bar", baz: "baz"}); restrictedCollectionWithTransform.remove(item1, expect(function (e, res) { test.isFalse(e); diff --git a/packages/mongo-livedata/collection.js b/packages/mongo-livedata/collection.js index 3ce91b36c85..6839a6e3f24 100644 --- a/packages/mongo-livedata/collection.js +++ b/packages/mongo-livedata/collection.js @@ -38,10 +38,11 @@ Meteor.Collection = function (name, options) { break; } - if (options.transform) - self._transform = Deps._makeNonreactive(options.transform); - else + if (options.transform) { + self._transform = Deps._makeNonreactive(transformTransform(options.transform)); + } else { self._transform = null; + } if (!name && (name !== null)) { Meteor._debug("Warning: creating anonymous collection. It will not be " + @@ -184,6 +185,39 @@ Meteor.Collection = function (name, options) { } }; +// Transform a transform function to return objects that have the +// original _id field. Used by transform functions passed into `new +// Meteor.Collection`. This ensures that subsystems such as the +// observe-sequence package that call `observe` can keep track of the +// documents identities. +// +// - Require that it returns objects +// - If the return value has an _id field, verify that it matches the +// original _id field +// - If the return value doesn't have an _id field, add it back. +var transformTransform = function(transform) { + return function (doc) { + var id = doc._id; + var transformed = transform(doc); + if (typeof transformed !== 'object' || + // Even though fine technically, don't let Mongo ObjectIDs + // through. It would suck to think your app works until + // you insert the first document using Meteor. + transformed instanceof Meteor.Collection.ObjectID) { + throw new Error("transform must return object"); + } + + if (transformed._id) { + if (transformed._id !== id) { + throw new Error("transformed document can't have different _id"); + } + } else { + transformed._id = id; + } + return transformed; + }; +}; + /// /// Main collection API /// diff --git a/packages/mongo-livedata/mongo_livedata_tests.js b/packages/mongo-livedata/mongo_livedata_tests.js index aee96704471..92daefade1d 100644 --- a/packages/mongo-livedata/mongo_livedata_tests.js +++ b/packages/mongo-livedata/mongo_livedata_tests.js @@ -789,6 +789,36 @@ testAsyncMulti('mongo-livedata - document with a date, ' + idGeneration, [ } ]); +testAsyncMulti('mongo-livedata - transform must return an object, ' + idGeneration, [ + function (test, expect) { + var self = this; + var justId = function (doc) { + return doc._id; + }; + TRANSFORMS["justId"] = justId; + var collectionOptions = { + idGeneration: idGeneration, + transform: justId, + transformName: "justId" + }; + var collectionName = Random.id(); + if (Meteor.isClient) { + Meteor.call('createInsecureCollection', collectionName, collectionOptions); + Meteor.subscribe('c-' + collectionName); + } + self.coll = new Meteor.Collection(collectionName, collectionOptions); + self.coll.insert({}, expect(function (err, id) { + test.isFalse(err); + test.isTrue(id); + test.throws(function () { + self.coll.findOne(); + }); + // you can still override the transform though. + test.equal(self.coll.findOne({}, {transform: null})._id, id); + })); + } +]); + testAsyncMulti('mongo-livedata - document goes through a transform, ' + idGeneration, [ function (test, expect) { var self = this; @@ -847,15 +877,63 @@ testAsyncMulti('mongo-livedata - document goes through a transform, ' + idGenera test.isTrue(id); self.id2 = id; })); - }, + } +]); + +testAsyncMulti('mongo-livedata - transformed object can\'t have conflicting _id, ' + idGeneration, [ + function (test, expect) { + var self = this; + var justId = function (doc) { + doc._id = "foo"; + return doc; + }; + TRANSFORMS["justId"] = justId; + var collectionOptions = { + idGeneration: idGeneration, + transform: justId, + transformName: "justId" + }; + var collectionName = Random.id(); + if (Meteor.isClient) { + Meteor.call('createInsecureCollection', collectionName, collectionOptions); + Meteor.subscribe('c-' + collectionName); + } + self.coll = new Meteor.Collection(collectionName, collectionOptions); + self.coll.insert({}, expect(function (err, id) { + test.isFalse(err); + test.isTrue(id); + test.throws(function () { + self.coll.findOne(); + }); + // you can still override the transform though. + test.equal(self.coll.findOne({}, {transform: null})._id, id); + })); + } +]); + +testAsyncMulti('mongo-livedata - transform sets _id if not present, ' + idGeneration, [ function (test, expect) { var self = this; - // Test that a transform that returns something other than a document with - // an _id (eg, a number) works. Regression test for #974. - test.equal(self.coll.find({}, { - transform: function (doc) { return doc.d.getSeconds(); }, - sort: {d: 1} - }).fetch(), [50, 51]); + var justId = function (doc) { + return _.omit(doc, '_id'); + }; + TRANSFORMS["justId"] = justId; + var collectionOptions = { + idGeneration: idGeneration, + transform: justId, + transformName: "justId" + }; + var collectionName = Random.id(); + if (Meteor.isClient) { + Meteor.call('createInsecureCollection', collectionName, collectionOptions); + Meteor.subscribe('c-' + collectionName); + } + self.coll = new Meteor.Collection(collectionName, collectionOptions); + self.coll.insert({}, expect(function (err, id) { + test.isFalse(err); + test.isTrue(id); + test.equal(self.coll.findOne()._id, id); + })); } ]);