Merge pull request #3757 from BridgeAR/fix-isip

Fix isIP: closes #2972, move integration tests to unit tests and…

Merge pull request #3757 from BridgeAR/fix-isip
Fix isIP: closes #2972, move integration tests to unit tests and…
Jan Aagaard Meier
Commit 341b0172 authored May 21, 2015 by Jan Aagaard Meier
Showing with 307 additions and 9 deletions
changelog.md
lib/hooks.js
lib/instance-validator.js
lib/sequelize.js
lib/utils.js
test/integration/instance.validations.test.js
test/unit/instance/build.test.js
test/unit/model/validation.test.js
--- a/changelog.md
+++ b/changelog.md
+# next
+- [BUG] Fix an issue with the build in isIP validator returning false negatives [#3756](https://github.com/sequelize/sequelize/pull/3756)
 # 3.0.1
 - [FIXED] `include.attributes = []` will no longer force the inclusion of the primary key, making it possible to write aggregates with includes.

--- a/lib/hooks.js
+++ b/lib/hooks.js
@@ -89,7 +89,7 @@ var Hooks = {
  runHooks: function(hooks) {
    var self = this
      , fn
-      , fnArgs = Array.prototype.slice.call(arguments, 1)
+      , fnArgs = Utils.sliceArgs(arguments, 1)
      , hookType;
    if (typeof fnArgs[fnArgs.length - 1] === 'function') {

--- a/lib/instance-validator.js
+++ b/lib/instance-validator.js
@@ -343,11 +343,16 @@ InstanceValidator.prototype._invokeBuiltinValidator = Promise.method(function(va
    'Validation ' + validatorType + ' failed';
  if (!Array.isArray(validatorArgs)) {
-    validatorArgs = [validatorArgs];
+    if (validatorType === 'isImmutable') {
+      validatorArgs = [validatorArgs, field];
+    } else if (validatorType === 'isIP') {
+      validatorArgs = [];
+    } else {
+      validatorArgs = [validatorArgs];
+    }
  } else {
    validatorArgs = validatorArgs.slice(0);
  }
-  validatorArgs.push(field);
  if (!Validator[validatorType].apply(Validator, [value].concat(validatorArgs))) {
    throw errorMessage;

--- a/lib/sequelize.js
+++ b/lib/sequelize.js
@@ -936,7 +936,7 @@ module.exports = (function() {
   * @return {Sequelize.fn}
   */
  Sequelize.fn = Sequelize.prototype.fn = function(fn) {
-    return new Utils.fn(fn, Array.prototype.slice.call(arguments, 1));
+    return new Utils.fn(fn, Utils.sliceArgs(arguments, 1));
  };
  /**
@@ -989,7 +989,7 @@ module.exports = (function() {
   * @return {Sequelize.and}
   */
  Sequelize.and = Sequelize.prototype.and = function() {
-    return new Utils.and(Array.prototype.slice.call(arguments));
+    return new Utils.and(Utils.sliceArgs(arguments));
  };
  /**
@@ -1002,7 +1002,7 @@ module.exports = (function() {
   * @return {Sequelize.or}
   */
  Sequelize.or = Sequelize.prototype.or = function() {
-    return new Utils.or(Array.prototype.slice.call(arguments));
+    return new Utils.or(Utils.sliceArgs(arguments));
  };
  /**
@@ -1136,7 +1136,7 @@ module.exports = (function() {
  };
  Sequelize.prototype.log = function() {
-    var args = [].slice.call(arguments)
+    var args = Utils.sliceArgs(arguments)
      , last = Utils._.last(args)
      , options;

--- a/lib/utils.js
+++ b/lib/utils.js
@@ -304,6 +304,15 @@ var Utils = module.exports = {
    return errStack;
  },
+  sliceArgs: function (args, begin) {
+    begin = begin || 0;
+    var tmp = new Array(args.length - begin);
+    for (var i = begin; i < args.length; ++i) {
+        tmp[i - begin] = args[i];
+    }
+    return tmp;
+  },
  now: function(dialect) {
    var now = new Date();
    if (dialect !== 'postgres') now.setMilliseconds(0);
@@ -339,7 +348,7 @@ var Utils = module.exports = {
  col: function(col) {
    if (arguments.length > 1) {
-      col = Array.prototype.slice.call(arguments);
+      col = this.sliceArgs(arguments);
    }
    this.col = col;
  },

--- a/test/integration/instance.validations.test.js
+++ b/test/integration/instance.validations.test.js
--- a/test/unit/instance/build.test.js
+++ b/test/unit/instance/build.test.js
@@ -20,17 +20,37 @@ describe(Support.getTestDialectTeaser('Instance'), function() {
            type: DataTypes.DATE,
            allowNull: true,
            defaultValue: DataTypes.NOW
+          },
+          ip: {
+            type: DataTypes.STRING,
+            validate: {
+              isIP: true
+            }
+          },
+          ip2: {
+            type: DataTypes.STRING,
+            validate: {
+              isIP: {
+                msg: 'test'
+              }
+            }
          }
+        }, {
+          timestamp: false
        })
        , instance;
-      instance = Model.build({});
+      instance = Model.build({ip: '127.0.0.1', ip2: '0.0.0.0'});
      expect(instance.get('created_time')).to.be.ok;
      expect(instance.get('created_time')).to.be.an.instanceof(Date);
      expect(instance.get('updated_time')).to.be.ok;
      expect(instance.get('updated_time')).to.be.an.instanceof(Date);
+      return instance.validate().then(function(err) {
+        expect(err).to.be.equal(undefined);
+      });
    });
    it('should popuplate explicitely undefined UUID primary keys', function () {

--- a/test/unit/model/validation.test.js
+++ b/test/unit/model/validation.test.js
+'use strict';
+/* jshint -W030 */
+/* jshint -W110 */
+var chai = require('chai')
+  , expect = chai.expect
+  , Sequelize = require(__dirname + '/../../../index')
+  , Support = require(__dirname + '/../support')
+  , config = require(__dirname + '/../../config/config');
+describe(Support.getTestDialectTeaser('InstanceValidator'), function() {
+  describe('validations', function() {
+    var checks = {
+      is: {
+        spec: { args: ['[a-z]', 'i'] },
+        fail: '0',
+        pass: 'a'
+      }
+    , not: {
+        spec: { args: ['[a-z]', 'i'] },
+        fail: 'a',
+        pass: '0'
+      }
+    , isEmail: {
+        fail: 'a',
+        pass: 'abc@abc.com'
+      }
+    , isUrl: {
+        fail: 'abc',
+        pass: 'http://abc.com'
+      }
+    , isIP: {
+        fail: 'abc',
+        pass: '129.89.23.1'
+      }
+    , isIPv4: {
+        fail: 'abc',
+        pass: '129.89.23.1'
+      }
+    , isIPv6: {
+        fail: '1111:2222:3333::5555:',
+        pass: 'fe80:0000:0000:0000:0204:61ff:fe9d:f156'
+      }
+    , isAlpha: {
+        fail: '012',
+        pass: 'abc'
+      }
+    , isAlphanumeric: {
+        fail: '_abc019',
+        pass: 'abc019'
+      }
+    , isNumeric: {
+        fail: 'abc',
+        pass: '019'
+      }
+    , isInt: {
+        fail: '9.2',
+        pass: '-9'
+      }
+    , isLowercase: {
+        fail: 'AB',
+        pass: 'ab'
+      }
+    , isUppercase: {
+        fail: 'ab',
+        pass: 'AB'
+      }
+    , isDecimal: {
+        fail: 'a',
+        pass: '0.2'
+      }
+    , isFloat: {
+        fail: 'a',
+        pass: '9.2'
+      }
+    , isNull: {
+        fail: 0,
+        pass: null
+      }
+    , notEmpty: {
+        fail: '       ',
+        pass: 'a'
+      }
+    , equals: {
+        spec: { args: 'bla bla bla' },
+        fail: 'bla',
+        pass: 'bla bla bla'
+      }
+    , contains: {
+        spec: { args: 'bla' },
+        fail: 'la',
+        pass: '0bla23'
+      }
+    , notContains: {
+        spec: { args: 'bla' },
+        fail: '0bla23',
+        pass: 'la'
+      }
+    , regex: {
+        spec: { args: ['[a-z]', 'i'] },
+        fail: '0',
+        pass: 'a'
+      }
+    , notRegex: {
+        spec: { args: ['[a-z]', 'i'] },
+        fail: 'a',
+        pass: '0'
+      }
+    , len: {
+        spec: { args: [2, 4] },
+        fail: ['1', '12345'],
+        pass: ['12', '123', '1234'],
+        raw: true
+      }
+    , len$: {
+        spec: [2, 4],
+        fail: ['1', '12345'],
+        pass: ['12', '123', '1234'],
+        raw: true
+      }
+    , isUUID: {
+        spec: { args: 4 },
+        fail: 'f47ac10b-58cc-3372-a567-0e02b2c3d479',
+        pass: 'f47ac10b-58cc-4372-a567-0e02b2c3d479'
+      }
+    , isDate: {
+        fail: 'not a date',
+        pass: '2011-02-04'
+      }
+    , isAfter: {
+        spec: { args: '2011-11-05' },
+        fail: '2011-11-04',
+        pass: '2011-11-06'
+      }
+    , isBefore: {
+        spec: { args: '2011-11-05' },
+        fail: '2011-11-06',
+        pass: '2011-11-04'
+      }
+    , isIn: {
+        spec: { args: 'abcdefghijk' },
+        fail: 'ghik',
+        pass: 'ghij'
+      }
+    , notIn: {
+        spec: { args: 'abcdefghijk' },
+        fail: 'ghij',
+        pass: 'ghik'
+      }
+    , max: {
+        spec: { args: 23 },
+        fail: '24',
+        pass: '23'
+      }
+    , max$: {
+        spec: 23,
+        fail: '24',
+        pass: '23'
+      }
+    , min: {
+        spec: { args: 23 },
+        fail: '22',
+        pass: '23'
+      }
+    , min$: {
+        spec: 23,
+        fail: '22',
+        pass: '23'
+      }
+    , isCreditCard: {
+        fail: '401288888888188f',
+        pass: '4012888888881881'
+      }
+    };
+    var applyFailTest = function applyFailTest(validatorDetails, i, validator) {
+        var failingValue = validatorDetails.fail[i];
+        it('correctly specifies an instance as invalid using a value of "' + failingValue + '" for the validation "' + validator + '"', function() {
+          var validations = {}
+            , message = validator + '(' + failingValue + ')';
+          if (validatorDetails.hasOwnProperty('spec')) {
+            validations[validator] = validatorDetails.spec;
+          } else {
+            validations[validator] = {};
+          }
+          validations[validator].msg = message;
+          var UserFail = this.sequelize.define('User' + config.rand(), {
+            name: {
+              type: Sequelize.STRING,
+              validate: validations
+            }
+          });
+          var failingUser = UserFail.build({ name: failingValue });
+          return failingUser.validate().then(function(_errors) {
+            expect(_errors).not.to.be.null;
+            expect(_errors).to.be.an.instanceOf(Error);
+            expect(_errors.get('name')[0].message).to.equal(message);
+          });
+        });
+      }
+      , applyPassTest = function applyPassTest(validatorDetails, j, validator, msg) {
+          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')) {
+              validations[validator] = validatorDetails.spec;
+            } else {
+              validations[validator] = {};
+            }
+            if (msg) {
+              validations[validator].msg = validator + '(' + succeedingValue + ')';
+            }
+            var UserSuccess = this.sequelize.define('User' + config.rand(), {
+              name: {
+                type: Sequelize.STRING,
+                validate: validations
+              }
+            });
+            var successfulUser = UserSuccess.build({ name: succeedingValue });
+            return successfulUser.validate().then(function(errors) {
+              expect(errors).to.be.undefined;
+            }).catch(function(err) {
+              expect(err).to.deep.equal({});
+            });
+          });
+        };
+      for (var validator in checks) {
+        if (checks.hasOwnProperty(validator)) {
+          validator = validator.replace(/\$$/, '');
+          var validatorDetails = checks[validator];
+          if (!validatorDetails.hasOwnProperty('raw')) {
+            validatorDetails.fail = Array.isArray(validatorDetails.fail) ? validatorDetails.fail : [validatorDetails.fail];
+            validatorDetails.pass = Array.isArray(validatorDetails.pass) ? validatorDetails.pass : [validatorDetails.pass];
+          }
+          for (var i = 0; i < validatorDetails.fail.length; i++) {
+            applyFailTest(validatorDetails, i, validator);
+          }
+          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);
+          }
+        }
+      }
+  });
+});