Merge pull request #2568 from jamonkko/master

Prefer index name when mapping unique constraint errors

changelog.md

@@ -4,6 +4,7 @@
 - [BUG] Add support for `field` named the same as the attribute in `reload`, `bulkCreate` and `save` [#2348](https://github.com/sequelize/sequelize/issues/2348)
 - [BUG] Add support for `field` named the same as the attribute in `reload`, `bulkCreate` and `save` [#2348](https://github.com/sequelize/sequelize/issues/2348)
 - [BUG] Copy the options object in association getters. [#2311](https://github.com/sequelize/sequelize/issues/2311)
 - [BUG] Copy the options object in association getters. [#2311](https://github.com/sequelize/sequelize/issues/2311)
 - [BUG] `Model#destroy()` now supports `field`, this also fixes an issue with `N:M#removeAssociation` and `field`
 - [BUG] `Model#destroy()` now supports `field`, this also fixes an issue with `N:M#removeAssociation` and `field`
+- [BUG] Customized error message can now be set for unique constraint that was created manually (not with sync, but e.g. with migrations) or that has fields with underscore naming. This was problem at least with postgres before.
 
 
 #### Backwards compatability changes
 #### Backwards compatability changes
 - When eager-loading a many-to-many association, the attributes of the through table are now accessible through an attribute named after the through model rather than the through table name singularized. i.e. `Task.find({include: Worker})` where the table name for through model `TaskWorker` is `TableTaskWorkers` used to produce `{ Worker: { ..., TableTaskWorker: {...} } }`. It now produces `{ Worker: { ..., TaskWorker: {...} } }`. Does not affect models where table name is auto-defined by Sequelize, or where table name is model name pluralized.
 - When eager-loading a many-to-many association, the attributes of the through table are now accessible through an attribute named after the through model rather than the through table name singularized. i.e. `Task.find({include: Worker})` where the table name for through model `TaskWorker` is `TableTaskWorkers` used to produce `{ Worker: { ..., TableTaskWorker: {...} } }`. It now produces `{ Worker: { ..., TaskWorker: {...} } }`. Does not affect models where table name is auto-defined by Sequelize, or where table name is model name pluralized.

lib/dialects/postgres/query-generator.js

@@ -162,13 +162,22 @@ module.exports = (function() {
     uniqueConstraintMapping: {
     uniqueConstraintMapping: {
       code: '23505',
       code: '23505',
       map: function(str) {
       map: function(str) {
-        var match = str.match(/duplicate key value violates unique constraint "(.*?)_key"/);
+        var match = str.match(/duplicate key value violates unique constraint "(.*?)"/);
         if (match === null || match.length < 2) {
         if (match === null || match.length < 2) {
           return false;
           return false;
         }
         }
 
 
+        var indexName = match[1];
+        var fields = [];
+
+        match = indexName.match(/(.*?)_key/);
+        if (!!match && match.length > 1) {
+          fields = match[1].split('_').splice(1);
+        }
+
         return {
         return {
-          fields: match[1].split('_').splice(1)
+          indexName: indexName,
+          fields: fields
         };
         };
       }
       }
     },
     },

lib/instance.js

@@ -614,15 +614,19 @@ module.exports = (function() {
 
 
             if (index !== false) {
             if (index !== false) {
               var fields = index.fields.filter(function(f) { return f !== self.Model.tableName; });
               var fields = index.fields.filter(function(f) { return f !== self.Model.tableName; });
-
               Utils._.each(self.__options.uniqueKeys, function(uniqueKey) {
               Utils._.each(self.__options.uniqueKeys, function(uniqueKey) {
-                if (!!uniqueKey.msg && (Utils._.isEqual(uniqueKey.fields, fields)) || uniqueKey.name === index.indexName) {
-                  err = new self.sequelize.UniqueConstraintError({
-                    message: uniqueKey.msg,
-                    fields: fields,
-                    index: index.indexName,
-                    parent: err.parent
-                  });
+                if (!!uniqueKey.msg) {
+                  if (uniqueKey.name === index.indexName) {
+                    fields = _.clone(uniqueKey.fields);
+                  }
+                  if (Utils._.isEqual(uniqueKey.fields, fields)) {
+                    err = new self.sequelize.UniqueConstraintError({
+                      message: uniqueKey.msg,
+                      fields: fields,
+                      index: index.indexName,
+                      parent: err.parent
+                    });
+                  }
                 }
                 }
               });
               });
             }
             }

test/dao-factory.test.js

@@ -342,9 +342,9 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
           }
           }
         });
         });
       });
       });
-    });
+    })
 
 
-    it('allows us to customize the error message for unique constraint', function(done) {
+    it('allows us to customize the error message for unique constraint', function() {
       var self = this
       var self = this
         , User = this.sequelize.define('UserWithUniqueUsername', {
         , User = this.sequelize.define('UserWithUniqueUsername', {
             username: { type: Sequelize.STRING, unique: { name: 'user_and_email', msg: 'User and email must be unique' }},
             username: { type: Sequelize.STRING, unique: { name: 'user_and_email', msg: 'User and email must be unique' }},
@@ -353,14 +353,46 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
             bCol: { type: Sequelize.STRING, unique: 'a_and_b' }
             bCol: { type: Sequelize.STRING, unique: 'a_and_b' }
           })
           })
 
 
-      User.sync({ force: true }).success(function() {
-        User.create({username: 'tobi', email: 'tobi@tobi.me'}).success(function() {
-          User.create({username: 'tobi', email: 'tobi@tobi.me'}).catch(self.sequelize.UniqueConstraintError, function(err) {
-            expect(err.message).to.equal('User and email must be unique')
-            done()
-          })
-        })
-      })
+      return User.sync({ force: true }).bind(this).then(function() {
+        return self.sequelize.Promise.all([
+          User.create({username: 'tobi', email: 'tobi@tobi.me'}),
+          User.create({username: 'tobi', email: 'tobi@tobi.me'})])
+      }).catch(self.sequelize.UniqueConstraintError, function(err) {
+        expect(err.message).to.equal('User and email must be unique')
+      });
+    })
+
+    // If you use migrations to create unique indexes that have explicit names and/or contain fields
+    // that have underscore in their name. Then sequelize must use the index name to map the custom message to the error thrown from db.
+    it('allows us to map the customized error message with unique constraint name', function() {
+      // Fake migration style index creation with explicit index definition
+      var self = this
+        , User = this.sequelize.define('UserWithUniqueUsername', {
+            user_id: { type: Sequelize.INTEGER},
+            email: { type: Sequelize.STRING}
+          }, {
+            indexes: [
+            {
+              name: 'user_and_email_index',
+              msg: 'User and email must be unique',
+              unique: true,
+              method: 'BTREE',
+              fields: ['user_id', {attribute:'email', collate: dialect === 'sqlite' ? 'RTRIM' : 'en_US', order: 'DESC', length: 5}]
+            }]
+          });
+
+      return User.sync({ force: true }).bind(this).then(function() {
+        // Redefine the model to use the index in database and override error message
+        User = self.sequelize.define('UserWithUniqueUsername', {
+            user_id: { type: Sequelize.INTEGER, unique: { name: 'user_and_email_index', msg: 'User and email must be unique' }},
+            email: { type: Sequelize.STRING, unique: 'user_and_email_index'}
+          });
+        return self.sequelize.Promise.all([
+          User.create({user_id: 1, email: 'tobi@tobi.me'}),
+          User.create({user_id: 1, email: 'tobi@tobi.me'})])
+      }).catch(self.sequelize.UniqueConstraintError, function(err) {
+        expect(err.message).to.equal('User and email must be unique')
+      });
     })
     })
 
 
     it('should allow the user to specify indexes in options', function () {
     it('should allow the user to specify indexes in options', function () {