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

Make 'reserved keywords' in validations extendable #482

Merged
merged 2 commits into from
Jul 2, 2014

Conversation

mphasize
Copy link
Contributor

This PR relates to #444:

I think the "ignored property keys" in waterline/core/validations should be exposed and extendable by an application developer. That would make it easier to write custom blueprints with specialized model definitions.

Example: I am using an async keyword in my model attributes to define which associations I want to have populated by default and which should be excluded by default. My customized blueprints handle this attribute, but I have to modify waterline/core/validations to prevent it from choking on the async property.

Now I modified the waterline core a little bit, to make the default keywords in core/validations extendable through setting a configuration object in sails.config.models. More info below.

However, after doing all the work, I came back and read about the custom types that waterline allows and now I am wondering just how different the use cases and possible benefits for each approach are.

Comments welcome!


Configuration option

In config/models I am adding a configuration key "validations" like this:

// These properties should be ignored in Waterline validations
validations: {
  ignoreProperties: [ 'async', 'special']
}

Changes in waterline core

In waterline/core/index.js I make sure that the configuration gets passed down to the initializer function of the Validator:

this._validator.initialize(_validations, this.types, this.defaults.validations); // extend the initializer

After that I modified waterline/core/validations.js to accept the config and concatenate any keywords in ignoreProperties to the reservedProperties.

this.reservedProperties = ['defaultsTo', 'primaryKey', 'autoIncrement', 'unique', 'index', 'collection', 'dominant', 'through',
        'columnName', 'foreignKey', 'references', 'on', 'groupKey', 'model', 'via', 'size',
        'example', 'validationMessage', 'validations', 'populateSettings', 'onKey', 'protected'];

if(defaults.ignoreProperties && Array.isArray(defaults.ignoreProperties)) {
  this.reservedProperties = this.reservedProperties.concat(defaults.ignoreProperties);
}

....

  // If property is reserved don't do anything with it
  if(self.reservedProperties.indexOf(prop) > -1) return;

Test

Added a unit test for this behaviour to be found in test/unit/validations/validations.ignoreProperties.js.

…dable

Pass a `defaults.validations` configuration object as additional argument to Validator.initialize() in the Waterline Core.

As `defaults` is already coming from sails.config.models, it seems a good idea to extend it there with a "validations" object holding the configuration for the model validator.
@mphasize
Copy link
Contributor Author

To elaborate on my own question:
Using custom types, it looks like we have to define the types per model. So there's a potential drawback, that we have to repeat ourselves over and over. Also, custom types pass through the validator which adds a few function calls. On the other hand, this way it's actually possible to use the validator.

The modification to extend the reserved properties allows us to use a global configuration across all models and it simply skips validation on those keywords, potentially saving a few processor loops.

To ask a new question: Is it possible to define custom types globally in Sails?

particlebanana added a commit that referenced this pull request Jul 2, 2014
Make 'reserved keywords' in validations extendable
@particlebanana particlebanana merged commit b714c55 into balderdashy:master Jul 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants