From 3e63142323d4ba80c69de8923ad3269a52efb052 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 21 Mar 2024 16:09:10 -0400 Subject: [PATCH 1/5] fix(document): avoid depopulating populated subdocs underneath document arrays when copying to another document Fix #14118 --- lib/document.js | 11 +++++++---- lib/model.js | 2 +- lib/schema/documentarray.js | 16 +++------------- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/document.js b/lib/document.js index 757f5101c32..d85288ac64a 100644 --- a/lib/document.js +++ b/lib/document.js @@ -1087,7 +1087,11 @@ Document.prototype.$set = function $set(path, val, type, options) { if (path.$__isNested) { path = path.toObject(); } else { - path = path._doc; + // This ternary is to support gh-7898 (copying virtuals if same schema) + // while not breaking gh-10819, which for some reason breaks if we use toObject() + path = path.$__schema === this.$__schema + ? applyVirtuals(path, { ...path._doc }) + : path._doc; } } if (path == null) { @@ -4012,11 +4016,11 @@ function applyVirtuals(self, json, options, toObjectOptions) { ? toObjectOptions.aliases : true; + options = options || {}; let virtualsToApply = null; if (Array.isArray(options.virtuals)) { virtualsToApply = new Set(options.virtuals); - } - else if (options.virtuals && options.virtuals.pathsToSkip) { + } else if (options.virtuals && options.virtuals.pathsToSkip) { virtualsToApply = new Set(paths); for (let i = 0; i < options.virtuals.pathsToSkip.length; i++) { if (virtualsToApply.has(options.virtuals.pathsToSkip[i])) { @@ -4029,7 +4033,6 @@ function applyVirtuals(self, json, options, toObjectOptions) { return json; } - options = options || {}; for (i = 0; i < numPaths; ++i) { path = paths[i]; diff --git a/lib/model.js b/lib/model.js index 8a9e4a3226f..313d1f4747f 100644 --- a/lib/model.js +++ b/lib/model.js @@ -5124,7 +5124,7 @@ function _assign(model, vals, mod, assignmentOpts) { } // flag each as result of population if (!lean) { - val.$__.wasPopulated = val.$__.wasPopulated || true; + val.$__.wasPopulated = val.$__.wasPopulated || { value: _val }; } } } diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js index 3867c512aa2..eb63ad759c7 100644 --- a/lib/schema/documentarray.js +++ b/lib/schema/documentarray.js @@ -443,19 +443,9 @@ DocumentArrayPath.prototype.cast = function(value, doc, init, prev, options) { const Constructor = getConstructor(this.casterConstructor, rawArray[i]); - // Check if the document has a different schema (re gh-3701) - if (rawArray[i].$__ != null && !(rawArray[i] instanceof Constructor)) { - const spreadDoc = handleSpreadDoc(rawArray[i], true); - if (rawArray[i] !== spreadDoc) { - rawArray[i] = spreadDoc; - } else { - rawArray[i] = rawArray[i].toObject({ - transform: false, - // Special case: if different model, but same schema, apply virtuals - // re: gh-7898 - virtuals: rawArray[i].schema === Constructor.schema - }); - } + const spreadDoc = handleSpreadDoc(rawArray[i], true); + if (rawArray[i] !== spreadDoc) { + rawArray[i] = spreadDoc; } if (rawArray[i] instanceof Subdocument) { From ed355731ab4b5d91e900a4de411ca132d7e56cfa Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 21 Mar 2024 16:14:29 -0400 Subject: [PATCH 2/5] test(document): add test case for #14418 --- test/document.test.js | 62 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test/document.test.js b/test/document.test.js index beefe366d1f..1a5c1809212 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12476,6 +12476,68 @@ describe('document', function() { doc.set({ nested: void 0 }); assert.strictEqual(doc.toObject().nested, void 0); }); + + it('avoids depopulating populated subdocs underneath document arrays when copying to another document (gh-14418)', async function() { + const cartSchema = new mongoose.Schema({ + products: [ + { + product: { + type: mongoose.Schema.Types.ObjectId, + ref: 'Product' + }, + quantity: Number + } + ], + singleProduct: { + type: mongoose.Schema.Types.ObjectId, + ref: 'Product' + } + }); + const purchaseSchema = new mongoose.Schema({ + products: [ + { + product: { + type: mongoose.Schema.Types.ObjectId, + ref: 'Product' + }, + quantity: Number + } + ], + singleProduct: { + type: mongoose.Schema.Types.ObjectId, + ref: 'Product' + } + }); + const productSchema = new mongoose.Schema({ + name: String + }); + + const Cart = db.model('Cart', cartSchema); + const Purchase = db.model('Purchase', purchaseSchema); + const Product = db.model('Product', productSchema); + + const dbProduct = await Product.create({ name: 'Bug' }); + + const dbCart = await Cart.create({ + products: [ + { + product: dbProduct, + quantity: 2 + } + ], + singleProduct: dbProduct + }); + + const foundCart = await Cart.findById(dbCart._id). + populate('products.product singleProduct'); + + const purchaseFromDbCart = new Purchase({ + products: foundCart.products, + singleProduct: foundCart.singleProduct + }); + assert.equal(purchaseFromDbCart.products[0].product.name, 'Bug'); + assert.equal(purchaseFromDbCart.singleProduct.name, 'Bug'); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { From bc483799e6b7520214b2ce60b5e62ca006674519 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 21 Mar 2024 17:24:44 -0400 Subject: [PATCH 3/5] fix(schematype): consistently set `wasPopulated` to object with `value` property rather than boolean Re: #14418 Re: #6048 --- lib/schematype.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/schematype.js b/lib/schematype.js index 85e0241b5ea..ebd5dbc3c3d 100644 --- a/lib/schematype.js +++ b/lib/schematype.js @@ -1505,7 +1505,7 @@ SchemaType.prototype._castRef = function _castRef(value, doc, init) { } if (value.$__ != null) { - value.$__.wasPopulated = value.$__.wasPopulated || true; + value.$__.wasPopulated = value.$__.wasPopulated || { value: value._id }; return value; } @@ -1531,7 +1531,7 @@ SchemaType.prototype._castRef = function _castRef(value, doc, init) { !doc.$__.populated[path].options.options || !doc.$__.populated[path].options.options.lean) { ret = new pop.options[populateModelSymbol](value); - ret.$__.wasPopulated = true; + ret.$__.wasPopulated = { value: ret._id }; } return ret; From 69a0581079a074876cfcb2ae1d1ae7ed5bb75f2f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 26 Mar 2024 14:55:34 -0400 Subject: [PATCH 4/5] fix(document): handle virtuals that are stored as objects but getter returns string with toJSON Fix #14446 --- lib/document.js | 7 ++++++- test/document.test.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/document.js b/lib/document.js index d85288ac64a..e82d2e73b71 100644 --- a/lib/document.js +++ b/lib/document.js @@ -4113,7 +4113,12 @@ function applyGetters(self, json, options) { for (let ii = 0; ii < plen; ++ii) { part = parts[ii]; v = cur[part]; - if (ii === last) { + // If we've reached a non-object part of the branch, continuing would + // cause "Cannot create property 'foo' on string 'bar'" error. + // Necessary for mongoose-intl plugin re: gh-14446 + if (branch != null && typeof branch !== 'object') { + break; + } else if (ii === last) { const val = self.$get(path); branch[part] = clone(val, options); } else if (v == null) { diff --git a/test/document.test.js b/test/document.test.js index 1a5c1809212..2aa0b332e2f 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12538,6 +12538,38 @@ describe('document', function() { assert.equal(purchaseFromDbCart.products[0].product.name, 'Bug'); assert.equal(purchaseFromDbCart.singleProduct.name, 'Bug'); }); + + it('handles virtuals that are stored as objects but getter returns string with toJSON (gh-14446)', async function() { + const childSchema = new mongoose.Schema(); + + childSchema.virtual('name') + .set(function(values) { + for (const [lang, val] of Object.entries(values)) { + this.set(`name.${lang}`, val); + } + }) + .get(function() { + return this.$__getValue(`name.${this.lang}`); + }); + + childSchema.add({ name: { en: { type: String }, de: { type: String } } }); + + const ChildModel = db.model('Child', childSchema); + const ParentModel = db.model('Parent', new mongoose.Schema({ + children: [childSchema] + })); + + const child = await ChildModel.create({ name: { en: 'Stephen', de: 'Stefan' } }); + child.lang = 'en'; + assert.equal(child.name, 'Stephen'); + + const parent = await ParentModel.create({ + children: [{ name: { en: 'Stephen', de: 'Stefan' } }] + }); + parent.children[0].lang = 'de'; + const obj = parent.toJSON({ getters: true }); + assert.equal(obj.children[0].name, 'Stefan'); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { From c00a715e97c6437a5ff1a503c2a50ebd0df2ba47 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 10 Apr 2024 17:44:25 -0400 Subject: [PATCH 5/5] chore: release 6.12.8 --- CHANGELOG.md | 6 ++++++ package.json | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1f7bb87b8b..4bdcdeec521 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +6.12.8 / 2024-04-10 +=================== + * fix(document): handle virtuals that are stored as objects but getter returns string with toJSON #14468 #14446 + * fix(schematype): consistently set wasPopulated to object with `value` property rather than boolean #14418 + * docs(model): add extra note about lean option for insertMany() skipping casting #14415 #14376 + 6.12.7 / 2024-03-01 =================== * perf(model): make insertMany() lean option skip hydrating Mongoose docs #14376 #14372 diff --git a/package.json b/package.json index 9f3a3889566..6d887a254e2 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "mongoose", "description": "Mongoose MongoDB ODM", - "version": "6.12.7", + "version": "6.12.8", "author": "Guillermo Rauch ", "keywords": [ "mongodb",