Merge remote-tracking branch 'upstream/master'

changelog.md

@@ -3,6 +3,8 @@
 - [BUG] Fix wrong count for `findAndCountAll` with required includes [#4016](https://github.com/sequelize/sequelize/pull/4016)
 - [BUG] Fix problems related to parsing of unique constraint errors [#4017](https://github.com/sequelize/sequelize/issues/4017) and [#4012](https://github.com/sequelize/sequelize/issues/4012)
 - [BUG] Fix postgres path variable being surrounded by quotes to often in unique constraint errors [#4034](https://github.com/sequelize/sequelize/pull/4034)
+- [BUG] Fix `removeAttributes(id)` not setting `this.primaryKeys` to null
+- [BUG] Run validations on the through model during add, set and create for `belongsToMany`
 
 # 3.3.2
 - [FIXED] upsert no longer updates with default values each time [#3994](https://github.com/sequelize/sequelize/pull/3994)

lib/associations/belongs-to-many.js

@@ -457,7 +457,7 @@ BelongsToMany.prototype.injectSetter = function(obj) {
           return attributes;
         }.bind(this));
 
-        promises.push(association.through.model.bulkCreate(bulk, options));
+        promises.push(association.through.model.bulkCreate(bulk, _.assign({ validate: true }, options)));
       }
 
       return Utils.Promise.all(promises);
@@ -525,7 +525,7 @@ BelongsToMany.prototype.injectSetter = function(obj) {
           return attributes;
         }.bind(this));
 
-        promises.push(association.through.model.bulkCreate(bulk, options));
+        promises.push(association.through.model.bulkCreate(bulk, _.assign({ validate: true }, options)));
       }
 
       changedAssociations.forEach(function(assoc) {

lib/model.js

@@ -174,7 +174,8 @@ var addDefaultAttributes = function() {
     , head = {};
 
   // Add id if no primary key was manually added to definition
-  if (!Object.keys(this.primaryKeys).length) {
+  // Can't use this.primaryKeys here, since this function is called before PKs are identified
+  if (!_.any(this.rawAttributes, 'primaryKey')) {
     head = {
       id: {
         type: new DataTypes.INTEGER(),
@@ -580,7 +581,6 @@ Model.prototype.init = function(modelManager) {
 
   this.modelManager = modelManager;
   this.primaryKeys = {};
-  self.options.uniqueKeys = {};
 
   // Setup names of timestamp attributes
   this._timestampAttributes = {};
@@ -596,51 +596,8 @@ Model.prototype.init = function(modelManager) {
     }
   }
 
-  // Identify primary and unique attributes
-  Utils._.each(this.rawAttributes, function(options, attribute) {
-    if (options.hasOwnProperty('unique')) {
-      var idxName;
-      if (options.unique === true) {
-        idxName = self.tableName + '_' + attribute + '_unique';
-        self.options.uniqueKeys[idxName] = {
-          name: idxName,
-          fields: [attribute],
-          singleField: true
-        };
-      } else if (options.unique !== false) {
-        idxName = options.unique;
-        if (typeof options.unique === 'object') {
-          idxName = options.unique.name;
-        }
-
-        self.options.uniqueKeys[idxName] = self.options.uniqueKeys[idxName] || {fields: [], msg: null};
-        self.options.uniqueKeys[idxName].fields.push(options.field || attribute);
-        self.options.uniqueKeys[idxName].msg = self.options.uniqueKeys[idxName].msg || options.unique.msg || null;
-        self.options.uniqueKeys[idxName].name = idxName || false;
-      }
-    }
-
-    if (options.primaryKey === true) {
-      self.primaryKeys[attribute] = self.attributes[attribute];
-    }
-  });
-
-  this.uniqueKeys = this.options.uniqueKeys;
-
   // Add head and tail default attributes (id, timestamps)
-  addDefaultAttributes.call(this);
   addOptionalClassMethods.call(this);
-  // Primary key convenience variables
-  this.primaryKeyAttributes = Object.keys(this.primaryKeys);
-  this.primaryKeyAttribute = this.primaryKeyAttributes[0];
-  this.primaryKeyField = this.rawAttributes[this.primaryKeyAttribute].field || this.primaryKeyAttribute;
-
-  this.primaryKeyCount = this.primaryKeyAttributes.length;
-  this._hasPrimaryKeys = this.options.hasPrimaryKeys = this.hasPrimaryKeys = this.primaryKeyCount > 0;
-
-  this._isPrimaryKey = Utils._.memoize(function(key) {
-    return self.primaryKeyAttributes.indexOf(key) !== -1;
-  });
 
   this.$scope = _.isPlainObject(this.options.defaultScope) ? this.options.defaultScope : {};
 
@@ -668,7 +625,10 @@ Model.prototype.init = function(modelManager) {
       self.Instance.prototype[name] = fct;
     });
   }
+
+  addDefaultAttributes.call(this);
   this.refreshAttributes();
+
   findAutoIncrementField.call(this);
 
   this.Instance.prototype.$Model =
@@ -743,6 +703,9 @@ Model.prototype.refreshAttributes = function() {
 
   this.fieldRawAttributesMap = {};
 
+  this.primaryKeys = {};
+  self.options.uniqueKeys = {};
+
   Utils._.each(this.rawAttributes, function(definition, name) {
     definition.type = self.sequelize.normalizeDataType(definition.type);
 
@@ -754,6 +717,10 @@ Model.prototype.refreshAttributes = function() {
       definition.field = name;
     }
 
+    if (definition.primaryKey === true) {
+      self.primaryKeys[name] = definition;
+    }
+
     self.fieldRawAttributesMap[definition.field] = definition;
 
     if (definition.type instanceof DataTypes.BOOLEAN) {
@@ -784,6 +751,28 @@ Model.prototype.refreshAttributes = function() {
       self._defaultValues[name] = Utils._.partial(Utils.toDefaultValue, definition.defaultValue);
     }
 
+    if (definition.hasOwnProperty('unique')) {
+      var idxName;
+      if (definition.unique === true) {
+        idxName = self.tableName + '_' + name + '_unique';
+        self.options.uniqueKeys[idxName] = {
+          name: idxName,
+          fields: [definition.field],
+          singleField: true
+        };
+      } else if (definition.unique !== false) {
+        idxName = definition.unique;
+        if (typeof definition.unique === 'object') {
+          idxName = definition.unique.name;
+        }
+
+        self.options.uniqueKeys[idxName] = self.options.uniqueKeys[idxName] || {fields: [], msg: null};
+        self.options.uniqueKeys[idxName].fields.push(definition.field);
+        self.options.uniqueKeys[idxName].msg = self.options.uniqueKeys[idxName].msg || definition.unique.msg || null;
+        self.options.uniqueKeys[idxName].name = idxName || false;
+      }
+    }
+
     if (definition.hasOwnProperty('validate')) {
       self.Instance.prototype.validators[name] = definition.validate;
     }
@@ -798,6 +787,8 @@ Model.prototype.refreshAttributes = function() {
     }
   });
 
+  this.uniqueKeys = this.options.uniqueKeys;
+
   this._hasBooleanAttributes = !!this._booleanAttributes.length;
   this._isBooleanAttribute = Utils._.memoize(function(key) {
     return self._booleanAttributes.indexOf(key) !== -1;
@@ -848,6 +839,20 @@ Model.prototype.refreshAttributes = function() {
   this.Instance.prototype._isAttribute = Utils._.memoize(function(key) {
     return self.Instance.prototype.attributes.indexOf(key) !== -1;
   });
+
+  // Primary key convenience variables
+  this.primaryKeyAttributes = Object.keys(this.primaryKeys);
+  this.primaryKeyAttribute = this.primaryKeyAttributes[0];
+  if (this.primaryKeyAttribute) {
+    this.primaryKeyField = this.rawAttributes[this.primaryKeyAttribute].field || this.primaryKeyAttribute;
+  }
+
+  this.primaryKeyCount = this.primaryKeyAttributes.length;
+  this._hasPrimaryKeys = this.options.hasPrimaryKeys = this.hasPrimaryKeys = this.primaryKeyCount > 0;
+
+  this._isPrimaryKey = Utils._.memoize(function(key) {
+    return self.primaryKeyAttributes.indexOf(key) !== -1;
+  });
 };
 
 /**

test/integration/associations/belongs-to-many.test.js

@@ -860,6 +860,56 @@ describe(Support.getTestDialectTeaser('BelongsToMany'), function() {
     });
   });
 
+  describe('through model validations', function () {
+    beforeEach(function () {
+      var Project = this.sequelize.define('Project', {
+        name: Sequelize.STRING
+      });
+
+      var Employee = this.sequelize.define('Employee', {
+        name: Sequelize.STRING
+      });
+
+      var Participation = this.sequelize.define('Participation', {
+        role: {
+          type: Sequelize.STRING,
+          allowNull: false,
+          validate: {
+            len: {
+              args: [2, 50],
+              msg: 'too bad'
+            }
+          }
+        }
+      });
+
+      Project.belongsToMany(Employee, { as: 'Participants', through: Participation });
+      Employee.belongsToMany(Project, { as: 'Participations', through: Participation });
+
+      return this.sequelize.sync({ force: true }).bind(this).then(function () {
+        return Promise.all([
+          Project.create({ name: 'project 1' }),
+          Employee.create({ name: 'employee 1' })
+        ]).bind(this).spread(function (project, employee) {
+          this.project = project;
+          this.employee = employee;
+        });
+      });
+    });
+
+    it('runs on add', function () {
+      return expect(this.project.addParticipant(this.employee, { role: ''})).to.be.rejected;
+    });
+
+    it('runs on set', function () {
+      return expect(this.project.setParticipants([this.employee], { role: ''})).to.be.rejected;
+    });
+
+    it('runs on create', function () {
+      return expect(this.project.createParticipant({ name: 'employee 2'}, { role: ''})).to.be.rejected;
+    });
+  });
+
   describe('optimizations using bulk create, destroy and update', function() {
     beforeEach(function() {
       this.User = this.sequelize.define('User', { username: DataTypes.STRING }, {timestamps: false});

test/unit/model/removeAttribute.test.js

+'use strict';
+
+/* jshint -W030 */
+var chai = require('chai')
+  , expect = chai.expect
+  , Support   = require(__dirname + '/../support')
+  , current   = Support.sequelize
+  , _ = require('lodash')
+  , DataTypes = require(__dirname + '/../../../lib/data-types');
+
+describe(Support.getTestDialectTeaser('Model'), function() {
+  describe('removeAttribute', function () {
+    it('should support removing the primary key', function () {
+      var Model = current.define('m', {
+        name: DataTypes.STRING
+      });
+
+      expect(Model.primaryKeyAttribute).not.to.be.undefined;
+      expect(_.size(Model.primaryKeys)).to.equal(1);
+
+      Model.removeAttribute('id');
+
+      expect(Model.primaryKeyAttribute).to.be.undefined;
+      expect(_.size(Model.primaryKeys)).to.equal(0);
+    });
+  });
+});