Merge pull request #4604 from sequelize/validationsRollback

feat(validations) Partial rollback of datatype validation by hiding b…

changelog.md

+# Next
+- [FIXED] Partial rollback of datatype validations by hiding it behind the `typeValidation` flag.
+
 # 3.11.0
 - [INTERNALS] Updated dependencies [#4594](https://github.com/sequelize/sequelize/pull/4594)
     + bluebird@2.10.1

lib/data-types.js

@@ -84,10 +84,8 @@ STRING.prototype.toSql = function() {
 };
 STRING.prototype.validate = function(value) {
   if (Object.prototype.toString.call(value) !== '[object String]') {
-    if (this.options.binary) {
-      if (Buffer.isBuffer(value)) {
-        return true;
-      }
+    if ((this.options.binary && Buffer.isBuffer(value)) || _.isNumber(value)) {
+      return true;
     }
     throw new sequelizeErrors.ValidationError(util.format('%j is not a valid string', value));
   }
@@ -352,7 +350,7 @@ DECIMAL.prototype.toSql = function() {
   return 'DECIMAL';
 };
 DECIMAL.prototype.validate = function(value) {
-  if (!_.isNumber(value)) {
+  if (!Validator.isDecimal(value)) {
     throw new sequelizeErrors.ValidationError(util.format('%j is not a valid decimal', value));
   }
 
@@ -374,7 +372,7 @@ BOOLEAN.prototype.toSql = function() {
   return 'TINYINT(1)';
 };
 BOOLEAN.prototype.validate = function(value) {
-  if (!_.isBoolean(value)) {
+  if (!Validator.isBoolean(value)) {
     throw new sequelizeErrors.ValidationError(util.format('%j is not a valid boolean', value));
   }
 

lib/dialects/abstract/index.js

@@ -30,7 +30,12 @@ AbstractDialect.prototype.supports = {
     /* does the dialect support updating autoincrement fields */
     update: true
   },
-
+  /* Do we need to say DEFAULT for bulk insert */
+  bulkDefault: false,
+  /* The dialect's words for INSERT IGNORE */
+  ignoreDuplicates: '',
+  /* Does the dialect support ON DUPLICATE KEY UPDATE */
+  updateOnDuplicate: false,
   schemas: false,
   transactions: true,
   migrations: true,

lib/dialects/abstract/query-generator.js

@@ -289,7 +289,7 @@ var QueryGenerator = {
             identityWrapperRequired = true;
           }
 
-          values.push(this.escape(value, (modelAttributeMap && modelAttributeMap[key]) || undefined));
+          values.push(this.escape(value, (modelAttributeMap && modelAttributeMap[key]) || undefined, { context: 'INSERT' }));
         }
       }
     }
@@ -314,13 +314,64 @@ var QueryGenerator = {
 
     return Utils._.template(query)(replacements);
   },
+
   /*
     Returns an insert into command for multiple values.
     Parameters: table name + list of hashes of attribute-value-pairs.
   */
-  /* istanbul ignore next */
-  bulkInsertQuery: function(tableName, attrValueHashes, options, modelAttributes) {
-    throwMethodUndefined('bulkInsertQuery');
+  bulkInsertQuery: function(tableName, attrValueHashes, options, rawAttributes) {
+    options = options || {};
+    rawAttributes = rawAttributes || {};
+
+    var query = 'INSERT<%= ignoreDuplicates %> INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %><%= onDuplicateKeyUpdate %><%= returning %>;'
+      , tuples = []
+      , serials = []
+      , allAttributes = []
+      , onDuplicateKeyUpdate = '';
+
+    attrValueHashes.forEach(function(attrValueHash) {
+      _.forOwn(attrValueHash, function(value, key) {
+        if (allAttributes.indexOf(key) === -1) {
+          allAttributes.push(key);
+        }
+
+        if (rawAttributes[key] && rawAttributes[key].autoIncrement === true) {
+          serials.push(key);
+        }
+      });
+    });
+
+    attrValueHashes.forEach(function(attrValueHash) {
+      tuples.push('(' +
+        allAttributes.map(function(key) {
+          if (this._dialect.supports.bulkDefault && serials.indexOf(key) !== -1) {
+            return attrValueHash[key] || 'DEFAULT';
+          }
+          return this.escape(attrValueHash[key], rawAttributes[key], { context: 'INSERT' });
+        }, this).join(',') +
+        ')');
+    }, this);
+
+    if (this._dialect.supports.updateOnDuplicate && options.updateOnDuplicate) {
+      onDuplicateKeyUpdate += ' ON DUPLICATE KEY UPDATE ' + options.updateOnDuplicate.map(function(attr) {
+        var field = rawAttributes && rawAttributes[attr] && rawAttributes[attr].field || attr;
+        var key = this.quoteIdentifier(field);
+        return key + '=VALUES(' + key + ')';
+      }, this).join(',');
+    }
+
+    var replacements = {
+      ignoreDuplicates: options.ignoreDuplicates ? this._dialect.supports.ignoreDuplicates : '',
+      table: this.quoteTable(tableName),
+      attributes: allAttributes.map(function(attr) {
+        return this.quoteIdentifier(attr);
+      }, this).join(','),
+      tuples: tuples.join(','),
+      onDuplicateKeyUpdate: onDuplicateKeyUpdate,
+      returning: this._dialect.supports.returnValues && options.returning ? ' RETURNING *' : ''
+    };
+
+    return _.template(query)(replacements);
   },
 
   /*
@@ -413,7 +464,7 @@ var QueryGenerator = {
       }
 
       var value = attrValueHash[key];
-      values.push(this.quoteIdentifier(key) + '=' + this.escape(value, (modelAttributeMap && modelAttributeMap[key] || undefined)));
+      values.push(this.quoteIdentifier(key) + '=' + this.escape(value, (modelAttributeMap && modelAttributeMap[key] || undefined), { context: 'UPDATE' }));
     }
 
     var replacements = {
@@ -894,11 +945,12 @@ var QueryGenerator = {
   /*
     Escape a value (e.g. a string, number or date)
   */
-  escape: function(value, field) {
+  escape: function(value, field, options) {
+    options = options || {};
     if (value && value._isSequelizeMethod) {
       return this.handleSequelizeMethod(value);
     } else {
-      if (field && field.type && value) {
+      if (['INSERT', 'UPDATE'].indexOf(options.context) !== -1 && this.typeValidation && field && field.type && value) {
         if (field.type.validate) {
           field.type.validate(value);
         }

lib/dialects/mysql/index.js

@@ -31,6 +31,8 @@ MysqlDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.support
     type: true,
     using: 1,
   },
+  ignoreDuplicates: ' IGNORE',
+  updateOnDuplicate: true,
   indexViaAlter: true,
   NUMERIC: true,
   GEOMETRY: true

lib/dialects/mysql/query-generator.js

@@ -167,47 +167,6 @@ var QueryGenerator = {
     return this.insertQuery(tableName, insertValues, rawAttributes, options);
   },
 
-  bulkInsertQuery: function(tableName, attrValueHashes, options, rawAttributes) {
-    var query = 'INSERT<%= ignoreDuplicates %> INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %><%= onDuplicateKeyUpdate %>;'
-      , tuples = []
-      , allAttributes = []
-      , onDuplicateKeyUpdate = '';
-
-    Utils._.forEach(attrValueHashes, function(attrValueHash) {
-      Utils._.forOwn(attrValueHash, function(value, key) {
-        if (allAttributes.indexOf(key) === -1) allAttributes.push(key);
-      });
-    });
-
-    Utils._.forEach(attrValueHashes, function(attrValueHash) {
-      tuples.push('(' +
-        allAttributes.map(function(key) {
-          return this.escape(attrValueHash[key]);
-        }.bind(this)).join(',') +
-      ')');
-    }.bind(this));
-
-    if (options && options.updateOnDuplicate) {
-      onDuplicateKeyUpdate += ' ON DUPLICATE KEY UPDATE ' + options.updateOnDuplicate.map(function(attr) {
-        var field = rawAttributes && rawAttributes[attr] && rawAttributes[attr].field || attr;
-        var key = this.quoteIdentifier(field);
-        return key + '=VALUES(' + key + ')';
-      }.bind(this)).join(',');
-    }
-
-    var replacements = {
-      ignoreDuplicates: options && options.ignoreDuplicates ? ' IGNORE' : '',
-      table: this.quoteTable(tableName),
-      attributes: allAttributes.map(function(attr) {
-                    return this.quoteIdentifier(attr);
-                  }.bind(this)).join(','),
-      tuples: tuples,
-      onDuplicateKeyUpdate: onDuplicateKeyUpdate
-    };
-
-    return Utils._.template(query)(replacements);
-  },
-
   deleteQuery: function(tableName, where, options) {
     options = options || {};
 
@@ -353,13 +312,15 @@ var QueryGenerator = {
     return Utils.addTicks(identifier, '`');
   },
 
-  escape: function(value, field) {
+  escape: function(value, field, options) {
+    options = options || {};
+
     if (value && value._isSequelizeMethod) {
       return this.handleSequelizeMethod(value);
     } else if (value && field && field.type instanceof DataTypes.GEOMETRY) {
       return 'GeomFromText(\'' + Wkt.stringify(value) + '\')';
     } else {
-      if (field && field.type && value) {
+      if (['INSERT', 'UPDATE'].indexOf(options.context) !== -1  && this.typeValidation && field && field.type && value) {
         if (field.type.validate) {
           field.type.validate(value);
         }

lib/dialects/postgres/index.js

@@ -30,6 +30,7 @@ PostgresDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.supp
   returnValues: {
     returning: true
   },
+  bulkDefault: true,
   schemas: true,
   lock: true,
   lockOf: true,

lib/dialects/postgres/query-generator.js

@@ -328,54 +328,6 @@ var QueryGenerator = {
     );
   },
 
-  bulkInsertQuery: function(tableName, attrValueHashes, options, modelAttributes) {
-    options = options || {};
-
-    var query = 'INSERT INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %>'
-      , tuples = []
-      , serials = []
-      , allAttributes = [];
-
-    if (options.returning) {
-      query += ' RETURNING *';
-    }
-
-    Utils._.forEach(attrValueHashes, function(attrValueHash) {
-      Utils._.forOwn(attrValueHash, function(value, key) {
-        if (allAttributes.indexOf(key) === -1) {
-          allAttributes.push(key);
-        }
-
-        if (modelAttributes && modelAttributes[key] && modelAttributes[key].autoIncrement === true) {
-          serials.push(key);
-        }
-      });
-    });
-
-    Utils._.forEach(attrValueHashes, function(attrValueHash) {
-      tuples.push('(' +
-        allAttributes.map(function(key) {
-          if (serials.indexOf(key) !== -1) {
-            return attrValueHash[key] || 'DEFAULT';
-          }
-          return this.escape(attrValueHash[key], modelAttributes && modelAttributes[key]);
-        }.bind(this)).join(',') +
-      ')');
-    }.bind(this));
-
-    var replacements = {
-      table: this.quoteTable(tableName)
-    , attributes: allAttributes.map(function(attr) {
-                    return this.quoteIdentifier(attr);
-                  }.bind(this)).join(',')
-    , tuples: tuples.join(',')
-    };
-
-    query = query + ';';
-
-    return Utils._.template(query)(replacements);
-  },
-
   deleteQuery: function(tableName, where, options, model) {
     var query;
 
@@ -895,12 +847,14 @@ var QueryGenerator = {
   /*
     Escape a value (e.g. a string, number or date)
   */
-  escape: function(value, field) {
+  escape: function(value, field, options) {
+    options = options || {};
+
     if (value && value._isSequelizeMethod) {
       return this.handleSequelizeMethod(value);
     }
 
-    if (field && field.type && value) {
+    if (['INSERT', 'UPDATE'].indexOf(options.context) !== -1 && this.typeValidation && field && field.type && value) {
       if (field.type.validate) {
         field.type.validate(value);
       }

lib/dialects/sqlite/index.js

@@ -26,7 +26,8 @@ SqliteDialect.prototype.supports = _.merge(_.cloneDeep(Abstract.prototype.suppor
     using: false
   },
   joinTableDependent: false,
-  groupedLimit: false
+  groupedLimit: false,
+  ignoreDuplicates: ' OR IGNORE'
 });
 
 ConnectionManager.prototype.defaultVersion = '3.8.0';

lib/dialects/sqlite/query-generator.js

@@ -137,37 +137,6 @@ var QueryGenerator = {
     return sql;
   },
 
-  bulkInsertQuery: function(tableName, attrValueHashes, options) {
-    var query = 'INSERT<%= ignoreDuplicates %> INTO <%= table %> (<%= attributes %>) VALUES <%= tuples %>;'
-      , tuples = []
-      , allAttributes = [];
-
-    Utils._.forEach(attrValueHashes, function(attrValueHash) {
-      Utils._.forOwn(attrValueHash, function(value, key) {
-        if (allAttributes.indexOf(key) === -1) allAttributes.push(key);
-      });
-    });
-
-    Utils._.forEach(attrValueHashes, function(attrValueHash) {
-      tuples.push('(' +
-        allAttributes.map(function (key) {
-          return this.escape(attrValueHash[key]);
-        }.bind(this)).join(',') +
-      ')');
-    }.bind(this));
-
-    var replacements  = {
-      ignoreDuplicates: options && options.ignoreDuplicates ? ' OR IGNORE' : '',
-      table: this.quoteTable(tableName),
-      attributes: allAttributes.map(function(attr) {
-                    return this.quoteIdentifier(attr);
-                  }.bind(this)).join(','),
-      tuples: tuples
-    };
-
-    return Utils._.template(query)(replacements);
-  },
-
   updateQuery: function(tableName, attrValueHash, where, options, attributes) {
     options = options || {};
     _.defaults(options, this.options);
@@ -189,7 +158,7 @@ var QueryGenerator = {
 
     for (var key in attrValueHash) {
       var value = attrValueHash[key];
-      values.push(this.quoteIdentifier(key) + '=' + this.escape(value, (modelAttributeMap && modelAttributeMap[key] || undefined)));
+      values.push(this.quoteIdentifier(key) + '=' + this.escape(value, (modelAttributeMap && modelAttributeMap[key] || undefined), { context: 'UPDATE' }));
     }
 
     var replacements = {

lib/sequelize.js

@@ -80,6 +80,7 @@ var url = require('url')
  * @param {Function} [options.pool.validateConnection] A function that validates a connection. Called with client. The default function checks that client is an object, and that its state is not disconnected
  * @param {Boolean}  [options.quoteIdentifiers=true] Set to `false` to make table names and attributes case-insensitive on Postgres and skip double quoting of them.
  * @param {String}   [options.isolationLevel='REPEATABLE_READ'] Set the default transaction isolation level. See `Sequelize.Transaction.ISOLATION_LEVELS` for possible options.
+ * @param {Boolean   [options.typeValidation=false] Run built in type validators on insert and update, e.g. validate that arguments passed to integer fields are integer-like
 */
 
 /**
@@ -143,7 +144,8 @@ var Sequelize = function(database, username, password, options) {
     quoteIdentifiers: true,
     hooks: {},
     isolationLevel: Transaction.ISOLATION_LEVELS.REPEATABLE_READ,
-    databaseVersion: 0
+    databaseVersion: 0,
+    typeValidation: false
   }, options || {});
 
   if (this.options.dialect === 'postgresql') {
@@ -203,6 +205,8 @@ var Sequelize = function(database, username, password, options) {
     throw new Error('The dialect ' + this.getDialect() + ' is not supported. ('+err+')');
   }
 
+  this.dialect.QueryGenerator.typeValidation = options.typeValidation;
+
   /**
    * Models are stored here under the name given to `sequelize.define`
    * @property models

test/integration/sequelize.test.js

@@ -945,6 +945,10 @@ describe(Support.getTestDialectTeaser('Sequelize'), function() {
     ].forEach(function(status) {
       describe('enum', function() {
         beforeEach(function() {
+          this.sequelize = Support.createSequelizeInstance({
+            typeValidation: true
+          });
+
           this.Review = this.sequelize.define('review', { status: status });
           return this.Review.sync({ force: true });
         });

test/unit/model/findone.test.js

@@ -12,10 +12,10 @@ var chai = require('chai')
 describe(Support.getTestDialectTeaser('Model'), function() {
   describe('method findOne', function () {
     before(function () {
-      this.oldfindAll = current.Model.prototype.findAll;
+      this.oldFindAll = current.Model.prototype.findAll;
     });
     after(function () {
-      current.Model.prototype.findAll = this.oldfindall;
+      current.Model.prototype.findAll = this.oldFindAll;
     });
 
     beforeEach(function () {

test/unit/model/validation.test.js

@@ -3,12 +3,15 @@
 /* jshint -W030 */
 /* jshint -W110 */
 var chai = require('chai')
+  , sinon = require('sinon')
   , expect = chai.expect
   , Sequelize = require(__dirname + '/../../../index')
   , Support = require(__dirname + '/../support')
   , current = Support.sequelize
+  , Promise = current.Promise
   , config = require(__dirname + '/../../config/config');
 
+
 describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
   describe('validations', function() {
     var checks = {
@@ -266,10 +269,115 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
   });
 
   describe('datatype validations', function () {
-    describe('should throw validationerror', function () {
-      var User = current.define('user', {
-        age: Sequelize.INTEGER
+    current = Support.createSequelizeInstance({
+      typeValidation: true
+    });
+
+    var User = current.define('user', {
+      age: Sequelize.INTEGER,
+      name: Sequelize.STRING,
+      awesome: Sequelize.BOOLEAN,
+      number: Sequelize.DECIMAL,
+      uid: Sequelize.UUID
+    });
+
+    before(function () {
+      this.stub = sinon.stub(current, 'query', function () {
+        return new Promise(function (resolve) {
+          resolve(User.build({}));
+        });
+      });
+    });
+
+    after(function () {
+      this.stub.restore();
+    });
+
+    describe('should not throw', function () {
+      describe('create', function () {
+        it('should allow number as a string', function () {
+          return expect(User.create({
+            age: '12'
+          })).not.to.be.rejected;
+        });
+
+        it('should allow decimal as a string', function () {
+          return expect(User.create({
+            number: '12.6'
+          })).not.to.be.rejected;
+        });
+
+        it('should allow decimal big numbers as a string', function () {
+          return expect(User.create({
+            number: '2321312301230128391820831289123012'
+          })).not.to.be.rejected;
+        });
+
+        it('should allow decimal as scientific notation', function () {
+          return Promise.join(
+            expect(User.create({
+              number: '2321312301230128391820e219'
+            })).not.to.be.rejected,
+            expect(User.create({
+              number: '2321312301230128391820e+219'
+            })).not.to.be.rejected,
+            expect(User.create({
+              number: '2321312301230128391820f219'
+            })).to.be.rejected
+          );
+        });
+
+        it('should allow string as a number', function () {
+          return expect(User.create({
+            name: 12
+          })).not.to.be.rejected;
+        });
+
+        it('should allow 0/1 as a boolean', function () {
+          return expect(User.create({
+            awesome: 1
+          })).not.to.be.rejected;
+        });
+
+        it('should allow 0/1 string as a boolean', function () {
+          return expect(User.create({
+            awesome: '1'
+          })).not.to.be.rejected;
+        });
+
+        it('should allow true/false string as a boolean', function () {
+          return expect(User.create({
+            awesome: 'true'
+          })).not.to.be.rejected;
+        });
+      });
+
+      describe('findAll', function () {
+        it('should allow $in', function () {
+          return expect(User.all({
+            where: {
+              name: {
+                $like: {
+                  $any: ['foo%', 'bar%']
+                }
+              }
+            }
+          })).not.to.be.rejected;
+        });
+
+        it('should allow $like for uuid', function () {
+          return expect(User.all({
+            where: {
+              uid: {
+                $like: '12345678%'
+              }
+            }
+          })).not.to.be.rejected;
+        });
       });
+    });
+
+    describe('should throw validationerror', function () {
 
       describe('create', function () {
         it('should throw when passing string', function () {

test/unit/sql/data-types.test.js

@@ -51,14 +51,6 @@ suite(Support.getTestDialectTeaser('SQL'), function() {
       });
 
       suite('validate', function () {
-        test('should throw an error if `value` is invalid', function() {
-          var type = DataTypes.STRING();
-
-          expect(function () {
-            type.validate(12345);
-          }).to.throw(Sequelize.ValidationError, '12345 is not a valid string');
-        });
-
         test('should return `true` if `value` is a string', function() {
           var type = DataTypes.STRING();
 
@@ -66,6 +58,7 @@ suite(Support.getTestDialectTeaser('SQL'), function() {
           /*jshint -W053 */
           expect(type.validate(new String('foobar'))).to.equal(true);
           /*jshint +W053 */
+          expect(type.validate(12)).to.equal(true);
         });
       });
     });