From 58bdfe024d5f0da2a26153638f56f85ff0cdb002 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Mon, 13 Nov 2017 15:53:11 -0500 Subject: [PATCH] Shallow-clone inserted documents more reliably. In a previous commit, I changed doc = _.extend({}, doc); to avoid using underscore, thus: doc = { ...doc }; While this may seem harmless, it broke a few Mongo.Collection tests because _.extend copies *all* properties, both own and inherited, whereas object ...spread only copies own properties. However, the correct way to fix this problem is *not* to revert to the old behavior, since flattening the inherited properties of a document was never actually what we wanted. The old behavior was subtly broken, too. Instead, we need to create a new object with the same prototoype as the provided document, then shallow-copy the own properties. Any properties or methods inherited from the original prototype will then be available on the new object, even though they didn't get copied over. I've intentionally left some trivial formatting changes in this commit to remind myself which broken tests were fixed by this change. --- packages/mongo/collection.js | 14 ++++++++++---- packages/mongo/mongo_livedata_tests.js | 12 +++++++++--- packages/mongo/package.js | 2 +- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/mongo/collection.js b/packages/mongo/collection.js index 6a1a6906a4..9793e4a57d 100644 --- a/packages/mongo/collection.js +++ b/packages/mongo/collection.js @@ -429,12 +429,18 @@ Mongo.Collection.prototype.insert = function insert(doc, callback) { throw new Error("insert requires an argument"); } - // Shallow-copy the document and possibly generate an ID - doc = _.extend({}, doc); + // Make a shallow clone of the document, preserving its prototype. + doc = Object.create( + Object.getPrototypeOf(doc), + Object.getOwnPropertyDescriptors(doc) + ); if ('_id' in doc) { - if (!doc._id || !(typeof doc._id === 'string' || doc._id instanceof Mongo.ObjectID)) { - throw new Error("Meteor requires document _id fields to be non-empty strings or ObjectIDs"); + if (! doc._id || + ! (typeof doc._id === 'string' || + doc._id instanceof Mongo.ObjectID)) { + throw new Error( + "Meteor requires document _id fields to be non-empty strings or ObjectIDs"); } } else { let generateId = true; diff --git a/packages/mongo/mongo_livedata_tests.js b/packages/mongo/mongo_livedata_tests.js index 932b98caca..98ef9955a2 100644 --- a/packages/mongo/mongo_livedata_tests.js +++ b/packages/mongo/mongo_livedata_tests.js @@ -1507,7 +1507,9 @@ testAsyncMulti('mongo-livedata - document with a custom type, ' + idGeneration, Meteor.call('createInsecureCollection', this.collectionName, collectionOptions); Meteor.subscribe('c-' + this.collectionName, expect()); } - }, function (test, expect) { + }, + + function (test, expect) { var self = this; self.coll = new Mongo.Collection(this.collectionName, collectionOptions); var docId; @@ -1525,13 +1527,17 @@ testAsyncMulti('mongo-livedata - document with a custom type, ' + idGeneration, test.isTrue(inColl); inColl && test.equal(inColl.d.speak(), "woof"); })); - }, function (test, expect) { + }, + + function (test, expect) { var self = this; self.coll.insert(new Dog("rover", "orange"), expect(function (err, id) { test.isTrue(err); test.isFalse(id); })); - }, function (test, expect) { + }, + + function (test, expect) { var self = this; self.coll.update( self.docId, new Dog("rover", "orange"), expect(function (err) { diff --git a/packages/mongo/package.js b/packages/mongo/package.js index 85c5a094fd..a765a8052f 100644 --- a/packages/mongo/package.js +++ b/packages/mongo/package.js @@ -9,7 +9,7 @@ Package.describe({ summary: "Adaptor for using MongoDB and Minimongo over DDP", - version: '1.2.2' + version: '1.2.3' }); Npm.depends({