From 17e9773dbd37b2eb6be6f8d8b8451def6328ac86 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 16 Feb 2021 11:27:34 -0500 Subject: [PATCH 1/5] test(populate): repro #9906 --- test/model.populate.test.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/model.populate.test.js b/test/model.populate.test.js index 97871f1ac92..779d801c713 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -9869,6 +9869,7 @@ describe('model: populate:', function() { assert.deepEqual(findCallOptions[0].virtuals, ['foo']); }); }); + it('gh-9833', function() { const Books = db.model('books', new Schema({ name: String, tags: [{ type: Schema.Types.ObjectId, ref: 'tags' }] })); const Tags = db.model('tags', new Schema({ author: Schema.Types.ObjectId })); @@ -9907,4 +9908,37 @@ describe('model: populate:', function() { assert.ok(!Array.isArray(populatedBooks[0].tags[0].author)); }); }); + + it('handles perDocumentLimit where multiple documents reference the same populated doc (gh-9906)', function() { + const postSchema = new Schema({ + title: String, + commentsIds: [{ type: Schema.ObjectId, ref: 'Comment' }] + }); + const Post = db.model('Post', postSchema); + + const commentSchema = new Schema({ content: String }); + const Comment = db.model('Comment', commentSchema); + + return co(function*() { + const commonComment = new Comment({ content: 'Im used in two posts' }); + yield commonComment.save(); + + const comments = yield Comment.create([ + { content: 'Nice first post' }, + { content: 'Nice second post' } + ]); + + let posts = yield Post.create([ + { title: 'First post', commentsIds: [commonComment, comments[0]] }, + { title: 'Second post', commentsIds: [commonComment, comments[1]] } + ]); + + posts = yield Post.find().populate({ path: 'commentsIds', perDocumentLimit: 2, sort: { content: 1 } }); + assert.equal(posts.length, 2); + assert.ok(!Array.isArray(posts[0].commentsIds[0])); + + assert.deepEqual(posts[0].toObject().commentsIds.map(c => c.content), ['Im used in two posts', 'Nice first post']); + assert.deepEqual(posts[1].toObject().commentsIds.map(c => c.content), ['Im used in two posts', 'Nice second post']); + }); + }); }); From 0757423a2ee15f4c6dad6adeb72c4bc35515c9a6 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 16 Feb 2021 11:27:46 -0500 Subject: [PATCH 2/5] fix(populate): handle `perDocumentLimit` when multiple documents reference the same populated doc Fix #9906 --- lib/model.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/model.js b/lib/model.js index 99002b330b3..82fd28c27cf 100644 --- a/lib/model.js +++ b/lib/model.js @@ -4592,7 +4592,12 @@ function _assign(model, vals, mod, assignmentOpts) { if (Array.isArray(rawDocs[key])) { rawDocs[key].push(val); rawOrder[key].push(i); - } else { + } else if (isVirtual || + rawDocs[key].constructor !== val.constructor || + String(rawDocs[key]._id) !== String(val._id)) { + // May need to store multiple docs with the same id if there's multiple models + // if we have discriminators or a ref function. But avoid converting to an array + // if we have multiple queries on the same model because of `perDocumentLimit` re: gh-9906 rawDocs[key] = [rawDocs[key], val]; rawOrder[key] = [rawOrder[key], i]; } From 18e5db1961ddcdd0f46081185c20f937b9c7337d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 16 Feb 2021 11:49:31 -0500 Subject: [PATCH 3/5] docs(connection): clarify that `Connection#transaction()` promise resolves to a command result Fix #9919 --- lib/connection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connection.js b/lib/connection.js index 185948e696b..83ff8f8c9a0 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -469,7 +469,7 @@ Connection.prototype.startSession = _wrapConnHelper(function startSession(option * @method transaction * @param {Function} fn Function to execute in a transaction * @param {mongodb.TransactionOptions} [options] Optional settings for the transaction - * @return {Promise} promise that resolves to the returned value of `fn` + * @return {Promise} promise that is fulfilled if Mongoose successfully committed the transaction, or rejects if the transaction was aborted or if Mongoose failed to commit the transaction. If fulfilled, the promise resolves to a MongoDB command result. * @api public */ From 881ee620ac43afdedd789f6493e71915666a6aaf Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 16 Feb 2021 12:24:19 -0500 Subject: [PATCH 4/5] test(populate): repro #9913 --- test/model.populate.test.js | 40 +++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/model.populate.test.js b/test/model.populate.test.js index 779d801c713..07639c5b104 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -9909,6 +9909,46 @@ describe('model: populate:', function() { }); }); + it('sets not-found values to null for paths that are not in the schema (gh-9913)', function() { + const Books = db.model('books', new Schema({ name: String, tags: [{ type: 'ObjectId', ref: 'tags' }] })); + const Tags = db.model('tags', new Schema({ authors: [{ author: 'ObjectId' }] })); + const Authors = db.model('authors', new Schema({ name: String })); + + return co(function*() { + const anAuthor = new Authors({ name: 'Author1' }); + yield anAuthor.save(); + + const aTag = new Tags({ authors: [{ author: anAuthor.id }, { author: new mongoose.Types.ObjectId() }] }); + yield aTag.save(); + + const aBook = new Books({ name: 'Book1', tags: [aTag.id] }); + yield aBook.save(); + + const aggregateOptions = [ + { $match: { + name: { $in: [aBook.name] } + } }, + { $lookup: { + from: 'tags', + localField: 'tags', + foreignField: '_id', + as: 'tags' + } } + ]; + const books = yield Books.aggregate(aggregateOptions).exec(); + + const populateOptions = [{ + path: 'tags.authors.author', + model: 'authors', + select: '_id name' + }]; + + const populatedBooks = yield Books.populate(books, populateOptions); + assert.strictEqual(populatedBooks[0].tags[0].authors[0].author.name, 'Author1'); + assert.strictEqual(populatedBooks[0].tags[0].authors[1].author, null); + }); + }); + it('handles perDocumentLimit where multiple documents reference the same populated doc (gh-9906)', function() { const postSchema = new Schema({ title: String, From aa7b529be017ec5c7f158d6bf0ed1b8e736d1343 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 16 Feb 2021 12:24:29 -0500 Subject: [PATCH 5/5] fix(populate): set not found values to `null` for paths that are not in the schema Fix #9913 --- lib/helpers/populate/assignVals.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/helpers/populate/assignVals.js b/lib/helpers/populate/assignVals.js index 9fd51d80967..a09b1007473 100644 --- a/lib/helpers/populate/assignVals.js +++ b/lib/helpers/populate/assignVals.js @@ -240,7 +240,7 @@ function valueFilter(val, assignmentOpts, populateOptions) { if (populateOptions.justOne === false) { return []; } - return val; + return val == null ? val : null; } /*!