Merge pull request #4927 from greenify/master

don't overwrite options.foreignKey in associations
sequelize · Nov 27, 2015 · 1b54275 · 1b54275
2 parents 4659be4 + 55c9607
commit 1b54275
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 5 deletions.
diff --git a/changelog.md b/changelog.md
@@ -1,3 +1,6 @@
+# Next
+- [FIXED] Don't overwrite options.foreignKey in associations [#4927](https://github.com/sequelize/sequelize/pull/4927)
+
 # 3.14.1
 - [FIXED] Issue with transaction options leaking and certain queries running outside of the transaction connection.
 

diff --git a/lib/associations/belongs-to-many.js b/lib/associations/belongs-to-many.js
@@ -323,8 +323,8 @@ BelongsToMany.prototype.injectAttributes = function() {
     , targetKey = this.target.rawAttributes[this.target.primaryKeyAttribute]
     , targetKeyType = targetKey.type
     , targetKeyField = targetKey.field || this.target.primaryKeyAttribute
-    , sourceAttribute = _.defaults(this.foreignKeyAttribute, { type: sourceKeyType })
-    , targetAttribute = _.defaults(this.otherKeyAttribute, { type: targetKeyType });
+    , sourceAttribute = _.defaults({}, this.foreignKeyAttribute, { type: sourceKeyType })
+    , targetAttribute = _.defaults({}, this.otherKeyAttribute, { type: targetKeyType });
 
   if (this.primaryKeyDeleted === true) {
     targetAttribute.primaryKey = sourceAttribute.primaryKey = true;

diff --git a/lib/associations/belongs-to.js b/lib/associations/belongs-to.js
@@ -109,7 +109,7 @@ util.inherits(BelongsTo, Association);
 BelongsTo.prototype.injectAttributes = function() {
   var newAttributes = {};
 
-  newAttributes[this.foreignKey] = _.defaults(this.foreignKeyAttribute, {
+  newAttributes[this.foreignKey] = _.defaults({}, this.foreignKeyAttribute, {
     type: this.options.keyType || this.target.rawAttributes[this.targetKey].type,
     allowNull : true
   });

diff --git a/lib/associations/has-many.js b/lib/associations/has-many.js
@@ -202,7 +202,7 @@ util.inherits(HasMany, Association);
 HasMany.prototype.injectAttributes = function() {
   var newAttributes = {};
   var constraintOptions = _.clone(this.options); // Create a new options object for use with addForeignKeyConstraints, to avoid polluting this.options in case it is later used for a n:m
-  newAttributes[this.foreignKey] = _.defaults(this.foreignKeyAttribute, {
+  newAttributes[this.foreignKey] = _.defaults({}, this.foreignKeyAttribute, {
     type: this.options.keyType || this.source.rawAttributes[this.source.primaryKeyAttribute].type,
     allowNull : true
   });

diff --git a/lib/associations/has-one.js b/lib/associations/has-one.js
@@ -103,7 +103,7 @@ HasOne.prototype.injectAttributes = function() {
   var newAttributes = {}
     , keyType = this.source.rawAttributes[this.source.primaryKeyAttribute].type;
 
-  newAttributes[this.foreignKey] = _.defaults(this.foreignKeyAttribute, {
+  newAttributes[this.foreignKey] = _.defaults({}, this.foreignKeyAttribute, {
     type: this.options.keyType || keyType,
     allowNull : true
   });

diff --git a/test/unit/associations/dont-modify-options.test.js b/test/unit/associations/dont-modify-options.test.js
@@ -0,0 +1,54 @@
+'use strict';
+
+/* jshint -W030 */
+var chai = require('chai')
+  , expect = chai.expect
+  , Support = require(__dirname + '/../support')
+  , DataTypes = require(__dirname + '/../../../lib/data-types')
+  , Sequelize = require('../../../index');
+
+describe(Support.getTestDialectTeaser('associations'), function() {
+  describe('Test options.foreignKey', function() {
+    beforeEach(function() {
+
+      this.A = this.sequelize.define('A', {
+        id: {
+          type: DataTypes.CHAR(20),
+          primaryKey: true
+        }
+      });
+      this.B = this.sequelize.define('B', {
+        id: {
+          type: Sequelize.CHAR(20),
+          primaryKey: true
+        }
+      });
+      this.C = this.sequelize.define('C', {});
+    });
+
+    it('should not be overwritten for belongsTo', function(){
+      var reqValidForeignKey = { foreignKey: { allowNull: false }};
+      this.A.belongsTo(this.B, reqValidForeignKey);
+      this.A.belongsTo(this.C, reqValidForeignKey);
+      expect(this.A.attributes.CId.type).to.deep.equal(this.C.attributes.id.type);
+    });
+    it('should not be overwritten for belongsToMany', function(){
+      var reqValidForeignKey = { foreignKey: { allowNull: false }, through: 'ABBridge'};
+      this.B.belongsToMany(this.A, reqValidForeignKey);
+      this.A.belongsTo(this.C, reqValidForeignKey);
+      expect(this.A.attributes.CId.type).to.deep.equal(this.C.attributes.id.type);
+    });
+    it('should not be overwritten for hasOne', function(){
+      var reqValidForeignKey = { foreignKey: { allowNull: false }};
+      this.B.hasOne(this.A, reqValidForeignKey);
+      this.A.belongsTo(this.C, reqValidForeignKey);
+      expect(this.A.attributes.CId.type).to.deep.equal(this.C.attributes.id.type);
+    });
+    it('should not be overwritten for hasMany', function(){
+      var reqValidForeignKey = { foreignKey: { allowNull: false }};
+      this.B.hasMany(this.A, reqValidForeignKey);
+      this.A.belongsTo(this.C, reqValidForeignKey);
+      expect(this.A.attributes.CId.type).to.deep.equal(this.C.attributes.id.type);
+    });
+  });
+});