fix(model): dont wipe out changes made while save() is in-flight

Fix #9327
Automattic · Aug 26, 2020 · 5629fac · 5629fac
1 parent 1b416bb
commit 5629fac
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 9 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -38,6 +38,7 @@ const clone = utils.clone;
 const deepEqual = utils.deepEqual;
 const isMongooseObject = utils.isMongooseObject;
 
+const arrayAtomicsBackupSymbol = Symbol('mongoose.Array#atomicsBackup');
 const arrayAtomicsSymbol = require('./helpers/symbols').arrayAtomicsSymbol;
 const documentArrayParent = require('./helpers/symbols').documentArrayParent;
 const documentSchemaSymbol = require('./helpers/symbols').documentSchemaSymbol;
@@ -2764,6 +2765,7 @@ Document.prototype.$__reset = function reset() {
 
       _this.$__.activePaths.init(array.$path());
 
+      array[arrayAtomicsBackupSymbol] = array[arrayAtomicsSymbol];
       array[arrayAtomicsSymbol] = {};
     });
 
@@ -2784,10 +2786,19 @@ Document.prototype.$__reset = function reset() {
     const type = dirt.value;
 
     if (type && type[arrayAtomicsSymbol]) {
+      type[arrayAtomicsBackupSymbol] = type[arrayAtomicsSymbol];
       type[arrayAtomicsSymbol] = {};
     }
   });
 
+  this.$__.backup = {};
+  this.$__.backup.activePaths = {
+    modify: Object.assign({}, this.$__.activePaths.states.modify),
+    default: Object.assign({}, this.$__.activePaths.states.default)
+  };
+  this.$__.backup.validationError = this.$__.validationError;
+  this.$__.backup.errors = this.errors;
+
   // Clear 'dirty' cache
   this.$__.activePaths.clear('modify');
   this.$__.activePaths.clear('default');
@@ -2801,6 +2812,34 @@ Document.prototype.$__reset = function reset() {
   return this;
 };
 
+/*!
+ * ignore
+ */
+
+Document.prototype.$__undoReset = function $__undoReset() {
+  if (this.$__.backup == null || this.$__.backup.activePaths == null) {
+    return;
+  }
+
+  this.$__.activePaths.states.modify = this.$__.backup.activePaths.modify;
+  this.$__.activePaths.states.default = this.$__.backup.activePaths.default;
+
+  this.$__.validationError = this.$__.backup.validationError;
+  this.errors = this.$__.backup.errors;
+
+  for (const dirt of this.$__dirty()) {
+    const type = dirt.value;
+
+    if (type && type[arrayAtomicsSymbol] && type[arrayAtomicsBackupSymbol]) {
+      type[arrayAtomicsSymbol] = type[arrayAtomicsBackupSymbol];
+    }
+  }
+
+  for (const subdoc of this.$__getAllSubdocs()) {
+    subdoc.$__undoReset();
+  }
+};
+
 /**
  * Returns this documents dirty paths / vals.
  *

diff --git a/lib/model.js b/lib/model.js
@@ -282,6 +282,7 @@ Model.prototype.$__handleSave = function(options, callback) {
 
       callback(null, ret);
     });
+
     this.$__reset();
     _setIsNew(this, false);
     // Make it possible to retry the insert
@@ -307,8 +308,10 @@ Model.prototype.$__handleSave = function(options, callback) {
 
       _applyCustomWhere(this, where);
 
-      this[modelCollectionSymbol].updateOne(where, delta[1], saveOptions, function(err, ret) {
+      this[modelCollectionSymbol].updateOne(where, delta[1], saveOptions, (err, ret) => {
         if (err) {
+          this.$__undoReset();
+
           callback(err);
           return;
         }
@@ -319,15 +322,21 @@ Model.prototype.$__handleSave = function(options, callback) {
       const optionsWithCustomValues = Object.assign({}, options, saveOptions);
       this.constructor.exists(this.$__where(), optionsWithCustomValues)
         .then((documentExists) => {
-          if (!documentExists) throw new DocumentNotFoundError(this.$__where(), this.constructor.modelName);
+          if (!documentExists) {
+            throw new DocumentNotFoundError(this.$__where(), this.constructor.modelName);
+          }
 
-          this.$__reset();
           callback();
         })
         .catch(callback);
       return;
     }
 
+    // store the modified paths before the document is reset
+    this.$__.modifiedPaths = this.modifiedPaths();
+
+    this.$__reset();
+
     _setIsNew(this, false);
   }
 };
@@ -345,11 +354,6 @@ Model.prototype.$__save = function(options, callback) {
       });
     }
 
-    // store the modified paths before the document is reset
-    const modifiedPaths = this.modifiedPaths();
-
-    this.$__reset();
-
     let numAffected = 0;
     if (get(options, 'safe.w') !== 0 && get(options, 'w') !== 0) {
       // Skip checking if write succeeded if writeConcern is set to
@@ -376,8 +380,9 @@ Model.prototype.$__save = function(options, callback) {
 
         if (numAffected <= 0) {
           // the update failed. pass an error back
+          this.$__undoReset();
           const err = this.$__.$versionError ||
-            new VersionError(this, version, modifiedPaths);
+            new VersionError(this, version, this.$__.modifiedPaths);
           return callback(err);
         }
 
@@ -388,6 +393,7 @@ Model.prototype.$__save = function(options, callback) {
       }
 
       if (result != null && numAffected <= 0) {
+        this.$__undoReset();
         error = new DocumentNotFoundError(result.$where,
           this.constructor.modelName, numAffected, result);
         return hooks.execPost('save:error', this, [this], { error: error }, (error) => {