Merge branch 'master' into milestones/2.0.0

Daniel Durante
Commit 3710b441 authored Sep 06, 2013 by Daniel Durante
Showing with 82 additions and 20 deletions
changelog.md
lib/dao-factory.js
lib/dialects/postgres/connector-manager.js
lib/dialects/postgres/query-generator.js
lib/dialects/postgres/query.js
lib/query-interface.js
package.json
test/dao-factory.test.js
test/dao.test.js
--- a/changelog.md
+++ b/changelog.md
@@ -36,6 +36,7 @@
 - [BUG] Fixed eager loading for many-to-many associations. [#834](https://github.com/sequelize/sequelize/pull/834). thanks to lemon-tree
 - [BUG] allowNull: true enums can now be null [#857](https://github.com/sequelize/sequelize/pull/857). thanks to durango
 - [BUG] Fixes Postgres' ability to search within arrays. [#879](https://github.com/sequelize/sequelize/pull/879). thanks to durango
+- [BUG] Find and finAll would modify the options objects, now the objects are cloned at the start of the method [#884](https://github.com/sequelize/sequelize/pull/884)
 - [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

--- a/lib/dao-factory.js
+++ b/lib/dao-factory.js
@@ -324,7 +324,8 @@ module.exports = (function() {
  DAOFactory.prototype.findAll = function(options, queryOptions) {
    var hasJoin = false
-    var options = Utils._.clone(options)
+    options = optClone(options)
    if (typeof options === 'object') {
      if (options.hasOwnProperty('include')) {
@@ -378,6 +379,7 @@ module.exports = (function() {
    var primaryKeys = this.primaryKeys
      , keys        = Object.keys(primaryKeys)
      , keysLength  = keys.length
+    options = optClone(options)
    // options is not a hash but an id
    if (typeof options === 'number') {
@@ -825,6 +827,13 @@ module.exports = (function() {
    return attributes
  }
+  var optClone = function (options) {
+    return Utils._.cloneDeep(options, function (elem) {
+      // The DAOFactories used for include are pass by ref, so don't clone them. Otherwise return undefined, meaning, 'handle this lodash'
+      return elem instanceof DAOFactory ? elem : undefined
+    })
+  }
  Utils._.extend(DAOFactory.prototype, require("./associations/mixin"))
  return DAOFactory

--- a/lib/dialects/postgres/connector-manager.js
+++ b/lib/dialects/postgres/connector-manager.js
@@ -19,9 +19,9 @@ module.exports = (function() {
    // set pooling parameters if specified
    if (this.pooling) {
-      this.pg.defaults.poolSize           = this.config.pool.maxConnections
+      this.pg.defaults.poolSize           = this.config.pool.maxConnections || 10
-      this.pg.defaults.poolIdleTimeout    = this.config.pool.maxIdleTime
+      this.pg.defaults.poolIdleTimeout    = this.config.pool.maxIdleTime    || 30000
-      this.pg.defaults.reapIntervalMillis = this.config.pool.reapInterval || 1000
+      this.pg.defaults.reapIntervalMillis = this.config.pool.reapInterval   || 1000
    }
    this.disconnectTimeoutId = null
@@ -142,7 +142,9 @@ module.exports = (function() {
    }
    if (this.client) {
-      this.client.end.bind(this.client)
+      // Closes a client correctly even if we have backed up queries
+      // https://github.com/brianc/node-postgres/pull/346
+      this.client.on('drain', this.client.end.bind(this.client))
    }
    this.isConnecting = false

--- a/lib/dialects/postgres/query-generator.js
+++ b/lib/dialects/postgres/query-generator.js
@@ -402,7 +402,7 @@ module.exports = (function() {
      return Utils._.template(query)(replacements)
    },
-    deleteQuery: function(tableName, where, options) {
+    deleteQuery: function(tableName, where, options, factory) {
      options = options || {}
      if (options.truncate === true) {
@@ -415,6 +415,10 @@ module.exports = (function() {
      primaryKeys[tableName] = primaryKeys[tableName] || [];
+      if (!!factory && primaryKeys[tableName].length < 1) {
+        primaryKeys[tableName] = Object.keys(factory.primaryKeys)
+      }
      var query = "DELETE FROM <%= table %> WHERE <%= primaryKeys %> IN (SELECT <%= primaryKeysSelection %> FROM <%= table %> WHERE <%= where %><%= limit %>)"
      var pks;

--- a/lib/dialects/postgres/query.js
+++ b/lib/dialects/postgres/query.js
@@ -20,34 +20,34 @@ module.exports = (function() {
  Query.prototype.run = function(sql) {
    this.sql = sql
    var self = this
+      , receivedError = false
+      , query         = this.client.query(sql)
+      , rows          = []
    if (this.options.logging !== false) {
      this.options.logging('Executing: ' + this.sql)
    }
-      var receivedError = false
-        , query         = this.client.query(sql)
-        , rows          = []
    query.on('row', function(row) {
      rows.push(row)
    })
    query.on('error', function(err) {
      receivedError = true
-      this.emit('error', err, this.callee)
+      self.emit('error', err, self.callee)
-    }.bind(this))
+    })
    query.on('end', function() {
-      this.emit('sql', this.sql)
+      self.emit('sql', self.sql)
      if (receivedError) {
        return
      }
-      onSuccess.call(this, rows, sql)
+      onSuccess.call(self, rows, sql)
-    }.bind(this))
+    })
    return this
  }

--- a/lib/query-interface.js
+++ b/lib/query-interface.js
@@ -481,7 +481,7 @@ module.exports = (function() {
  QueryInterface.prototype.delete = function(dao, tableName, identifier) {
    var self = this
      , restrict = false
-      , sql = self.QueryGenerator.deleteQuery(tableName, identifier)
+      , sql = self.QueryGenerator.deleteQuery(tableName, identifier, null, dao.daoFactory)
    // Check for a restrict field
    if (!!dao.daoFactory && !!dao.daoFactory.associations) {

--- a/package.json
+++ b/package.json
@@ -51,7 +51,7 @@
  "devDependencies": {
    "sqlite3": "~2.1.12",
    "mysql": "~2.0.0-alpha8",
-    "pg": "~2.3.1",
+    "pg": "~2.6.0",
    "watchr": "~2.4.3",
    "yuidocjs": "~0.3.36",
    "chai": "~1.7.2",

--- a/test/dao-factory.test.js
+++ b/test/dao-factory.test.js
@@ -1426,14 +1426,14 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        var self = this
        this.User.create({username: 'barfooz'}).success(function(user) {
          self.UserPrimary = self.sequelize.define('UserPrimary', {
-            specialKey: {
+            specialkey: {
              type: DataTypes.STRING,
              primaryKey: true
            }
          })
          self.UserPrimary.sync({force: true}).success(function() {
-            self.UserPrimary.create({specialKey: 'a string'}).success(function() {
+            self.UserPrimary.create({specialkey: 'a string'}).success(function() {
              self.user = user
              done()
            })
@@ -1441,9 +1441,18 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        })
      })
+      it('does not modify the passed arguments', function (done) {
+        var options = { where: ['specialkey = ?', 'awesome']}
+        this.UserPrimary.find(options).success(function(user) {
+          expect(options).to.deep.equal({ where: ['specialkey = ?', 'awesome']})
+          done()
+        })
+      })
      it('doesn\'t throw an error when entering in a non integer value for a specified primary field', function(done) {
        this.UserPrimary.find('a string').success(function(user) {
-          expect(user.specialKey).to.equal('a string')
+          expect(user.specialkey).to.equal('a string')
          done()
        })
      })
@@ -2458,6 +2467,15 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        })
      })
+      it('does not modify the passed arguments', function (done) {
+        var options = { where: ['username = ?', 'awesome']}
+        this.User.findAll(options).success(function(user) {
+          expect(options).to.deep.equal({ where: ['username = ?', 'awesome']})
+          done()
+        })
+      })
      it("finds all users matching the passed conditions", function(done) {
        this.User.findAll({where: "id != " + this.users[1].id}).success(function(users) {
          expect(users.length).to.equal(1)

--- a/test/dao.test.js
+++ b/test/dao.test.js
@@ -1000,6 +1000,34 @@ describe(Support.getTestDialectTeaser("DAO"), function () {
      })
    })
+    it('destroys a record with a primary key of something other than id', function(done) {
+      var UserDestroy = this.sequelize.define('UserDestroy', {
+        newId: {
+          type: DataTypes.STRING,
+          primaryKey: true
+        },
+        email: DataTypes.STRING
+      })
+      UserDestroy.sync().success(function() {
+        UserDestroy.create({newId: '123ABC', email: 'hello'}).success(function() {
+          UserDestroy.find({where: {email: 'hello'}}).success(function(user) {
+            user.destroy().on('sql', function(sql) {
+              if (dialect === "postgres" || dialect === "postgres-native") {
+                expect(sql).to.equal('DELETE FROM "UserDestroys" WHERE "newId" IN (SELECT "newId" FROM "UserDestroys" WHERE "newId"=\'123ABC\' LIMIT 1)')
+              }
+              else if (dialect === "mysql") {
+                expect(sql).to.equal("DELETE FROM `UserDestroys` WHERE `newId`='123ABC' LIMIT 1")
+              } else {
+                expect(sql).to.equal("DELETE FROM `UserDestroys` WHERE `newId`='123ABC'")
+              }
+              done()
+            })
+          })
+        })
+      })
+    })
    it("sets deletedAt property to a specific date when deleting an instance", function(done) {
      var self = this
      this.ParanoidUser.create({ username: 'fnord' }).success(function() {