From 965b7d596d3c62e1a6917dcc29bbd748e404dfed Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 1 Dec 2023 14:53:22 -0500 Subject: [PATCH 1/2] fix(populate): call `transform` object with single id instead of array when populating a justOne path under an array Fix #14073 --- lib/helpers/populate/assignVals.js | 2 ++ test/model.populate.test.js | 55 ++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/lib/helpers/populate/assignVals.js b/lib/helpers/populate/assignVals.js index 017066add17..858846b42c0 100644 --- a/lib/helpers/populate/assignVals.js +++ b/lib/helpers/populate/assignVals.js @@ -80,6 +80,8 @@ module.exports = function assignVals(o) { return valueFilter(val[0], options, populateOptions, _allIds); } else if (o.justOne === false && !Array.isArray(val)) { return valueFilter([val], options, populateOptions, _allIds); + } else if (o.justOne === true && !Array.isArray(val) && Array.isArray(_allIds)) { + return valueFilter(val, options, populateOptions, val == null ? val : _allIds[o.rawOrder[val._id]]); } return valueFilter(val, options, populateOptions, _allIds); } diff --git a/test/model.populate.test.js b/test/model.populate.test.js index 4e638a90178..4072a2a081a 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -10776,4 +10776,59 @@ describe('model: populate:', function() { new Date('2015-06-01').toString() ); }); + + it('calls transform with single ObjectId when populating justOne path underneath array (gh-14073)', async function() { + const mySchema = mongoose.Schema({ + name: { type: String }, + items: [{ + _id: false, + name: { type: String }, + brand: { type: mongoose.Schema.Types.ObjectId, ref: 'Brand' } + }] + }); + + const brandSchema = mongoose.Schema({ + name: 'String', + quantity: Number + }); + + const myModel = db.model('MyModel', mySchema); + const brandModel = db.model('Brand', brandSchema); + const { _id: id1 } = await brandModel.create({ + name: 'test', + quantity: 1 + }); + const { _id: id2 } = await brandModel.create({ + name: 'test1', + quantity: 1 + }); + const { _id: id3 } = await brandModel.create({ + name: 'test2', + quantity: 2 + }); + const brands = await brandModel.find(); + const test = new myModel({ name: 'Test Model' }); + for (let i = 0; i < brands.length; i++) { + test.items.push({ name: `${i}`, brand: brands[i]._id }); + } + await test.save(); + + const ids = []; + await myModel + .findOne() + .populate([ + { + path: 'items.brand', + transform: (doc, id) => { + ids.push(id); + return doc; + } + } + ]); + assert.equal(ids.length, 3); + assert.deepStrictEqual( + ids.map(id => id.toHexString()), + [id1.toString(), id2.toString(), id3.toString()] + ); + }); }); From 0282f50b5fe696ca9ae869c07f3cec41f2b40cb6 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 1 Dec 2023 15:13:07 -0500 Subject: [PATCH 2/2] fix(populate): make sure to call `transform` with the correct index, even if no document found Re: #14073 --- lib/helpers/populate/assignVals.js | 5 ++++- test/model.populate.test.js | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/helpers/populate/assignVals.js b/lib/helpers/populate/assignVals.js index 858846b42c0..44de64fbc39 100644 --- a/lib/helpers/populate/assignVals.js +++ b/lib/helpers/populate/assignVals.js @@ -39,8 +39,10 @@ module.exports = function assignVals(o) { const options = o.options; const count = o.count && o.isVirtual; let i; + let setValueIndex = 0; function setValue(val) { + ++setValueIndex; if (count) { return val; } @@ -81,12 +83,13 @@ module.exports = function assignVals(o) { } else if (o.justOne === false && !Array.isArray(val)) { return valueFilter([val], options, populateOptions, _allIds); } else if (o.justOne === true && !Array.isArray(val) && Array.isArray(_allIds)) { - return valueFilter(val, options, populateOptions, val == null ? val : _allIds[o.rawOrder[val._id]]); + return valueFilter(val, options, populateOptions, val == null ? val : _allIds[setValueIndex - 1]); } return valueFilter(val, options, populateOptions, _allIds); } for (i = 0; i < docs.length; ++i) { + setValueIndex = 0; const _path = o.path.endsWith('.$*') ? o.path.slice(0, -3) : o.path; const existingVal = mpath.get(_path, docs[i], lookupLocalFields); if (existingVal == null && !getVirtual(o.originalModel.schema, _path)) { diff --git a/test/model.populate.test.js b/test/model.populate.test.js index 4072a2a081a..9f629f28844 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -10811,6 +10811,9 @@ describe('model: populate:', function() { for (let i = 0; i < brands.length; i++) { test.items.push({ name: `${i}`, brand: brands[i]._id }); } + + const id4 = new mongoose.Types.ObjectId(); + test.items.push({ name: '4', brand: id4 }); await test.save(); const ids = []; @@ -10825,10 +10828,10 @@ describe('model: populate:', function() { } } ]); - assert.equal(ids.length, 3); + assert.equal(ids.length, 4); assert.deepStrictEqual( - ids.map(id => id.toHexString()), - [id1.toString(), id2.toString(), id3.toString()] + ids.map(id => id?.toHexString()), + [id1.toString(), id2.toString(), id3.toString(), id4.toString()] ); }); });