Merge pull request #13167 from Automattic/vkarpov15/gh-13159

Validate array elements when passing array path to `validateSync()` in `pathsToValidate`
Automattic · Mar 14, 2023 · c05ba1a · c05ba1a
2 parents 2fedff1 + dcf7209
commit c05ba1a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 25 deletions.
diff --git a/lib/document.js b/lib/document.js
@@ -2557,7 +2557,7 @@ function _evaluateRequiredFunctions(doc) {
  * ignore
  */
 
-function _getPathsToValidate(doc) {
+function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {
   const skipSchemaValidators = {};
 
   _evaluateRequiredFunctions(doc);
@@ -2628,6 +2628,22 @@ function _getPathsToValidate(doc) {
     }
   }
 
+  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;
+    }
+  }
+
+  if (Array.isArray(pathsToValidate)) {
+    paths = _handlePathsToValidate(paths, pathsToValidate);
+  } else if (Array.isArray(pathsToSkip)) {
+    paths = _handlePathsToSkip(paths, pathsToSkip);
+  }
+
   // 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
@@ -2687,13 +2703,6 @@ function _getPathsToValidate(doc) {
   }
 
   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;
@@ -2776,19 +2785,14 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
   };
 
   // only validate required fields when necessary
-  const pathDetails = _getPathsToValidate(this);
+  const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip);
   let paths = shouldValidateModifiedOnly ?
     pathDetails[0].filter((path) => this.$isModified(path)) :
     pathDetails[0];
   const skipSchemaValidators = pathDetails[1];
   if (typeof pathsToValidate === 'string') {
     pathsToValidate = pathsToValidate.split(' ');
   }
-  if (Array.isArray(pathsToValidate)) {
-    paths = _handlePathsToValidate(paths, pathsToValidate);
-  } else if (pathsToSkip) {
-    paths = _handlePathsToSkip(paths, pathsToSkip);
-  }
 
   if (paths.length === 0) {
     return immediate(function() {
@@ -2907,12 +2911,12 @@ function _handlePathsToValidate(paths, pathsToValidate) {
     }
   }
 
-  const ret = [];
+  const ret = new Set();
   for (const path of paths) {
     if (_pathsToValidate.has(path)) {
-      ret.push(path);
+      ret.add(path);
     } else if (parentPaths.has(path)) {
-      ret.push(parentPaths.get(path));
+      ret.add(parentPaths.get(path));
     }
   }
   return ret;
@@ -2924,8 +2928,8 @@ function _handlePathsToValidate(paths, pathsToValidate) {
 
 function _handlePathsToSkip(paths, pathsToSkip) {
   pathsToSkip = new Set(pathsToSkip);
-  paths = paths.filter(p => !pathsToSkip.has(p));
-  return paths;
+  paths = Array.from(paths).filter(p => !pathsToSkip.has(p));
+  return new Set(paths);
 }
 
 /**
@@ -2981,17 +2985,12 @@ Document.prototype.validateSync = function(pathsToValidate, options) {
   }
 
   // only validate required fields when necessary
-  const pathDetails = _getPathsToValidate(this);
+  const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip);
   let paths = shouldValidateModifiedOnly ?
     pathDetails[0].filter((path) => this.$isModified(path)) :
     pathDetails[0];
   const skipSchemaValidators = pathDetails[1];
 
-  if (Array.isArray(pathsToValidate)) {
-    paths = _handlePathsToValidate(paths, pathsToValidate);
-  } else if (Array.isArray(pathsToSkip)) {
-    paths = _handlePathsToSkip(paths, pathsToSkip);
-  }
   const validating = {};
 
   for (let i = 0, len = paths.length; i < len; ++i) {

diff --git a/test/schema.validation.test.js b/test/schema.validation.test.js
@@ -1134,6 +1134,23 @@ describe('schema', function() {
       done();
     });
 
+    it('validateSync validates array elements when setting pathsToValidate (gh-13159)', function() {
+      const schema = new Schema({
+        permissions: [{ type: String, enum: ['users', 'anotherPermission'] }]
+      });
+
+      const Model = mongoose.model('gh13159', schema);
+
+      const doc = new Model({
+        permissions: ['avocado']
+      });
+
+      const error = doc.validateSync('permissions');
+      assert.ok(error);
+      assert.equal(Object.keys(error.errors).length, 1);
+      assert.ok(error.errors['permissions.0']);
+    });
+
     it('adds required validators to the front of the list (gh-2843)', async function() {
       const breakfast = new Schema({ description: { type: String, maxlength: 50, required: true } });