不要怂,就是干,撸起袖子干!

Commit 5296da9c by Pedro Augusto de Paula Barbosa Committed by Sushant

fix(instance-validator): don't skip custom validators on null (#9143) (#10310)

1 parent 2a4e1113
...@@ -224,15 +224,17 @@ set(title) { ...@@ -224,15 +224,17 @@ set(title) {
## Validations ## Validations
Model validations, allow you to specify format/content/inheritance validations for each attribute of the model. Model validations allow you to specify format/content/inheritance validations for each attribute of the model.
Validations are automatically run on `create`, `update` and `save`. You can also call `validate()` to manually validate an instance. Validations are automatically run on `create`, `update` and `save`. You can also call `validate()` to manually validate an instance.
The validations are implemented by [validator.js][3]. ### Per-attribute validations
You can define your custom validators or use several built-in validators, implemented by [validator.js][3], as shown below.
```js ```js
const ValidateMe = sequelize.define('foo', { const ValidateMe = sequelize.define('foo', {
foo: { bar: {
type: Sequelize.STRING, type: Sequelize.STRING,
validate: { validate: {
is: ["^[a-z]+$",'i'], // will only allow letters is: ["^[a-z]+$",'i'], // will only allow letters
...@@ -268,12 +270,15 @@ const ValidateMe = sequelize.define('foo', { ...@@ -268,12 +270,15 @@ const ValidateMe = sequelize.define('foo', {
min: 23, // only allow values >= 23 min: 23, // only allow values >= 23
isCreditCard: true, // check for valid credit card numbers isCreditCard: true, // check for valid credit card numbers
// custom validations are also possible: // Examples of custom validators:
isEven(value) { isEven(value) {
if (parseInt(value) % 2 != 0) { if (parseInt(value) % 2 !== 0) {
throw new Error('Only even values are allowed!') throw new Error('Only even values are allowed!');
// we also are in the model's context here, so this.otherField }
// would get the value of otherField if it existed }
isGreaterThanOtherField(value) {
if (parseInt(value) <= parseInt(this.otherField)) {
throw new Error('Bar must be greater than otherField.');
} }
} }
} }
...@@ -283,7 +288,7 @@ const ValidateMe = sequelize.define('foo', { ...@@ -283,7 +288,7 @@ const ValidateMe = sequelize.define('foo', {
Note that where multiple arguments need to be passed to the built-in validation functions, the arguments to be passed must be in an array. But if a single array argument is to be passed, for instance an array of acceptable strings for `isIn`, this will be interpreted as multiple string arguments instead of one array argument. To work around this pass a single-length array of arguments, such as `[['one', 'two']]` as shown above. Note that where multiple arguments need to be passed to the built-in validation functions, the arguments to be passed must be in an array. But if a single array argument is to be passed, for instance an array of acceptable strings for `isIn`, this will be interpreted as multiple string arguments instead of one array argument. To work around this pass a single-length array of arguments, such as `[['one', 'two']]` as shown above.
To use a custom error message instead of that provided by validator.js, use an object instead of the plain value or array of arguments, for example a validator which needs no argument can be given a custom message with To use a custom error message instead of that provided by [validator.js][3], use an object instead of the plain value or array of arguments, for example a validator which needs no argument can be given a custom message with
```js ```js
isInt: { isInt: {
...@@ -291,7 +296,7 @@ isInt: { ...@@ -291,7 +296,7 @@ isInt: {
} }
``` ```
or if arguments need to also be passed add an`args`property: or if arguments need to also be passed add an `args` property:
```js ```js
isIn: { isIn: {
...@@ -300,19 +305,52 @@ isIn: { ...@@ -300,19 +305,52 @@ isIn: {
} }
``` ```
When using custom validator functions the error message will be whatever message the thrown`Error`object holds. When using custom validator functions the error message will be whatever message the thrown `Error` object holds.
See [the validator.js project][3] for more details on the built in validation methods. See [the validator.js project][3] for more details on the built in validation methods.
**Hint: **You can also define a custom function for the logging part. Just pass a function. The first parameter will be the string that is logged. **Hint:** You can also define a custom function for the logging part. Just pass a function. The first parameter will be the string that is logged.
### Per-attribute validators and `allowNull`
If a particular field of a model is set to not allow null (with `allowNull: false`) and that value has been set to `null`, all validators will be skipped and a `ValidationError` will be thrown.
On the other hand, if it is set to allow null (with `allowNull: true`) and that value has been set to `null`, only the built-in validators will be skipped, while the custom validators will still run.
This means you can, for instance, have a string field which validates its length to be between 5 and 10 characters, but which also allows `null` (since the length validator will be skipped automatically when the value is `null`):
### Validators and `allowNull` ```js
const User = sequelize.define('user', {
username: {
type: Sequelize.STRING,
allowNull: true,
validate: {
len: [5, 10]
}
}
});
```
If a particular field of a model is set to allow null (with `allowNull: true`) and that value has been set to `null` , its validators do not run. You also can conditionally allow `null` values, with a custom validator, since it won't be skipped:
This means you can, for instance, have a string field which validates its length to be at least 5 characters, but which also allows `null`. ```js
const User = sequelize.define('user', {
age: Sequelize.INTEGER,
name: {
type: Sequelize.STRING,
allowNull: true,
validate: {
customValidator: function(value) {
if (value === null && this.age !== 10) {
throw new Error("name can't be null unless age is 10");
}
})
}
}
});
```
You can customize `allowNull` error message by setting `notNull` validator, like this You can customize `allowNull` error message by setting the `notNull` validator:
```js ```js
const User = sequelize.define('user', { const User = sequelize.define('user', {
...@@ -328,7 +366,7 @@ const User = sequelize.define('user', { ...@@ -328,7 +366,7 @@ const User = sequelize.define('user', {
}); });
``` ```
### Model validations ### Model-wide validations
Validations can also be defined to check the model after the field-specific validators. Using this you could, for example, ensure either neither of `latitude` and `longitude` are set or both, and fail if one but not the other is set. Validations can also be defined to check the model after the field-specific validators. Using this you could, for example, ensure either neither of `latitude` and `longitude` are set or both, and fail if one but not the other is set.
...@@ -374,6 +412,8 @@ In this simple case an object fails validation if either latitude or longitude i ...@@ -374,6 +412,8 @@ In this simple case an object fails validation if either latitude or longitude i
} }
``` ```
Such validation could have also been done with a custom validator defined on a single attribute (such as the `latitude` attribute, by checking `(value === null) !== (this.longitude === null)`), but the model-wide validation approach is cleaner.
## Configuration ## Configuration
You can also influence the way Sequelize handles your column names: You can also influence the way Sequelize handles your column names:
......
...@@ -26,6 +26,10 @@ With v5 Sequelize now use `sequelize-pool` which is a modernized fork of `generi ...@@ -26,6 +26,10 @@ With v5 Sequelize now use `sequelize-pool` which is a modernized fork of `generi
### Model ### Model
**Validators**
Custom validators defined per attribute (as opposed to the custom validators defined in the model's options) now run when the attribute's value is `null` and `allowNull` is `true` (while previously they didn't run and the validation succeeded immediately). To avoid problems when upgrading, please check all your custom validators defined per attribute, where `allowNull` is `true`, and make sure all these validators behave correctly when the value is `null`. See [#9143](https://github.com/sequelize/sequelize/issues/9143).
**Attributes** **Attributes**
`Model.attributes` now removed, use `Model.rawAttributes`. [#5320](https://github.com/sequelize/sequelize/issues/5320) `Model.attributes` now removed, use `Model.rawAttributes`. [#5320](https://github.com/sequelize/sequelize/issues/5320)
......
...@@ -67,7 +67,7 @@ class InstanceValidator { ...@@ -67,7 +67,7 @@ class InstanceValidator {
this.inProgress = true; this.inProgress = true;
return Promise.all([ return Promise.all([
this._builtinValidators().reflect(), this._perAttributeValidators().reflect(),
this._customValidators().reflect() this._customValidators().reflect()
]).then(() => { ]).then(() => {
if (this.errors.length) { if (this.errors.length) {
...@@ -113,12 +113,12 @@ class InstanceValidator { ...@@ -113,12 +113,12 @@ class InstanceValidator {
} }
/** /**
* Will run all the built-in validators. * Will run all the validators defined per attribute (built-in validators and custom validators)
* *
* @returns {Promise<Array.<Promise.PromiseInspection>>} A promise from .reflect(). * @returns {Promise<Array.<Promise.PromiseInspection>>} A promise from .reflect().
* @private * @private
*/ */
_builtinValidators() { _perAttributeValidators() {
// promisify all attribute invocations // promisify all attribute invocations
const validators = []; const validators = [];
...@@ -139,7 +139,7 @@ class InstanceValidator { ...@@ -139,7 +139,7 @@ class InstanceValidator {
} }
if (this.modelInstance.validators.hasOwnProperty(field)) { if (this.modelInstance.validators.hasOwnProperty(field)) {
validators.push(this._builtinAttrValidate.call(this, value, field).reflect()); validators.push(this._singleAttrValidate.call(this, value, field, rawAttribute.allowNull).reflect());
} }
}); });
...@@ -147,7 +147,7 @@ class InstanceValidator { ...@@ -147,7 +147,7 @@ class InstanceValidator {
} }
/** /**
* Will run all the custom validators. * Will run all the custom validators defined in the model's options.
* *
* @returns {Promise<Array.<Promise.PromiseInspection>>} A promise from .reflect(). * @returns {Promise<Array.<Promise.PromiseInspection>>} A promise from .reflect().
* @private * @private
...@@ -171,18 +171,20 @@ class InstanceValidator { ...@@ -171,18 +171,20 @@ class InstanceValidator {
} }
/** /**
* Validate a single attribute with all the defined built-in validators. * Validate a single attribute with all the defined built-in validators and custom validators.
* *
* @private * @private
* *
* @param {*} value Anything. * @param {*} value Anything.
* @param {string} field The field name. * @param {string} field The field name.
* @param {boolean} allowNull Whether or not the schema allows null values
* *
* @returns {Promise} A promise, will always resolve, auto populates error on this.error local object. * @returns {Promise} A promise, will always resolve, auto populates error on this.error local object.
*/ */
_builtinAttrValidate(value, field) { _singleAttrValidate(value, field, allowNull) {
// check if value is null (if null not allowed the Schema pass will capture it) // If value is null and allowNull is false, no validators should run (see #9143)
if (value === null || value === undefined) { if ((value === null || value === undefined) && !allowNull) {
// The schema validator (_validateSchema) has already generated the validation error. Nothing to do here.
return Promise.resolve(); return Promise.resolve();
} }
...@@ -201,9 +203,15 @@ class InstanceValidator { ...@@ -201,9 +203,15 @@ class InstanceValidator {
} }
} }
// Check for custom validator. // Custom validators should always run, except if value is null and allowNull is false (see #9143)
if (typeof test === 'function') { if (typeof test === 'function') {
return validators.push(this._invokeCustomValidator(test, validatorType, true, value, field).reflect()); validators.push(this._invokeCustomValidator(test, validatorType, true, value, field).reflect());
return;
}
// If value is null, built-in validators should not run (only custom validators have to run) (see #9134).
if (value === null || value === undefined) {
return;
} }
const validatorPromise = this._invokeBuiltinValidator(value, test, validatorType, field); const validatorPromise = this._invokeBuiltinValidator(value, test, validatorType, field);
...@@ -231,7 +239,7 @@ class InstanceValidator { ...@@ -231,7 +239,7 @@ class InstanceValidator {
* @returns {Promise} A promise. * @returns {Promise} A promise.
*/ */
_invokeCustomValidator(validator, validatorType, optAttrDefined, optValue, optField) { _invokeCustomValidator(validator, validatorType, optAttrDefined, optValue, optField) {
let validatorFunction = null; // the validation function to call let validatorFunction = null; // the validation function to call
let isAsync = false; let isAsync = false;
const validatorArity = validator.length; const validatorArity = validator.length;
......
...@@ -856,7 +856,7 @@ class Model { ...@@ -856,7 +856,7 @@ class Model {
* @param {string} [attributes.column.onDelete] What should happen when the referenced key is deleted. One of CASCADE, RESTRICT, SET DEFAULT, SET NULL or NO ACTION * @param {string} [attributes.column.onDelete] What should happen when the referenced key is deleted. One of CASCADE, RESTRICT, SET DEFAULT, SET NULL or NO ACTION
* @param {Function} [attributes.column.get] Provide a custom getter for this column. Use `this.getDataValue(String)` to manipulate the underlying values. * @param {Function} [attributes.column.get] Provide a custom getter for this column. Use `this.getDataValue(String)` to manipulate the underlying values.
* @param {Function} [attributes.column.set] Provide a custom setter for this column. Use `this.setDataValue(String, Value)` to manipulate the underlying values. * @param {Function} [attributes.column.set] Provide a custom setter for this column. Use `this.setDataValue(String, Value)` to manipulate the underlying values.
* @param {Object} [attributes.column.validate] An object of validations to execute for this column every time the model is saved. Can be either the name of a validation provided by validator.js, a validation function provided by extending validator.js (see the `DAOValidator` property for more details), or a custom validation function. Custom validation functions are called with the value of the field, and can possibly take a second callback argument, to signal that they are asynchronous. If the validator is sync, it should throw in the case of a failed validation, it it is async, the callback should be called with the error text. * @param {Object} [attributes.column.validate] An object of validations to execute for this column every time the model is saved. Can be either the name of a validation provided by validator.js, a validation function provided by extending validator.js (see the `DAOValidator` property for more details), or a custom validation function. Custom validation functions are called with the value of the field and the instance itself as the `this` binding, and can possibly take a second callback argument, to signal that they are asynchronous. If the validator is sync, it should throw in the case of a failed validation; if it is async, the callback should be called with the error text.
* @param {Object} options These options are merged with the default define options provided to the Sequelize constructor * @param {Object} options These options are merged with the default define options provided to the Sequelize constructor
* @param {Object} options.sequelize Define the sequelize instance to attach to the new Model. Throw error if none is provided. * @param {Object} options.sequelize Define the sequelize instance to attach to the new Model. Throw error if none is provided.
* @param {string} [options.modelName] Set name of the model. By default its same as Class name. * @param {string} [options.modelName] Set name of the model. By default its same as Class name.
......
...@@ -604,4 +604,154 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), () => { ...@@ -604,4 +604,154 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), () => {
}); });
}); });
describe('custom validation functions and null values', () => {
before(function() {
this.customValidator = sinon.fake(function(value) {
if (value === null && this.age !== 10) {
throw new Error("name can't be null unless age is 10");
}
});
});
describe('with allowNull set to true', () => {
before(function() {
this.User = current.define('user', {
age: Sequelize.INTEGER,
name: {
type: Sequelize.STRING,
allowNull: true,
validate: {
customValidator: this.customValidator
}
}
});
this.stub = sinon.stub(current, 'query').returns(Promise.resolve([this.User.build(), 1]));
});
after(function() {
this.stub.restore();
});
describe('should call validator and not throw', () => {
beforeEach(function() {
this.customValidator.resetHistory();
});
it('on create', function() {
return expect(this.User.create({
age: 10,
name: null
})).not.to.be.rejected.then(() => {
return expect(this.customValidator).to.have.been.calledOnce;
});
});
it('on update', function() {
return expect(this.User.update({
age: 10,
name: null
}, { where: {} })).not.to.be.rejected.then(() => {
return expect(this.customValidator).to.have.been.calledOnce;
});
});
});
describe('should call validator and throw ValidationError', () => {
beforeEach(function() {
this.customValidator.resetHistory();
});
it('on create', function() {
return expect(this.User.create({
age: 11,
name: null
})).to.be.rejectedWith(Sequelize.ValidationError).then(() => {
return expect(this.customValidator).to.have.been.calledOnce;
});
});
it('on update', function() {
return expect(this.User.update({
age: 11,
name: null
}, { where: {} })).to.be.rejectedWith(Sequelize.ValidationError).then(() => {
return expect(this.customValidator).to.have.been.calledOnce;
});
});
});
});
describe('with allowNull set to false', () => {
before(function() {
this.User = current.define('user', {
age: Sequelize.INTEGER,
name: {
type: Sequelize.STRING,
allowNull: false,
validate: {
customValidator: this.customValidator
}
}
});
this.stub = sinon.stub(current, 'query').returns(Promise.resolve([this.User.build(), 1]));
});
after(function() {
this.stub.restore();
});
describe('should not call validator and throw ValidationError', () => {
beforeEach(function() {
this.customValidator.resetHistory();
});
it('on create', function() {
return expect(this.User.create({
age: 99,
name: null
})).to.be.rejectedWith(Sequelize.ValidationError).then(() => {
return expect(this.customValidator).to.have.not.been.called;
});
});
it('on update', function() {
return expect(this.User.update({
age: 99,
name: null
}, { where: {} })).to.be.rejectedWith(Sequelize.ValidationError).then(() => {
return expect(this.customValidator).to.have.not.been.called;
});
});
});
describe('should call validator and not throw', () => {
beforeEach(function() {
this.customValidator.resetHistory();
});
it('on create', function() {
return expect(this.User.create({
age: 99,
name: 'foo'
})).not.to.be.rejected.then(() => {
return expect(this.customValidator).to.have.been.calledOnce;
});
});
it('on update', function() {
return expect(this.User.update({
age: 99,
name: 'foo'
}, { where: {} })).not.to.be.rejected.then(() => {
return expect(this.customValidator).to.have.been.calledOnce;
});
});
});
});
});
}); });
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!