From 2918f79b2cb4f893823bc3401175a4787f3c2c59 Mon Sep 17 00:00:00 2001 From: Mohamed Yousef Date: Fri, 9 Aug 2024 21:07:58 +0300 Subject: [PATCH 1/2] fix populate called with lean when multiple documents with same foreignField exist --- lib/model.js | 4 ++-- test/model.populate.test.js | 48 +++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/model.js b/lib/model.js index b3497669be5..808a595811c 100644 --- a/lib/model.js +++ b/lib/model.js @@ -4459,7 +4459,7 @@ function _assign(model, vals, mod, assignmentOpts) { } } else { if (_val instanceof Document) { - _val = _val._doc._id; + _val = _val._id; } key = String(_val); if (rawDocs[key]) { @@ -4468,7 +4468,7 @@ function _assign(model, vals, mod, assignmentOpts) { rawOrder[key].push(i); } else if (isVirtual || rawDocs[key].constructor !== val.constructor || - String(rawDocs[key]._doc._id) !== String(val._doc._id)) { + 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 diff --git a/test/model.populate.test.js b/test/model.populate.test.js index 48427bf6ae4..bbdfadfe281 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -11048,4 +11048,52 @@ describe('model: populate:', function() { assert.equal(pet1.owner.name, 'Alice'); assert.equal(pet2.owner.name, 'Alice'); }); + + it('makes sure that populate works correctly with duplicate foreignField with lean(); (gh-14794)', async function() { + const authorSchema = new mongoose.Schema({ + group: String, + name: String + }); + + const postSchema = new mongoose.Schema({ + authorGroup: String, + title: String, + content: String + }); + + const Author = db.model('Author', authorSchema); + const Post = db.model('Post', postSchema); + + await Author.create({ group: 'AUTH1', name: 'John Doe' }); + await Author.create({ group: 'AUTH2', name: 'Jane Smith' }); + await Author.create({ group: 'AUTH2', name: 'Will Jons' }); + + await Post.create({ + authorGroup: 'AUTH1', + title: 'First Post', + content: 'Content 1' + }); + + await Post.create({ + authorGroup: 'AUTH2', + title: 'Second Post', + content: 'Content 2' + }); + + const posts = await Post.find() + .populate({ + path: 'authorGroup', + model: 'Author', + select: { _id: 1, name: 1 }, + foreignField: 'group' + }) + .select({ _id: 1, authorGroup: 1, title: 1 }) + .lean(); + + for (const post of posts) { + assert.ok(post.authorGroup._id instanceof mongoose.Types.ObjectId); + assert.ok(typeof post.authorGroup.name === 'string'); + } + assert.equal(posts.length, 2); + }); }); From 0cb96b92a2bf549c6e8233b68425622f260460bc Mon Sep 17 00:00:00 2001 From: Mohamed Yousef Date: Sun, 11 Aug 2024 02:32:43 +0300 Subject: [PATCH 2/2] check if rawDocs[key] and val are instance of Document --- lib/model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/model.js b/lib/model.js index 808a595811c..da841ae28f7 100644 --- a/lib/model.js +++ b/lib/model.js @@ -4459,7 +4459,7 @@ function _assign(model, vals, mod, assignmentOpts) { } } else { if (_val instanceof Document) { - _val = _val._id; + _val = _val._doc._id; } key = String(_val); if (rawDocs[key]) { @@ -4468,7 +4468,7 @@ function _assign(model, vals, mod, assignmentOpts) { rawOrder[key].push(i); } else if (isVirtual || rawDocs[key].constructor !== val.constructor || - String(rawDocs[key]._id) !== String(val._id)) { + (rawDocs[key] instanceof Document ? String(rawDocs[key]._doc._id) : String(rawDocs[key]._id)) !== (val instanceof Document ? String(val._doc._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