Merge pull request #961 from sequelize/hotfix/foreign_key_deletion_for_mixed_associations

fixed foreign key deletion for mixed associations

changelog.md

@@ -41,6 +41,7 @@
 - [BUG] Fix for counts queries with no result [#906](https://github.com/sequelize/sequelize/pull/906). thanks to iamjochem
 - [BUG] Allow include when the same table is referenced multiple times using hasMany [#913](https://github.com/sequelize/sequelize/pull/913). thanks to janmeier
 - [BUG] Allow definition of defaultValue for the timestamp columns (createdAt, updatedAt, deletedAt) [#930](https://github.com/sequelize/sequelize/pull/930). Thank to durango
+- [BUG] Don't delete foreign keys of many-to-many associations, if still needed. [#961](https://github.com/sequelize/sequelize/pull/961). thanks to sdepold
 - [FEATURE] Validate a model before it gets saved. [#601](https://github.com/sequelize/sequelize/pull/601). thanks to durango
 - [FEATURE] Schematics. [#564](https://github.com/sequelize/sequelize/pull/564). thanks to durango
 - [FEATURE] Foreign key constraints. [#595](https://github.com/sequelize/sequelize/pull/595). thanks to optilude

lib/associations/has-many.js

@@ -47,7 +47,8 @@ module.exports = (function() {
   // or in an extra table which connects two tables
   HasMany.prototype.injectAttributes = function() {
     var multiAssociation = this.target.associations.hasOwnProperty(this.associationAccessor)
-      , self = this
+      , self             = this
+
     this.identifier = this.options.foreignKey || Utils._.underscoredIf(Utils.singularize(this.source.tableName, this.source.options.language) + "Id", this.options.underscored)
 
     // is there already a single sided association between the source and the target?
@@ -59,8 +60,14 @@ module.exports = (function() {
       } else {
         this.foreignIdentifier = this.target.associations[this.associationAccessor].identifier
         this.target.associations[this.associationAccessor].foreignIdentifier = this.identifier
-        delete this.source.rawAttributes[this.foreignIdentifier]
-        delete this.target.associations[this.associationAccessor].source.rawAttributes[this.identifier]
+
+        if (isForeignKeyDeletionAllowedFor.call(this, this.source, this.foreignIdentifier)) {
+          delete this.source.rawAttributes[this.foreignIdentifier]
+        }
+
+        if (isForeignKeyDeletionAllowedFor.call(this, this.target, this.identifier)) {
+          delete this.target.associations[this.associationAccessor].source.rawAttributes[this.identifier]
+        }
       }
 
       // define a new model, which connects the models
@@ -256,5 +263,27 @@ module.exports = (function() {
     return this
   }
 
+  /**
+   * The method checks if it is ok to delete the previously defined foreign key.
+   * This is done because we need to keep the foreign key if another association
+   * is depending on it.
+   *
+   * @param  {DaoFactory}  daoFactory The source or target DaoFactory of this assocation
+   * @param  {[type]}  identifier     The name of the foreign key identifier
+   * @return {Boolean}                Whether or not the deletion of the foreign key is ok.
+   */
+  var isForeignKeyDeletionAllowedFor = function(daoFactory, identifier) {
+    var isAllowed        = true
+      , associationNames = Utils._.without(Object.keys(daoFactory.associations), this.associationAccessor)
+
+    associationNames.forEach(function(associationName) {
+      if (daoFactory.associations[associationName].identifier === identifier) {
+        isAllowed = false
+      }
+    })
+
+    return isAllowed
+  }
+
   return HasMany
 })()

test/associations/has-many.test.js

@@ -710,6 +710,71 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
         })
       })
     })
+
+    describe('belongsTo and hasMany at once', function() {
+      beforeEach(function() {
+        this.A = this.sequelize.define('a', { name: Sequelize.STRING })
+        this.B = this.sequelize.define('b', { name: Sequelize.STRING })
+      })
+
+      describe('source belongs to target', function() {
+        beforeEach(function(done) {
+          this.A.belongsTo(this.B, { as: 'relation1' })
+          this.A.hasMany(this.B, { as: 'relation2' })
+          this.B.hasMany(this.A, { as: 'relation2' })
+
+          this.sequelize.sync({ force: true }).success(function() {
+            done()
+          })
+        })
+
+        it('correctly uses bId in A', function(done) {
+          var self = this
+
+          var a1 = this.A.build({ name: 'a1' })
+            , b1 = this.B.build({ name: 'b1' })
+
+          a1
+            .save()
+            .then(function() { return b1.save() })
+            .then(function() { return a1.setRelation1(b1) })
+            .then(function() { return self.A.find({ where: { name: 'a1' } }) })
+            .done(function(a) {
+              expect(a.bId).to.be.eq(b1.id)
+              done()
+            })
+        })
+      })
+
+      describe('target belongs to source', function() {
+        beforeEach(function(done) {
+          this.B.belongsTo(this.A, { as: 'relation1' })
+          this.A.hasMany(this.B, { as: 'relation2' })
+          this.B.hasMany(this.A, { as: 'relation2' })
+
+          this.sequelize.sync({ force: true }).success(function() {
+            done()
+          })
+        })
+
+        it('correctly uses bId in A', function(done) {
+          var self = this
+
+          var a1 = this.A.build({ name: 'a1' })
+            , b1 = this.B.build({ name: 'b1' })
+
+          a1
+            .save()
+            .then(function() { return b1.save() })
+            .then(function() { return b1.setRelation1(a1) })
+            .then(function() { return self.B.find({ where: { name: 'b1' } }) })
+            .done(function(b) {
+              expect(b.aId).to.be.eq(a1.id)
+              done()
+            })
+        })
+      })
+    })
   })
 
   describe("Foreign key constraints", function() {
@@ -876,4 +941,4 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
       })
     })
   })
-})
\ No newline at end of file
+})