Skip to content

Commit

Permalink
feat(query): support runValidators as a global option
Browse files Browse the repository at this point in the history
Fix #6578
  • Loading branch information
vkarpov15 committed Jun 29, 2018
1 parent 4b905a9 commit f8686d1
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ Mongoose.prototype.STATES = STATES;
* - 'cloneSchemas': false by default. Set to `true` to `clone()` all schemas before compiling into a model.
* - 'applyPluginsToDiscriminators': false by default. Set to true to apply global plugins to discriminator schemas. This typically isn't necessary because plugins are applied to the base schema and discriminators copy all middleware, methods, statics, and properties from the base schema.
* - 'objectIdGetter': true by default. Mongoose adds a getter to MongoDB ObjectId's called `_id` that returns `this` for convenience with populate. Set this to false to remove the getter.
* - 'runValidators': false by default. Set to true to enable [update validators](/docs/validation.html#update-validators) for all validators by default.
*
* @param {String} key
* @param {String|Function|Boolean} value
Expand Down
24 changes: 21 additions & 3 deletions lib/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -2417,6 +2417,22 @@ Query.prototype._findOneAndRemove = function(callback) {
this._findAndModify('remove', callback);
};

/*!
* Get options from query opts, falling back to the base mongoose object.
*/

function _getOption(query, option, def) {
const opts = query._optionsForExec(query.model);

if (option in opts) {
return opts[option];
}
if ('option' in query.model.base.options) {
return query.model.base.options[option];
}
return def;
}

/*!
* Override mquery.prototype._findAndModify to provide casting etc.
*
Expand Down Expand Up @@ -2525,6 +2541,7 @@ Query.prototype._findAndModify = function(type, callback) {
var _callback;

var useFindAndModify = true;
var runValidators = _getOption(this, 'runValidators', false);
var base = _this.model && _this.model.base;
if ('useFindAndModify' in base.options) {
useFindAndModify = base.get('useFindAndModify');
Expand Down Expand Up @@ -2552,7 +2569,7 @@ Query.prototype._findAndModify = function(type, callback) {
return this;
}

if (opts.runValidators && doValidate) {
if (runValidators && doValidate) {
_callback = function(error) {
if (error) {
return callback(error);
Expand Down Expand Up @@ -2582,7 +2599,7 @@ Query.prototype._findAndModify = function(type, callback) {
return this;
}

if (opts.runValidators && doValidate) {
if (runValidators && doValidate) {
_callback = function(error) {
if (error) {
return callback(error);
Expand Down Expand Up @@ -2707,7 +2724,8 @@ function _updateThunk(op, callback) {
castedDoc, options);
}

if (this.options.runValidators) {
var runValidators = _getOption(this, 'runValidators', false);
if (runValidators) {
if (isOverwriting) {
doValidate = function(callback) {
castedDoc.validate(callback);
Expand Down

5 comments on commit f8686d1

@AbdelrahmanHafez
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to find the test for that option, 4b905a9 seems to test passing the option on the update instead of as a global option.

@vkarpov15
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbdelrahmanHafez thanks for reporting, I missed a test case for that. Will add a test #6865 . Is there a problem with this option?

@vkarpov15
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbdelrahmanHafez found the issue and added a test, fix will be in 5.2.9, ETA Thursday

@vkarpov15
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbdelrahmanHafez
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I missed that. Thank you.

Please sign in to comment.