Merge pull request #13843 from Automattic/vkarpov15/gh-13782

BREAKING CHANGE: apply minimize by default when updating document

Author: vkarpov15

lib/document.js

@@ -33,6 +33,7 @@ const isExclusive = require('./helpers/projection/isExclusive');
 const inspect = require('util').inspect;
 const internalToObjectOptions = require('./options').internalToObjectOptions;
 const markArraySubdocsPopulated = require('./helpers/populate/markArraySubdocsPopulated');
+const minimize = require('./helpers/minimize');
 const mpath = require('mpath');
 const queryhelpers = require('./queryHelpers');
 const utils = require('./utils');
@@ -3938,42 +3939,6 @@ Document.prototype.toObject = function(options) {
   return this.$toObject(options);
 };
 
-/**
- * Minimizes an object, removing undefined values and empty objects
- *
- * @param {Object} object to minimize
- * @return {Object}
- * @api private
- */
-
-function minimize(obj) {
-  const keys = Object.keys(obj);
-  let i = keys.length;
-  let hasKeys;
-  let key;
-  let val;
-
-  while (i--) {
-    key = keys[i];
-    val = obj[key];
-
-    if (utils.isPOJO(val)) {
-      obj[key] = minimize(val);
-    }
-
-    if (undefined === obj[key]) {
-      delete obj[key];
-      continue;
-    }
-
-    hasKeys = true;
-  }
-
-  return hasKeys
-    ? obj
-    : undefined;
-}
-
 /*!
  * Applies virtuals properties to `json`.
  */

lib/helpers/minimize.js

@@ -0,0 +1,41 @@
+'use strict';
+
+const { isPOJO } = require('../utils');
+
+module.exports = minimize;
+
+/**
+ * Minimizes an object, removing undefined values and empty objects
+ *
+ * @param {Object} object to minimize
+ * @return {Object|undefined}
+ * @api private
+ */
+
+function minimize(obj) {
+  const keys = Object.keys(obj);
+  let i = keys.length;
+  let hasKeys;
+  let key;
+  let val;
+
+  while (i--) {
+    key = keys[i];
+    val = obj[key];
+
+    if (isPOJO(val)) {
+      obj[key] = minimize(val);
+    }
+
+    if (undefined === obj[key]) {
+      delete obj[key];
+      continue;
+    }
+
+    hasKeys = true;
+  }
+
+  return hasKeys
+    ? obj
+    : undefined;
+}

lib/model.js

@@ -66,6 +66,7 @@ const STATES = require('./connectionState');
 const util = require('util');
 const utils = require('./utils');
 const MongooseBulkWriteError = require('./error/bulkWriteError');
+const minimize = require('./helpers/minimize');
 
 const VERSION_WHERE = 1;
 const VERSION_INC = 2;
@@ -342,7 +343,19 @@ Model.prototype.$__handleSave = function(options, callback) {
     }
 
     _applyCustomWhere(this, where);
-    this[modelCollectionSymbol].updateOne(where, delta[1], saveOptions).then(
+
+    const update = delta[1];
+    if (this.$__schema.options.minimize) {
+      minimize(update);
+      // minimize might leave us with an empty object, which would
+      // lead to MongoDB throwing a "Update document requires atomic operators" error
+      if (Object.keys(update).length === 0) {
+        handleEmptyUpdate.call(this);
+        return;
+      }
+    }
+
+    this[modelCollectionSymbol].updateOne(where, update, saveOptions).then(
       ret => {
         ret.$where = where;
         callback(null, ret);
@@ -354,6 +367,17 @@ Model.prototype.$__handleSave = function(options, callback) {
       }
     );
   } else {
+    handleEmptyUpdate.call(this);
+    return;
+  }
+
+  // store the modified paths before the document is reset
+  this.$__.modifiedPaths = this.modifiedPaths();
+  this.$__reset();
+
+  _setIsNew(this, false);
+
+  function handleEmptyUpdate() {
     const optionsWithCustomValues = Object.assign({}, options, saveOptions);
     const where = this.$__where();
     const optimisticConcurrency = this.$__schema.options.optimisticConcurrency;
@@ -370,14 +394,7 @@ Model.prototype.$__handleSave = function(options, callback) {
         callback(null, { $where: where, matchedCount });
       })
       .catch(callback);
-    return;
   }
-
-  // store the modified paths before the document is reset
-  this.$__.modifiedPaths = this.modifiedPaths();
-  this.$__reset();
-
-  _setIsNew(this, false);
 };
 
 /*!

test/document.test.js

@@ -3243,7 +3243,7 @@ describe('document', function() {
             field1: { type: Number, default: 1 }
           }
         }
-      });
+      }, { minimize: false });
 
       const MyModel = db.model('Test', schema);
 
@@ -3262,7 +3262,7 @@ describe('document', function() {
             field1: { type: Number, default: 1 }
           }
         }
-      });
+      }, { minimize: false });
 
       const MyModel = db.model('Test', schema);
 
@@ -5002,7 +5002,7 @@ describe('document', function() {
         }).
         then(function(doc) {
           doc.child = {};
-          return doc.save();
+          return Parent.updateOne({ _id: doc._id }, { $set: { child: {} } }, { minimize: false });
         }).
         then(function() {
           return Parent.findOne();