have belongsTo respect aliases and non-id/non-integer primary keys

Mick Hansen
Commit 8b036963 authored Feb 28, 2014 by Mick Hansen
Showing with 92 additions and 51 deletions
changelog.md
lib/associations/belongs-to.js
test/associations/belongs-to.test.js
test/associations/has-many.test.js
test/associations/self.test.js
--- a/changelog.md
+++ b/changelog.md
 Notice: All 1.7.x changes are present in 2.0.x aswell
-# v1.7.0 (next)
+# v2.0.0-dev10
+#### Backwards compatability changes
+- foreign keys will now correctly be based on the alias of the model
+- foreign keys for non-id primary keys will now be named for the foreign key, i.e. pub_name rather than pub_id - if you have non-id primary keys you should go through your associations and set the foreignKey option if relying on a incorrect _id foreign key
+# v1.7.0
 - [FEATURE] covers more advanced include cases with limiting and filtering (specifically cases where a include would be in the subquery but its child include wouldnt be, for cases where a 1:1 association had a 1:M association as a nested include)
 - [BUG] fixes issue where connection would timeout before calling COMMIT resulting in data never reaching the database [#1429](https://github.com/sequelize/sequelize/pull/1429)

--- a/lib/associations/belongs-to.js
+++ b/lib/associations/belongs-to.js
@@ -2,6 +2,7 @@ var Utils       = require("./../utils")
  , DataTypes   = require('./../data-types')
  , Helpers     = require('./helpers')
  , Transaction = require('../transaction')
+  , _           = require('lodash')
 module.exports = (function() {
  var BelongsTo = function(source, target, options) {
@@ -10,23 +11,26 @@ module.exports = (function() {
    this.target               = target
    this.options              = options
    this.isSingleAssociation  = true
-    this.isSelfAssociation    = (this.source.name == this.target.name)
+    this.isSelfAssociation    = (this.source == this.target)
    this.as                   = this.options.as
-    if (this.isSelfAssociation && !this.options.foreignKey && !!this.as) {
-      this.options.foreignKey = Utils._.underscoredIf(Utils.singularize(this.as, this.source.options.language) + "Id", this.source.options.underscored)
-    }
    if (this.as) {
      this.isAliased = true
    } else {
      this.as = Utils.singularize(this.target.name, this.target.options.language)
    }
-    this.associationAccessor = this.isSelfAssociation
+    if (!this.options.foreignKey) {
-      ? Utils.combineTableNames(this.target.name, this.as)
+      console.log(this.as);
-      : this.as
+      this.options.foreignKey = Utils._.camelizeIf(
+        [this.as, this.target.primaryKeyAttribute].join("_"),
+        !this.source.options.underscored
+      )
+    }
+    this.identifier = this.options.foreignKey
+    this.targetIdentifier = this.target.primaryKeyAttribute
+    this.associationAccessor = this.as
    this.options.useHooks = options.useHooks
    this.accessors = {
@@ -39,10 +43,7 @@ module.exports = (function() {
  // the id is in the source table
  BelongsTo.prototype.injectAttributes = function() {
    var newAttributes  = {}
-      , targetKeys     = Object.keys(this.target.primaryKeys)
+      , keyType        = this.target.rawAttributes[this.targetIdentifier].type
-      , keyType        = ((this.target.hasPrimaryKeys && targetKeys.length === 1) ? this.target.rawAttributes[targetKeys[0]].type : DataTypes.INTEGER)
-    this.identifier = this.options.foreignKey || Utils._.underscoredIf(Utils.singularize(this.target.name, this.target.options.language) + "Id", this.source.options.underscored)
    newAttributes[this.identifier] = { type: this.options.keyType || keyType }
    Helpers.addForeignKeyConstraints(newAttributes[this.identifier], this.target, this.source, this.options)
@@ -54,44 +55,31 @@ module.exports = (function() {
    return this
  }
-  BelongsTo.prototype.injectGetter = function(obj) {
+  // Add getAssociation method to the prototype of the model instance
-    var self     = this
+  BelongsTo.prototype.injectGetter = function(instancePrototype) {
-      , primaryKeys = Object.keys(self.target.primaryKeys)
+    var association = this
-      , primaryKey = primaryKeys.length === 1 ? primaryKeys[0] : 'id'
+    instancePrototype[this.accessors.get] = function(params) {
-    obj[this.accessors.get] = function(params) {
+      params = params || {}
-      var id      = this[self.identifier]
+      params.where = params.where || {}
-        , where   = {}
+      params.where[association.targetIdentifier] = this.get(association.identifier)
-      where[primaryKey] = id
-      if (!Utils._.isUndefined(params)) {
-        if (!Utils._.isUndefined(params.where)) {
-          params.where = Utils._.extend(where, params.where)
-        } else {
-          params.where = where
-        }
-      } else {
-        params = id
-      }
-      return self.target.find(params)
+      return association.target.find(params)
    }
    return this
  }
-  BelongsTo.prototype.injectSetter = function(obj) {
+  // Add setAssociaton method to the prototype of the model instance
-    var self     = this
+  BelongsTo.prototype.injectSetter = function(instancePrototype) {
+    var association = this
-    obj[this.accessors.set] = function(associatedObject, options) {
+    instancePrototype[this.accessors.set] = function(instance, options) {
-      var primaryKeys = !!associatedObject && !!associatedObject.Model ? Object.keys(associatedObject.Model.primaryKeys) : []
+      this.set(association.identifier, instance ? instance[association.targetIdentifier] : null)
-        , primaryKey = primaryKeys.length === 1 ? primaryKeys[0] : 'id'
-      this[self.identifier] = associatedObject ? associatedObject[primaryKey] : null
      options = Utils._.extend({
-        fields: [ self.identifier ],
+        fields: [ association.identifier ],
-        allowNull: [self.identifier],
+        allowNull: [association.identifier ],
        association: true
      }, options)
@@ -102,10 +90,11 @@ module.exports = (function() {
    return this
  }
-  BelongsTo.prototype.injectCreator = function(obj) {
+  // Add createAssociation method to the prototype of the model instance
-    var self = this
+  BelongsTo.prototype.injectCreator = function(instancePrototype) {
+    var association = this
-    obj[this.accessors.create] = function(values, fieldsOrOptions) {
+    instancePrototype[this.accessors.create] = function(values, fieldsOrOptions) {
      var instance = this
        , options = {}
@@ -114,11 +103,11 @@ module.exports = (function() {
      }
      return new Utils.CustomEventEmitter(function(emitter) {
-        self.target
+        association.target
          .create(values, fieldsOrOptions)
          .proxy(emitter, { events: ['error', 'sql'] })
          .success(function(newAssociatedObject) {
-            instance[self.accessors.set](newAssociatedObject, options)
+            instance[association.accessors.set](newAssociatedObject, options)
              .proxy(emitter)
          })
      }).run()

--- a/test/associations/belongs-to.test.js
+++ b/test/associations/belongs-to.test.js
@@ -344,7 +344,7 @@ describe(Support.getTestDialectTeaser("BelongsTo"), function() {
  })
  describe("Association column", function() {
-    it('has correct type for non-id primary keys with non-integer type', function(done) {
+    it('has correct type and name for non-id primary keys with non-integer type', function(done) {
      var User = this.sequelize.define('UserPKBT', {
        username: {
          type: DataTypes.STRING
@@ -362,7 +362,7 @@ describe(Support.getTestDialectTeaser("BelongsTo"), function() {
      User.belongsTo(Group)
      self.sequelize.sync({ force: true }).success(function() {
-        expect(User.rawAttributes.GroupPKBTId.type.toString()).to.equal(DataTypes.STRING.toString())
+        expect(User.rawAttributes.GroupPKBTName.type.toString()).to.equal(DataTypes.STRING.toString())
        done()
      })
    })

--- a/test/associations/has-many.test.js
+++ b/test/associations/has-many.test.js
@@ -901,6 +901,19 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
    })
    describe('primary key handling for join table', function () {
+      beforeEach(function (done) {
+        var self = this
+        this.User = this.sequelize.define('User',
+          { username: DataTypes.STRING },
+          { tableName: 'users'}
+        )
+        this.Task = this.sequelize.define('Task',
+          { title: DataTypes.STRING },
+          { tableName: 'tasks' }
+        )
+        done()
+      })
      it('removes the primary key if it was added by sequelize', function () {
        var self = this
        this.UserTasks = this.sequelize.define('usertasks', {});
@@ -1238,7 +1251,7 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
            .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)
+              expect(a.relation1Id).to.be.eq(b1.id)
              done()
            })
        })
@@ -1267,7 +1280,7 @@ describe(Support.getTestDialectTeaser("HasMany"), function() {
            .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)
+              expect(b.relation1Id).to.be.eq(a1.id)
              done()
            })
        })

--- a/test/associations/self.test.js
+++ b/test/associations/self.test.js
+/* jshint camelcase: false, expr: true */
+var chai      = require('chai')
+  , expect    = chai.expect
+  , Support   = require(__dirname + '/../support')
+  , DataTypes = require(__dirname + "/../../lib/data-types")
+chai.Assertion.includeStack = true
+describe(Support.getTestDialectTeaser("Self"), function() {
+  it('supports freezeTableName', function (done) {
+    var Group = this.sequelize.define('Group', {
+    }, {
+      tableName: 'user_group',
+      timestamps: false,
+      underscored: true,
+      freezeTableName: true
+    });
+    Group.belongsTo(Group, { foreignKey: 'parent' });
+    Group.sync({force: true}).done(function (err) {
+      expect(err).not.to.be.ok
+      Group.findAll({
+        include: [{
+          model: Group
+        }]
+      }).done(function (err) {
+        expect(err).not.to.be.ok
+        done()
+      })
+    })
+  })
+})
\ No newline at end of file