Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid depopulating populated subdocs underneath document arrays when copying to another document #14458

Merged
merged 3 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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])) {
Expand All @@ -4029,7 +4033,6 @@ function applyVirtuals(self, json, options, toObjectOptions) {
return json;
}

options = options || {};
for (i = 0; i < numPaths; ++i) {
path = paths[i];

Expand Down
2 changes: 1 addition & 1 deletion lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
}
}
Expand Down
16 changes: 3 additions & 13 deletions lib/schema/documentarray.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions lib/schematype.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down
62 changes: 62 additions & 0 deletions test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading