From da4eb754528665fae8c1766ef2ef1f8f4d8eadc4 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 23 Dec 2024 15:41:17 -0500 Subject: [PATCH 1/4] fix(populate): handle hydrating deeply nested populated docs underneath virtual populate re: #15110 --- lib/document.js | 2 +- lib/model.js | 2 +- lib/schema.js | 9 +++++- test/model.hydrate.test.js | 57 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 3 deletions(-) diff --git a/lib/document.js b/lib/document.js index 14a33ef323..87c8e91b10 100644 --- a/lib/document.js +++ b/lib/document.js @@ -806,7 +806,7 @@ function init(self, obj, doc, opts, prefix) { reason: e })); } - } else if (opts.hydratedPopulatedDocs) { + } else if (schemaType && opts.hydratedPopulatedDocs) { doc[i] = schemaType.cast(value, self, true); if (doc[i] && doc[i].$__ && doc[i].$__.wasPopulated) { diff --git a/lib/model.js b/lib/model.js index 7edbce6ce2..b50ea3b09c 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3688,7 +3688,7 @@ Model.castObject = function castObject(obj, options) { } if (schemaType.$isMongooseDocumentArray) { - const castNonArraysOption = schemaType.options?.castNonArrays ??schemaType.constructor.options.castNonArrays; + const castNonArraysOption = schemaType.options?.castNonArrays ?? schemaType.constructor.options.castNonArrays; if (!Array.isArray(val)) { if (!castNonArraysOption) { if (!options.ignoreCastErrors) { diff --git a/lib/schema.js b/lib/schema.js index ddde465929..5c9cb2e944 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -25,6 +25,7 @@ const setPopulatedVirtualValue = require('./helpers/populate/setPopulatedVirtual const setupTimestamps = require('./helpers/timestamps/setupTimestamps'); const utils = require('./utils'); const validateRef = require('./helpers/populate/validateRef'); +const { populateModelSymbol } = require('./helpers/symbols'); const hasNumericSubpathRegex = /\.\d+(\.|$)/; @@ -2382,9 +2383,15 @@ Schema.prototype.virtual = function(name, options) { const PopulateModel = this.db.model(modelNames[0]); for (let i = 0; i < populatedVal.length; ++i) { if (!populatedVal[i].$__) { - populatedVal[i] = PopulateModel.hydrate(populatedVal[i]); + populatedVal[i] = PopulateModel.hydrate(populatedVal[i], null, { hydratedPopulatedDocs: true }); } } + const foreignField = options.foreignField; + this.$populated( + name, + populatedVal.map(doc => doc == null ? doc : doc.get(typeof foreignField === 'function' ? foreignField.call(doc, doc) : foreignField)), + { populateModelSymbol: PopulateModel } + ); } } diff --git a/test/model.hydrate.test.js b/test/model.hydrate.test.js index 447cc2be85..0f24d6887b 100644 --- a/test/model.hydrate.test.js +++ b/test/model.hydrate.test.js @@ -198,5 +198,62 @@ describe('model', function() { assert.ok(c.populated('users')); assert.ok(c.users[0] instanceof User); }); + + it('marks deeply nested docs as hydrated (gh-15110)', async function() { + const ArticleSchema = new Schema({ title: String }); + + const StorySchema = new Schema({ + title: String, + userId: Schema.Types.ObjectId, + article: { + type: Schema.Types.ObjectId, + ref: 'Article' + } + }); + + const UserSchema = new Schema({ + name: String + }); + + UserSchema.virtual('stories', { + ref: 'Story', + localField: '_id', + foreignField: 'userId' + }); + + const User = db.model('User', UserSchema); + const Story = db.model('Story', StorySchema); + const Article = db.model('Article', ArticleSchema); + await Promise.all([ + User.deleteMany({}), + Story.deleteMany({}), + Article.deleteMany({}) + ]); + + const article = await Article.create({ title: 'Cinema' }); + const user = await User.create({ name: 'Alex' }); + await Story.create({ title: 'Ticket 1', userId: user._id, article }); + await Story.create({ title: 'Ticket 2', userId: user._id }); + + const populated = await User.findOne({ name: 'Alex' }).populate({ + path: 'stories', + populate: ['article'] + }).lean(); + + const hydrated = User.hydrate( + JSON.parse(JSON.stringify(populated)), + null, + { hydratedPopulatedDocs: true } + ); + + assert.ok(hydrated.populated('stories')); + assert.ok(hydrated.stories[0].populated('article')); + assert.equal(hydrated.stories[0].article._id.toString(), article._id.toString()); + assert.ok(typeof hydrated.stories[0].article._id === 'object'); + assert.ok(hydrated.stories[0].article._id instanceof mongoose.Types.ObjectId); + assert.equal(hydrated.stories[0].article.title, 'Cinema'); + + assert.ok(!hydrated.stories[1].article); + }); }); }); From 5a04290f4b5911046f3f2774873a136e785cde29 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 26 Dec 2024 14:02:48 -0500 Subject: [PATCH 2/4] fix(model): handle nested conventional populate with hydratedPopulatedDocs option for hydrate() --- lib/document.js | 2 +- lib/model.js | 2 +- lib/schema.js | 11 +++--- lib/schema/array.js | 3 ++ lib/schema/buffer.js | 4 +-- lib/schema/decimal128.js | 4 +-- lib/schema/number.js | 4 +-- lib/schema/objectId.js | 4 +-- lib/schema/string.js | 4 +-- lib/schema/uuid.js | 4 +-- lib/schemaType.js | 4 +-- test/model.hydrate.test.js | 69 +++++++++++++++++++++++++++++++++++++- 12 files changed, 91 insertions(+), 24 deletions(-) diff --git a/lib/document.js b/lib/document.js index 87c8e91b10..ed9e347adc 100644 --- a/lib/document.js +++ b/lib/document.js @@ -807,7 +807,7 @@ function init(self, obj, doc, opts, prefix) { })); } } else if (schemaType && opts.hydratedPopulatedDocs) { - doc[i] = schemaType.cast(value, self, true); + doc[i] = schemaType.cast(value, self, true, undefined, { hydratedPopulatedDocs: true }); if (doc[i] && doc[i].$__ && doc[i].$__.wasPopulated) { self.$populated(path, doc[i].$__.wasPopulated.value, doc[i].$__.wasPopulated.options); diff --git a/lib/model.js b/lib/model.js index b50ea3b09c..bd8e60ec97 100644 --- a/lib/model.js +++ b/lib/model.js @@ -1461,7 +1461,7 @@ function getIndexesToDrop(schema, schemaIndexes, dbIndexes) { * @param {Object} [options] * @param {Array} [options.toDrop] if specified, contains a list of index names to drop * @param {Boolean} [options.hideIndexes=false] set to `true` to hide indexes instead of dropping. Requires MongoDB server 4.4 or higher - * @return {Promise} list of dropped or hidden index names + * @return {Promise>} list of dropped or hidden index names * @api public */ diff --git a/lib/schema.js b/lib/schema.js index 5c9cb2e944..60124f68e3 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -25,7 +25,6 @@ const setPopulatedVirtualValue = require('./helpers/populate/setPopulatedVirtual const setupTimestamps = require('./helpers/timestamps/setupTimestamps'); const utils = require('./utils'); const validateRef = require('./helpers/populate/validateRef'); -const { populateModelSymbol } = require('./helpers/symbols'); const hasNumericSubpathRegex = /\.\d+(\.|$)/; @@ -1940,13 +1939,11 @@ Schema.prototype.pre = function(name) { * const Model = mongoose.model('Model', schema); * * const m = new Model(..); - * m.save(function(err) { - * console.log('this fires after the `post` hook'); - * }); + * await m.save(); + * console.log('this fires after the `post` hook'); * - * m.find(function(err, docs) { - * console.log('this fires after the post find hook'); - * }); + * await m.find(); + * console.log('this fires after the post find hook'); * * @param {String|RegExp|String[]} methodName The method name or regular expression to match method name * @param {Object} [options] diff --git a/lib/schema/array.js b/lib/schema/array.js index e424731e4d..a555c308cc 100644 --- a/lib/schema/array.js +++ b/lib/schema/array.js @@ -403,6 +403,9 @@ SchemaArray.prototype.cast = function(value, doc, init, prev, options) { opts.arrayPathIndex = i; } } + if (options.hydratedPopulatedDocs) { + opts.hydratedPopulatedDocs = options.hydratedPopulatedDocs; + } rawValue[i] = caster.applySetters(rawValue[i], doc, init, void 0, opts); } } catch (e) { diff --git a/lib/schema/buffer.js b/lib/schema/buffer.js index e5cec2e015..4d5c1af7d5 100644 --- a/lib/schema/buffer.js +++ b/lib/schema/buffer.js @@ -140,7 +140,7 @@ SchemaBuffer.prototype.checkRequired = function(value, doc) { * @api private */ -SchemaBuffer.prototype.cast = function(value, doc, init) { +SchemaBuffer.prototype.cast = function(value, doc, init, prev, options) { let ret; if (SchemaType._isRef(this, value, doc, init)) { if (value && value.isMongooseBuffer) { @@ -167,7 +167,7 @@ SchemaBuffer.prototype.cast = function(value, doc, init) { } if (value == null || utils.isNonBuiltinObject(value)) { - return this._castRef(value, doc, init); + return this._castRef(value, doc, init, options); } } diff --git a/lib/schema/decimal128.js b/lib/schema/decimal128.js index 70fcfecc60..136529ec04 100644 --- a/lib/schema/decimal128.js +++ b/lib/schema/decimal128.js @@ -180,13 +180,13 @@ SchemaDecimal128.prototype.checkRequired = function checkRequired(value, doc) { * @api private */ -SchemaDecimal128.prototype.cast = function(value, doc, init) { +SchemaDecimal128.prototype.cast = function(value, doc, init, prev, options) { if (SchemaType._isRef(this, value, doc, init)) { if (isBsonType(value, 'Decimal128')) { return value; } - return this._castRef(value, doc, init); + return this._castRef(value, doc, init, options); } let castDecimal128; diff --git a/lib/schema/number.js b/lib/schema/number.js index d89ab7d63c..a5188a81cc 100644 --- a/lib/schema/number.js +++ b/lib/schema/number.js @@ -354,10 +354,10 @@ SchemaNumber.prototype.enum = function(values, message) { * @api private */ -SchemaNumber.prototype.cast = function(value, doc, init) { +SchemaNumber.prototype.cast = function(value, doc, init, prev, options) { if (typeof value !== 'number' && SchemaType._isRef(this, value, doc, init)) { if (value == null || utils.isNonBuiltinObject(value)) { - return this._castRef(value, doc, init); + return this._castRef(value, doc, init, options); } } diff --git a/lib/schema/objectId.js b/lib/schema/objectId.js index cad05198ea..927a168df4 100644 --- a/lib/schema/objectId.js +++ b/lib/schema/objectId.js @@ -223,7 +223,7 @@ SchemaObjectId.prototype.checkRequired = function checkRequired(value, doc) { * @api private */ -SchemaObjectId.prototype.cast = function(value, doc, init) { +SchemaObjectId.prototype.cast = function(value, doc, init, prev, options) { if (!(isBsonType(value, 'ObjectId')) && SchemaType._isRef(this, value, doc, init)) { // wait! we may need to cast this to a document if ((getConstructorName(value) || '').toLowerCase() === 'objectid') { @@ -231,7 +231,7 @@ SchemaObjectId.prototype.cast = function(value, doc, init) { } if (value == null || utils.isNonBuiltinObject(value)) { - return this._castRef(value, doc, init); + return this._castRef(value, doc, init, options); } } diff --git a/lib/schema/string.js b/lib/schema/string.js index d62e233765..b832dbd988 100644 --- a/lib/schema/string.js +++ b/lib/schema/string.js @@ -586,9 +586,9 @@ SchemaString.prototype.checkRequired = function checkRequired(value, doc) { * @api private */ -SchemaString.prototype.cast = function(value, doc, init) { +SchemaString.prototype.cast = function(value, doc, init, prev, options) { if (typeof value !== 'string' && SchemaType._isRef(this, value, doc, init)) { - return this._castRef(value, doc, init); + return this._castRef(value, doc, init, options); } let castString; diff --git a/lib/schema/uuid.js b/lib/schema/uuid.js index 1fbfc38654..6eb5d2f5ae 100644 --- a/lib/schema/uuid.js +++ b/lib/schema/uuid.js @@ -268,10 +268,10 @@ SchemaUUID.prototype.checkRequired = function checkRequired(value) { * @api private */ -SchemaUUID.prototype.cast = function(value, doc, init) { +SchemaUUID.prototype.cast = function(value, doc, init, prev, options) { if (utils.isNonBuiltinObject(value) && SchemaType._isRef(this, value, doc, init)) { - return this._castRef(value, doc, init); + return this._castRef(value, doc, init, options); } let castFn; diff --git a/lib/schemaType.js b/lib/schemaType.js index ed63c47bbc..d57cc775e6 100644 --- a/lib/schemaType.js +++ b/lib/schemaType.js @@ -1555,7 +1555,7 @@ SchemaType._isRef = function(self, value, doc, init) { * ignore */ -SchemaType.prototype._castRef = function _castRef(value, doc, init) { +SchemaType.prototype._castRef = function _castRef(value, doc, init, options) { if (value == null) { return value; } @@ -1587,7 +1587,7 @@ SchemaType.prototype._castRef = function _castRef(value, doc, init) { !doc.$__.populated[path].options.options || !doc.$__.populated[path].options.options.lean) { const PopulatedModel = pop ? pop.options[populateModelSymbol] : doc.constructor.db.model(this.options.ref); - ret = new PopulatedModel(value); + ret = PopulatedModel.hydrate(value, null, options); ret.$__.wasPopulated = { value: ret._doc._id, options: { [populateModelSymbol]: PopulatedModel } }; } diff --git a/test/model.hydrate.test.js b/test/model.hydrate.test.js index 0f24d6887b..0ce7ebf538 100644 --- a/test/model.hydrate.test.js +++ b/test/model.hydrate.test.js @@ -199,7 +199,7 @@ describe('model', function() { assert.ok(c.users[0] instanceof User); }); - it('marks deeply nested docs as hydrated (gh-15110)', async function() { + it('marks deeply nested docs as hydrated underneath virtuals (gh-15110)', async function() { const ArticleSchema = new Schema({ title: String }); const StorySchema = new Schema({ @@ -221,6 +221,9 @@ describe('model', function() { foreignField: 'userId' }); + db.deleteModel(/User/); + db.deleteModel(/Story/); + db.deleteModel(/Article/); const User = db.model('User', UserSchema); const Story = db.model('Story', StorySchema); const Article = db.model('Article', ArticleSchema); @@ -255,5 +258,69 @@ describe('model', function() { assert.ok(!hydrated.stories[1].article); }); + + it('marks deeply nested docs as hydrated underneath conventional (gh-15110)', async function() { + const ArticleSchema = new Schema({ + title: { + type: String + } + }); + + const StorySchema = new Schema({ + title: { + type: String + }, + article: { + type: Schema.Types.ObjectId, + ref: 'Article' + } + }); + + const UserSchema = new Schema({ + name: String, + stories: [{ + type: Schema.Types.ObjectId, + ref: 'Story' + }] + }); + + db.deleteModel(/User/); + db.deleteModel(/Story/); + db.deleteModel(/Article/); + const User = db.model('User', UserSchema); + const Story = db.model('Story', StorySchema); + const Article = db.model('Article', ArticleSchema); + await Promise.all([ + User.deleteMany({}), + Story.deleteMany({}), + Article.deleteMany({}) + ]); + + const article = await Article.create({ title: 'Cinema' }); + const story1 = await Story.create({ title: 'Ticket 1', article }); + const story2 = await Story.create({ title: 'Ticket 2' }); + + await User.create({ name: 'Alex', stories: [story1, story2] }); + + const populated = await User.findOne({ name: 'Alex' }).populate({ + path: 'stories', + populate: ['article'] + }).lean(); + + const hydrated = User.hydrate( + JSON.parse(JSON.stringify(populated)), + null, + { hydratedPopulatedDocs: true } + ); + + assert.ok(hydrated.populated('stories')); + assert.ok(hydrated.stories[0].populated('article')); + assert.equal(hydrated.stories[0].article._id.toString(), article._id.toString()); + assert.ok(typeof hydrated.stories[0].article._id === 'object'); + assert.ok(hydrated.stories[0].article._id instanceof mongoose.Types.ObjectId); + assert.equal(hydrated.stories[0].article.title, 'Cinema'); + + assert.ok(!hydrated.stories[1].article); + }); }); }); From acacc7aeac07b88503a87133ff4c1d6daa5c4c57 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 30 Dec 2024 15:10:51 -0500 Subject: [PATCH 3/4] Update test/model.hydrate.test.js Co-authored-by: hasezoey --- test/model.hydrate.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/model.hydrate.test.js b/test/model.hydrate.test.js index 0ce7ebf538..9dcfaba297 100644 --- a/test/model.hydrate.test.js +++ b/test/model.hydrate.test.js @@ -261,9 +261,7 @@ describe('model', function() { it('marks deeply nested docs as hydrated underneath conventional (gh-15110)', async function() { const ArticleSchema = new Schema({ - title: { - type: String - } + title: String }); const StorySchema = new Schema({ From 79de0ff04be6bcc6ad75c6838c934bd3a5b43113 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 30 Dec 2024 15:11:03 -0500 Subject: [PATCH 4/4] Update test/model.hydrate.test.js Co-authored-by: hasezoey --- test/model.hydrate.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/model.hydrate.test.js b/test/model.hydrate.test.js index 9dcfaba297..98ca46f0f7 100644 --- a/test/model.hydrate.test.js +++ b/test/model.hydrate.test.js @@ -265,9 +265,7 @@ describe('model', function() { }); const StorySchema = new Schema({ - title: { - type: String - }, + title: String, article: { type: Schema.Types.ObjectId, ref: 'Article'