refactor(model): changed signature of findOrBuild from (where, defaults, options) to (options)

changelog.md

@@ -12,9 +12,10 @@
 #### Backwards compatability changes
 #### Backwards compatability changes
 - The `fieldName` property, used in associations with a foreign key object `(A.hasMany(B, { foreignKey: { ... }})`, has been renamed to `name` to avoid confusion with `field`. 
 - The `fieldName` property, used in associations with a foreign key object `(A.hasMany(B, { foreignKey: { ... }})`, has been renamed to `name` to avoid confusion with `field`. 
 - The naming of the join table entry for N:M association getters is now singular (like includes)
 - The naming of the join table entry for N:M association getters is now singular (like includes)
-- Signature of hooks has changed to pass options to all hooks
+- Signature of hooks has changed to pass options to all hooks. Any hooks previously defined like `Model.beforeCreate(values)` now need to be `Model.beforeCreate(values, options)` etc.
 - Results returned by hooks are ignored - changes to results by hooks should be made by reference
 - Results returned by hooks are ignored - changes to results by hooks should be made by reference
 - `Model.destroy()` signature has been changed from `(where, options)` to `(options)`, options now take a where parameter.
 - `Model.destroy()` signature has been changed from `(where, options)` to `(options)`, options now take a where parameter.
+- The syntax for `Model.findOrBuild` has changed, to be more in line with the rest of the library. `Model.findOrBuild(where, defaults);` becomes `Model.findOrBuild({ where: where, defaults: defaults });`.
 
 
 # v2.0.0-dev13
 # v2.0.0-dev13
 We are working our way to the first 2.0.0 release candidate.
 We are working our way to the first 2.0.0 release candidate.

lib/model.js

@@ -1035,44 +1035,38 @@ module.exports = (function() {
    * Find a row that matches the query, or build (but don't save) the row if none is found.
    * Find a row that matches the query, or build (but don't save) the row if none is found.
    * The successfull result of the promise will be (instance, initialized) - Make sure to use .spread()
    * The successfull result of the promise will be (instance, initialized) - Make sure to use .spread()
    *
    *
-   * @param {Object}  where A hash of search attributes. Note that this method differs from finders, in that the syntax is `{ attr1: 42 }` and NOT `{ where: { attr1: 42}}`. This may be subject to change in 2.0
-   * @param {Object}  [defaults] Default values to use if building a new instance
-   * @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
+   * @param {Object}  options
+   * @param {Object}  options.where A hash of search attributes.
+   * @param {Object}  [options.defaults] Default values to use if building a new instance
+   * @param {Object}  [options.transaction] Transaction to run query under
    *
    *
    * @return {Promise<Instance>}
    * @return {Promise<Instance>}
    * @method
    * @method
    * @alias findOrBuild
    * @alias findOrBuild
    */
    */
-  Model.prototype.findOrInitialize = Model.prototype.findOrBuild = function(params, defaults, options) {
-    defaults = defaults || {};
-    options = options || {};
+  Model.prototype.findOrInitialize = Model.prototype.findOrBuild = function(options) {
+    if (!options || !options.where) {
+      throw new Error('Missing where attribute in the options parameter passed to findOrCreate. Please note that the API has changed, and is now options (an object with where and defaults keys), queryOptions (transaction etc.)');
+    }
 
 
     var self = this
     var self = this
-      , defaultKeys = Object.keys(defaults)
-      , defaultLength = defaultKeys.length;
-
-    if (!options.transaction && defaults.transaction && (defaults.transaction instanceof Transaction)) {
-      options.transaction = defaults.transaction;
-      delete defaults.transaction;
-    }
+      , values;
 
 
     return self.find({
     return self.find({
-      where: params
+      where: options.where
     }, options).then(function(instance) {
     }, options).then(function(instance) {
       if (instance === null) {
       if (instance === null) {
-        var i = 0;
-
-        for (i = 0; i < defaultLength; i++) {
-          params[defaultKeys[i]] = defaults[defaultKeys[i]];
+        values = Utils._.clone(options.defaults) || {};
+        if (Utils._.isPlainObject(options.where)) {
+          values = Utils._.defaults(values, options.where);
         }
         }
 
 
-        var build = self.build(params);
+        instance = self.build(values);
+        options.skip = Object.keys(values);
 
 
-        options.skip = Object.keys(params);
-        return build.hookValidate(options).then(function() {
+        return instance.hookValidate(options).then(function() {
           delete options.skip;
           delete options.skip;
-          return Promise.resolve([build, true]);
+          return Promise.resolve([instance, true]);
         });
         });
       }
       }
 
 

test/dao-factory.test.js

@@ -674,9 +674,18 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
         User.sync({ force: true }).success(function() {
         User.sync({ force: true }).success(function() {
           sequelize.transaction().then(function(t) {
           sequelize.transaction().then(function(t) {
             User.create({ username: 'foo' }, { transaction: t }).success(function() {
             User.create({ username: 'foo' }, { transaction: t }).success(function() {
-              User.findOrInitialize({ username: 'foo' }).spread(function(user1) {
-                User.findOrInitialize({ username: 'foo' }, { transaction: t }).spread(function(user2) {
-                  User.findOrInitialize({ username: 'foo' }, { foo: 'asd' }, { transaction: t }).spread(function(user3) {
+              User.findOrInitialize({ 
+                where: {username: 'foo'}
+              }).spread(function(user1) {
+                User.findOrInitialize({ 
+                  where: {username: 'foo'},
+                  transaction: t
+                }).spread(function(user2) {
+                  User.findOrInitialize({ 
+                    where: {username: 'foo'},
+                    defaults: { foo: 'asd' },
+                    transaction: t
+                  }).spread(function(user3) {
                     expect(user1.isNewRecord).to.be.true
                     expect(user1.isNewRecord).to.be.true
                     expect(user2.isNewRecord).to.be.false
                     expect(user2.isNewRecord).to.be.false
                     expect(user3.isNewRecord).to.be.false
                     expect(user3.isNewRecord).to.be.false
@@ -696,7 +705,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
 
 
         this.User.create({ username: 'Username' }).success(function (user) {
         this.User.create({ username: 'Username' }).success(function (user) {
           self.User.findOrInitialize({
           self.User.findOrInitialize({
-            username: user.username
+            where: { username: user.username }
           }).spread(function (_user, initialized) {
           }).spread(function (_user, initialized) {
             expect(_user.id).to.equal(user.id)
             expect(_user.id).to.equal(user.id)
             expect(_user.username).to.equal('Username')
             expect(_user.username).to.equal('Username')
@@ -710,10 +719,10 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
         var self = this
         var self = this
 
 
         this.User.create({ username: 'Username', data: 'data' }).success(function (user) {
         this.User.create({ username: 'Username', data: 'data' }).success(function (user) {
-          self.User.findOrInitialize({
+          self.User.findOrInitialize({ where: {
             username: user.username,
             username: user.username,
             data: user.data
             data: user.data
-          }).spread(function (_user, initialized) {
+          }}).spread(function (_user, initialized) {
             expect(_user.id).to.equal(user.id)
             expect(_user.id).to.equal(user.id)
             expect(_user.username).to.equal('Username')
             expect(_user.username).to.equal('Username')
             expect(_user.data).to.equal('data')
             expect(_user.data).to.equal('data')
@@ -731,7 +740,10 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
             data: 'ThisIsData'
             data: 'ThisIsData'
           }
           }
 
 
-        this.User.findOrInitialize(data, default_values).spread(function(user, initialized) {
+        this.User.findOrInitialize({
+          where: data,
+          defaults: default_values
+        }).spread(function(user, initialized) {
           expect(user.id).to.be.null
           expect(user.id).to.be.null
           expect(user.username).to.equal('Username')
           expect(user.username).to.equal('Username')
           expect(user.data).to.equal('ThisIsData')
           expect(user.data).to.equal('ThisIsData')