Merge pull request #4151 from sequelize/fix-btm-pairing

fix(belongs-to-many): properly pair association based on the through …

lib/associations/belongs-to-many.js

@@ -36,19 +36,35 @@ var Utils = require('./../utils')
 var BelongsToMany = function(source, target, options) {
 var BelongsToMany = function(source, target, options) {
   Association.call(this);
   Association.call(this);
 
 
+  options = options || {};
+
+  if (options.through === undefined || options.through === true || options.through === null) {
+    throw new Error('belongsToMany must be given a through option, either a string or a model');
+  }
+
+  if (!options.through.model) {
+    options.through = {
+      model: options.through
+    };
+  }
+
   this.associationType = 'BelongsToMany';
   this.associationType = 'BelongsToMany';
   this.source = source;
   this.source = source;
   this.target = target;
   this.target = target;
   this.targetAssociation = null;
   this.targetAssociation = null;
-  this.options = options || {};
+  this.options = options;
   this.sequelize = source.modelManager.sequelize;
   this.sequelize = source.modelManager.sequelize;
-  this.through = options.through;
+  this.through = _.assign({}, options.through);
   this.scope = options.scope;
   this.scope = options.scope;
   this.isMultiAssociation = true;
   this.isMultiAssociation = true;
   this.isSelfAssociation = this.source === this.target;
   this.isSelfAssociation = this.source === this.target;
   this.doubleLinked = false;
   this.doubleLinked = false;
   this.as = this.options.as;
   this.as = this.options.as;
 
 
+  if (!this.as && this.isSelfAssociation) {
+    throw new Error('\'as\' must be defined for many-to-many self-associations');
+  }
+
   if (this.as) {
   if (this.as) {
     this.isAliased = true;
     this.isAliased = true;
 
 
@@ -71,24 +87,10 @@ var BelongsToMany = function(source, target, options) {
     this.isSelfAssociation ? (this.as || this.target.tableName) : this.target.tableName
     this.isSelfAssociation ? (this.as || this.target.tableName) : this.target.tableName
   );
   );
 
 
-  if (this.through === undefined || this.through === true || this.through === null) {
-    throw new Error('belongsToMany must be given a through option, either a string or a model');
-  }
-
-  if (!this.through.model) {
-    this.through = {
-      model: this.through
-    };
-  }
-
   /*
   /*
    * If self association, this is the target association - Unless we find a pairing association
    * If self association, this is the target association - Unless we find a pairing association
    */
    */
   if (this.isSelfAssociation) {
   if (this.isSelfAssociation) {
-    if (!this.as) {
-      throw new Error('\'as\' must be defined for many-to-many self-associations');
-    }
-
     this.targetAssociation = this;
     this.targetAssociation = this;
   }
   }
 
 
@@ -145,6 +147,7 @@ var BelongsToMany = function(source, target, options) {
 
 
     if (this.options.through.model === association.options.through.model) {
     if (this.options.through.model === association.options.through.model) {
       this.paired = association;
       this.paired = association;
+      association.paired = this;
     }
     }
   }, this);
   }, this);
 
 

test/unit/associations/belongs-to-many.test.js

@@ -63,8 +63,99 @@ describe(Support.getTestDialectTeaser('belongsToMany'), function() {
         expect(this.destroy).to.have.been.calledOnce;
         expect(this.destroy).to.have.been.calledOnce;
       });
       });
     });
     });
+  });
+
+  describe('foreign keys', function() {
+    it('should infer otherKey from paired BTM relationship with a through string defined', function () {
+      var User = this.sequelize.define('User', {});
+      var Place = this.sequelize.define('Place', {});
+
+      var Places = User.belongsToMany(Place, { through: 'user_places', foreignKey: 'user_id' });
+      var Users = Place.belongsToMany(User, { through: 'user_places', foreignKey: 'place_id' });
+
+      expect(Places.paired).to.equal(Users);
+      expect(Users.paired).to.equal(Places);
+
+      expect(Places.foreignKey).to.equal('user_id');
+      expect(Users.foreignKey).to.equal('place_id');
+
+      expect(Places.otherKey).to.equal('place_id');
+      expect(Users.otherKey).to.equal('user_id');
+    });
+
+    it('should infer otherKey from paired BTM relationship with a through model defined', function () {
+      var User = this.sequelize.define('User', {});
+      var Place = this.sequelize.define('User', {});
+      var UserPlace = this.sequelize.define('UserPlace', {
+        id: {
+          primaryKey: true,
+          type: DataTypes.INTEGER,
+          autoIncrement: true
+        }
+      }, {timestamps: false});
+
+      var Places = User.belongsToMany(Place, { through: UserPlace, foreignKey: 'user_id' });
+      var Users = Place.belongsToMany(User, { through: UserPlace, foreignKey: 'place_id' });
+
+      expect(Places.paired).to.equal(Users);
+      expect(Users.paired).to.equal(Places);
+
+      expect(Places.foreignKey).to.equal('user_id');
+      expect(Users.foreignKey).to.equal('place_id');
+
+      expect(Places.otherKey).to.equal('place_id');
+      expect(Users.otherKey).to.equal('user_id');
+
+      expect(Object.keys(UserPlace.rawAttributes).length).to.equal(3); // Defined primary key and two foreign keys
+    });
+  });
+
+  describe('self-associations', function () {
+    it('does not pair multiple self associations with different through arguments', function () {
+      var User = current.define('user', {})
+        , UserFollowers = current.define('userFollowers', {})
+        , Invite = current.define('invite', {});
+
+      User.Followers = User.belongsToMany(User, {
+        as: 'Followers',
+        through: UserFollowers,
+      });
+
+      User.Invites = User.belongsToMany(User, {
+        as: 'Invites',
+        foreignKey: 'InviteeId',
+        through: Invite
+      });
+
+      expect(User.Followers.paired).not.to.be.ok;
+      expect(User.Invites.paired).not.to.be.ok;
+
+      expect(User.Followers.otherKey).not.to.equal(User.Invites.foreignKey);
+    });
+
+    it('correctly generates a foreign/other key when none are defined', function () {
+      var User = current.define('user', {})
+        , UserFollowers = current.define('userFollowers', {
+            id: {
+              type: DataTypes.INTEGER,
+              primaryKey: true,
+              autoIncrement: true
+            }
+          }, {
+            timestamps: false
+          });
+
+      User.Followers = User.belongsToMany(User, {
+        as: 'Followers',
+        through: UserFollowers
+      });
+
+      expect(User.Followers.foreignKey).to.be.ok;
+      expect(User.Followers.otherKey).to.be.ok;
+
+      expect(Object.keys(UserFollowers.rawAttributes).length).to.equal(3);
+    });
 
 
-    describe('belongsToMany', function () {
     it('works with singular and plural name for self-associations', function () {
     it('works with singular and plural name for self-associations', function () {
       // Models taken from https://github.com/sequelize/sequelize/issues/3796
       // Models taken from https://github.com/sequelize/sequelize/issues/3796
       var Service = current.define('service', {})
       var Service = current.define('service', {})
@@ -85,5 +176,4 @@ describe(Support.getTestDialectTeaser('belongsToMany'), function() {
       expect(Instance.prototype).not.to.have.property('addSupplementeds').which.is.a.function;
       expect(Instance.prototype).not.to.have.property('addSupplementeds').which.is.a.function;
     });
     });
   });
   });
-  });
 });
 });