Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(document): add validateAllPaths option to validate() and validateSync() #14467

Merged
merged 1 commit into from
Mar 27, 2024
Merged

feat(document): add validateAllPaths option to validate() and validateSync() #14467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 125 additions & 54 deletions lib/document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2756,47 +2756,7 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {

// gh-661: if a whole array is modified, make sure to run validation on all
// the children as well
for (const path of paths) {
const _pathType = doc.$__schema.path(path);
if (!_pathType) {
continue;
}

if (!_pathType.$isMongooseArray ||
// To avoid potential performance issues, skip doc arrays whose children
// are not required. `getPositionalPathType()` may be slow, so avoid
// it unless we have a case of #6364
(!Array.isArray(_pathType) &&
_pathType.$isMongooseDocumentArray &&
!(_pathType && _pathType.schemaOptions && _pathType.schemaOptions.required))) {
continue;
}

// gh-11380: optimization. If the array isn't a document array and there's no validators
// on the array type, there's no need to run validation on the individual array elements.
if (_pathType.$isMongooseArray &&
!_pathType.$isMongooseDocumentArray && // Skip document arrays...
!_pathType.$embeddedSchemaType.$isMongooseArray && // and arrays of arrays
_pathType.$embeddedSchemaType.validators.length === 0) {
continue;
}

const val = doc.$__getValue(path);
_pushNestedArrayPaths(val, paths, path);
}

function _pushNestedArrayPaths(val, paths, path) {
if (val != null) {
const numElements = val.length;
for (let j = 0; j < numElements; ++j) {
if (Array.isArray(val[j])) {
_pushNestedArrayPaths(val[j], paths, path + '.' + j);
} else {
paths.add(path + '.' + j);
}
}
}
}
_addArrayPathsToValidate(doc, paths);

const flattenOptions = { skipArrays: true };
for (const pathToCheck of paths) {
Expand Down Expand Up @@ -2841,12 +2801,55 @@ function _getPathsToValidate(doc, pathsToValidate, pathsToSkip) {
return [paths, doValidateOptions];
}

function _addArrayPathsToValidate(doc, paths) {
for (const path of paths) {
const _pathType = doc.$__schema.path(path);
if (!_pathType) {
continue;
}

if (!_pathType.$isMongooseArray ||
// To avoid potential performance issues, skip doc arrays whose children
// are not required. `getPositionalPathType()` may be slow, so avoid
// it unless we have a case of #6364
(!Array.isArray(_pathType) &&
_pathType.$isMongooseDocumentArray &&
!(_pathType && _pathType.schemaOptions && _pathType.schemaOptions.required))) {
continue;
}

// gh-11380: optimization. If the array isn't a document array and there's no validators
// on the array type, there's no need to run validation on the individual array elements.
if (_pathType.$isMongooseArray &&
!_pathType.$isMongooseDocumentArray && // Skip document arrays...
!_pathType.$embeddedSchemaType.$isMongooseArray && // and arrays of arrays
_pathType.$embeddedSchemaType.validators.length === 0) {
continue;
}

const val = doc.$__getValue(path);
_pushNestedArrayPaths(val, paths, path);
}
}

function _pushNestedArrayPaths(val, paths, path) {
if (val != null) {
const numElements = val.length;
for (let j = 0; j < numElements; ++j) {
if (Array.isArray(val[j])) {
_pushNestedArrayPaths(val[j], paths, path + '.' + j);
} else {
paths.add(path + '.' + j);
}
}
}
}

/*!
* ignore
*/

Document.prototype.$__validate = function(pathsToValidate, options, callback) {

if (this.$__.saveOptions && this.$__.saveOptions.pathsToSave && !pathsToValidate) {
pathsToValidate = [...this.$__.saveOptions.pathsToSave];
} else if (typeof pathsToValidate === 'function') {
Expand All @@ -2871,6 +2874,19 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
shouldValidateModifiedOnly = this.$__schema.options.validateModifiedOnly;
}

const validateAllPaths = options && options.validateAllPaths;
if (validateAllPaths) {
if (pathsToSkip) {
throw new TypeError('Cannot set both `validateAllPaths` and `pathsToSkip`');
}
if (pathsToValidate) {
throw new TypeError('Cannot set both `validateAllPaths` and `pathsToValidate`');
}
if (hasValidateModifiedOnlyOption && shouldValidateModifiedOnly) {
throw new TypeError('Cannot set both `validateAllPaths` and `validateModifiedOnly`');
}
}

const _this = this;
const _complete = () => {
let validationError = this.$__.validationError;
Expand Down Expand Up @@ -2908,11 +2924,33 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
};

// only validate required fields when necessary
const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip);
const paths = shouldValidateModifiedOnly ?
pathDetails[0].filter((path) => this.$isModified(path)) :
pathDetails[0];
const doValidateOptionsByPath = pathDetails[1];
let paths;
let doValidateOptionsByPath;
if (validateAllPaths) {
paths = new Set(Object.keys(this.$__schema.paths));
// gh-661: if a whole array is modified, make sure to run validation on all
// the children as well
for (const path of paths) {
const schemaType = this.$__schema.path(path);
if (!schemaType || !schemaType.$isMongooseArray) {
continue;
}
const val = this.$__getValue(path);
if (!val) {
continue;
}
_pushNestedArrayPaths(val, paths, path);
}
paths = [...paths];
doValidateOptionsByPath = {};
} else {
const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip);
paths = shouldValidateModifiedOnly ?
pathDetails[0].filter((path) => this.$isModified(path)) :
pathDetails[0];
doValidateOptionsByPath = pathDetails[1];
}

if (typeof pathsToValidate === 'string') {
pathsToValidate = pathsToValidate.split(' ');
}
Expand Down Expand Up @@ -2993,7 +3031,8 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) {
const doValidateOptions = {
...doValidateOptionsByPath[path],
path: path,
validateModifiedOnly: shouldValidateModifiedOnly
validateModifiedOnly: shouldValidateModifiedOnly,
validateAllPaths
};

schemaType.doValidate(val, function(err) {
Expand Down Expand Up @@ -3111,6 +3150,16 @@ Document.prototype.validateSync = function(pathsToValidate, options) {

let pathsToSkip = options && options.pathsToSkip;

const validateAllPaths = options && options.validateAllPaths;
if (validateAllPaths) {
if (pathsToSkip) {
throw new TypeError('Cannot set both `validateAllPaths` and `pathsToSkip`');
}
if (pathsToValidate) {
throw new TypeError('Cannot set both `validateAllPaths` and `pathsToValidate`');
}
}

if (typeof pathsToValidate === 'string') {
const isOnePathOnly = pathsToValidate.indexOf(' ') === -1;
pathsToValidate = isOnePathOnly ? [pathsToValidate] : pathsToValidate.split(' ');
Expand All @@ -3119,11 +3168,32 @@ Document.prototype.validateSync = function(pathsToValidate, options) {
}

// only validate required fields when necessary
const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip);
const paths = shouldValidateModifiedOnly ?
pathDetails[0].filter((path) => this.$isModified(path)) :
pathDetails[0];
const skipSchemaValidators = pathDetails[1];
let paths;
let skipSchemaValidators;
if (validateAllPaths) {
paths = new Set(Object.keys(this.$__schema.paths));
// gh-661: if a whole array is modified, make sure to run validation on all
// the children as well
for (const path of paths) {
const schemaType = this.$__schema.path(path);
if (!schemaType || !schemaType.$isMongooseArray) {
continue;
}
const val = this.$__getValue(path);
if (!val) {
continue;
}
_pushNestedArrayPaths(val, paths, path);
}
paths = [...paths];
skipSchemaValidators = {};
} else {
const pathDetails = _getPathsToValidate(this, pathsToValidate, pathsToSkip);
paths = shouldValidateModifiedOnly ?
pathDetails[0].filter((path) => this.$isModified(path)) :
pathDetails[0];
skipSchemaValidators = pathDetails[1];
}

const validating = {};

Expand All @@ -3148,7 +3218,8 @@ Document.prototype.validateSync = function(pathsToValidate, options) {
const err = p.doValidateSync(val, _this, {
skipSchemaValidators: skipSchemaValidators[path],
path: path,
validateModifiedOnly: shouldValidateModifiedOnly
validateModifiedOnly: shouldValidateModifiedOnly,
validateAllPaths
});
if (err) {
const isSubdoc = p.$isSingleNested ||
Expand Down
4 changes: 2 additions & 2 deletions lib/schema/documentArray.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ SchemaDocumentArray.prototype.doValidate = function(array, fn, scope, options) {
continue;
}

doc.$__validate(callback);
doc.$__validate(null, options, callback);
}
}
};
Expand Down Expand Up @@ -330,7 +330,7 @@ SchemaDocumentArray.prototype.doValidateSync = function(array, scope, options) {
continue;
}

const subdocValidateError = doc.validateSync();
const subdocValidateError = doc.validateSync(options);

if (subdocValidateError && resultError == null) {
resultError = subdocValidateError;
Expand Down
118 changes: 118 additions & 0 deletions test/document.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13087,6 +13087,124 @@ describe('document', function() {
const savedDoc = await MainModel.findById(doc.id).orFail();
assert.strictEqual(savedDoc.sub, null);
});

it('validate supports validateAllPaths', async function() {
const schema = new mongoose.Schema({
name: {
type: String,
validate: v => !!v
},
age: {
type: Number,
validate: v => v == null || v < 200
},
subdoc: {
type: new Schema({
url: String
}, { _id: false }),
validate: v => v == null || v.url.length > 0
},
docArr: [{
subprop: {
type: String,
validate: v => v == null || v.length > 0
}
}]
});

const TestModel = db.model('Test', schema);

const doc = await TestModel.create({});
doc.name = '';
doc.age = 201;
doc.subdoc = { url: '' };
doc.docArr = [{ subprop: '' }];
await doc.save({ validateBeforeSave: false });
await doc.validate();

const err = await doc.validate({ validateAllPaths: true }).then(() => null, err => err);
assert.ok(err);
assert.equal(err.name, 'ValidationError');
assert.ok(err.errors['name']);
assert.ok(
err.errors['name'].message.includes('Validator failed for path `name` with value ``'),
err.errors['name'].message
);
assert.ok(err.errors['age']);
assert.ok(
err.errors['age'].message.includes('Validator failed for path `age` with value `201`'),
err.errors['age'].message
);
assert.ok(err.errors['subdoc']);
assert.ok(
err.errors['subdoc'].message.includes('Validator failed for path `subdoc` with value `{ url: \'\' }`'),
err.errors['subdoc'].message
);
assert.ok(err.errors['docArr.0.subprop']);
assert.ok(
err.errors['docArr.0.subprop'].message.includes('Validator failed for path `subprop` with value ``'),
err.errors['docArr.0.subprop'].message
);
});

it('validateSync() supports validateAllPaths', async function() {
const schema = new mongoose.Schema({
name: {
type: String,
validate: v => !!v
},
age: {
type: Number,
validate: v => v == null || v < 200
},
subdoc: {
type: new Schema({
url: String
}, { _id: false }),
validate: v => v == null || v.url.length > 0
},
docArr: [{
subprop: {
type: String,
validate: v => v == null || v.length > 0
}
}]
});

const TestModel = db.model('Test', schema);

const doc = await TestModel.create({});
doc.name = '';
doc.age = 201;
doc.subdoc = { url: '' };
doc.docArr = [{ subprop: '' }];
await doc.save({ validateBeforeSave: false });
await doc.validate();

const err = await doc.validateSync({ validateAllPaths: true });
assert.ok(err);
assert.equal(err.name, 'ValidationError');
assert.ok(err.errors['name']);
assert.ok(
err.errors['name'].message.includes('Validator failed for path `name` with value ``'),
err.errors['name'].message
);
assert.ok(err.errors['age']);
assert.ok(
err.errors['age'].message.includes('Validator failed for path `age` with value `201`'),
err.errors['age'].message
);
assert.ok(err.errors['subdoc']);
assert.ok(
err.errors['subdoc'].message.includes('Validator failed for path `subdoc` with value `{ url: \'\' }`'),
err.errors['subdoc'].message
);
assert.ok(err.errors['docArr.0.subprop']);
assert.ok(
err.errors['docArr.0.subprop'].message.includes('Validator failed for path `subprop` with value ``'),
err.errors['docArr.0.subprop'].message
);
});
});

describe('Check if instance function that is supplied in schema option is availabe', function() {
Expand Down
Loading