Skip to content

Commit

Permalink
Merge pull request #14728 from Automattic/vkarpov15/gh-14722
Browse files Browse the repository at this point in the history
Avoid leaving subdoc defaults on top-level doc when setting subdocument to same value
  • Loading branch information
vkarpov15 authored Jul 9, 2024
2 parents c4c932a + 2a83e26 commit bb7e929
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
4 changes: 3 additions & 1 deletion lib/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ function Document(obj, fields, skipId, options) {
// By default, defaults get applied **before** setting initial values
// Re: gh-6155
if (defaults) {
applyDefaults(this, fields, exclude, hasIncludedChildren, true, null);
applyDefaults(this, fields, exclude, hasIncludedChildren, true, null, {
skipParentChangeTracking: true
});
}
}
if (obj) {
Expand Down
13 changes: 7 additions & 6 deletions lib/helpers/document/applyDefaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

const isNestedProjection = require('../projection/isNestedProjection');

module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildren, isBeforeSetters, pathsToSkip) {
module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildren, isBeforeSetters, pathsToSkip, options) {
const paths = Object.keys(doc.$__schema.paths);
const plen = paths.length;
const skipParentChangeTracking = options && options.skipParentChangeTracking;

for (let i = 0; i < plen; ++i) {
let def;
Expand Down Expand Up @@ -80,7 +81,7 @@ module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildre

if (typeof def !== 'undefined') {
doc_[piece] = def;
applyChangeTracking(doc, p);
applyChangeTracking(doc, p, skipParentChangeTracking);
}
} else if (included) {
// selected field
Expand All @@ -93,7 +94,7 @@ module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildre

if (typeof def !== 'undefined') {
doc_[piece] = def;
applyChangeTracking(doc, p);
applyChangeTracking(doc, p, skipParentChangeTracking);
}
}
} else {
Expand All @@ -106,7 +107,7 @@ module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildre

if (typeof def !== 'undefined') {
doc_[piece] = def;
applyChangeTracking(doc, p);
applyChangeTracking(doc, p, skipParentChangeTracking);
}
}
} else {
Expand All @@ -120,9 +121,9 @@ module.exports = function applyDefaults(doc, fields, exclude, hasIncludedChildre
* ignore
*/

function applyChangeTracking(doc, fullPath) {
function applyChangeTracking(doc, fullPath, skipParentChangeTracking) {
doc.$__.activePaths.default(fullPath);
if (doc.$isSubdocument && doc.$isSingleNested && doc.$parent() != null) {
if (!skipParentChangeTracking && doc.$isSubdocument && doc.$isSingleNested && doc.$parent() != null) {
doc.$parent().$__.activePaths.default(doc.$__pathRelativeToParent(fullPath));
}
}
32 changes: 31 additions & 1 deletion test/document.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12805,7 +12805,7 @@ describe('document', function() {
await user.save();
await TestModel.deleteOne({ userId });

assert.equal(Object.keys(TestModel.schema.subpaths).length, 3);
assert.ok(Object.keys(TestModel.schema.subpaths).length <= 3);
});

it('handles embedded discriminators defined using Schema.prototype.discriminator (gh-13898)', async function() {
Expand Down Expand Up @@ -13552,6 +13552,36 @@ describe('document', function() {
assert.ok(blogPost.comment.jsonField.isDirectModified('fieldA'));
});

it('avoids leaving subdoc _id in default state when setting subdocument to same value (gh-14722)', async function() {
const getUser = () => ({
_id: new mongoose.Types.ObjectId('66852317541ef0e22ae5214c'),
name: 'test1',
email: 'test@test.com'
});

const updateInfoSchema = new mongoose.Schema({
name: {
type: String, required: true
},
email: {
type: String,
required: true
}
}, {
versionKey: false
});
const schema = new mongoose.Schema({ name: String, updater: updateInfoSchema });

const TestModel = db.model('Test', schema);
const { _id } = await TestModel.create({ name: 'taco', updater: getUser() });
const doc = await TestModel.findById(_id).orFail();

doc.name = 'taco-edit';
doc.updater = getUser();
assert.deepStrictEqual(doc.getChanges(), { $set: { name: 'taco-edit' } });
assert.ok(!doc.$isDefault('updater._id'));
});

Check failure on line 13584 in test/document.test.js

View workflow job for this annotation

GitHub Actions / Lint JS-Files

Trailing spaces not allowed
it('$clearModifiedPaths (gh-14268)', async function() {
const schema = new Schema({
name: String,
Expand Down

0 comments on commit bb7e929

Please sign in to comment.