correctly support find with primarykeys

lib/sequelize/model-definition.js

@@ -21,8 +21,7 @@ ModelDefinition.prototype.addDefaultAttributes = function() {
   var defaultAttributes = {id: {type: DataTypes.INTEGER, allowNull: false, primaryKey: true, autoIncrement: true}}
     , self = this
 
-  if(Utils._.keys(this.primaryKeys).length > 0)
-    defaultAttributes = {}
+  if(this.hasPrimaryKeys) defaultAttributes = {}
   
   if(this.options.timestamps) {
     defaultAttributes[this.options.camelcase ? 'created_at' : 'createdAt'] = {type: DataTypes.DATE, allowNull: false}
@@ -78,7 +77,17 @@ ModelDefinition.prototype.find = function(options) {
   if(typeof options == 'number')
     options = {where: options}
   else {
-    // if(arguments.length == this.primaryKeys.length)
+    if (Utils.argsArePrimaryKeys(arguments, this.primaryKeys)) {
+      var where = {}
+        , self  = this
+        
+      Utils._.each(arguments, function(arg, i) {
+        var key = Utils._.keys(self.primaryKeys)[i]
+        where[key] = arg
+      })
+
+      options = {where: where}
+    }
   }
   
   options.limit = 1
@@ -88,9 +97,9 @@ ModelDefinition.prototype.find = function(options) {
 }
 
 ModelDefinition.prototype.build = function(values) {
-  var instance = new Model(values, this.options)
+  var instance = new Model(values, Utils._.extend(this.options, {hasPrimaryKeys: this.hasPrimaryKeys}))
     , self     = this
- 
+  
   instance.definition = this
   
   return instance
@@ -102,13 +111,21 @@ ModelDefinition.prototype.create = function(values) {
 
 ModelDefinition.prototype.__defineGetter__('primaryKeys', function() {
   var result = {}
-  
+
   Utils._.each(this.attributes, function(dataTypeString, attributeName) {
-    if(dataTypeString.indexOf('PRIMARY KEY') > -1)
+    if((attributeName != 'id') && (dataTypeString.indexOf('PRIMARY KEY') > -1))
       result[attributeName] = dataTypeString
   })
-  
+
   return result
 })
 
+ModelDefinition.prototype.__defineGetter__('primaryKeyCount', function() {
+  return Utils._.keys(this.primaryKeys).length
+})
+
+ModelDefinition.prototype.__defineGetter__('hasPrimaryKeys', function() {
+  return this.primaryKeyCount > 0
+})
+
 Utils._.map(require("./association-mixin").classMethods, function(fct, name) { ModelDefinition.prototype[name] = fct })
\ No newline at end of file

lib/sequelize/model.js

@@ -5,7 +5,7 @@ var Utils          = require("./utils")
 var Model = module.exports = function(values, options) {
   var self = this
   
-  this.definition = null // will be set on Model.build or Model.create
+  this.definition = null // will be set in Model.build
   this.attributes = []
   this.options = options || {}
   
@@ -14,8 +14,7 @@ var Model = module.exports = function(values, options) {
 
   // set id to null if not passed as value
   // a newly created model has no id
-  
-  var defaults = { id: null }
+  var defaults = this.options.hasPrimaryKeys ? {} : { id: null }
 
   if(this.options.timestamps) {
     defaults[this.options.camelcase ? 'created_at' : 'createdAt'] = new Date()
@@ -60,8 +59,10 @@ Model.prototype.save = function() {
   
   if(this.isNewRecord)
     return this.query(QueryGenerator.insertQuery(this.definition.tableName, this.values))
-  else
-    return this.query(QueryGenerator.updateQuery(this.definition.tableName, this.values, this.id))
+  else {
+    var identifier = this.options.hasPrimaryKeys ? {where: this.definition.primaryKeys} : this.id
+    return this.query(QueryGenerator.updateQuery(this.definition.tableName, this.values, identifier))
+  }
 }
 
 Model.prototype.destroy = function() {

lib/sequelize/query-generator.js

@@ -41,7 +41,7 @@ var QueryGenerator = module.exports = {
     options.table = Utils.addTicks(tableName)
     options.attributes = options.attributes && options.attributes.map(function(attr){return Utils.addTicks(attr)}).join(", ")
     options.attributes = options.attributes || '*'
-
+    
     var query = "SELECT <%= attributes %> FROM <%= table %>"
     
     if(options.where) {
@@ -59,7 +59,7 @@ var QueryGenerator = module.exports = {
     }
     
     query += ";"
-    
+
     return Utils._.template(query)(options)
   },
   

lib/sequelize/query.js

@@ -34,9 +34,9 @@ Query.prototype.onSuccess = function(query, results, fields) {
     , self   = this
   
   // add the inserted row id to the instance
-  if (this.callee && (query.indexOf('INSERT INTO') == 0) && (results.hasOwnProperty('insertId')))
+  if (this.callee && !this.callee.options.hasPrimaryKeys && (query.indexOf('INSERT INTO') == 0) && (results.hasOwnProperty('insertId')))
     this.callee.id = results.insertId
-    
+
   // transform results into real model instances
   // return the first real model instance if options.plain is set (e.g. Model.find)
   if (query.indexOf('SELECT') == 0) {
@@ -45,7 +45,7 @@ Query.prototype.onSuccess = function(query, results, fields) {
     if(this.options.plain)
       result = (result.length == 0) ? null : result[0]
   }
-    
+
   this.emit('success', result)
 }
 

lib/sequelize/utils.js

@@ -90,5 +90,18 @@ var Utils = module.exports = {
       ].join("-"),
       date.toLocaleTimeString()
     ].join(" ")
+  },
+  argsArePrimaryKeys: function(args, primaryKeys) {
+    var result = (args.length == Utils._.keys(primaryKeys).length)
+    Utils._.each(args, function(arg) {
+      if(result) {
+        if(['number', 'string'].indexOf(typeof arg) > -1)
+          result = true
+        else
+          result = (arg instanceof Date)
+
+      }
+    })
+    return result
   }
 }
\ No newline at end of file

test/Model/find-findAll-all.js

@@ -60,6 +60,22 @@ module.exports = {
       })
     })
   },
+  'find should find records by primaryKeys': function(exit) {
+    var User = sequelize.define('User' + parseInt(Math.random() * 999999999), {
+      identifier: {type: Sequelize.STRING, primaryKey: true},
+      name: Sequelize.STRING
+    })
+    User.sync({force:true}).on('success', function() {
+      User.create({identifier: 'an identifier', name: 'John'}).on('success', function(u) {
+        assert.isUndefined(u.id)
+        User.find('an identifier').on('success', function(u2) {
+          assert.eql(u.identifier, 'an identifier')
+          assert.eql(u.name, 'John')
+          exit()
+        })
+      })
+    })
+  },
   'findAll should find all records': function(exit) {
     initUsers(2, function(_, User) {
       User.findAll().on('success', function(users) {

test/Model/is-new-record.js

@@ -16,7 +16,7 @@ var initUsers = function(num, callback) {
 module.exports = {
   'build should not create database entries': function(exit) {
     initUsers(1, function(users, User) {
-      assert.eql(users[0].id, null)
+      assert.isNull(users[0].id)
       assert.eql(users[0].isNewRecord, true)
       exit()
     })

test/Utils/argsArePrimaryKeys.js

+var assert = require("assert")
+  , Utils  = require("../../lib/sequelize/utils")
+
+module.exports = {
+  'it should be false if primaryKeys and args have different lengths': function() {
+    assert.eql(false, Utils.argsArePrimaryKeys([1,2,3], [1]))
+  },
+  'it should be false if primaryKeys are hashes or arrays': function() {
+    assert.eql(false, Utils.argsArePrimaryKeys([[]], [1]))
+  },
+  'it should be true if primaryKeys are primitive data types and lengths are matching': function() {
+    assert.eql(true, Utils.argsArePrimaryKeys([1,2,3], ["INT", "INT", "INT"]))
+  },
+  'it should be true if primaryKeys are dates and lengths are matching': function() {
+    assert.eql(true, Utils.argsArePrimaryKeys([new Date()], ['foo']))
+  }
+}
\ No newline at end of file