fix N:M self associations with foreign keys and through models

lib/associations/has-many-double-linked.js

@@ -82,7 +82,7 @@ module.exports = (function() {
     var self                 = this
       , chainer              = new Utils.QueryChainer()
       , targetAssociation    = self.association.targetAssociation
-      , foreignIdentifier    = targetAssociation.isSelfAssociation ? targetAssociation.foreignIdentifier : targetAssociation.identifier
+      , foreignIdentifier    = self.association.foreignIdentifier
       , sourceKeys           = Object.keys(self.association.source.primaryKeys)
       , targetKeys           = Object.keys(self.association.target.primaryKeys)
       , obsoleteAssociations = []
@@ -175,7 +175,7 @@ module.exports = (function() {
   HasManyDoubleLinked.prototype.injectAdder = function(emitterProxy, newAssociation, additionalAttributes, exists) {
     var attributes          = {}
       , targetAssociation   = this.association.targetAssociation
-      , foreignIdentifier   = targetAssociation.isSelfAssociation ? targetAssociation.foreignIdentifier : targetAssociation.identifier
+      , foreignIdentifier   = targetAssociation.identifier
       , options = {}
 
     var sourceKeys = Object.keys(this.association.source.primaryKeys);

lib/associations/has-many.js

@@ -58,7 +58,7 @@ module.exports = (function() {
     }
 
     /*
-     * If self association, this association is target association
+     * If self association, this is the target association - Unless we find a pairing association
      */
     if (this.isSelfAssociation) {
       this.targetAssociation = this
@@ -67,7 +67,7 @@ module.exports = (function() {
     /*
      * Else find partner DAOFactory if present, to identify double linked association
      */
-    else if (this.through) {
+    if (this.through) {
       _.each(this.target.associations, function (association, accessor) {
         if (self.source === association.target) {
           var paired = false
@@ -91,7 +91,6 @@ module.exports = (function() {
         }
       })
     }
-
     /*
      * If we are double linked, and through is either default or a string, we create the through model and set it on both associations
      */
@@ -145,24 +144,34 @@ module.exports = (function() {
 
     // is there already a single sided association between the source and the target?
     // or is the association on the model itself?
+
     if ((this.isSelfAssociation && Object(this.through) === this.through) || doubleLinked) {
-      // remove the obsolete association identifier from the source
-      if (this.isSelfAssociation) {
+      // We need to remove the keys that 1:M have added
+      if (this.isSelfAssociation && doubleLinked) {
+        if (self.through.rawAttributes[this.targetAssociation.identifier]._autoGenerated) {
+          delete self.through.rawAttributes[this.targetAssociation.identifier];
+        }
+        if (self.through.rawAttributes[this.targetAssociation.foreignIdentifier]._autoGenerated) {
+          delete self.through.rawAttributes[this.targetAssociation.foreignIdentifier];
+        }
+      }
+
+      this.foreignIdentifier = this.targetAssociation.identifier
+      this.targetAssociation.foreignIdentifier = this.identifier
+
+      if (this.isSelfAssociation && this.foreignIdentifier === this.identifier) {
         this.foreignIdentifier = Utils._.camelizeIf(
-          [(this.options.as || this.target.name), this.target.primaryKeyAttribute].join('_'),
-          !this.target.options.underscored
-        )
-      } else {
-        this.foreignIdentifier = this.targetAssociation.identifier
-        this.targetAssociation.foreignIdentifier = this.identifier
+          [Utils.singularize(this.as, this.source.options.language), this.source.primaryKeyAttribute].join("_"),
+          !this.source.options.underscored
+        );
+      }
 
-        if (isForeignKeyDeletionAllowedFor.call(this, this.source, this.foreignIdentifier)) {
-          delete this.source.rawAttributes[this.foreignIdentifier]
-        }
+      if (isForeignKeyDeletionAllowedFor.call(this, this.source, this.foreignIdentifier)) {
+        delete this.source.rawAttributes[this.foreignIdentifier]
+      }
 
-        if (isForeignKeyDeletionAllowedFor.call(this, this.target, this.identifier)) {
-          delete this.targetAssociation.source.rawAttributes[this.identifier]
-        }
+      if (isForeignKeyDeletionAllowedFor.call(this, this.target, this.identifier)) {
+        delete this.targetAssociation.source.rawAttributes[this.identifier]
       }
 
       // remove any PKs previously defined by sequelize
@@ -204,7 +213,21 @@ module.exports = (function() {
       combinedTableAttributes[this.identifier] = sourceAttribute
       combinedTableAttributes[this.foreignIdentifier] = targetAttribute
 
-      this.through.rawAttributes = Utils._.merge(this.through.rawAttributes, combinedTableAttributes)
+      if (!this.through.rawAttributes[this.identifier]) {
+        this.through.rawAttributes[this.identifier] = {
+          _autoGenerated: true
+        }
+      }
+
+      if (!this.through.rawAttributes[this.foreignIdentifier]) {
+        this.through.rawAttributes[this.foreignIdentifier] = {
+          _autoGenerated: true
+        }
+      }
+
+      this.through.rawAttributes[this.identifier] = Utils._.merge(this.through.rawAttributes[this.identifier], sourceAttribute);
+      this.through.rawAttributes[this.foreignIdentifier] = Utils._.merge(this.through.rawAttributes[this.foreignIdentifier], targetAttribute);
+
       this.through.init(this.through.daoFactoryManager)
     } else {
       var newAttributes = {}

lib/dialects/abstract/query.js

@@ -338,6 +338,7 @@ module.exports = (function() {
   */
 
   // includeOptions are 'level'-specific where options is a general directive
+  var i = 0;
   var groupJoinData = function(data, includeOptions, options) {
     var results = []
       , existingResult
@@ -413,7 +414,6 @@ module.exports = (function() {
       if (!options.checkExisting) {
         parseChildren(existingResult)
       }
-
     })
     
     // parseChildren after row parsing if duplicate values are possible

test/associations/self.test.js

@@ -3,6 +3,7 @@ var chai      = require('chai')
   , expect    = chai.expect
   , Support   = require(__dirname + '/../support')
   , DataTypes = require(__dirname + "/../../lib/data-types")
+  , _ = require('lodash')
 
 chai.config.includeStack = true
 
@@ -31,4 +32,90 @@ describe(Support.getTestDialectTeaser("Self"), function() {
       })
     })
   })
+
+  it('can handle 1:m associations', function (done) {
+    var Person = this.sequelize.define('Person', { name: DataTypes.STRING });
+
+    Person.hasMany(Person, { as: 'Children', foreignKey: 'parent_id'});
+
+    expect(Person.rawAttributes.parent_id).to.be.ok
+
+    this.sequelize.sync({force: true}).done(function () {
+      Person.create({ name: 'Mary' }).complete(function(err, mary) {
+        expect(err).to.not.be.ok
+        Person.create({ name: 'John' }).complete(function(err, john) {
+          expect(err).to.not.be.ok
+          Person.create({ name: 'Chris' }).complete(function(err, chris) {
+            expect(err).to.not.be.ok
+            mary.setChildren([john, chris]).done(function (err) {
+              expect(err).not.to.be.ok
+              done();
+            });
+          });
+        });
+      });
+    });
+  });
+
+  it('can handle n:m associations', function(done) {
+    var Person = this.sequelize.define('Person', { name: DataTypes.STRING });
+    var Family = this.sequelize.define('Family', {
+      preexisting_child: {
+        type: DataTypes.INTEGER,
+        primaryKey: true
+      },
+      preexisting_parent: {
+        type: DataTypes.INTEGER,
+        primaryKey: true
+      }
+    }, { timestamps: false });
+
+    Person.hasMany(Person, { as: 'Parents', through: Family, foreignKey: 'preexisting_child' });
+    Person.hasMany(Person, { as: 'Children', through: Family, foreignKey: 'preexisting_parent' });
+
+    var foreignIdentifiers = _.map(_.values(Person.associations), 'foreignIdentifier')
+    var rawAttributes = _.keys(Family.rawAttributes)
+
+    expect(foreignIdentifiers.length).to.equal(2)
+    expect(rawAttributes.length).to.equal(2)
+
+    expect(foreignIdentifiers).to.have.members([ 'preexisting_parent', 'preexisting_child' ]);
+    expect(rawAttributes).to.have.members([ 'preexisting_parent', 'preexisting_child' ]);
+
+    this.sequelize.sync({ force: true }).complete(function() {
+      Person.create({ name: 'Mary' }).complete(function(err, mary) {
+        expect(err).to.not.be.ok
+        Person.create({ name: 'John' }).complete(function(err, john) {
+          expect(err).to.not.be.ok
+          Person.create({ name: 'Chris' }).complete(function(err, chris) {
+            expect(err).to.not.be.ok
+            mary.setParents([john]).done(function (err) {
+              expect(err).to.not.be.ok
+              mary.addParent(chris).on('sql', function(sql) {
+                if (sql.match(/INSERT/)) {
+                  expect(sql).to.have.string('preexisting_child');
+                  expect(sql).to.have.string('preexisting_parent');
+                }
+              }).complete(function(err) {
+                expect(err).to.not.be.ok
+                john.getChildren().on('sql', function(sql) {
+                  var whereClause = sql.split('WHERE')[1]; // look only in the whereClause
+                  expect(whereClause).to.have.string('preexisting_child');
+                  expect(whereClause).to.have.string('preexisting_parent');
+                }).complete(function(err, children) {
+                  expect(_.map(children, 'id')).to.have.members([mary.id]);
+                  done()
+                })
+              })
+            }).on('sql', function(sql) {
+              if (sql.match(/INSERT/)) {
+                expect(sql).to.have.string('preexisting_child');
+                expect(sql).to.have.string('preexisting_parent');
+              }
+            });
+          })
+        })
+      })
+    })
+  })
 })
\ No newline at end of file

test/include.test.js

@@ -565,6 +565,7 @@ describe(Support.getTestDialectTeaser("Include"), function () {
       Group.hasMany(Group, { through: 'groups_outsourcing_companies', as: 'OutsourcingCompanies'});
 
       this.sequelize.sync({force: true}).done(function (err) {
+        expect(err).not.to.be.ok;
         Group.bulkCreate([
           {name: 'SoccerMoms'},
           {name: 'Coca Cola'},