Merge pull request #5086 from Automattic/3556

When overwriting doc with update+overwrite, validate the whole doc

Author: vkarpov15

lib/query.js

@@ -7,6 +7,7 @@ var QueryCursor = require('./querycursor');
 var QueryStream = require('./querystream');
 var cast = require('./cast');
 var castUpdate = require('./services/query/castUpdate');
+var hasDollarKeys = require('./services/query/hasDollarKeys');
 var helpers = require('./queryhelpers');
 var mquery = require('mquery');
 var readPref = require('./drivers').ReadPreference;
@@ -2001,29 +2002,36 @@ Query.prototype._findAndModify = function(type, callback) {
       opts.remove = false;
     }
 
-    castedDoc = castDoc(this, opts.overwrite);
-    castedDoc = setDefaultsOnInsert(this, schema, castedDoc, opts);
-    if (!castedDoc) {
-      if (opts.upsert) {
-        // still need to do the upsert to empty doc
-        var doc = utils.clone(castedQuery);
-        delete doc._id;
-        castedDoc = {$set: doc};
-      } else {
-        return this.findOne(callback);
-      }
-    } else if (castedDoc instanceof Error) {
-      return callback(castedDoc);
+    if (opts.overwrite && !hasDollarKeys(this._update)) {
+      castedDoc = new model(this._update, null, true);
+      doValidate = function(callback) {
+        castedDoc.validate(callback);
+      };
     } else {
-      // In order to make MongoDB 2.6 happy (see
-      // https://jira.mongodb.org/browse/SERVER-12266 and related issues)
-      // if we have an actual update document but $set is empty, junk the $set.
-      if (castedDoc.$set && Object.keys(castedDoc.$set).length === 0) {
-        delete castedDoc.$set;
+      castedDoc = castDoc(this, opts.overwrite);
+      castedDoc = setDefaultsOnInsert(this, schema, castedDoc, opts);
+      if (!castedDoc) {
+        if (opts.upsert) {
+          // still need to do the upsert to empty doc
+          var doc = utils.clone(castedQuery);
+          delete doc._id;
+          castedDoc = {$set: doc};
+        } else {
+          return this.findOne(callback);
+        }
+      } else if (castedDoc instanceof Error) {
+        return callback(castedDoc);
+      } else {
+        // In order to make MongoDB 2.6 happy (see
+        // https://jira.mongodb.org/browse/SERVER-12266 and related issues)
+        // if we have an actual update document but $set is empty, junk the $set.
+        if (castedDoc.$set && Object.keys(castedDoc.$set).length === 0) {
+          delete castedDoc.$set;
+        }
       }
-    }
 
-    doValidate = updateValidators(this, schema, castedDoc, opts);
+      doValidate = updateValidators(this, schema, castedDoc, opts);
+    }
   }
 
   this._applyPaths();
@@ -2181,7 +2189,14 @@ Query.prototype._execUpdate = function(callback) {
 
   if (this.options.runValidators) {
     _this = this;
-    doValidate = updateValidators(this, schema, castedDoc, options);
+    if (this.options.overwrite && !hasDollarKeys(castedDoc)) {
+      castedDoc = new _this.model(castedDoc, null, true);
+      doValidate = function(callback) {
+        castedDoc.validate(callback);
+      };
+    } else {
+      doValidate = updateValidators(this, schema, castedDoc, options);
+    }
     var _callback = function(err) {
       if (err) {
         return callback(err);

lib/services/query/hasDollarKeys.js

@@ -0,0 +1,12 @@
+'use strict';
+
+module.exports = function(obj) {
+  var keys = Object.keys(obj);
+  var len = keys.length;
+  for (var i = 0; i < len; ++i) {
+    if (keys[i].charAt(0) === '$') {
+      return true;
+    }
+  }
+  return false;
+};

test/model.findOneAndUpdate.test.js

@@ -1828,6 +1828,27 @@ describe('model: findOneAndUpdate:', function() {
         });
     });
 
+    it('overwrite doc with update validators (gh-3556)', function(done) {
+      var testSchema = new Schema({
+        name: {
+          type: String,
+          required: true
+        },
+        otherName: String
+      });
+      var Test = db.model('gh3556', testSchema);
+
+      var opts = { overwrite: true, runValidators: true };
+      Test.findOneAndUpdate({}, { otherName: 'test' }, opts, function(error) {
+        assert.ok(error);
+        assert.ok(error.errors['name']);
+        Test.findOneAndUpdate({}, { $set: { otherName: 'test' } }, opts, function(error) {
+          assert.ifError(error);
+          done();
+        });
+      });
+    });
+
     it('properly handles casting nested objects in update (gh-4724)', function(done) {
       var locationSchema = new Schema({
         _id: false,

test/model.update.test.js

@@ -2465,6 +2465,27 @@ describe('model: update:', function() {
       });
     });
 
+    it('overwrite doc with update validators (gh-3556)', function(done) {
+      var testSchema = new Schema({
+        name: {
+          type: String,
+          required: true
+        },
+        otherName: String
+      });
+      var Test = db.model('gh3556', testSchema);
+
+      var opts = { overwrite: true, runValidators: true };
+      Test.update({}, { otherName: 'test' }, opts, function(error) {
+        assert.ok(error);
+        assert.ok(error.errors['name']);
+        Test.update({}, { $set: { otherName: 'test' } }, opts, function(error) {
+          assert.ifError(error);
+          done();
+        });
+      });
+    });
+
     it('does not fail if passing whole doc (gh-5088)', function(done) {
       var schema = new Schema({
         username: String,