Merge pull request #1305 from janmeier/1117fix

Change the way PKs for through table are handled (Closes #1117)

lib/associations/has-many.js

@@ -128,8 +128,9 @@ module.exports = (function() {
   // the id is in the target table
   // the id is in the target table
   // or in an extra table which connects two tables
   // or in an extra table which connects two tables
   HasMany.prototype.injectAttributes = function() {
   HasMany.prototype.injectAttributes = function() {
-    var doubleLinked = this.doubleLinked
-      , self         = this
+    var doubleLinked      = this.doubleLinked
+      , self              = this
+      , primaryKeyDeleted = false
 
 
     this.identifier = this.options.foreignKey || Utils._.underscoredIf(Utils.singularize(this.source.tableName, this.source.options.language) + "Id", this.options.underscored)
     this.identifier = this.options.foreignKey || Utils._.underscoredIf(Utils.singularize(this.source.tableName, this.source.options.language) + "Id", this.options.underscored)
 
 
@@ -152,21 +153,29 @@ module.exports = (function() {
         }
         }
       }
       }
 
 
+      // remove any PKs previously defined by sequelize
+      Utils._.each(this.through.attributes, function(dataTypeString, attributeName) {
+        if (dataTypeString.toString().indexOf('PRIMARY KEY') !== -1 && self.through.rawAttributes[attributeName]._autoGenerated === true) {
+          delete self.through.rawAttributes[attributeName]
+          primaryKeyDeleted = true
+        }
+      })
+
       // define a new model, which connects the models
       // define a new model, which connects the models
       var combinedTableAttributes = {}
       var combinedTableAttributes = {}
       var sourceKeys = Object.keys(this.source.primaryKeys);
       var sourceKeys = Object.keys(this.source.primaryKeys);
       var sourceKeyType = ((!this.source.hasPrimaryKeys || sourceKeys.length !== 1) ? DataTypes.INTEGER : this.source.rawAttributes[sourceKeys[0]].type)
       var sourceKeyType = ((!this.source.hasPrimaryKeys || sourceKeys.length !== 1) ? DataTypes.INTEGER : this.source.rawAttributes[sourceKeys[0]].type)
       var targetKeys = Object.keys(this.target.primaryKeys);
       var targetKeys = Object.keys(this.target.primaryKeys);
       var targetKeyType = ((!this.target.hasPrimaryKeys || targetKeys.length !== 1) ? DataTypes.INTEGER : this.target.rawAttributes[targetKeys[0]].type)
       var targetKeyType = ((!this.target.hasPrimaryKeys || targetKeys.length !== 1) ? DataTypes.INTEGER : this.target.rawAttributes[targetKeys[0]].type)
-      combinedTableAttributes[this.identifier] = {type: sourceKeyType, primaryKey: true}
-      combinedTableAttributes[this.foreignIdentifier] = {type: targetKeyType, primaryKey: true}
-
-      // remove any previously defined PKs
-      Utils._.each(this.through.attributes, function(dataTypeString, attributeName) {
-        if (dataTypeString.toString().indexOf('PRIMARY KEY') !== -1) {
-          delete self.through.rawAttributes[attributeName]
-        }
-      })
+      
+      if (primaryKeyDeleted) {
+        combinedTableAttributes[this.identifier] = {type: sourceKeyType, primaryKey: true}
+        combinedTableAttributes[this.foreignIdentifier] = {type: targetKeyType, primaryKey: true}
+      } else { 
+        var uniqueKey = [this.through.tableName, this.identifier, this.foreignIdentifier, 'unique'].join('_')
+        combinedTableAttributes[this.identifier] = {type: sourceKeyType, unique: uniqueKey}
+        combinedTableAttributes[this.foreignIdentifier] = {type: targetKeyType, unique: uniqueKey}
+      }
 
 
       this.through.rawAttributes = Utils._.merge(this.through.rawAttributes, combinedTableAttributes)
       this.through.rawAttributes = Utils._.merge(this.through.rawAttributes, combinedTableAttributes)
       this.through.init(this.through.daoFactoryManager)
       this.through.init(this.through.daoFactoryManager)

lib/dao-factory.js

@@ -1265,7 +1265,8 @@ module.exports = (function() {
           type: DataTypes.INTEGER,
           type: DataTypes.INTEGER,
           allowNull: false,
           allowNull: false,
           primaryKey: true,
           primaryKey: true,
-          autoIncrement: true
+          autoIncrement: true,
+          _autoGenerated: true
         }
         }
       }
       }
 
 

test/associations/has-many.test.js

@@ -889,7 +889,53 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
         }, 50)
         }, 50)
       })
       })
     })
     })
+    
+    describe('primary key handling for join table', function () {
+      it('removes the primary key if it was added by sequelize', function () {
+        var self = this
+        this.UserTasks = this.sequelize.define('usertasks', {});
+
+        this.User.hasMany(this.Task, { through: this.UserTasks })
+        this.Task.hasMany(this.User, { through: this.UserTasks })
 
 
+        expect(Object.keys(self.UserTasks.primaryKeys)).to.deep.equal(['taskId', 'userId'])
+      })
+
+      it('keeps the primary key if it was added by the user', function () {
+        var self = this
+          , fk
+         
+        this.UserTasks = this.sequelize.define('usertasks', {
+          id: {
+            type: Sequelize.INTEGER,
+            autoincrement: true,
+            primaryKey: true
+          }
+        });
+        this.UserTasks2 = this.sequelize.define('usertasks2', {
+          userTasksId: {
+            type: Sequelize.INTEGER,
+            autoincrement: true,
+            primaryKey: true
+          }
+        }); 
+
+        this.User.hasMany(this.Task, { through: this.UserTasks })
+        this.Task.hasMany(this.User, { through: this.UserTasks })
+ 
+        this.User.hasMany(this.Task, { through: this.UserTasks2 })
+        this.Task.hasMany(this.User, { through: this.UserTasks2 })
+ 
+        expect(Object.keys(self.UserTasks.primaryKeys)).to.deep.equal(['id'])
+        expect(Object.keys(self.UserTasks2.primaryKeys)).to.deep.equal(['userTasksId'])
+
+        _.each([self.UserTasks, self.UserTasks2], function (model) {
+          fk = Object.keys(model.options.uniqueKeys)[0]
+          expect(model.options.uniqueKeys[fk].fields).to.deep.equal([ 'taskId', 'userId' ])
+        })
+      })
+    })
+    
     describe('join table model', function () {
     describe('join table model', function () {
       beforeEach(function (done) {
       beforeEach(function (done) {
         this.User = this.sequelize.define('User', {})
         this.User = this.sequelize.define('User', {})