fix(document): dont double-validate doc array elements

Fix #4145
Automattic · May 13, 2016 · b6be960 · b6be960
1 parent 8d28cdb
commit b6be960
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 57 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -1115,6 +1115,59 @@ Document.prototype.validate = function(options, callback) {
   });
 };
 
+/*!
+ * ignore
+ */
+
+function _getPathsToValidate(doc) {
+  // only validate required fields when necessary
+  var paths = Object.keys(doc.$__.activePaths.states.require).filter(function(path) {
+    if (!doc.isSelected(path) && !doc.isModified(path)) {
+      return false;
+    }
+    var p = doc.schema.path(path);
+    if (typeof p.originalRequiredValue === 'function') {
+      return p.originalRequiredValue.call(doc);
+    }
+    return true;
+  });
+
+  paths = paths.concat(Object.keys(doc.$__.activePaths.states.init));
+  paths = paths.concat(Object.keys(doc.$__.activePaths.states.modify));
+  paths = paths.concat(Object.keys(doc.$__.activePaths.states.default));
+
+  // gh-661: if a whole array is modified, make sure to run validation on all
+  // the children as well
+  for (var i = 0; i < paths.length; ++i) {
+    var path = paths[i];
+    var val = doc.getValue(path);
+    if (val && val.isMongooseArray && !Buffer.isBuffer(val) && !val.isMongooseDocumentArray) {
+      var numElements = val.length;
+      for (var j = 0; j < numElements; ++j) {
+        paths.push(path + '.' + j);
+      }
+    }
+  }
+
+  var flattenOptions = { skipArrays: true };
+  for (i = 0; i < paths.length; ++i) {
+    var pathToCheck = paths[i];
+    if (doc.schema.nested[pathToCheck]) {
+      var _v = doc.getValue(pathToCheck);
+      if (isMongooseObject(_v)) {
+        _v = _v.toObject({ virtuals: false });
+      }
+      var flat = flatten(_v, '', flattenOptions);
+      var _subpaths = Object.keys(flat).map(function(p) {
+        return pathToCheck + '.' + p;
+      });
+      paths = paths.concat(_subpaths);
+    }
+  }
+
+  return paths;
+}
+
 /*!
  * ignore
  */
@@ -1138,20 +1191,7 @@ Document.prototype.$__validate = function(callback) {
   };
 
   // only validate required fields when necessary
-  var paths = Object.keys(this.$__.activePaths.states.require).filter(function(path) {
-    if (!_this.isSelected(path) && !_this.isModified(path)) {
-      return false;
-    }
-    var p = _this.schema.path(path);
-    if (typeof p.originalRequiredValue === 'function') {
-      return p.originalRequiredValue.call(_this);
-    }
-    return true;
-  });
-
-  paths = paths.concat(Object.keys(this.$__.activePaths.states.init));
-  paths = paths.concat(Object.keys(this.$__.activePaths.states.modify));
-  paths = paths.concat(Object.keys(this.$__.activePaths.states.default));
+  var paths = _getPathsToValidate(this);
 
   if (paths.length === 0) {
     process.nextTick(function() {
@@ -1167,33 +1207,6 @@ Document.prototype.$__validate = function(callback) {
   var validating = {},
       total = 0;
 
-  // gh-661: if a whole array is modified, make sure to run validation on all
-  // the children as well
-  for (var i = 0; i < paths.length; ++i) {
-    var path = paths[i];
-    var val = _this.getValue(path);
-    if (val && val.isMongooseArray && !Buffer.isBuffer(val) && !val.isMongooseDocumentArray) {
-      var numElements = val.length;
-      for (var j = 0; j < numElements; ++j) {
-        paths.push(path + '.' + j);
-      }
-    }
-  }
-
-  for (i = 0; i < paths.length; ++i) {
-    var pathToCheck = paths[i];
-    if (this.schema.nested[pathToCheck]) {
-      var _v = this.getValue(pathToCheck);
-      if (isMongooseObject(_v)) {
-        _v = _v.toObject({ virtuals: false });
-      }
-      var _subpaths = Object.keys(flatten(_v)).map(function(p) {
-        return pathToCheck + '.' + p;
-      });
-      paths = paths.concat(_subpaths);
-    }
-  }
-
   var complete = function() {
     var err = _complete();
     if (err) {
@@ -1265,20 +1278,7 @@ Document.prototype.validateSync = function(pathsToValidate) {
   }
 
   // only validate required fields when necessary
-  var paths = Object.keys(this.$__.activePaths.states.require).filter(function(path) {
-    if (!_this.isSelected(path) && !_this.isModified(path)) {
-      return false;
-    }
-    var p = _this.schema.path(path);
-    if (typeof p.originalRequiredValue === 'function') {
-      return p.originalRequiredValue.call(_this);
-    }
-    return true;
-  });
-
-  paths = paths.concat(Object.keys(this.$__.activePaths.states.init));
-  paths = paths.concat(Object.keys(this.$__.activePaths.states.modify));
-  paths = paths.concat(Object.keys(this.$__.activePaths.states.default));
+  var paths = _getPathsToValidate(this);
 
   if (pathsToValidate && pathsToValidate.length) {
     var tmp = [];

diff --git a/lib/services/common.js b/lib/services/common.js
@@ -14,7 +14,7 @@ exports.modifiedPaths = modifiedPaths;
  * ignore
  */
 
-function flatten(update, path) {
+function flatten(update, path, options) {
   var keys = Object.keys(update || {});
   var numKeys = keys.length;
   var result = {};
@@ -27,6 +27,9 @@ function flatten(update, path) {
       val = val.toObject({ virtuals: false });
     }
     if (shouldFlatten(val)) {
+      if (options && options.skipArrays && Array.isArray(val)) {
+        continue;
+      }
       var flat = flatten(val, path + key);
       for (var k in flat) {
         result[k] = flat[k];

diff --git a/test/schema.validation.test.js b/test/schema.validation.test.js
@@ -856,7 +856,8 @@ describe('schema', function() {
       var myValidator = function() {
         ++calls;
         return true;
-      }
+      };
+
       var InnerSchema = new mongoose.Schema({
         myfield: {
           type: String,