feat(promises): more refactoring

Mick Hansen
Commit d8c32ff4 authored Apr 27, 2014 by Mick Hansen
Showing with 70 additions and 105 deletions
changelog.md
lib/associations/belongs-to.js
lib/dao-factory.js
lib/dao.js
lib/promise.js
test/dao-factory.test.js
test/dao-factory/create.test.js
test/dao-factory/scopes.test.js
test/promise.test.js
--- a/changelog.md
+++ b/changelog.md
 Notice: All 1.7.x changes are present in 2.0.x aswell
+# next
+#### Breaking changes
+- Sequelize now returns promises instead of its custom event emitter from most calls. This affects methods that return multiple values (like `findOrCreate` or `findOrInitialize`). If your current callbacks do not accept the 2nd success parameter you might be seeing an array as the first param. Either use `.spread()` for these methods or add another argument to your callback: `.success(instance)` -> `.success(instance, created)`.
+- `.success()`/`.done()` and any other non promise methods are now deprecated (we will keep the codebase around for a few versions though). on('sql') persists for debugging purposes.
 # v2.0.0-dev11
 ### Caution: This release contains many changes and is highly experimental
 - [PERFORMANCE] increased build performance when using include, which speeds up findAll etc.

--- a/lib/associations/belongs-to.js
+++ b/lib/associations/belongs-to.js
@@ -113,15 +113,9 @@ module.exports = (function() {
        options.transaction = fieldsOrOptions.transaction
      }
-      return new Utils.CustomEventEmitter(function(emitter) {
+      return association.target.create(values, fieldsOrOptions).then(function(newAssociatedObject) {
-        association.target
+        return instance[association.accessors.set](newAssociatedObject, options)
-          .create(values, fieldsOrOptions)
-          .proxy(emitter, { events: ['error', 'sql'] })
-          .success(function(newAssociatedObject) {
-            instance[association.accessors.set](newAssociatedObject, options)
-              .proxy(emitter)
      })
-      }).run()
    }
    return this

--- a/lib/dao-factory.js
+++ b/lib/dao-factory.js
@@ -330,7 +330,7 @@ module.exports = (function() {
  /**
   * Sync this DAOFactory to the DB, that is create the table. Upon success, the callback will be called with the model instance (this)
   * @see {Sequelize#sync} for options
-   * @return {EventEmitter} 
+   * @return {Promise} 
   */
  DAOFactory.prototype.sync = function(options) {
    options = Utils._.extend({}, this.options, options || {})
@@ -353,7 +353,7 @@ module.exports = (function() {
   * Drop the table represented by this Model
   * @param {Object}  [options]
   * @param {Boolean} [options.cascade=false] Also drop all objects depending on this table, such as views. Only works in postgres
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.drop = function(options) {
    return this.QueryInterface.dropTable(this.getTableName(), options)
@@ -593,7 +593,7 @@ module.exports = (function() {
   * @param  {Transaction}               [queryOptions.transaction]
   *
   * @see    {Sequelize#query}
-   * @return {EventEmitter}
+   * @return {Promise}
   * @alias all
   */
  DAOFactory.prototype.findAll = function(options, queryOptions) {
@@ -649,7 +649,7 @@ module.exports = (function() {
  * @param  {Object}                    [queryOptions]
  * 
  * @see {DAOFactory#findAll}           for an explanation of options and queryOptions
-  * @return {EventEmitter}
+  * @return {Promise}
  */
  DAOFactory.prototype.find = function(options, queryOptions) {
    var hasJoin = false
@@ -657,9 +657,7 @@ module.exports = (function() {
    // no options defined?
    // return an emitter which emits null
    if ([null, undefined].indexOf(options) !== -1) {
-      return new Utils.CustomEventEmitter(function(emitter) {
+      return Utils.Promise.resolve(null)
-        setTimeout(function() { emitter.emit('success', null) }, 10)
-      }).run()
    }
    var primaryKeys = this.primaryKeys
@@ -742,7 +740,7 @@ module.exports = (function() {
   * @param {Object}          [options] Query options. See sequelize.query for full options
   * @param {DataType|String} [options.dataType] The type of the result. If field is a field in the DAO, the default will be the type of that field, otherwise defaults to float.
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.aggregate = function(field, aggregateFunction, options) {
    var tableField;
@@ -773,7 +771,6 @@ module.exports = (function() {
  DAOFactory.prototype.count = function(options) {
    options = Utils._.clone(options || {})
-    return new Utils.CustomEventEmitter(function (emitter) {
    var col = this.sequelize.col('*')
    if (options.include) {
      col = this.sequelize.col(this.name+'.'+(this.primaryKeyAttribute))
@@ -786,10 +783,12 @@ module.exports = (function() {
    options.includeIgnoreAttributes = false
    options.limit = null
-      this.find(options, {raw: true, transaction: options.transaction}).proxy(emitter, {events: ['sql', 'error']}).success(function (result) {
+    return this.find(options, {
-        emitter.emit('success', parseInt(result.count, 10))
+      raw: true,
+      transaction: options.transaction
+    }).then(function (result) {
+      return parseInt(result.count, 10)
    })
-    }.bind(this)).run()
  }
  /**
@@ -809,38 +808,27 @@ module.exports = (function() {
   * @param {Object} [queryOptions] See Sequelize.query
   *
   * @see {DAOFactory#findAll} for a specification of find and query options 
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.findAndCountAll = function(findOptions, queryOptions) {
    var self  = this
      // no limit, offset, order, attributes for the options given to count()
      , countOptions = Utils._.omit(findOptions ? Utils._.merge({}, findOptions) : {}, ['offset', 'limit', 'order', 'attributes'])
-    return new Utils.CustomEventEmitter(function (emitter) {
+    return self.count(countOptions).then(function (count) {
-      var emit = {
+      if (count === 0) {
-        okay : function(count, results) {     // emit success
+        return {
-          emitter.emit('success', {
          count: count || 0,
-            rows : (results && Array.isArray(results) ? results : [])
+          rows: []
-          })
        }
      }
+      return self.findAll(findOptions, queryOptions).then(function (results) {
-      self.count(countOptions)
+        return {
-        .proxy(emitter, {events: ['sql', 'error']})
+          count: count || 0,
-        .success(function(count) {
+          rows: (results && Array.isArray(results) ? results : [])
-          if (count === 0) {
-            return emit.okay(count) // no records, no need for another query
        }
-          self.findAll(findOptions, queryOptions)
-            .proxy(emitter, {events: ['sql', 'error']})
-            .success(function(results) {
-              emit.okay(count, results)
      })
    })
-    }).run()
  }
  /**
@@ -850,7 +838,7 @@ module.exports = (function() {
   * @param {Object} [options] See aggregate
   * @see {DAOFactory#aggregate} for options
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.max = function(field, options) {
    return this.aggregate(field, 'max', options)
@@ -863,7 +851,7 @@ module.exports = (function() {
   * @param {Object} [options] See aggregate
   * @see {DAOFactory#aggregate} for options
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.min = function(field, options) {
    return this.aggregate(field, 'min', options)
@@ -876,7 +864,7 @@ module.exports = (function() {
   * @param {Object} [options] See aggregate
   * @see {DAOFactory#aggregate} for options
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.sum = function(field, options) {
    return this.aggregate(field, 'sum', options)
@@ -947,7 +935,7 @@ module.exports = (function() {
   * @param {Array}         [options.include] an array of include options - Used to build prefetched/included model instances
   * @param {Transaction}   [options.transaction]
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */  
  DAOFactory.prototype.create = function(values, options) {
    Utils.validateParameter(values, Object, { optional: true })
@@ -975,7 +963,7 @@ module.exports = (function() {
   * @param {Object}  [options] Options passed to the find call
   * @deprecated The syntax is due for change, in order to make `where` more consistent with the rest of the API
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   * @alias findOrBuild
   */
  DAOFactory.prototype.findOrInitialize = DAOFactory.prototype.findOrBuild = function (params, defaults, options) {
@@ -991,10 +979,9 @@ module.exports = (function() {
      delete defaults.transaction
    }
-    return new Utils.CustomEventEmitter(function (emitter) {
+    return self.find({
-      self.find({
      where: params
-      }, options).success(function (instance) {
+    }, options).then(function (instance) {
      if (instance === null) {
        var i = 0
@@ -1004,19 +991,13 @@ module.exports = (function() {
        var build = self.build(params)
-          build.hookValidate({skip: Object.keys(params)}).success(function (instance) {
+        return build.hookValidate({skip: Object.keys(params)}).then(function () {
-            emitter.emit('success', build, true)
+          return Utils.Promise.resolve([build, true])
-          })
-          .error(function (error) {
-            emitter.emit('error', error)
        })
-        } else {
-          emitter.emit('success', instance, false)
      }
-      }).error(function (error) {
-        emitter.emit('error', error)
+      return Utils.Promise.resolve([instance, false])
    })
-    }).run()
  }
  /**
@@ -1027,7 +1008,7 @@ module.exports = (function() {
   * @param {Object}  [options] Options passed to the find and create calls
   * @deprecated The syntax is due for change, in order to make `where` more consistent with the rest of the API
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.findOrCreate = function (where, defaults, options) {
    var self   = this
@@ -1043,32 +1024,23 @@ module.exports = (function() {
      }
    }
-    return new Utils.CustomEventEmitter(function (emitter) {
+    return self.find({
-      self.find({
      where: where
    }, {
      transaction: options.transaction
-      }).success(function (instance) {
+    }).then(function (instance) {
      if (instance === null) {
        for (var attrname in defaults) {
          values[attrname] = defaults[attrname]
        }
-          self
+        return self.create(values, options).then(function (instance) {
-            .create(values, options)
+          return Utils.Promise.resolve([instance, true])
-            .success(function (instance) {
-              emitter.emit('success', instance, true)
-            })
-            .error( function (error) {
-              emitter.emit('error', error)
-            })
-        } else {
-          emitter.emit('success', instance, false)
-        }
-      }).error(function (error) {
-        emitter.emit('error', error)
        });
-    }).run()
+      }
+      return Utils.Promise.resolve([instance, false])
+    })
  }
  /**
@@ -1084,16 +1056,16 @@ module.exports = (function() {
   * @param  {Boolean}      [options.hooks=false] Run before / after bulkCreate hooks?
   * @param  {Boolean}      [options.ignoreDuplicates=false] Ignore duplicate values for primary keys? (not supported by postgres)
   * 
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.bulkCreate = function(records, fieldsOrOptions, options) {
    Utils.validateParameter(fieldsOrOptions, Object, { deprecated: Array, optional: true, index: 2, method: 'DAOFactory#bulkCreate' })
    Utils.validateParameter(options, 'undefined', { deprecated: Object, optional: true, index: 3, method: 'DAOFactory#bulkCreate' })
    if (!records.length) {
-      return new Utils.CustomEventEmitter(function(emitter) {
+      return new Utils.Promise(function(resolve) {
-        emitter.emit('success', [])
+        resolve([])
-      }).run();
+      })
    }
    options = Utils._.extend({
@@ -1110,9 +1082,9 @@ module.exports = (function() {
    }
    if(this.daoFactoryManager.sequelize.options.dialect === 'postgres' && options.ignoreDuplicates ) {
-      return new Utils.CustomEventEmitter(function(emitter) {
+      return new Utils.Promise(function(resolve, reject) {
-        emitter.emit('error', new Error('Postgres does not support the \'ignoreDuplicates\' option.'))
+        reject(new Error('Postgres does not support the \'ignoreDuplicates\' option.'))
-      }).run();
+      });
    }
    var self          = this
@@ -1269,7 +1241,7 @@ module.exports = (function() {
   * @param  {Number}   [options.limit]     How many rows to delete
   * @param  {Boolean}  [options.truncate]  If set to true, dialects that support it will use TRUNCATE instead of DELETE FROM. If a table is truncated the where and limit options are ignored
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAOFactory.prototype.destroy = function(where, options) {
    options = options || {}

--- a/lib/dao.js
+++ b/lib/dao.js
@@ -400,7 +400,7 @@ module.exports = (function() {
   * @param {Object} [options.fields] An alternative way of setting which fields should be persisted
   * @param {Transaction} [options.transaction]
   *
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAO.prototype.save = function(fieldsOrOptions, options) {
    if (fieldsOrOptions instanceof Array) {
@@ -585,7 +585,7 @@ module.exports = (function() {
   * @param {Array} [options.skip] An array of strings. All properties that are in this array will not be validated
   * @see {DAOValidator}
   * 
-   * @return {EventEmitter}
+   * @return {Promise}
   */
  DAO.prototype.validate = function(options) {
    return new DaoValidator(this, options).validate()

--- a/lib/promise.js
+++ b/lib/promise.js
@@ -139,7 +139,7 @@ SequelizePromise.prototype.emit = function(evt) {
  if (evt === 'success') {
    this.seqResolve.apply(this, args);
-  } else if (evt === 'error') {;
+  } else if (evt === 'error') {
    this.seqReject.apply(this, args);
  } else {
    // Needed to transfer sql across .then() calls
@@ -170,7 +170,11 @@ SequelizePromise.prototype.emit = function(evt) {
 SequelizePromise.prototype.success =
 SequelizePromise.prototype.ok = function(fct) {
+  if (fct.length > 1) {
+    return this.spread(fct);
+  } else {
    return this.then(fct);
+  }
 }
 /**

--- a/test/dao-factory.test.js
+++ b/test/dao-factory.test.js
@@ -569,9 +569,9 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        User.sync({ force: true }).success(function() {
          sequelize.transaction(function(t) {
            User.create({ username: 'foo' }, { transaction: t }).success(function() {
-              User.findOrInitialize({ username: 'foo' }).success(function(user1) {
+              User.findOrInitialize({ username: 'foo' }).spread(function(user1) {
-                User.findOrInitialize({ username: 'foo' }, { transaction: t }).success(function(user2) {
+                User.findOrInitialize({ username: 'foo' }, { transaction: t }).spread(function(user2) {
-                  User.findOrInitialize({ username: 'foo' }, { foo: 'asd' }, { transaction: t }).success(function(user3) {
+                  User.findOrInitialize({ username: 'foo' }, { foo: 'asd' }, { transaction: t }).spread(function(user3) {
                    expect(user1.isNewRecord).to.be.true
                    expect(user2.isNewRecord).to.be.false
                    expect(user3.isNewRecord).to.be.false
@@ -592,7 +592,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        this.User.create({ username: 'Username' }).success(function (user) {
          self.User.findOrInitialize({
            username: user.username
-          }).success(function (_user, initialized) {
+          }).spread(function (_user, initialized) {
            expect(_user.id).to.equal(user.id)
            expect(_user.username).to.equal('Username')
            expect(initialized).to.be.false
@@ -608,7 +608,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
          self.User.findOrInitialize({
            username: user.username,
            data: user.data
-          }).success(function (_user, initialized) {
+          }).spread(function (_user, initialized) {
            expect(_user.id).to.equal(user.id)
            expect(_user.username).to.equal('Username')
            expect(_user.data).to.equal('data')
@@ -626,7 +626,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
            data: 'ThisIsData'
          }
-        this.User.findOrInitialize(data, default_values).success(function(user, initialized) {
+        this.User.findOrInitialize(data, default_values).spread(function(user, initialized) {
          expect(user.id).to.be.null
          expect(user.username).to.equal('Username')
          expect(user.data).to.equal('ThisIsData')

--- a/test/dao-factory/create.test.js
+++ b/test/dao-factory/create.test.js
@@ -67,7 +67,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
      this.User.create(data).success(function (user) {
        self.User.findOrCreate({
          username: user.username
-        }).success(function (_user, created) {
+        }).spread(function (_user, created) {
          expect(_user.id).to.equal(user.id)
          expect(_user.username).to.equal('Username')
          expect(created).to.be.false

--- a/test/dao-factory/scopes.test.js
+++ b/test/dao-factory/scopes.test.js
@@ -245,7 +245,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
    })
    it("should have no problem performing findOrCreate", function(done) {
-      this.ScopeMe.findOrCreate({username: 'fake'}).success(function(user) {
+      this.ScopeMe.findOrCreate({username: 'fake'}).spread(function(user) {
        expect(user.username).to.equal('fake')
        done()
      })

--- a/test/promise.test.js
+++ b/test/promise.test.js
@@ -319,16 +319,6 @@ describe(Support.getTestDialectTeaser("Promise"), function () {
      this.User.create({ id: 1, aNumber: 0, bNumber: 0 }).done(done)
    })
-    it('with then', function (done) {
-      this.User
-        .findOrCreate({ id: 1})
-        .then(function(user) {
-          expect(user.id).to.equal(1)
-          expect(arguments.length).to.equal(1)
-          done()
-        })
-    })
    describe('with spread', function () {
      it('user not created', function (done) {
        this.User