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

Add default validation based on attribute type #3472

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
133 changes: 132 additions & 1 deletion lib/data-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
var util = require('util')
, _ = require('lodash')
, Wkt = require('wellknown')
, warnings = {};
, sequelizeErrors = require('./errors')
, warnings = {}
, Validator = require('validator');

/**
* A convenience class holding commonly used data types. The datatypes are used when definining a new model using `Sequelize.define`, like this:
Expand Down Expand Up @@ -80,6 +82,13 @@ STRING.prototype.key = STRING.key = 'STRING';
STRING.prototype.toSql = function() {
return 'VARCHAR(' + this._length + ')' + ((this._binary) ? ' BINARY' : '');
};
STRING.prototype.validate = function(value) {
if (typeof value !== 'string') {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid string', value));
}

return true;
};
Object.defineProperty(STRING.prototype, 'BINARY', {
get: function() {
this._binary = true;
Expand Down Expand Up @@ -137,6 +146,13 @@ TEXT.prototype.toSql = function() {
return this.key;
}
};
TEXT.prototype.validate = function(value) {
if (typeof value !== 'string') {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid string', value));
}

return true;
};

var NUMBER = function(options) {
this.options = options;
Expand Down Expand Up @@ -167,6 +183,13 @@ NUMBER.prototype.toSql = function() {
}
return result;
};
NUMBER.prototype.validate = function(value) {
if (!_.isNumber(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid number', value));
}

return true;
};

Object.defineProperty(NUMBER.prototype, 'UNSIGNED', {
get: function() {
Expand Down Expand Up @@ -200,6 +223,13 @@ var INTEGER = function(length) {
util.inherits(INTEGER, NUMBER);

INTEGER.prototype.key = INTEGER.key = 'INTEGER';
INTEGER.prototype.validate = function(value) {
if (!Validator.isInt(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid integer', value));
}

return true;
};

/**
* A 64 bit integer.
Expand All @@ -219,6 +249,13 @@ var BIGINT = function(length) {
util.inherits(BIGINT, NUMBER);

BIGINT.prototype.key = BIGINT.key = 'BIGINT';
BIGINT.prototype.validate = function(value) {
if (!Validator.isInt(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid bigint', value));
}

return true;
};

/**
* Floating point number (4-byte precision). Accepts one or two arguments for precision
Expand All @@ -238,6 +275,13 @@ var FLOAT = function(length, decimals) {
util.inherits(FLOAT, NUMBER);

FLOAT.prototype.key = FLOAT.key = 'FLOAT';
FLOAT.prototype.validate = function(value) {
if (!Validator.isFloat(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid float', value));
}

return true;
};

/**
* Floating point number (4-byte precision). Accepts one or two arguments for precision
Expand Down Expand Up @@ -302,6 +346,13 @@ DECIMAL.prototype.toSql = function() {

return 'DECIMAL';
};
DECIMAL.prototype.validate = function(value) {
if (!_.isNumber(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid decimal', value));
}

return true;
};

/**
* A boolean / tinyint column, depending on dialect
Expand All @@ -317,6 +368,13 @@ BOOLEAN.prototype.key = BOOLEAN.key = 'BOOLEAN';
BOOLEAN.prototype.toSql = function() {
return 'TINYINT(1)';
};
BOOLEAN.prototype.validate = function(value) {
if (!_.isBoolean(value)) {

Choose a reason for hiding this comment

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

I vote for using Validator.isBoolean(), _.isBoolean() is too strict even the value is number 1 or 0

throw new sequelizeErrors.ValidationError(util.format('%j is not a valid boolean', value));
}

return true;
};

/**
* A time column
Expand Down Expand Up @@ -348,7 +406,13 @@ DATE.prototype.key = DATE.key = 'DATE';
DATE.prototype.toSql = function() {
return 'DATETIME';
};
DATE.prototype.validate = function(value) {
if (!Validator.isDate(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid date', value));
}

return true;
};
/**
* A date only column
* @property DATEONLY
Expand Down Expand Up @@ -377,6 +441,13 @@ var HSTORE = function() {
util.inherits(HSTORE, ABSTRACT);

HSTORE.prototype.key = HSTORE.key = 'HSTORE';
HSTORE.prototype.validate = function(value) {
if (!_.isPlainObject(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid hstore', value));
}

return true;
Copy link
Member

Choose a reason for hiding this comment

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

_.isPlainObject and maybe check that it is not more than one level deep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do you suggest to check for more than one level deep here?

};

/**
* A JSON string column. Only available in postgres.
Expand All @@ -389,6 +460,9 @@ var JSONTYPE = function() {
util.inherits(JSONTYPE, ABSTRACT);

JSONTYPE.prototype.key = JSONTYPE.key = 'JSON';
JSONTYPE.prototype.validate = function(value) {
return true;
Copy link
Member

Choose a reason for hiding this comment

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

Can't really validate here I think - json objects, plain strings, booleans and nulls are all allowed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

};

/**
* A pre-processed JSON data column. Only available in postgres.
Expand Down Expand Up @@ -442,6 +516,13 @@ BLOB.prototype.toSql = function() {
return this.key;
}
};
BLOB.prototype.validate = function(value) {
if (!_.isString(value) && !Buffer.isBuffer(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid blob', value));
}

return true;
Copy link
Member

Choose a reason for hiding this comment

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

String or buffer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

};

/**
* Range types are data types representing a range of values of some element type (called the range's subtype).
Expand Down Expand Up @@ -474,6 +555,21 @@ RANGE.prototype.key = RANGE.key = 'RANGE';
RANGE.prototype.toSql = function() {
return pgRangeSubtypes[this._subtype.toLowerCase()];
};
RANGE.prototype.validate = function(value) {
if (_.isPlainObject(value) && value.inclusive) {
value = value.inclusive;
}

if (!_.isArray(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid range', value));
}

if (value.length !== 2) {
throw new sequelizeErrors.ValidationError('A range must be an array with two elements');
}

return true;
Copy link
Member

Choose a reason for hiding this comment

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

An array with two elements

Might have an inclusive property which is also an array with two elements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

};

/**
* A column storing a unique univeral identifier. Use with `UUIDV1` or `UUIDV4` for default values.
Expand All @@ -486,6 +582,13 @@ var UUID = function() {
util.inherits(UUID, ABSTRACT);

UUID.prototype.key = UUID.key = 'UUID';
UUID.prototype.validate = function(value) {
if (!Validator.isUUID(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid uuid', value));
}

return true;
};

/**
* A default unique universal identifier generated following the UUID v1 standard
Expand All @@ -499,6 +602,13 @@ var UUIDV1 = function() {
util.inherits(UUIDV1, ABSTRACT);

UUIDV1.prototype.key = UUIDV1.key = 'UUIDV1';
UUIDV1.prototype.validate = function(value) {
if (!Validator.isUUID(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid uuid', value));
}

return true;
};

/**
* A default unique universal identifier generated following the UUID v2 standard
Expand All @@ -512,6 +622,13 @@ var UUIDV4 = function() {
util.inherits(UUIDV4, ABSTRACT);

UUIDV4.prototype.key = UUIDV4.key = 'UUIDV4';
UUIDV4.prototype.validate = function(value) {
if (!Validator.isUUID(value, 4)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid uuidv4', value));
}

return true;
};

/**
* A virtual value that is not stored in the DB. This could for example be useful if you want to provide a default value in your model that is returned to the user but not stored in the DB.
Expand Down Expand Up @@ -565,6 +682,13 @@ var ENUM = function(value) {
util.inherits(ENUM, ABSTRACT);

ENUM.prototype.key = ENUM.key = 'ENUM';
ENUM.prototype.validate = function(value) {
if (!_.contains(this.values, value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid choice in %j', value, this.values));
}

return true;
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, maybe check that the value is in the allowed ones? Not sure you have access to those here though..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

};

/**
* An array of `type`, e.g. `DataTypes.ARRAY(DataTypes.DECIMAL)`. Only available in postgres.
Expand All @@ -583,6 +707,13 @@ ARRAY.prototype.key = ARRAY.key = 'ARRAY';
ARRAY.prototype.toSql = function() {
return this.type.toSql() + '[]';
};
ARRAY.prototype.validate = function(value) {
if (!Array.isArray(value)) {
throw new sequelizeErrors.ValidationError(util.format('%j is not a valid array', value));
}

return true;
};
ARRAY.is = function(obj, type) {
return obj instanceof ARRAY && obj.type instanceof type;
};
Expand Down
6 changes: 6 additions & 0 deletions lib/dialects/postgres/query-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,12 @@ var QueryGenerator = {
return this.handleSequelizeMethod(value);
}

if (field && field.type && value) {
if (field.type.validate) {
field.type.validate(value);
}
}

if (Utils._.isObject(value) && field && (field.type instanceof DataTypes.HSTORE || DataTypes.ARRAY.is(field.type, DataTypes.HSTORE))) {
if (field.type instanceof DataTypes.HSTORE){
return "'" + hstore.stringify(value) + "'";
Expand Down
22 changes: 2 additions & 20 deletions lib/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,26 +91,8 @@ var Model = function(name, attributes, options) {
throw new Error('Values for ENUM have not been defined.');
}

attribute.validate = attribute.validate || {
_checkEnum: function(value, next) {
var hasValue = value !== undefined
, isMySQL = ['mysql', 'mariadb'].indexOf(options.sequelize.options.dialect) !== -1
, ciCollation = !!options.collate && options.collate.match(/_ci$/i) !== null
, valueOutOfScope;

if (isMySQL && ciCollation && hasValue) {
var scopeIndex = (attributes[name].values || []).map(function(d) { return d.toLowerCase(); }).indexOf(value.toLowerCase());
valueOutOfScope = scopeIndex === -1;
} else {
valueOutOfScope = ((attributes[name].values || []).indexOf(value) === -1);
}

if (hasValue && valueOutOfScope && attributes[name].allowNull !== true) {
return next('Value "' + value + '" for ENUM ' + name + ' is out of allowed scope. Allowed values: ' + attributes[name].values.join(', '));
}
next();
}
};
// BC compatible.
attribute.type.values = attribute.values;
}

return attribute;
Expand Down
2 changes: 1 addition & 1 deletion lib/query-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ QueryInterface.prototype.renameColumn = function(tableName, attrNameBefore, attr

_options[attrNameAfter] = {
attribute: attrNameAfter,
type: data.type,
type: DataTypes[data.type],
Copy link
Member

Choose a reason for hiding this comment

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

@fixe This was breaking a mysql test so I removed it. Tests seem to pass fine without this change

allowNull: data.allowNull,
defaultValue: data.defaultValue
};
Expand Down
6 changes: 3 additions & 3 deletions test/integration/dialects/postgres/query-generator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ if (dialect.match(/^postgres/)) {
beforeEach(function() {
this.User = this.sequelize.define('User', {
username: DataTypes.STRING,
email: {type: DataTypes.ARRAY(DataTypes.TEXT)},
numbers: {type: DataTypes.ARRAY(DataTypes.FLOAT)},
document: {type: DataTypes.HSTORE, defaultValue: '"default"=>"value"'}
email: { type: DataTypes.ARRAY(DataTypes.TEXT) },
numbers: { type: DataTypes.ARRAY(DataTypes.FLOAT) },
document: { type: DataTypes.HSTORE, defaultValue: { default: '"value"' } }
});
return this.User.sync({ force: true });
});
Expand Down
2 changes: 1 addition & 1 deletion test/integration/instance.validations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,4 +718,4 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
expect(errors.get('name')[0].message).to.equal('Validation isExactly7Characters failed');
});
});
});
});
2 changes: 1 addition & 1 deletion test/integration/sequelize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ describe(Support.getTestDialectTeaser('Sequelize'), function() {
it("doesn't save an instance if value is not in the range of enums", function() {
return this.Review.create({status: 'fnord'}).catch(function(err) {
expect(err).to.be.instanceOf(Error);
expect(err.get('status')[0].message).to.equal('Value "fnord" for ENUM status is out of allowed scope. Allowed values: scheduled, active, finished');
expect(err.message).to.equal('"fnord" is not a valid choice in ["scheduled","active","finished"]');
});
});
});
Expand Down
Loading