Merge pull request #13827 from Automattic/vkarpov15/gh-12668

BREAKING CHANGE: make `Model.validate()` use `Model.castObject()` to cast, and return casted copy of object instead of modifying in place

lib/model.js

@@ -4064,7 +4064,7 @@ Model.aggregate = function aggregate(pipeline, options) {
  * @param {Object} obj
  * @param {Object|Array|String} pathsOrOptions
  * @param {Object} [context]
- * @return {Promise|undefined}
+ * @return {Promise<Object>} casted and validated copy of `obj` if validation succeeded
  * @api public
  */
 
@@ -4123,8 +4123,19 @@ Model.validate = async function validate(obj, pathsOrOptions, context) {
     pushNestedArrayPaths(paths, val, path);
   }
 
-  let remaining = paths.length;
   let error = null;
+  paths = new Set(paths);
+
+  try {
+    obj = this.castObject(obj);
+  } catch (err) {
+    error = err;
+    for (const key of Object.keys(error.errors || {})) {
+      paths.delete(key);
+    }
+  }
+
+  let remaining = paths.size;
 
   return new Promise((resolve, reject) => {
     for (const path of paths) {
@@ -4140,20 +4151,7 @@ Model.validate = async function validate(obj, pathsOrOptions, context) {
         cur = cur[pieces[i]];
       }
 
-      let val = get(obj, path, void 0);
-
-      if (val != null) {
-        try {
-          val = schemaType.cast(val);
-          cur[pieces[pieces.length - 1]] = val;
-        } catch (err) {
-          error = error || new ValidationError();
-          error.addError(path, err);
-
-          _checkDone();
-          continue;
-        }
-      }
+      const val = get(obj, path, void 0);
 
       schemaType.doValidate(val, err => {
         if (err) {
@@ -4169,7 +4167,7 @@ Model.validate = async function validate(obj, pathsOrOptions, context) {
         if (error) {
           reject(error);
         } else {
-          resolve();
+          resolve(obj);
         }
       }
     }

test/model.validate.test.js

@@ -57,7 +57,7 @@ describe('model: validate: ', function() {
     assert.deepEqual(Object.keys(err.errors), ['comments.name']);
 
     obj = { age: '42' };
-    await Model.validate(obj, ['age']);
+    obj = await Model.validate(obj, ['age']);
     assert.strictEqual(obj.age, 42);
   });
 
@@ -106,7 +106,7 @@ describe('model: validate: ', function() {
 
     const test = { docs: ['6132655f2cdb9d94eaebc09b'] };
 
-    const err = await Test.validate(test);
+    const err = await Test.validate(test).then(() => null, err => err);
     assert.ifError(err);
   });
 
@@ -124,7 +124,7 @@ describe('model: validate: ', function() {
     const User = mongoose.model('User', userSchema);
 
     const user = new User({ name: 'test', nameRequired: false });
-    const err = await User.validate(user).catch(err => err);
+    const err = await User.validate(user).then(() => null, err => err);
 
     assert.ifError(err);
 
@@ -181,6 +181,29 @@ describe('model: validate: ', function() {
     await assert.rejects(async() => {
       await Test.validate(test, pathsOrOptions);
     }, { message: 'Validation failed: name: Validator failed for path `name` with value `1`' });
+  });
 
+  it('runs validation on casted paths even if cast error happened', async function() {
+    const Model = mongoose.model('Test', new Schema({
+      invalid1: {
+        type: String,
+        validate: () => false
+      },
+      myNumber: {
+        type: Number,
+        required: true
+      },
+      invalid2: {
+        type: String,
+        validate: () => false
+      }
+    }));
+
+    const err = await Model.validate({ invalid1: 'foo', myNumber: 'not a number', invalid2: 'bar' }).
+      then(() => null, err => err);
+    assert.ok(err);
+    assert.deepEqual(Object.keys(err.errors).sort(), ['invalid1', 'invalid2', 'myNumber']);
+    assert.equal(err.errors['myNumber'].name, 'CastError');
+    assert.equal(err.errors['invalid1'].name, 'ValidatorError');
   });
 });