Merge pull request #8548 from taxilian/feature/single_validate_subpaths_and_instrumentation

Feature/single validate subpaths and instrumentation

Author: vkarpov15

lib/document.js

@@ -2030,9 +2030,12 @@ Document.prototype.validate = function(pathsToValidate, options, callback) {
   let parallelValidate;
 
   if (this.$__.validating) {
-    parallelValidate = new ParallelValidateError(this);
+    parallelValidate = new ParallelValidateError(this, {
+      parentStack: options && options.parentStack,
+      conflictStack: this.$__.validating.stack
+    });
   } else {
-    this.$__.validating = new ParallelValidateError(this);
+    this.$__.validating = new ParallelValidateError(this, {parentStack: options && options.parentStack});
   }
 
   if (typeof pathsToValidate === 'function') {
@@ -2076,53 +2079,54 @@ function _evaluateRequiredFunctions(doc) {
  */
 
 function _getPathsToValidate(doc) {
-  let i;
-  let len;
   const skipSchemaValidators = {};
 
   _evaluateRequiredFunctions(doc);
 
   // only validate required fields when necessary
-  let paths = Object.keys(doc.$__.activePaths.states.require).filter(function(path) {
+  let paths = new Set(Object.keys(doc.$__.activePaths.states.require).filter(function(path) {
     if (!doc.isSelected(path) && !doc.isModified(path)) {
       return false;
     }
     if (path in doc.$__.cachedRequired) {
       return doc.$__.cachedRequired[path];
     }
     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));
+  function addToPaths(p) { paths.add(p); }
+  Object.keys(doc.$__.activePaths.states.init).forEach(addToPaths);
+  Object.keys(doc.$__.activePaths.states.modify).forEach(addToPaths);
+  Object.keys(doc.$__.activePaths.states.default).forEach(addToPaths);
 
   const subdocs = doc.$__getAllSubdocs();
-  let subdoc;
-  len = subdocs.length;
   const modifiedPaths = doc.modifiedPaths();
-  for (i = 0; i < len; ++i) {
-    subdoc = subdocs[i];
-    if (subdoc.$basePath &&
-        doc.isModified(subdoc.$basePath, modifiedPaths) &&
-        !doc.isDirectModified(subdoc.$basePath) &&
-        !doc.$isDefault(subdoc.$basePath)) {
+  for (const subdoc of subdocs) {
+    if (subdoc.$basePath) {
       // Remove child paths for now, because we'll be validating the whole
       // subdoc
-      paths = paths.filter(function(p) {
-        return p != null && p.indexOf(subdoc.$basePath + '.') !== 0;
-      });
-      paths.push(subdoc.$basePath); // This can cause duplicates, make unique below
-      skipSchemaValidators[subdoc.$basePath] = true;
+      for (const p of paths) {
+        if (p === null || p.startsWith(subdoc.$basePath + '.')) {
+          paths.delete(p);
+        }
+      }
+
+      if (doc.isModified(subdoc.$basePath, modifiedPaths) &&
+            !doc.isDirectModified(subdoc.$basePath) &&
+            !doc.$isDefault(subdoc.$basePath)) {
+        paths.add(subdoc.$basePath);
+
+        skipSchemaValidators[subdoc.$basePath] = true;
+      }
     }
   }
 
+  // from here on we're not removing items from paths
+
   // gh-661: if a whole array is modified, make sure to run validation on all
   // the children as well
-  len = paths.length;
-  for (i = 0; i < len; ++i) {
-    const path = paths[i];
-
+  for (const path of paths) {
     const _pathType = doc.schema.path(path);
     if (!_pathType ||
         !_pathType.$isMongooseArray ||
@@ -2144,34 +2148,33 @@ function _getPathsToValidate(doc) {
         if (Array.isArray(val[j])) {
           _pushNestedArrayPaths(val[j], paths, path + '.' + j);
         } else {
-          paths.push(path + '.' + j);
+          paths.add(path + '.' + j);
         }
       }
     }
   }
 
   const flattenOptions = { skipArrays: true };
-  len = paths.length;
-  for (i = 0; i < len; ++i) {
-    const pathToCheck = paths[i];
+  for (const pathToCheck of paths) {
     if (doc.schema.nested[pathToCheck]) {
       let _v = doc.$__getValue(pathToCheck);
       if (isMongooseObject(_v)) {
         _v = _v.toObject({ transform: false });
       }
       const flat = flatten(_v, pathToCheck, flattenOptions, doc.schema);
-      paths = paths.concat(Object.keys(flat));
+      Object.keys(flat).forEach(addToPaths);
     }
   }
 
-  // Single nested paths (paths embedded under single nested subdocs) will
-  // be validated on their own when we call `validate()` on the subdoc itself.
-  // Re: gh-8468
-  paths = paths.filter(p => !doc.schema.singleNestedPaths.hasOwnProperty(p));
 
-  len = paths.length;
-  for (i = 0; i < len; ++i) {
-    const path = paths[i];
+  for (const path of paths) {
+    // Single nested paths (paths embedded under single nested subdocs) will
+    // be validated on their own when we call `validate()` on the subdoc itself.
+    // Re: gh-8468
+    if (doc.schema.singleNestedPaths.hasOwnProperty(path)) {
+      paths.delete(path);
+      continue;
+    }
     const _pathType = doc.schema.path(path);
     if (!_pathType || !_pathType.$isSchemaMap) {
       continue;
@@ -2182,11 +2185,11 @@ function _getPathsToValidate(doc) {
       continue;
     }
     for (const key of val.keys()) {
-      paths.push(path + '.' + key);
+      paths.add(path + '.' + key);
     }
   }
 
-  paths = Array.from(new Set(paths));
+  paths = Array.from(paths);
   return [paths, skipSchemaValidators];
 }
 
@@ -2208,6 +2211,15 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
       (typeof options === 'object') &&
       ('validateModifiedOnly' in options);
 
+  const saveDeepStacktrace = (
+    options &&
+      (typeof options === 'object') &&
+      !!options['deepStackTrace'])
+      || !!this.schema.options.deepStackTrace;
+  const parentStack = saveDeepStacktrace && options &&
+      (typeof options === 'object') &&
+      options.parentStack || [];
+
   let shouldValidateModifiedOnly;
   if (hasValidateModifiedOnlyOption) {
     shouldValidateModifiedOnly = !!options.validateModifiedOnly;
@@ -2294,6 +2306,9 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
     validated[path] = true;
     total++;
 
+    const stackToHere = saveDeepStacktrace ?
+      [(new Error().stack), path].concat(parentStack) : void 0;
+
     process.nextTick(function() {
       const p = _this.schema.path(path);
 
@@ -2320,6 +2335,11 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
         _this.$__.pathsToScopes[path] :
         _this;
 
+      const doValidateOptions = {
+        skipSchemaValidators: skipSchemaValidators[path],
+        path: path,
+        parentStack: stackToHere
+      };
       p.doValidate(val, function(err) {
         if (err && (!p.$isMongooseDocumentArray || err.$isArrayValidatorError)) {
           if (p.$isSingleNested &&
@@ -2330,7 +2350,7 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
           _this.invalidate(path, err, undefined, true);
         }
         --total || complete();
-      }, scope, { skipSchemaValidators: skipSchemaValidators[path], path: path });
+      }, scope, doValidateOptions);
     });
   };
 

lib/error/parallelValidate.js

@@ -13,10 +13,18 @@ const MongooseError = require('./mongooseError');
  * @api private
  */
 
-function ParallelValidateError(doc) {
+function ParallelValidateError(doc, opts) {
   const msg = 'Can\'t validate() the same doc multiple times in parallel. Document: ';
   MongooseError.call(this, msg + doc._id);
   this.name = 'ParallelValidateError';
+  if (opts && opts.parentStack) {
+    // Provide a full async stack, most recent first
+    this.stack = this.stack + '\n\n' + opts.parentStack.join('\n\n');
+  }
+  // You need to know to look for this, but having it can be very helpful
+  // for tracking down issues when combined with the deepStackTrace schema
+  // option
+  this.conflictStack = opts && opts.conflictStack;
 }
 
 /*!

lib/schema.js

@@ -402,6 +402,9 @@ Schema.prototype.defaultOptions = function(options) {
     strict: 'strict' in baseOptions ? baseOptions.strict : true,
     bufferCommands: true,
     capped: false, // { size, max, autoIndexId }
+    // Setting to true may help with debugging but will have performance
+    // consequences
+    deepStackTrace: false,
     versionKey: '__v',
     discriminatorKey: '__t',
     minimize: true,

lib/schema/SingleNestedPath.js

@@ -237,9 +237,17 @@ SingleNestedPath.prototype.doValidate = function(value, fn, scope, options) {
     if (!(value instanceof Constructor)) {
       value = new Constructor(value, null, scope);
     }
-
-    return value.validate(fn);
+    try {
+      return value.validate(fn);
+    } catch (err) {
+      // Save the parent stack on the error
+      if (options.parentStack) {
+        err.parentStack = options.parentStack;
+      }
+      throw err;
+    }
   }
+  const parentStack = options && options.parentStack;
 
   SchemaType.prototype.doValidate.call(this, value, function(error) {
     if (error) {
@@ -249,8 +257,11 @@ SingleNestedPath.prototype.doValidate = function(value, fn, scope, options) {
       return fn(null);
     }
 
-    value.validate(fn);
-  }, scope);
+    value.validate(fn, {
+      deepStackTrace: !!parentStack,
+      parentStack: parentStack,
+    });
+  }, scope, options);
 };
 
 /**

lib/schema/documentarray.js

@@ -197,7 +197,7 @@ DocumentArrayPath.prototype.doValidate = function(array, fn, scope, options) {
 
   const _this = this;
   try {
-    SchemaType.prototype.doValidate.call(this, array, cb, scope);
+    SchemaType.prototype.doValidate.call(this, array, cb, scope, options);
   } catch (err) {
     err.$isArrayValidatorError = true;
     return fn(err);
@@ -251,7 +251,10 @@ DocumentArrayPath.prototype.doValidate = function(array, fn, scope, options) {
         doc = array[i] = new Constructor(doc, array, undefined, undefined, i);
       }
 
-      doc.$__validate(callback);
+      doc.$__validate(null, {
+        parentStack: options && options.parentStack,
+        deepStackTrace: !!(options && options.parentStack)
+      }, callback);
     }
   }
 };

lib/schematype.js

@@ -1060,6 +1060,9 @@ SchemaType.prototype.doValidate = function(value, fn, scope, options) {
       const ErrorConstructor = validatorProperties.ErrorConstructor || ValidatorError;
       err = new ErrorConstructor(validatorProperties);
       err[validatorErrorSymbol] = true;
+      if (options && options.parentStack) {
+        err.parentStack = options.parentStack;
+      }
       immediate(function() {
         fn(err);
       });
@@ -1101,6 +1104,9 @@ SchemaType.prototype.doValidate = function(value, fn, scope, options) {
           if (error.message) {
             validatorProperties.message = error.message;
           }
+          if (options && options.parentStack) {
+            validatorProperties.deepStack = options.parentStack;
+          }
         }
         if (ok != null && typeof ok.then === 'function') {
           ok.then(

test/document.test.js

@@ -586,7 +586,7 @@ describe('document', function() {
     const Model = db.model('gh8468-2', Schema({
       name: String,
       keys: [keySchema],
-    }));
+    }, {deepStackTrace: true}));
     const doc = new Model({
       name: 'test',
       keys: [