From 3e63142323d4ba80c69de8923ad3269a52efb052 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 21 Mar 2024 16:09:10 -0400 Subject: [PATCH 1/3] 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/3] 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/3] 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;