Fix two minor issues. Closes #3812 and #3770

changelog.md

@@ -2,6 +2,8 @@
 - [FEATURE] Add support for keyword `after` in options of a field (useful for migrations), only for MySQL. [#3166](https://github.com/sequelize/sequelize/pull/3166)
 - [FIXED] Fix a case where `type` in `sequelize.query` was not being set to raw. [#3800](https://github.com/sequelize/sequelize/pull/3800)
 - [FIXED] Fix an issue where include all was not being properly expanded for self-references [#3804](https://github.com/sequelize/sequelize/issues/3804)
+- [FIXED] Fix instance.changed regression to not return false negatives for not changed null values [#3812](https://github.com/sequelize/sequelize/issues/3812)
+- [FIXED] Fix isEmail validator to allow args: true [#3770](https://github.com/sequelize/sequelize/issues/3770)
 
 # 3.1.1
 - [FIXED] Always quote aliases, even when quoteIdentifiers is false [#1589](https://github.com/sequelize/sequelize/issues/1589)

lib/instance-validator.js

@@ -256,12 +256,17 @@ InstanceValidator.prototype._builtinAttrValidate = function(value, field) {
   Utils._.forIn(this.modelInstance.validators[field], function(test,
     validatorType) {
 
-    if (['isUrl', 'isURL', 'isEmail'].indexOf(validatorType) !== -1 && test === true) {
+    if (['isUrl', 'isURL', 'isEmail'].indexOf(validatorType) !== -1) {
       // Preserve backwards compat. Validator.js now expects the second param to isURL and isEmail to be an object
-      test = {};
+      if (typeof test === 'object' && test !== null && test.msg) {
+        test = {
+          msg: test.msg
+        };
+      } else if (test === true) {
+        test = {};
+      }
     }
 
-
     // Check for custom validator.
     if (typeof test === 'function') {
       return validators.push(self._invokeCustomValidator(test, validatorType, true, value, field));
@@ -273,8 +278,7 @@ InstanceValidator.prototype._builtinAttrValidate = function(value, field) {
     validators.push(validatorPromise);
   });
 
-  return Promise.settle(validators)
-    .then(this._handleSettledResult.bind(this, field));
+  return Promise.settle(validators).then(this._handleSettledResult.bind(this, field));
 };
 
 /**
@@ -311,11 +315,9 @@ InstanceValidator.prototype._invokeCustomValidator = Promise.method(function(val
     } else {
       validatorFunction = Promise.promisify(validator.bind(this.modelInstance));
     }
-    return validatorFunction()
-      .catch(this._pushError.bind(this, false, errorKey));
+    return validatorFunction().catch(this._pushError.bind(this, false, errorKey));
   } else {
-    return Promise.try(validator.bind(this.modelInstance, invokeArgs))
-      .catch(this._pushError.bind(this, false, errorKey));
+    return Promise.try(validator.bind(this.modelInstance, invokeArgs)).catch(this._pushError.bind(this, false, errorKey));
   }
 });
 
@@ -336,11 +338,7 @@ InstanceValidator.prototype._invokeBuiltinValidator = Promise.method(function(va
   }
 
   // extract extra arguments for the validator
-  var validatorArgs = test.hasOwnProperty('args') || test.hasOwnProperty('msg') ? test.args : test;
-
-  // extract the error msg
-  var errorMessage = test.hasOwnProperty('msg') && test.msg ? test.msg :
-    'Validation ' + validatorType + ' failed';
+  var validatorArgs = test.args || test;
 
   if (!Array.isArray(validatorArgs)) {
     if (validatorType === 'isImmutable') {
@@ -355,7 +353,8 @@ InstanceValidator.prototype._invokeBuiltinValidator = Promise.method(function(va
   }
 
   if (!Validator[validatorType].apply(Validator, [value].concat(validatorArgs))) {
-    throw errorMessage;
+  // extract the error msg
+    throw test.msg || 'Validation ' + validatorType + ' failed';
   }
 });
 

lib/instance.js

@@ -321,8 +321,7 @@ module.exports = (function() {
               value = (value === 1) ? true : (value === 0) ? false : value;
             }
           }
-
-          if (!options.raw && (primitives.indexOf(typeof value) === -1 || value !== originalValue)) {
+          if (!options.raw && ((primitives.indexOf(typeof value) === -1 && value !== null) || value !== originalValue)) {
             this._previousDataValues[key] = originalValue;
             this.changed(key, true);
           }

lib/sequelize.js

@@ -658,15 +658,15 @@ module.exports = (function() {
     }
 
     if (Utils._.isPlainObject(sql)) {
-      if (Object.prototype.hasOwnProperty.call(sql,'values')) {
-        if (Object.prototype.hasOwnProperty.call(options,'replacements')) {
+      if (sql.values !== undefined) {
+        if (options.replacements !== undefined) {
           throw new Error('Both `sql.values` and `options.replacements` cannot be set at the same time');
         }
 
         options.replacements = sql.values;
       }
 
-      if (Object.prototype.hasOwnProperty.call(sql,'query')) {
+      if (sql.query !== undefined) {
         sql = sql.query;
       }
     }

lib/sql-string.js

@@ -156,7 +156,7 @@ SqlString.formatNamedParameters = function(sql, values, timeZone, dialect) {
       return value;
     }
 
-    if (Object.prototype.hasOwnProperty.call(values,key)) {
+    if (values[key] !== undefined) {
       return SqlString.escape(values[key], false, timeZone, dialect);
     } else {
       throw new Error('Named parameter "' + value + '" has no value in the given object.');

test/unit/instance/changed.test.js

@@ -25,20 +25,26 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
         raw: true
       });
 
+      expect(user.changed('meta')).to.equal(false);
       user.set('name', 'b');
+      user.set('meta', null);
       expect(user.changed('name')).to.equal(true);
+      expect(user.changed('meta')).to.equal(true);
     });
 
     it('should return falsy for unchanged primitive', function () {
       var user = this.User.build({
-        name: 'a'
+        name: 'a',
+        meta: null
       }, {
         isNewRecord: false,
         raw: true
       });
 
       user.set('name', 'a');
+      user.set('meta', null);
       expect(user.changed('name')).to.equal(false);
+      expect(user.changed('meta')).to.equal(false);
     });
 
     it('should return true for multiple changed values', function () {

test/unit/model/validation.test.js

@@ -179,7 +179,7 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
           var validations = {}
             , message = validator + '(' + failingValue + ')';
 
-          if (validatorDetails.hasOwnProperty('spec')) {
+          if (validatorDetails.spec) {
             validations[validator] = validatorDetails.spec;
           } else {
             validations[validator] = {};
@@ -203,19 +203,24 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
           });
         });
       }
-      , applyPassTest = function applyPassTest(validatorDetails, j, validator, msg) {
+      , applyPassTest = function applyPassTest(validatorDetails, j, validator, type) {
           var succeedingValue = validatorDetails.pass[j];
           it('correctly specifies an instance as valid using a value of "' + succeedingValue + '" for the validation "' + validator + '"', function() {
             var validations = {};
 
-            if (validatorDetails.hasOwnProperty('spec')) {
+            if (validatorDetails.spec !== undefined) {
               validations[validator] = validatorDetails.spec;
             } else {
               validations[validator] = {};
             }
 
-            if (msg) {
+            if (type === 'msg') {
               validations[validator].msg = validator + '(' + succeedingValue + ')';
+            } else if (type === 'args') {
+              validations[validator].args = validations[validator].args || true;
+              validations[validator].msg = validator + '(' + succeedingValue + ')';
+            } else if (type === 'true') {
+              validations[validator] = true;
             }
 
             var UserSuccess = this.sequelize.define('User' + config.rand(), {
@@ -238,7 +243,7 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
           validator = validator.replace(/\$$/, '');
           var validatorDetails = checks[validator];
 
-          if (!validatorDetails.hasOwnProperty('raw')) {
+          if (!validatorDetails.raw) {
             validatorDetails.fail = Array.isArray(validatorDetails.fail) ? validatorDetails.fail : [validatorDetails.fail];
             validatorDetails.pass = Array.isArray(validatorDetails.pass) ? validatorDetails.pass : [validatorDetails.pass];
           }
@@ -248,11 +253,12 @@ describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
           }
 
           for (i = 0; i < validatorDetails.pass.length; i++) {
-            applyPassTest(validatorDetails, i, validator, false);
-          }
-
-          for (i = 0; i < validatorDetails.pass.length; i++) {
-            applyPassTest(validatorDetails, i, validator, true);
+            applyPassTest(validatorDetails, i, validator);
+            applyPassTest(validatorDetails, i, validator, 'msg');
+            applyPassTest(validatorDetails, i, validator, 'args');
+            if (validatorDetails.spec === undefined) {
+              applyPassTest(validatorDetails, i, validator, 'true');
+            }
           }
         }
       }