fix(*): fixes a few bugs with validate and hooks, closes #2934

lib/instance.js

@@ -507,7 +507,15 @@ module.exports = (function() {
       // Validate
       if (options.validate) {
         options.skip = _.difference(Object.keys(this.Model.rawAttributes), options.fields);
-        return (options.hooks ? this.hookValidate(options) : this.validate(options)).then(function() {
+
+        return Promise.bind(this).then(function () {
+          // hookValidate rejects with errors, validate returns with errors
+          if (options.hooks) return this.hookValidate(options);
+
+          return this.validate(options).then(function (err) {
+            if (err) throw err;
+          });
+        }).then(function() {
           delete options.skip;
         });
       }
@@ -539,15 +547,21 @@ module.exports = (function() {
             }            
 
             if (hookChanged) {
-              return Promise.bind(this).then(function() {
+              if (options.validate) {
                 // Validate again
-                if (options.validate) {
-                  options.skip = _.difference(Object.keys(this.Model.rawAttributes), hookChanged);
-                  return (options.hooks ? this.hookValidate(options) : this.validate(options)).then(function() {
-                    delete options.skip;
+                
+                options.skip = _.difference(Object.keys(this.Model.rawAttributes), hookChanged);
+                return Promise.bind(this).then(function () {
+                  // hookValidate rejects with errors, validate returns with errors
+                  if (options.hooks) return this.hookValidate(options);
+
+                  return this.validate(options).then(function (err) {
+                    if (err) throw err;
                   });
-                }
-              });
+                }).then(function() {
+                  delete options.skip;
+                });
+              }
             }
           });
         }

lib/model.js

@@ -1303,21 +1303,21 @@ module.exports = (function() {
       , now = Utils.now(self.modelManager.sequelize.options.dialect);
 
     // build DAOs
-    var daos = records.map(function(values) {
+    var instances = records.map(function(values) {
       return self.build(values, {isNewRecord: true});
     });
 
     return Promise.try(function() {
       // Run before hook
       if (options.hooks) {
-        return self.runHooks('beforeBulkCreate', daos, options);
+        return self.runHooks('beforeBulkCreate', instances, options);
       }
     }).then(function() {
-      daos.forEach(function(dao) {
+      instances.forEach(function(instance) {
         // Filter dataValues by options.fields
         var values = {};
         options.fields.forEach(function(field) {
-          values[field] = dao.dataValues[field];
+          values[field] = instance.dataValues[field];
         });
 
         // set createdAt/updatedAt attributes
@@ -1328,7 +1328,7 @@ module.exports = (function() {
           values[updatedAtAttr] = now;
         }
 
-        dao.dataValues = values;
+        instance.dataValues = values;
       });
 
       // Validate
@@ -1336,13 +1336,17 @@ module.exports = (function() {
         options.skip = Utils._.difference(Object.keys(self.attributes), options.fields);
 
         var errors = [];
-        return Promise.map(daos, function(dao) {
-          var fn = options.individualHooks ? 'hookValidate' : 'validate';
-          return dao[fn](options).then(function(err) {
-            if (!!err) {
-              errors.push({record: dao, errors: err});
-            }
-          });
+        return Promise.map(instances, function(instance) {
+          // hookValidate rejects with errors, validate returns with errors
+          if (options.individualHooks) {
+            return instance.hookValidate(options);
+          } else {
+            return instance.validate(options).then(function (err) {
+              if (err) {
+                errors.push({record: instance, errors: err});
+              }
+            });
+          }
         }).then(function() {
           delete options.skip;
           if (errors.length) {
@@ -1352,8 +1356,8 @@ module.exports = (function() {
       }
     }).then(function() {
       if (options.individualHooks) {
-        // Create each dao individually
-        return Promise.map(daos, function(dao) {
+        // Create each instance individually
+        return Promise.map(instances, function(instance) {
           var individualOptions = Utils._.clone(options);
           delete individualOptions.fields;
           delete individualOptions.individualHooks;
@@ -1361,15 +1365,15 @@ module.exports = (function() {
           individualOptions.validate = false;
           individualOptions.hooks = true;
 
-          return dao.save(individualOptions);
-        }).then(function(_daos) {
-          daos = _daos;
+          return instance.save(individualOptions);
+        }).then(function(_instances) {
+          instances = _instances;
         });
       } else {
         // Create all in one query
-        // Recreate records from daos to represent any changes made in hooks or validation
-        records = daos.map(function(dao) {
-          return Utils._.omit(dao.dataValues, self._virtualAttributes);
+        // Recreate records from instances to represent any changes made in hooks or validation
+        records = instances.map(function(instance) {
+          return Utils._.omit(instance.dataValues, self._virtualAttributes);
         });
 
         var rawAttribute;
@@ -1400,7 +1404,7 @@ module.exports = (function() {
         return self.QueryInterface.bulkInsert(self.getTableName(options), records, options, attributes).then(function (results) {
           if (Array.isArray(results)) {
             results.forEach(function (result, i) {
-              daos[i].set(self.primaryKeyAttribute, result[self.rawAttributes[self.primaryKeyAttribute].field], {raw: true});
+              instances[i].set(self.primaryKeyAttribute, result[self.rawAttributes[self.primaryKeyAttribute].field], {raw: true});
             });
           }
           return results;
@@ -1409,10 +1413,10 @@ module.exports = (function() {
     }).then(function() {
       // Run after hook
       if (options.hooks) {
-        return self.runHooks('afterBulkCreate', daos, options);
+        return self.runHooks('afterBulkCreate', instances, options);
       }
     }).then(function() {
-      return daos;
+      return instances;
     });
   };
 

test/instance.test.js

@@ -1121,6 +1121,17 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
       });
     });
 
+    it('should fail a validation upon creating with hooks false', function(done) {
+      this.User.create({aNumber: 0, validateTest: 'hello'}, {hooks: false}).error(function(err) {
+        expect(err).to.exist;
+        expect(err).to.be.instanceof(Object);
+        expect(err.get('validateTest')).to.be.instanceof(Array);
+        expect(err.get('validateTest')[0]).to.exist;
+        expect(err.get('validateTest')[0].message).to.equal('Validation isInt failed');
+        done();
+      });
+    });
+
     it('should fail a validation upon building', function(done) {
       this.User.build({aNumber: 0, validateCustom: 'aaaaaaaaaaaaaaaaaaaaaaaaaa'}).save()
       .error(function(err) {

test/model/create.test.js

@@ -1191,6 +1191,18 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       });
     });
 
+    it('should not fail on validate: true and individualHooks: true', function () {
+      var User = this.sequelize.define('user', {
+        name: Sequelize.STRING
+      });
+
+      return User.sync({force: true}).then(function () {
+        return User.bulkCreate([
+          {name: 'James'}
+        ], {validate: true, individualHooks: true});
+      });
+    });
+
     it('properly handles disparate field lists', function(done) {
       var self = this
         , data = [{username: 'Peter', secretValue: '42', uniqueName: '1' },