Skip to content

Commit 7c2cfa8

Browse files
committed
fix(model+document): avoid depopulating manually populated doc as getter value
Fix #14759
1 parent e4462c6 commit 7c2cfa8

File tree

5 files changed

+40
-9
lines changed

5 files changed

+40
-9
lines changed

lib/document.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,7 +1394,7 @@ Document.prototype.$set = function $set(path, val, type, options) {
13941394

13951395
let didPopulate = false;
13961396
if (refMatches && val instanceof Document && (!val.$__.wasPopulated || utils.deepEqual(val.$__.wasPopulated.value, val._id))) {
1397-
const unpopulatedValue = (schema && schema.$isSingleNested) ? schema.cast(val, this) : val._id;
1397+
const unpopulatedValue = (schema && schema.$isSingleNested) ? schema.cast(val, this) : val._doc._id;
13981398
this.$populated(path, unpopulatedValue, { [populateModelSymbol]: val.constructor });
13991399
val.$__.wasPopulated = { value: unpopulatedValue };
14001400
didPopulate = true;
@@ -1412,7 +1412,7 @@ Document.prototype.$set = function $set(path, val, type, options) {
14121412
this.$populated(path, val.map(function(v) { return v._id; }), popOpts);
14131413

14141414
for (const doc of val) {
1415-
doc.$__.wasPopulated = { value: doc._id };
1415+
doc.$__.wasPopulated = { value: doc._doc._id };
14161416
}
14171417
didPopulate = true;
14181418
}
@@ -3840,7 +3840,7 @@ Document.prototype.$toObject = function(options, json) {
38403840
// _isNested will only be true if this is not the top level document, we
38413841
// should never depopulate the top-level document
38423842
if (depopulate && options._isNested && this.$__.wasPopulated) {
3843-
return clone(this.$__.wasPopulated.value || this._id, options);
3843+
return clone(this.$__.wasPopulated.value || this._doc._id, options);
38443844
}
38453845

38463846
// merge default options with input options.

lib/model.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4459,7 +4459,7 @@ function _assign(model, vals, mod, assignmentOpts) {
44594459
}
44604460
} else {
44614461
if (_val instanceof Document) {
4462-
_val = _val._id;
4462+
_val = _val._doc._id;
44634463
}
44644464
key = String(_val);
44654465
if (rawDocs[key]) {
@@ -4468,7 +4468,7 @@ function _assign(model, vals, mod, assignmentOpts) {
44684468
rawOrder[key].push(i);
44694469
} else if (isVirtual ||
44704470
rawDocs[key].constructor !== val.constructor ||
4471-
String(rawDocs[key]._id) !== String(val._id)) {
4471+
String(rawDocs[key]._doc._id) !== String(val._doc._id)) {
44724472
// May need to store multiple docs with the same id if there's multiple models
44734473
// if we have discriminators or a ref function. But avoid converting to an array
44744474
// if we have multiple queries on the same model because of `perDocumentLimit` re: gh-9906

lib/schemaType.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,7 +1542,7 @@ SchemaType.prototype._castRef = function _castRef(value, doc, init) {
15421542
}
15431543

15441544
if (value.$__ != null) {
1545-
value.$__.wasPopulated = value.$__.wasPopulated || { value: value._id };
1545+
value.$__.wasPopulated = value.$__.wasPopulated || { value: value._doc._id };
15461546
return value;
15471547
}
15481548

@@ -1568,7 +1568,7 @@ SchemaType.prototype._castRef = function _castRef(value, doc, init) {
15681568
!doc.$__.populated[path].options.options ||
15691569
!doc.$__.populated[path].options.options.lean) {
15701570
ret = new pop.options[populateModelSymbol](value);
1571-
ret.$__.wasPopulated = { value: ret._id };
1571+
ret.$__.wasPopulated = { value: ret._doc._id };
15721572
}
15731573

15741574
return ret;

lib/types/map.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,15 @@ class MongooseMap extends Map {
116116
v = new populated.options[populateModelSymbol](v);
117117
}
118118
// Doesn't support single nested "in-place" populate
119-
v.$__.wasPopulated = { value: v._id };
119+
v.$__.wasPopulated = { value: v._doc._id };
120120
return v;
121121
});
122122
} else if (value != null) {
123123
if (value.$__ == null) {
124124
value = new populated.options[populateModelSymbol](value);
125125
}
126126
// Doesn't support single nested "in-place" populate
127-
value.$__.wasPopulated = { value: value._id };
127+
value.$__.wasPopulated = { value: value._doc._id };
128128
}
129129
} else {
130130
try {

test/model.populate.test.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11014,4 +11014,35 @@ describe('model: populate:', function() {
1101411014
assert.equal(latestClass.students[1].name, 'Robert');
1101511015
assert.equal(latestClass.students[1].grade.grade, 'B');
1101611016
});
11017+
11018+
it('avoids depopulating manually populated doc as getter value (gh-14759)', async function() {
11019+
const ownerSchema = new mongoose.Schema({
11020+
_id: {
11021+
type: 'ObjectId',
11022+
get(value) {
11023+
return value == null ? value : value.toString();
11024+
}
11025+
},
11026+
name: 'String'
11027+
});
11028+
const petSchema = new mongoose.Schema({
11029+
name: 'String',
11030+
owner: { type: 'ObjectId', ref: 'Owner' }
11031+
});
11032+
11033+
const Owner = db.model('Owner', ownerSchema);
11034+
const Pet = db.model('Pet', petSchema);
11035+
11036+
const ownerId = new mongoose.Types.ObjectId();
11037+
const owner = new Owner({
11038+
_id: ownerId,
11039+
name: 'Alice'
11040+
});
11041+
await owner.save();
11042+
const pet = new Pet({ name: 'Kitty', owner: owner });
11043+
await pet.save();
11044+
11045+
const fromDb = await Pet.findOne({ owner: ownerId }).lean().orFail();
11046+
assert.ok(fromDb.owner instanceof mongoose.Types.ObjectId);
11047+
});
1101711048
});

0 commit comments

Comments
 (0)