merge skipped fields refactor

Mick Hansen
Commit 0750a939 authored Jan 11, 2014 by Mick Hansen
Showing with 123 additions and 24 deletions
README.md
changelog.md
lib/dao-factory.js
lib/dao-validator.js
lib/dialects/postgres/query.js
lib/utils.js
test/dao-factory/create.test.js
test/dao.validations.test.js
test/dao/values.test.js
test/mysql/query-generator.test.js
test/postgres/query-generator.test.js
test/sqlite/query-generator.test.js
--- a/README.md
+++ b/README.md
@@ -4,7 +4,7 @@ MySQL, MariaDB, PostgresSQL, and SQLite Object Relational Mapper (ORM) for [node
 ### Install
-To install 1.x.x (currently 1.7.x) - which has a stable API and is backwards compatible:
+To install 1.x.x (currently 1.7.x) - which has a stable API and is mostly backwards compatible:
 `npm install sequelize`
@@ -26,7 +26,7 @@ To install 2.x.x branch - which has a unstable API and will break backwards comp
 ### 2.0.0 ###
 There is a parallel "branch" of the project, released as `2.0.0-alphaX` in NPM. All those releases are based on the master
-and will get all the changes of the master. However, `2.0.0` will contain backwards compatibility breaking changes. Check the
+and will get all the changes of the master. However, `2.0.0` will contain major backwards compatibility breaking changes. Check the
 changelog of the branch: https://github.com/sequelize/sequelize/blob/milestones/2.0.0/changelog.md
 ##### 2.0.0 API should be considered unstable

--- a/changelog.md
+++ b/changelog.md
@@ -8,6 +8,9 @@ Notice: All 1.7.x changed are present in 2.0.x aswell
 - fixes a few bugs with transactions in regards to associations
 - add error handling for transaction creation
 - `sequelize --undo` will now actually undo migrations. Its basically an alias for `sequelize --migrate --undo`. [#1059](https://github.com/sequelize/sequelize/pull/1059)
+- fix bug where `{where: {ne: null}}` would result in `!= NULL` instead of `IS NOT NULL` [#1231](https://github.com/sequelize/sequelize/pull/1059)
+- fixes a bug with validation skipping using the `fields` options. [#1233](https://github.com/sequelize/sequelize/pull/1233)
+- fixes a bug with postgres and setters [#1234](https://github.com/sequelize/sequelize/issues/123)
 #### Backwards compatability changes
 - Hooks are no longer passing value hashes. Instead, they are now passing instances of the model.

--- a/lib/dao-factory.js
+++ b/lib/dao-factory.js
@@ -875,9 +875,13 @@ module.exports = (function() {
        options.fields = newFields  || options.fields
        if (options.validate === true) {
+          if (options.fields.length) {
+            var skippedFields = Utils._.difference(Object.keys(self.attributes), options.fields);
+          }
          if (options.hooks === true) {
            var iterate = function(i) {
-              daos[i].hookValidate({skip: options.fields}).complete(function (err) {
+              daos[i].hookValidate({skip: skippedFields}).complete(function (err) {
                if (!!err) {
                  errors.push({record: v, errors: err})
                }
@@ -890,13 +894,13 @@ module.exports = (function() {
                iterate(i)
              })
            }
-          } else {
+          } else 
-             var afterDaos = Utils._.after(daos.length, function() {
+            var afterDaos = Utils._.after(daos.length, function() {
              next(errors.length > 0 ? errors : null)
            })
            daos.forEach(function(v) {
-              v.validate({skip: options.fields}).success(function(err) {
+              v.validate({skip: skippedFields}).success(function(err) {
                if (!!err) {
                  errors.push({record: v, errors: err})
                }
@@ -1149,12 +1153,22 @@ module.exports = (function() {
      }
      if (options.validate === true) {
+<<<<<<< HEAD
        var build    = self.build(attrValueHash)
          , attrKeys = Object.keys(attrValueHash)
        build.hookValidate({
          skip: Object.keys(build.dataValues).filter(function(val) { return attrKeys.indexOf(val) !== -1 })
        }).error(function(err) {
+=======
+        var build = self.build(attrValueHash)
+        // We want to skip validations for all other fields
+        var updatedFields = Object.keys(attrValueHash)
+        var skippedFields = Utils._.difference(Object.keys(self.attributes), updatedFields)
+        build.hookValidate({skip: skippedFields}).error(function(err) {
+>>>>>>> master
          emitter.emit('error', err)
        }).success(function(attributes) {
          if (!!attributes && !!attributes.dataValues) {

--- a/lib/dao-validator.js
+++ b/lib/dao-validator.js
@@ -93,7 +93,7 @@ var validateAttributes = function() {
  Utils._.each(this.model.rawAttributes, function(rawAttribute, field) {
    var value          = self.model.dataValues[field] || undefined
      , hasAllowedNull = ((rawAttribute === undefined || rawAttribute.allowNull === true) && ((value === null) || (value === undefined)))
-      , isSkipped      = self.options.skip.length > 0 && self.options.skip.indexOf(field) === -1
+      , isSkipped      = self.options.skip.length > 0 && self.options.skip.indexOf(field) !== -1
    if (self.model.validators.hasOwnProperty(field) && !hasAllowedNull && !isSkipped) {
      errors = Utils._.merge(errors, validateAttribute.call(self, value, field))

--- a/lib/dialects/postgres/query.js
+++ b/lib/dialects/postgres/query.js
@@ -139,7 +139,7 @@ module.exports = (function() {
            if (!!this.callee.daoFactory && !!this.callee.daoFactory.rawAttributes && !!this.callee.daoFactory.rawAttributes[key] && !!this.callee.daoFactory.rawAttributes[key].type && !!this.callee.daoFactory.rawAttributes[key].type.type && this.callee.daoFactory.rawAttributes[key].type.type === DataTypes.HSTORE.type) {
              record = hstore.parse(record)
            }
-            this.callee[key] = record
+            this.callee.dataValues[key] = record
          }
        }
      }  
@@ -152,7 +152,7 @@ module.exports = (function() {
            if (!!this.callee.daoFactory && !!this.callee.daoFactory.rawAttributes && !!this.callee.daoFactory.rawAttributes[key] && !!this.callee.daoFactory.rawAttributes[key].type && !!this.callee.daoFactory.rawAttributes[key].type.type && this.callee.daoFactory.rawAttributes[key].type.type === DataTypes.HSTORE.type) {
              record = hstore.parse(record)
            }
-            this.callee[key] = record
+            this.callee.dataValues[key] = record
          }
        }
      }

--- a/lib/utils.js
+++ b/lib/utils.js
@@ -299,7 +299,7 @@ var Utils = module.exports = {
    case 'eq':
      return '='
    case 'ne':
-      return val ? '!=' : 'IS NOT'
+      return val === null ? 'IS NOT' : '!='
    case 'between':
    case '..':
      return 'BETWEEN'
@@ -451,7 +451,7 @@ var Utils = module.exports = {
      for (var key in hash) {
        if (key instanceof Utils.literal) {
-          _hash[key] = hash[key]          
+          _hash[key] = hash[key]
        } else if (key.indexOf('.') === -1) {
          _hash[tableName + '.' + key] = hash[key]
        } else {

--- a/test/dao-factory/create.test.js
+++ b/test/dao-factory/create.test.js
@@ -597,7 +597,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
    it('should allow blank creates (with timestamps: false)', function (done) {
      var Worker = this.sequelize.define('Worker', {}, {timestamps: false})
-      Worker.sync().done(function(err) { 
+      Worker.sync().done(function(err) {
        Worker.create({}, {fields: []}).done(function (err, worker) {
          expect(err).not.to.be.ok
          expect(worker).to.be.ok
@@ -608,7 +608,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
    it('should allow truly blank creates', function (done) {
      var Worker = this.sequelize.define('Worker', {}, {timestamps: false})
-      Worker.sync().done(function(err) { 
+      Worker.sync().done(function(err) {
        Worker.create({}, {fields: []}).done(function (err, worker) {
          expect(err).not.to.be.ok
          expect(worker).to.be.ok
@@ -897,7 +897,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
    it('should allow blank arrays (return immediatly)', function (done) {
      var Worker = this.sequelize.define('Worker', {})
-      Worker.sync().done(function(err) { 
+      Worker.sync().done(function(err) {
        Worker.bulkCreate([]).done(function (err, workers) {
          expect(err).not.to.be.ok
          expect(workers).to.be.ok
@@ -909,7 +909,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
    it('should allow blank creates (with timestamps: false)', function (done) {
      var Worker = this.sequelize.define('Worker', {}, {timestamps: false})
-      Worker.sync().done(function(err) { 
+      Worker.sync().done(function(err) {
        Worker.bulkCreate([{}, {}]).done(function (err, workers) {
          expect(err).not.to.be.ok
          expect(workers).to.be.ok
@@ -920,7 +920,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
    it('should allow autoincremented attributes to be set', function (done) {
      var Worker = this.sequelize.define('Worker', {}, {timestamps: false})
-      Worker.sync().done(function(err) { 
+      Worker.sync().done(function(err) {
        Worker.bulkCreate([
          {id: 5},
          {id: 10}
@@ -954,4 +954,4 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
      })
    })
  })
 })
\ No newline at end of file
--- a/test/dao.validations.test.js
+++ b/test/dao.validations.test.js
@@ -650,5 +650,24 @@ describe(Support.getTestDialectTeaser("DaoValidator"), function() {
        expect(errors.field[0]).to.equal("Unexpected value or invalid argument")
      })
    })
+    it('skips validations for the given fields', function() {
+      var values = ['value1', 'value2']
+      var Bar = this.sequelize.define('Bar' + config.rand(), {
+        field: {
+          type: Sequelize.ENUM,
+          values: values,
+          validate: {
+            isIn: [values]
+          }
+        }
+      })
+      var failingBar = Bar.build({ field: 'value3' })
+        , errors     = failingBar.validate({ skip: ['field'] })
+      expect(errors).to.be.null
+    })
  })
 })
--- a/test/dao/values.test.js
+++ b/test/dao/values.test.js
@@ -193,6 +193,40 @@ describe(Support.getTestDialectTeaser("DAO"), function () {
        })
        expect(product.toJSON()).to.deep.equal({withTaxes: 1250, price: 1000, id: null})
      })
+      it('should work with save', function (done) {
+        var Contact = this.sequelize.define('Contact', {
+          first: { type: Sequelize.STRING },
+          last: { type: Sequelize.STRING },
+          tags: { 
+            type: Sequelize.STRING,
+            get: function(field) {
+              var val = this.getDataValue(field);
+              return JSON.parse(val);
+            },
+            set: function(val, field) {
+              this.setDataValue(field, JSON.stringify(val));
+            }
+          }
+        });
+        this.sequelize.sync().done(function () {
+          var contact = Contact.build({
+            first: 'My',
+            last: 'Name',
+            tags: ['yes','no']
+          });
+          expect(contact.get('tags')).to.deep.equal(['yes', 'no'])
+          contact.save().done(function(err, me) {
+            expect(err).not.to.be.ok
+            var idToTest = me.id;
+            expect(me.get('tags')).to.deep.equal(['yes', 'no'])
+            done();
+          });
+        });
+      })
    })
    describe('changed', function () {

--- a/test/mysql/query-generator.test.js
+++ b/test/mysql/query-generator.test.js
@@ -273,6 +273,16 @@ if (Support.dialectIsMySQL()) {
          arguments: ['myTable', {where: { field: new Buffer("Sequelize")}}],
          expectation: "SELECT * FROM `myTable` WHERE `myTable`.`field`=X'53657175656c697a65';",
          context: QueryGenerator
+        }, {
+          title: 'use != if ne !== null',
+          arguments: ['myTable', {where: {field: {ne: 0}}}],
+          expectation: "SELECT * FROM `myTable` WHERE `myTable`.`field` != 0;",
+          context: QueryGenerator
+        }, {
+          title: 'use IS NOT if ne === null',
+          arguments: ['myTable', {where: {field: {ne: null}}}],
+          expectation: "SELECT * FROM `myTable` WHERE `myTable`.`field` IS NOT NULL;",
+          context: QueryGenerator
        }
      ],

--- a/test/postgres/query-generator.test.js
+++ b/test/postgres/query-generator.test.js
@@ -268,8 +268,8 @@ if (dialect.match(/^postgres/)) {
        }, {
          title: 'functions can take functions as arguments',
          arguments: ['myTable', function (sequelize) {
-            return { 
+            return {
-              order: [[sequelize.fn('f1', sequelize.fn('f2', sequelize.col('id'))), 'DESC']] 
+              order: [[sequelize.fn('f1', sequelize.fn('f2', sequelize.col('id'))), 'DESC']]
            }
          }],
          expectation: 'SELECT * FROM "myTable" ORDER BY f1(f2("id")) DESC;',
@@ -280,7 +280,7 @@ if (dialect.match(/^postgres/)) {
          arguments: ['myTable', function (sequelize) {
            return {
              order: [
-                [sequelize.fn('f1', sequelize.col('myTable.id')), 'DESC'], 
+                [sequelize.fn('f1', sequelize.col('myTable.id')), 'DESC'],
                [sequelize.fn('f2', 12, 'lalala', new Date(Date.UTC(2011, 2, 27, 10, 1, 55))), 'ASC']
              ]
            }
@@ -410,6 +410,16 @@ if (dialect.match(/^postgres/)) {
          arguments: ['mySchema.myTable', {where: {name: "foo';DROP TABLE mySchema.myTable;"}}],
          expectation: "SELECT * FROM mySchema.myTable WHERE mySchema.myTable.name='foo'';DROP TABLE mySchema.myTable;';",
          context: {options: {quoteIdentifiers: false}}
+        }, {
+          title: 'use != if ne !== null',
+          arguments: ['myTable', {where: {field: {ne: 0}}}],
+          expectation: "SELECT * FROM myTable WHERE myTable.field != 0;",
+          context: {options: {quoteIdentifiers: false}}
+        }, {
+          title: 'use IS NOT if ne === null',
+          arguments: ['myTable', {where: {field: {ne: null}}}],
+          expectation: "SELECT * FROM myTable WHERE myTable.field IS NOT NULL;",
+          context: {options: {quoteIdentifiers: false}}
        }
      ],

--- a/test/sqlite/query-generator.test.js
+++ b/test/sqlite/query-generator.test.js
@@ -165,8 +165,8 @@ if (dialect === 'sqlite') {
        }, {
          title: 'functions can take functions as arguments',
          arguments: ['myTable', function (sequelize) {
-            return { 
+            return {
-              order: [[sequelize.fn('f1', sequelize.fn('f2', sequelize.col('id'))), 'DESC']] 
+              order: [[sequelize.fn('f1', sequelize.fn('f2', sequelize.col('id'))), 'DESC']]
            }
          }],
          expectation: "SELECT * FROM `myTable` ORDER BY f1(f2(`id`)) DESC;",
@@ -177,7 +177,7 @@ if (dialect === 'sqlite') {
          arguments: ['myTable', function (sequelize) {
            return {
              order: [
-                [sequelize.fn('f1', sequelize.col('myTable.id')), 'DESC'], 
+                [sequelize.fn('f1', sequelize.col('myTable.id')), 'DESC'],
                [sequelize.fn('f2', 12, 'lalala', new Date(Date.UTC(2011, 2, 27, 10, 1, 55))), 'ASC']
              ]
            }
@@ -260,6 +260,16 @@ if (dialect === 'sqlite') {
          arguments: ['myTable', {where: { field: new Buffer("Sequelize")}}],
          expectation: "SELECT * FROM `myTable` WHERE `myTable`.`field`=X'53657175656c697a65';",
          context: QueryGenerator
+        }, {
+          title: 'use != if ne !== null',
+          arguments: ['myTable', {where: {field: {ne: 0}}}],
+          expectation: "SELECT * FROM `myTable` WHERE `myTable`.`field` != 0;",
+          context: QueryGenerator
+        }, {
+          title: 'use IS NOT if ne === null',
+          arguments: ['myTable', {where: {field: {ne: null}}}],
+          expectation: "SELECT * FROM `myTable` WHERE `myTable`.`field` IS NOT NULL;",
+          context: QueryGenerator
        }
      ],