Fix object syntax for belongsto and hasMany. Closes #2164. Also properly merge e…

…xisting attributes with those added by associations

changelog.md

@@ -12,6 +12,7 @@ We are working our way to the first 2.0.0 release candidate.
 - [BUG] Fix problem with minConnections. [#2048](https://github.com/sequelize/sequelize/issues/2048)
 - [BUG] Fix default scope being overwritten [#2087](https://github.com/sequelize/sequelize/issues/2087)
 - [BUG] Fixed updatedAt timestamp not being set in bulk create when validate = true. [#1962](https://github.com/sequelize/sequelize/issues/1962)
+- [BUG] Fixed an issue with foreign key object syntax for hasOne and belongsTo
 - [INTERNALS] Replaced lingo with inflection
 - [INTERNALS] Removed underscore.string dependency and moved a couple of helper functions from `Utils._` to `Utils` 
 - [INTERNALS] Update dependencies

lib/associations/belongs-to.js

@@ -42,7 +42,14 @@ module.exports = (function() {
       );
     }
 
-    this.identifier = this.options.foreignKey;
+    this.identifier = this.foreignKey || Utils._.camelizeIf(
+      [
+        Utils._.underscoredIf(this.options.name.singular, this.target.options.underscored),
+        this.target.primaryKeyAttribute
+      ].join('_'),
+      !this.target.options.underscored
+    );
+
     this.targetIdentifier = this.target.primaryKeyAttribute;
     this.associationAccessor = this.as;
     this.options.useHooks = options.useHooks;
@@ -67,7 +74,7 @@ module.exports = (function() {
       this.options.onUpdate = this.options.onUpdate || 'CASCADE';
     }
     Helpers.addForeignKeyConstraints(newAttributes[this.identifier], this.target, this.source, this.options);
-    Utils._.defaults(this.source.rawAttributes, newAttributes);
+    Utils.mergeDefaults(this.source.rawAttributes, newAttributes);
 
     // Sync attributes and setters/getters to DAO prototype
     this.source.refreshAttributes();

lib/associations/has-many.js

@@ -273,7 +273,7 @@ module.exports = (function() {
         constraintOptions.onUpdate = constraintOptions.onUpdate || 'CASCADE';
       }
       Helpers.addForeignKeyConstraints(newAttributes[this.identifier], this.source, this.target, constraintOptions);
-      Utils._.defaults(this.target.rawAttributes, newAttributes);
+      Utils.mergeDefaults(this.target.rawAttributes, newAttributes);
     }
 
     // Sync attributes and setters/getters to DAO prototype

lib/associations/has-one.js

@@ -42,7 +42,14 @@ module.exports = (function() {
       );
     }
 
-    this.identifier = this.options.foreignKey;
+    this.identifier = this.foreignKey || Utils._.camelizeIf(
+      [
+        Utils._.underscoredIf(this.source.options.name.singular, this.source.options.underscored),
+        this.source.primaryKeyAttribute
+      ].join('_'),
+      !this.source.options.underscored
+    );
+
     this.sourceIdentifier = this.source.primaryKeyAttribute;
     this.associationAccessor = this.as;
     this.options.useHooks = options.useHooks;
@@ -63,7 +70,7 @@ module.exports = (function() {
       , keyType = this.source.rawAttributes[this.sourceIdentifier].type;
 
     newAttributes[this.identifier] = Utils._.defaults(this.foreignKeyAttribute, { type: this.options.keyType || keyType });
-    Utils._.defaults(this.target.rawAttributes, newAttributes);
+    Utils.mergeDefaults(this.target.rawAttributes, newAttributes);
 
     if (this.options.constraints !== false) {
       this.options.onDelete = this.options.onDelete || 'SET NULL';

lib/utils.js

@@ -57,6 +57,15 @@ var Utils = module.exports = {
 
     return _;
   })(),
+  // Same concept as _.merge, but don't overwrite properties that have already been assigned
+  mergeDefaults: function (a, b) {
+    return this._.merge(a, b, function (objectValue, sourceValue) {
+      // If it's an object, let _ handle it this time, we will be called again for each property
+      if (!this._.isPlainObject(objectValue) && objectValue !== undefined) {
+        return objectValue;
+      }
+    }, this);
+  },
   lowercaseFirst: function (s) {
     return s[0].toLowerCase() + s.slice(1);
   },

test/associations/belongs-to.test.js

@@ -532,6 +532,35 @@ describe(Support.getTestDialectTeaser("BelongsTo"), function() {
       expect(Profile.rawAttributes.user_id.allowNull).to.be.false
     })
 
+    it('allows the user to provide an attribute definition as foreignKey', function () {
+      var Task = this.sequelize.define('task', {})
+        , User = this.sequelize.define('user', {
+            uid: {
+              type: Sequelize.INTEGER,
+              primaryKey: true
+            }
+          });
+
+      Task.belongsTo(User, {
+        foreignKey: {
+          allowNull: false
+        }
+      });
+
+      expect(Task.rawAttributes.userUid.allowNull).to.be.false;
+
+      var Project = this.sequelize.define('project', {
+        user_id: {
+          type: Sequelize.INTEGER
+        }
+      });
+
+      Project.belongsTo(User, { foreignKey: Project.rawAttributes.user_id});
+
+      expect(Project.rawAttributes.user_id.references).to.equal(User.getTableName());
+      expect(Project.rawAttributes.user_id.referencesKey).to.equal('uid');
+    });
+
     it('should throw an error if foreignKey and as result in a name clash', function () {
       var Person = this.sequelize.define('person', {})
         , Car = this.sequelize.define('car', {})

test/associations/has-many.test.js

@@ -504,11 +504,7 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
           'title': DataTypes.STRING
 
         }), Label   = this.sequelize.define('Label', {
-          'text': DataTypes.STRING,
-          'ArticleId': {
-            type: DataTypes.INTEGER,
-            allowNull: false
-          }
+          'text': DataTypes.STRING
         });
 
         Article.hasMany(Label);
@@ -1337,7 +1333,7 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
         }).then(function (user) {
           return user.setTasks(null).on('sql', spy).on('sql', _.after(2, function (sql) {
             expect(sql).to.have.string("DELETE FROM");
-            expect(sql).to.have.string("IN (1,2)");
+            expect(sql).to.match(/IN \(1,2\)|IN \(2,1\)/);
           }));
         }).then(function () {
           expect(spy.calledTwice).to.be.ok; // Once for SELECT, once for DELETE

test/associations/has-one.test.js

@@ -487,6 +487,20 @@ describe(Support.getTestDialectTeaser("HasOne"), function() {
 
       User.hasOne(Profile, { foreignKey: Profile.rawAttributes.user_id})
 
+      var Project = this.sequelize.define('project', {
+        user_id: {
+          type: Sequelize.INTEGER
+        }
+      });
+
+      User.hasOne(Project, {
+        foreignKey: {
+          allowNull: false
+        }
+      })
+
+      expect(Project.rawAttributes.userUid.allowNull).to.be.false;
+
       expect(Profile.rawAttributes.user_id).to.be.defined
       expect(Profile.rawAttributes.user_id.references).to.equal(User.getTableName())
       expect(Profile.rawAttributes.user_id.referencesKey).to.equal('uid')

test/dao-factory/findAll.test.js

@@ -284,32 +284,30 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
 
         User.belongsTo(Binary, { foreignKey: 'binary' })
 
-        User.sync({ force: true }).success(function() {
-          Binary.sync({ force: true }).success(function() {
-            User.bulkCreate([
-              {username: 'boo5', aBool: false},
-              {username: 'boo6', aBool: true}
+        this.sequelize.sync({ force: true }).success(function() {
+          User.bulkCreate([
+            {username: 'boo5', aBool: false},
+            {username: 'boo6', aBool: true}
+          ]).success(function() {
+            Binary.bulkCreate([
+              {id: buf1},
+              {id: buf2}
             ]).success(function() {
-              Binary.bulkCreate([
-                {id: buf1},
-                {id: buf2}
-              ]).success(function() {
-                User.find(1).success(function(user) {
-                  Binary.find(buf1).success(function(binary) {
-                    user.setBinary(binary).success(function() {
-                      User.find(2).success(function(_user) {
-                        Binary.find(buf2).success(function(_binary) {
-                          _user.setBinary(_binary).success(function() {
-                            _user.getBinary().success(function(_binaryRetrieved) {
-                              user.getBinary().success(function(binaryRetrieved) {
-                                expect(binaryRetrieved.id).to.be.an.instanceof.string
-                                expect(_binaryRetrieved.id).to.be.an.instanceof.string
-                                expect(binaryRetrieved.id).to.have.length(16)
-                                expect(_binaryRetrieved.id).to.have.length(16)
-                                expect(binaryRetrieved.id.toString()).to.be.equal(buf1.toString())
-                                expect(_binaryRetrieved.id.toString()).to.be.equal(buf2.toString())
-                                done()
-                              })
+              User.find(1).success(function(user) {
+                Binary.find(buf1).success(function(binary) {
+                  user.setBinary(binary).success(function() {
+                    User.find(2).success(function(_user) {
+                      Binary.find(buf2).success(function(_binary) {
+                        _user.setBinary(_binary).success(function() {
+                          _user.getBinary().success(function(_binaryRetrieved) {
+                            user.getBinary().success(function(binaryRetrieved) {
+                              expect(binaryRetrieved.id).to.be.an.instanceof.string
+                              expect(_binaryRetrieved.id).to.be.an.instanceof.string
+                              expect(binaryRetrieved.id).to.have.length(16)
+                              expect(_binaryRetrieved.id).to.have.length(16)
+                              expect(binaryRetrieved.id.toString()).to.be.equal(buf1.toString())
+                              expect(_binaryRetrieved.id.toString()).to.be.equal(buf2.toString())
+                              done()
                             })
                           })
                         })

test/include/findAndCountAll.test.js

@@ -29,13 +29,13 @@ describe(Support.getTestDialectTeaser("Include"), function () {
       User.hasMany( SomeConnection, { foreignKey: 'u' })
 
       SomeConnection.belongsTo( User, { foreignKey: 'u' })
-      SomeConnection.belongsTo( A, { foreignKey: 'fk' })
-      SomeConnection.belongsTo( B, { foreignKey: 'fk' })
-      SomeConnection.belongsTo( C, { foreignKey: 'fk' })
+      SomeConnection.belongsTo( A, { foreignKey: 'fk', constraints: false })
+      SomeConnection.belongsTo( B, { foreignKey: 'fk', constraints: false })
+      SomeConnection.belongsTo( C, { foreignKey: 'fk', constraints: false })
 
-      A.hasMany( SomeConnection, { foreignKey: 'fk' })
-      B.hasMany( SomeConnection, { foreignKey: 'fk' })
-      C.hasMany( SomeConnection, { foreignKey: 'fk' })
+      A.hasMany( SomeConnection, { foreignKey: 'fk', constraints: false })
+      B.hasMany( SomeConnection, { foreignKey: 'fk', constraints: false })
+      C.hasMany( SomeConnection, { foreignKey: 'fk', constraints: false })
 
       // Sync them
       S.sync({ force: true }).done( function ( err ) { expect( err ).not.to.be.ok;
@@ -142,4 +142,4 @@ describe(Support.getTestDialectTeaser("Include"), function () {
     })
 
   })
-})
\ No newline at end of file
+})