From 9739dfcf05ffd42f0484a2227087e0cc1c06bf6e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 1 Feb 2024 11:27:05 -0500 Subject: [PATCH 1/3] fix(populate): call setter on virtual populated path with populated doc instead of undefined Fix #14285 --- lib/schema.js | 18 +++++++------ test/model.populate.test.js | 54 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/lib/schema.js b/lib/schema.js index 6f25a00ccbe..48ca4365bfd 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -2288,28 +2288,30 @@ Schema.prototype.virtual = function(name, options) { virtual.options = options; virtual. - set(function(_v) { + set(function(v) { if (!this.$$populatedVirtuals) { this.$$populatedVirtuals = {}; } if (options.justOne || options.count) { - this.$$populatedVirtuals[name] = Array.isArray(_v) ? - _v[0] : - _v; + this.$$populatedVirtuals[name] = Array.isArray(v) ? + v[0] : + v; if (typeof this.$$populatedVirtuals[name] !== 'object') { - this.$$populatedVirtuals[name] = options.count ? _v : null; + this.$$populatedVirtuals[name] = options.count ? v : null; } } else { - this.$$populatedVirtuals[name] = Array.isArray(_v) ? - _v : - _v == null ? [] : [_v]; + this.$$populatedVirtuals[name] = Array.isArray(v) ? + v : + v == null ? [] : [v]; this.$$populatedVirtuals[name] = this.$$populatedVirtuals[name].filter(function(doc) { return doc && typeof doc === 'object'; }); } + + return this.$$populatedVirtuals[name]; }); if (typeof options.get === 'function') { diff --git a/test/model.populate.test.js b/test/model.populate.test.js index 0a74994be3a..2252265cb8a 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -10868,4 +10868,58 @@ describe('model: populate:', function() { { name: 'foo', prop: 'bar' } ); }); + + it('calls setter on virtual populated path with populated doc (gh-14285)', async function() { + const userSchema = new Schema({ + email: String, + name: 'String' + }); + + const User = db.model('User', userSchema); + + const user = await User.create({ + email: 'admin@example.com', + name: 'Admin' + }); + + const personSchema = new Schema({ + userId: ObjectId, + userType: String + }); + + personSchema. + virtual('user', { + ref() { + return this.userType; + }, + localField: 'userId', + foreignField: '_id', + justOne: true + }). + set(function(user) { + if (user) { + this.userId = user._id; + this.userType = user.constructor.modelName; + } else { + this.userId = null; + this.userType = null; + } + + return user; + }); + + const Person = db.model('Person', personSchema); + + const person = new Person({ + userId: user._id, + userType: 'User' + }); + + await person.save(); + + const personFromDb = await Person.findById(person._id).populate('user'); + assert.equal(personFromDb.user.name, 'Admin'); + assert.equal(personFromDb.userType, 'User'); + assert.equal(personFromDb.userId.toHexString(), user._id.toHexString()); + }); }); From f2e4d64f166f290344a0c1aaeda01acc454d1557 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 1 Feb 2024 11:44:48 -0500 Subject: [PATCH 2/3] refactor: move populate virtual setter logic into populate helper --- .../populate/setPopulatedVirtualValue.js | 23 ++++++++++++++++ lib/schema.js | 26 +++++-------------- 2 files changed, 30 insertions(+), 19 deletions(-) create mode 100644 lib/helpers/populate/setPopulatedVirtualValue.js diff --git a/lib/helpers/populate/setPopulatedVirtualValue.js b/lib/helpers/populate/setPopulatedVirtualValue.js new file mode 100644 index 00000000000..b99ff798f22 --- /dev/null +++ b/lib/helpers/populate/setPopulatedVirtualValue.js @@ -0,0 +1,23 @@ +'use strict'; + +module.exports = function setPopulatedVirtualValue(populatedVirtuals, name, v, options) { + if (options.justOne || options.count) { + populatedVirtuals[name] = Array.isArray(v) ? + v[0] : + v; + + if (typeof populatedVirtuals[name] !== 'object') { + populatedVirtuals[name] = options.count ? v : null; + } + } else { + populatedVirtuals[name] = Array.isArray(v) ? + v : + v == null ? [] : [v]; + + populatedVirtuals[name] = populatedVirtuals[name].filter(function(doc) { + return doc && typeof doc === 'object'; + }); + } + + return populatedVirtuals[name]; +}; diff --git a/lib/schema.js b/lib/schema.js index 48ca4365bfd..5941b8ebf5e 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -20,6 +20,7 @@ const handleReadPreferenceAliases = require('./helpers/query/handleReadPreferenc const idGetter = require('./helpers/schema/idGetter'); const merge = require('./helpers/schema/merge'); const mpath = require('mpath'); +const setPopulatedVirtualValue = require('./helpers/populate/setPopulatedVirtualValue'); const setupTimestamps = require('./helpers/timestamps/setupTimestamps'); const utils = require('./utils'); const validateRef = require('./helpers/populate/validateRef'); @@ -2293,25 +2294,12 @@ Schema.prototype.virtual = function(name, options) { this.$$populatedVirtuals = {}; } - if (options.justOne || options.count) { - this.$$populatedVirtuals[name] = Array.isArray(v) ? - v[0] : - v; - - if (typeof this.$$populatedVirtuals[name] !== 'object') { - this.$$populatedVirtuals[name] = options.count ? v : null; - } - } else { - this.$$populatedVirtuals[name] = Array.isArray(v) ? - v : - v == null ? [] : [v]; - - this.$$populatedVirtuals[name] = this.$$populatedVirtuals[name].filter(function(doc) { - return doc && typeof doc === 'object'; - }); - } - - return this.$$populatedVirtuals[name]; + return setPopulatedVirtualValue( + this.$$populatedVirtuals, + name, + v, + options + ); }); if (typeof options.get === 'function') { From fc9df6bd168abab2866a357b11d3ddc699bb398e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 2 Feb 2024 10:25:51 -0500 Subject: [PATCH 3/3] docs: add jsdoc to new `setPopulatedVirtualValue()` function --- lib/helpers/populate/setPopulatedVirtualValue.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/helpers/populate/setPopulatedVirtualValue.js b/lib/helpers/populate/setPopulatedVirtualValue.js index b99ff798f22..786e540051a 100644 --- a/lib/helpers/populate/setPopulatedVirtualValue.js +++ b/lib/helpers/populate/setPopulatedVirtualValue.js @@ -1,5 +1,15 @@ 'use strict'; +/** + * Set a populated virtual value on a document's `$$populatedVirtuals` value + * + * @param {*} populatedVirtuals A document's `$$populatedVirtuals` + * @param {*} name The virtual name + * @param {*} v The result of the populate query + * @param {*} options The populate options. This function handles `justOne` and `count` options. + * @returns {Array|Document|Object|Array} the populated virtual value that was set + */ + module.exports = function setPopulatedVirtualValue(populatedVirtuals, name, v, options) { if (options.justOne || options.count) { populatedVirtuals[name] = Array.isArray(v) ?