Merge pull request #13644 from Automattic/vkarpov15/gh-13609

Correctly clean up nested subdocs modified state on save()
Automattic · Jul 23, 2023 · ae3894c · ae3894c
2 parents faf6bf7 + ec6ecb5
commit ae3894c
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 5 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -3321,8 +3321,8 @@ Document.prototype.$__reset = function reset() {
   const resetArrays = new Set();
   for (const subdoc of subdocs) {
     const fullPathWithIndexes = subdoc.$__fullPathWithIndexes();
+    subdoc.$__reset();
     if (this.isModified(fullPathWithIndexes) || isParentInit(fullPathWithIndexes)) {
-      subdoc.$__reset();
       if (subdoc.$isDocumentArrayElement) {
         resetArrays.add(subdoc.parentArray());
       } else {

diff --git a/lib/types/ArraySubdocument.js b/lib/types/ArraySubdocument.js
@@ -33,7 +33,15 @@ function ArraySubdocument(obj, parentArr, skipId, fields, index) {
   this.$setIndex(index);
   this.$__parent = this[documentArrayParent];
 
-  Subdocument.call(this, obj, fields, this[documentArrayParent], skipId, { isNew: true });
+  let options;
+  if (typeof skipId === 'object' && skipId != null) {
+    options = { isNew: true, ...skipId };
+    skipId = undefined;
+  } else {
+    options = { isNew: true };
+  }
+
+  Subdocument.call(this, obj, fields, this[documentArrayParent], skipId, options);
 }
 
 /*!

diff --git a/lib/types/DocumentArray/methods/index.js b/lib/types/DocumentArray/methods/index.js
@@ -38,7 +38,7 @@ const methods = {
    * @memberOf MongooseDocumentArray
    */
 
-  _cast(value, index) {
+  _cast(value, index, options) {
     if (this[arraySchemaSymbol] == null) {
       return value;
     }
@@ -89,7 +89,7 @@ const methods = {
     if (Constructor.$isMongooseDocumentArray) {
       return Constructor.cast(value, this, undefined, undefined, index);
     }
-    const ret = new Constructor(value, this, undefined, undefined, index);
+    const ret = new Constructor(value, this, options, undefined, index);
     ret.isNew = true;
     return ret;
   },

diff --git a/lib/types/array/methods/index.js b/lib/types/array/methods/index.js
@@ -595,7 +595,7 @@ const methods = {
    */
 
   pull() {
-    const values = [].map.call(arguments, this._cast, this);
+    const values = [].map.call(arguments, (v, i) => this._cast(v, i, { defaults: false }), this);
     const cur = this[arrayParentSymbol].get(this[arrayPathSymbol]);
     let i = cur.length;
     let mem;

diff --git a/lib/types/subdocument.js b/lib/types/subdocument.js
@@ -16,6 +16,10 @@ module.exports = Subdocument;
  */
 
 function Subdocument(value, fields, parent, skipId, options) {
+  if (typeof skipId === 'object' && skipId != null && options == null) {
+    options = skipId;
+    skipId = undefined;
+  }
   if (parent != null) {
     // If setting a nested path, should copy isNew from parent re: gh-7048
     const parentOptions = { isNew: parent.isNew };

diff --git a/test/document.test.js b/test/document.test.js
@@ -12290,6 +12290,29 @@ describe('document', function() {
     assert.deepStrictEqual(doc.elements[1].modifiedPaths(), []);
   });
 
+  it('cleans up all nested subdocs modified state on save (gh-13609)', async function() {
+    const TwoElementSchema = new mongoose.Schema({
+      elementName: String
+    });
+
+    const TwoNestedSchema = new mongoose.Schema({
+      nestedName: String,
+      elements: [TwoElementSchema]
+    });
+
+    const TwoDocDefaultSchema = new mongoose.Schema({
+      docName: String,
+      nested: { type: TwoNestedSchema, default: {} }
+    });
+
+    const Test = db.model('Test', TwoDocDefaultSchema);
+    const doc = new Test({ docName: 'MyDocName' });
+    doc.nested.nestedName = 'qdwqwd';
+    doc.nested.elements.push({ elementName: 'ElementName1' });
+    await doc.save();
+    assert.deepStrictEqual(doc.nested.modifiedPaths(), []);
+  });
+
   it('avoids prototype pollution on init', async function() {
     const Example = db.model('Example', new Schema({ hello: String }));