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.

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

Validators called with inconsistent "thisArg" between save/update logic #3430

Closed
viveleroi opened this issue Oct 3, 2015 · 4 comments
Closed
Milestone

Comments

@viveleroi
Copy link

When a user model calls save(), the method SchemaType.prototype.doValidate is given a scope value which is passed to the validators, validator.call(scope, ....

However, when using update methods and { runValidators: true }, the updateValidators code passes null for the scope.

This breaks the plugin because it's trying to call this.model inside the validator (necessary to dynamically query for any duplicate records).

I was looking for any alternative method to dynamically get a reference to the current model inside the validator, but am not seeing any - which led me to discover this issue.

      schema.path(updates[i]).doValidate(
        updatedValues[updates[i]],
        function(err) {
          if (err) {
            validationErrors.push(err);
          }
          callback(null);
        },
        null);
    });

To resolve, all calls should be able to ensure this is consistent, or a way to dynamically get a reference to the current model should be provided separately.

@vkarpov15
Copy link
Collaborator

See validation docs. That's one of the reasons why update validators are behind a flag - there's no real way to get consistent scope between document validators and update validators, because update validators by definition don't have the document being updated in memory. The best we can do is set the update validators scope to be the query, but it makes it easier to understand if we just explicitly set the scope to null.

@viveleroi
Copy link
Author

Thanks, that makes a lot of sense. Is there an alternate way to get a reference to the current model at least? Some method on the schema we could make use of? If not, can it be added? The plugin I was debugging uses that to build a new query for duplicate records.

@vkarpov15 vkarpov15 added this to the 4.2 milestone Oct 5, 2015
@vkarpov15
Copy link
Collaborator

Yeah that should be doable. We'd have to add an option, because the "null scope" behavior is the canonical way to determine "is this validation running as an update validator or a document validator?". The way that update validators were designed was as a way to run mongoose's built-in validators on update, while allowing you to skip your custom validators if they were incompatible. However, should be easy enough to have a schema option that will give you the query as the context, which means you should be able to access the underlying model.

@viveleroi
Copy link
Author

That would work perfectly. If/when that's added, I can PR a fix for this plugin. For the time being, I'm avoiding find/update combos and just running find and then save separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants