fix(Model): findOrCreate will now accurately throw an error when `defaults` valu…

…es trigger a unique error

For cases where the primary `where` passed to findOrCreate does not trigger a unique error but the default values does the code would then attempt a find on the were resulting in the perceived impossible return value of [null, true]

changelog.md

@@ -10,6 +10,7 @@
 - [BUG] Fixed issue with creating and updating values of a `DataTypes.ARRAY(DataTypes.JSON)` attribute
 - [BUG] Fixed issue with creating and updating values of a `DataTypes.ARRAY(DataTypes.JSON)` attribute
 - [BUG] `Model.bulkCreate([{}], {returning: true})` will now correctly result in instances with primary key values.
 - [BUG] `Model.bulkCreate([{}], {returning: true})` will now correctly result in instances with primary key values.
 - [BUG] `instance.save()` with `fields: []` (as a result of `.changed()` being `[]`) will no result in a noop instead of an empty update query.
 - [BUG] `instance.save()` with `fields: []` (as a result of `.changed()` being `[]`) will no result in a noop instead of an empty update query.
+- [BUG] Fixed case where `findOrCreate` could return `[null, true]` when given a `defaults` value that triggered a unique constraint error.
 
 
 #### Backwards compatability changes
 #### Backwards compatability changes
 - `instance.update()` using default fields will now automatically also save and validate values provided via `beforeUpdate` hooks
 - `instance.update()` using default fields will now automatically also save and validate values provided via `beforeUpdate` hooks

lib/dialects/mssql/query.js

@@ -189,7 +189,8 @@ module.exports = (function() {
       return new sequelizeErrors.UniqueConstraintError({
       return new sequelizeErrors.UniqueConstraintError({
         message: message,
         message: message,
         errors: errors,
         errors: errors,
-        parent: err
+        parent: err,
+        fields: fields
       });
       });
     }
     }
 
 

lib/dialects/mysql/query.js

@@ -133,7 +133,8 @@ module.exports = (function() {
         return new sequelizeErrors.UniqueConstraintError({
         return new sequelizeErrors.UniqueConstraintError({
           message: message,
           message: message,
           errors: errors,
           errors: errors,
-          parent: err
+          parent: err,
+          fields: fields
         });
         });
 
 
       case 1451:
       case 1451:

lib/dialects/postgres/query.js

@@ -287,7 +287,8 @@ module.exports = (function() {
           return new sequelizeErrors.UniqueConstraintError({
           return new sequelizeErrors.UniqueConstraintError({
             message: message,
             message: message,
             errors: errors,
             errors: errors,
-            parent: err
+            parent: err,
+            fields: fields
           });
           });
         } else {
         } else {
           return new sequelizeErrors.UniqueConstraintError({
           return new sequelizeErrors.UniqueConstraintError({

lib/dialects/sqlite/query.js

@@ -232,7 +232,8 @@ module.exports = (function() {
         return new sequelizeErrors.UniqueConstraintError({
         return new sequelizeErrors.UniqueConstraintError({
           message: message,
           message: message,
           errors: errors,
           errors: errors,
-          parent: err
+          parent: err,
+          fields: fields
         });
         });
 
 
       case 'SQLITE_BUSY':
       case 'SQLITE_BUSY':

lib/errors.js

@@ -120,6 +120,7 @@ error.UniqueConstraintError = function (options) {
   this.name = 'SequelizeUniqueConstraintError';
   this.name = 'SequelizeUniqueConstraintError';
   this.message = options.message;
   this.message = options.message;
   this.errors = options.errors;
   this.errors = options.errors;
+  this.fields = options.fields;
 };
 };
 util.inherits(error.UniqueConstraintError, error.ValidationError);
 util.inherits(error.UniqueConstraintError, error.ValidationError);
 
 

lib/model.js

@@ -1117,7 +1117,9 @@ module.exports = (function() {
   Model.prototype.findOrCreate = function(options, queryOptions) {
   Model.prototype.findOrCreate = function(options, queryOptions) {
     var self = this
     var self = this
       , internalTransaction = !(queryOptions && queryOptions.transaction)
       , internalTransaction = !(queryOptions && queryOptions.transaction)
-      , values;
+      , values
+      , whereFields
+      , defaultFields;
 
 
     queryOptions = queryOptions ? Utils._.clone(queryOptions) : {};
     queryOptions = queryOptions ? Utils._.clone(queryOptions) : {};
 
 
@@ -1125,6 +1127,9 @@ module.exports = (function() {
       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.)');
       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.)');
     }
     }
 
 
+    whereFields = Object.keys(options.where);
+    if (options.defaults) defaultFields = Object.keys(options.defaults);
+
     // Create a transaction or a savepoint, depending on whether a transaction was passed in
     // Create a transaction or a savepoint, depending on whether a transaction was passed in
     return self.sequelize.transaction(queryOptions).bind({}).then(function (transaction) {
     return self.sequelize.transaction(queryOptions).bind({}).then(function (transaction) {
       this.transaction = transaction;
       this.transaction = transaction;
@@ -1155,11 +1160,20 @@ module.exports = (function() {
         }
         }
 
 
         return [instance, true];
         return [instance, true];
-      }).catch(self.sequelize.UniqueConstraintError, function () {
+      }).catch(self.sequelize.UniqueConstraintError, function (err) {
+        if (defaultFields) {
+          if (!_.intersection(err.fields, whereFields).length && _.intersection(err.fields, defaultFields).length) {
+            throw err;
+          }
+        }
+
         // Someone must have created a matching instance inside the same transaction since we last did a find. Let's find it!
         // Someone must have created a matching instance inside the same transaction since we last did a find. Let's find it!
         return self.find(options, {
         return self.find(options, {
           transaction: internalTransaction ? null : this.transaction,
           transaction: internalTransaction ? null : this.transaction,
         }).then(function(instance) {
         }).then(function(instance) {
+          // Sanity check, ideally we caught this at the defaultFeilds/err.fields check
+          // But if we didn't and instance is null, we will throw
+          if (instance === null) throw err;
           return [instance, false];
           return [instance, false];
         });
         });
       });
       });

test/model/create.test.js

@@ -74,6 +74,34 @@ describe(Support.getTestDialectTeaser('Model'), function() {
       });
       });
     }
     }
 
 
+    it('should error correctly when defaults contain a unique key', function () {
+      var User = this.sequelize.define('user', {
+        objectId: {
+          type: DataTypes.STRING,
+          unique: true
+        },
+        username: {
+          type: DataTypes.STRING,
+          unique: true
+        }
+      });
+
+      return User.sync({force: true}).then(function () {
+        return User.create({
+          username: 'gottlieb'
+        });
+      }).then(function () {
+        return expect(User.findOrCreate({
+          where: {
+            objectId: 'asdasdasd'
+          },
+          defaults: {
+            username: 'gottlieb'
+          }
+        })).to.be.rejectedWith(Sequelize.UniqueConstraintError);
+      });
+    });
+
     it('returns instance if already existent. Single find field.', function(done) {
     it('returns instance if already existent. Single find field.', function(done) {
       var self = this,
       var self = this,
         data = {
         data = {