fix(document): avoid leaving subdoc defaults on top-level doc when se…

…tting subdocument to same value Fix #14722
Automattic · Jul 5, 2024 · f28514e · f28514e
1 parent c9f6591
commit f28514e
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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() {