From c9f6591d9947eba1a9b09fff58b949e58e8bf12e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 5 Jul 2024 11:24:27 -0400 Subject: [PATCH 1/2] proof of concept for cleaning up dangling defaults on unmodified subdocuments re: #14722 --- lib/document.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/document.js b/lib/document.js index ffaa066ef58..fffbf1efbdc 100644 --- a/lib/document.js +++ b/lib/document.js @@ -1691,6 +1691,13 @@ Document.prototype.$__set = function(pathToMark, path, options, constructing, pa } }); } + } else if (schema && schema.instance === 'Embedded') { + const defaultPaths = Object.keys(this.$__.activePaths.states['default'] ?? {}); + for (const defaultPath of defaultPaths) { + if (defaultPath.startsWith(path + '.')) { + this.$__.activePaths.clearPath(defaultPath); + } + } } let obj = this._doc; From f28514ec11b7221bde3322cd6bed5b80038e8ba1 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 5 Jul 2024 12:16:27 -0400 Subject: [PATCH 2/2] fix(document): avoid leaving subdoc defaults on top-level doc when setting subdocument to same value Fix #14722 --- lib/document.js | 11 +++------ lib/helpers/document/applyDefaults.js | 13 ++++++----- test/document.test.js | 32 ++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/lib/document.js b/lib/document.js index fffbf1efbdc..cf204974f0c 100644 --- a/lib/document.js +++ b/lib/document.js @@ -155,7 +155,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) { @@ -1691,13 +1693,6 @@ Document.prototype.$__set = function(pathToMark, path, options, constructing, pa } }); } - } else if (schema && schema.instance === 'Embedded') { - const defaultPaths = Object.keys(this.$__.activePaths.states['default'] ?? {}); - for (const defaultPath of defaultPaths) { - if (defaultPath.startsWith(path + '.')) { - this.$__.activePaths.clearPath(defaultPath); - } - } } let obj = this._doc; diff --git a/lib/helpers/document/applyDefaults.js b/lib/helpers/document/applyDefaults.js index 631d5151e5b..258e570ec30 100644 --- a/lib/helpers/document/applyDefaults.js +++ b/lib/helpers/document/applyDefaults.js @@ -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; @@ -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 @@ -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 { @@ -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 { @@ -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)); } } diff --git a/test/document.test.js b/test/document.test.js index a160f2edb62..339bf833383 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -12789,7 +12789,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() { @@ -13535,6 +13535,36 @@ describe('document', function() { assert.ok(blogPost.isDirectModified('comment.jsonField.fieldA')); 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')); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() {