Merge branch 'master' into milestones/2.0.0

changelog.md

@@ -5,6 +5,7 @@ Notice: All 1.7.x changes are present in 2.0.x aswell
 - fixes a hangup issue for mysql [#1244](https://github.com/sequelize/sequelize/pull/1244)
 - improves handling of uncaught errors in eventemitter [#1245](https://github.com/sequelize/sequelize/pull/1245)
 - fixes bug with mysql replication and pool settings [#1251](https://github.com/sequelize/sequelize/pull/1251)
+- through models created by N:M associations no longer inherit hooks [#1263](https://github.com/sequelize/sequelize/issues/1263)
 
 # v2.0.0 (alpha1) #
 - [FEATURE] async validations. [#580](https://github.com/sequelize/sequelize/pull/580). thanks to Interlock

lib/associations/has-many.js

@@ -67,28 +67,28 @@ module.exports = (function() {
      * Else find partner DAOFactory if present, to identify double linked association
      */
     else if (this.through) {
-    _.each(this.target.associations, function (association, accessor) {
-      if (self.source === association.target) {
-        var paired = false
+      _.each(this.target.associations, function (association, accessor) {
+        if (self.source === association.target) {
+          var paired = false
 
-        // If through is default, we determine pairing by the accesor value (i.e. DAOFactory's using as won't pair, but regular ones will)
-        if (self.through === true && accessor === self.associationAccessor) {
-          paired = true
-        }
-        // If through is not default, determine pairing by through value (model/string)
-        if (self.through !== true && self.options.through === association.options.through) {
-          paired = true
-        }
-        // If paired, set properties identifying both associations as double linked, and allow them to each eachtoerh
-        if (paired) {
-          self.doubleLinked = true
-          association.doubleLinked = true
+          // If through is default, we determine pairing by the accesor value (i.e. DAOFactory's using as won't pair, but regular ones will)
+          if (self.through === true && accessor === self.associationAccessor) {
+            paired = true
+          }
+          // If through is not default, determine pairing by through value (model/string)
+          if (self.through !== true && self.options.through === association.options.through) {
+            paired = true
+          }
+          // If paired, set properties identifying both associations as double linked, and allow them to each eachtoerh
+          if (paired) {
+            self.doubleLinked = true
+            association.doubleLinked = true
 
-          self.targetAssociation = association
-          association.targetAssociation = self
+            self.targetAssociation = association
+            association.targetAssociation = self
+          }
         }
-      }
-    })
+      })
     }
 
     /*
@@ -105,7 +105,9 @@ module.exports = (function() {
         tableName: this.through
       }))
 
-      if (this.targetAssociation) this.targetAssociation.through = this.through
+      if (this.targetAssociation) {
+        this.targetAssociation.through = this.through
+      }
     }
 
     this.options.tableName = this.combinedName = (this.through === Object(this.through) ? this.through.tableName : this.through)

lib/associations/mixin.js

@@ -13,7 +13,7 @@ Mixin.hasOne = function(associatedDAOFactory, options) {
   options.useHooks = options.hooks
 
   // the id is in the foreign table
-  var association = new HasOne(this, associatedDAOFactory, Utils._.extend((options||{}), this.options))
+  var association = new HasOne(this, associatedDAOFactory, Utils._.extend(options, this.options))
   this.associations[association.associationAccessor] = association.injectAttributes()
 
   association.injectGetter(this.DAO.prototype);
@@ -46,8 +46,10 @@ Mixin.hasMany = function(associatedDAOFactory, options) {
   options.hooks = options.hooks === undefined ? false : Boolean(options.hooks)
   options.useHooks = options.hooks
 
+  options = Utils._.extend(options, Utils._.omit(this.options, ['hooks']))
+
   // the id is in the foreign table or in a connecting table
-  var association = new HasMany(this, associatedDAOFactory, Utils._.extend((options||{}), this.options))
+  var association = new HasMany(this, associatedDAOFactory, options)
   this.associations[association.associationAccessor] = association.injectAttributes()
 
   association.injectGetter(this.DAO.prototype)

lib/dao-factory.js

@@ -1430,7 +1430,14 @@ module.exports = (function() {
   var optClone = function (options) {
     return Utils._.cloneDeep(options, function (elem) {
       // The DAOFactories used for include are pass by ref, so don't clone them. Otherwise return undefined, meaning, 'handle this lodash'
-      return elem instanceof DAOFactory ? elem : undefined
+      if (elem instanceof DAOFactory) {
+        return elem
+      }
+      if (elem instanceof Utils.col || elem instanceof Utils.literal || elem instanceof Utils.cast || elem instanceof Utils.fn) {
+        return elem
+      }
+
+      return undefined
     })
   }
 

test/dao-factory/findAll.test.js

@@ -857,6 +857,22 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
           })
         })
       })
+
+      it.only('should be possible to order by sequelize.col()', function (done) {
+        var self = this
+        var Company = this.sequelize.define('Company', {
+          name: Sequelize.STRING
+        });
+
+        Company.sync().done(function () {
+          Company.findAll({
+            order: [self.sequelize.col('name')]
+          }).done(function(err) {
+            expect(err).not.to.be.ok
+            done()
+          })
+        })
+      })
     })
   })
 

test/hooks.test.js

@@ -4,6 +4,8 @@ var chai      = require('chai')
   , Support   = require(__dirname + '/support')
   , DataTypes = require(__dirname + '/../lib/data-types')
   , _         = require('lodash')
+  , Sequelize = Support.Sequelize
+  , sinon     = require('sinon')
 
 chai.Assertion.includeStack = true
 
@@ -1866,6 +1868,43 @@ describe(Support.getTestDialectTeaser("Hooks"), function () {
         })
       })
     })
+
+    it('should not trigger hooks on parent when using N:M association setters', function (done) {
+      var A = this.sequelize.define('A', {
+        name: Sequelize.STRING
+      })
+      var B = this.sequelize.define('B', {
+        name: Sequelize.STRING
+      })
+
+      var hookCalled = 0
+
+      A.addHook('afterCreate', function (instance, next) {
+        hookCalled++
+        next()
+      })
+ 
+      B.hasMany(A)
+      A.hasMany(B)
+
+      this.sequelize.sync({force: true}).done(function (err) {
+        expect(err).not.to.be.ok
+
+        var chainer = new Sequelize.Utils.QueryChainer([
+          A.create({name: 'a'}),
+          B.create({name: 'b'})
+        ])
+
+        chainer.run().done(function (err, res, a, b) {
+          expect(err).not.to.be.ok
+          a.addB(b).done(function (err) {
+            expect(err).not.to.be.ok
+            expect(hookCalled).to.equal(1)
+            done()
+          })
+        })
+      })
+    })
   })
 
   describe('#updateAttributes', function() {