Merge branch 'master' of github.com:sequelize/sequelize

Mick Hansen
Commit 82d8b26d authored Sep 20, 2014 by Mick Hansen
Showing with 161 additions and 35 deletions
changelog.md
lib/dialects/abstract/query-generator.js
lib/dialects/mysql/query-generator.js
lib/dialects/mysql/query.js
lib/dialects/sqlite/query-generator.js
lib/model.js
lib/query-interface.js
lib/sequelize.js
test/dao-factory.test.js
test/dao-factory/create.test.js
test/query-interface.test.js
test/sequelize.test.js
--- a/changelog.md
+++ b/changelog.md
@@ -3,6 +3,7 @@
 - [FEATURE] Added `field` and `name` to the object form of foreign key definitions
 - [FEATURE] Added support for calling `Promise.done`, thus explicitly ending the promise chain by calling done with no arguments. Done with a function argument still continues the promise chain, to maintain BC.
 - [FEATURE] Added `scope` to hasMany association definitions, provides default values to association setters/finders [#2268](https://github.com/sequelize/sequelize/pull/2268)
+- [BUG] Only try to create indexes which don't already exist. Closes [#2162](https://github.com/sequelize/sequelize/issues/2162)
 #### 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`. 

--- a/lib/dialects/abstract/query-generator.js
+++ b/lib/dialects/abstract/query-generator.js
@@ -333,6 +333,20 @@ module.exports = (function() {
      return Utils._.template(query)(replacements);
    },
+    nameIndexes: function (indexes, rawTablename) {
+      return Utils._.map(indexes, function (index) {
+        if (!index.hasOwnProperty('name')) {
+          var onlyAttributeNames = index.fields.map(function(attribute) {
+            return (typeof attribute === 'string') ? attribute : attribute.attribute;
+          }.bind(this));
+          index.name = Utils.inflection.underscore(rawTablename + '_' + onlyAttributeNames.join('_'));
+        }
+        return index;
+      });
+    },
    /*
      Returns an add index query.
      Parameters:
@@ -380,15 +394,18 @@ module.exports = (function() {
        }
      }.bind(this));
-      var onlyAttributeNames = attributes.map(function(attribute) {
+      if (!options.name) {
-        return (typeof attribute === 'string') ? attribute : attribute.attribute;
+        // Mostly for cases where addIndex is called directly by the user without an options object (for example in migrations)
-      }.bind(this));
+        // All calls that go through sequelize should already have a name
+        options.fields = options.fields || attributes;
+        options = this.nameIndexes([options], rawTablename)[0];
+      }
      options = Utils._.defaults(options, {
        type: '',
        indicesType: options.type || '',
        indexType: options.method || undefined,
-        indexName: options.name || Utils.inflection.underscore(rawTablename + '_' + onlyAttributeNames.join('_')),
+        indexName: options.name,
        parser: null
      });

--- a/lib/dialects/mysql/query-generator.js
+++ b/lib/dialects/mysql/query-generator.js
@@ -223,7 +223,7 @@ module.exports = (function() {
    showIndexQuery: function(tableName, options) {
      var sql = 'SHOW INDEX FROM <%= tableName %><%= options %>';
      return Utils._.template(sql)({
-        tableName: this.quoteIdentifiers(tableName),
+        tableName: this.quoteTable(tableName),
        options: (options || {}).database ? ' FROM `' + options.database + '`' : ''
      });
    },

--- a/lib/dialects/mysql/query.js
+++ b/lib/dialects/mysql/query.js
@@ -53,7 +53,7 @@ module.exports = (function() {
    switch (err.errno || err.code) {
      case 1062:
-        match = err.message.match(/Duplicate entry '(.*)' for key '(.*?)'$/);
+        match = err.message.match(/Duplicate entry '(.*)' for key '?(.*?)$/);
        return new sequelizeErrors.UniqueConstraintError({
          fields: null,

--- a/lib/dialects/sqlite/query-generator.js
+++ b/lib/dialects/sqlite/query-generator.js
@@ -318,7 +318,7 @@ module.exports = (function() {
    showIndexQuery: function(tableName) {
      var sql = "PRAGMA INDEX_LIST(<%= tableName %>)";
-      return Utils._.template(sql, { tableName: this.quoteIdentifiers(tableName) });
+      return Utils._.template(sql, { tableName: this.quoteTable(tableName) });
    },
    removeIndexQuery: function(tableName, indexNameOrAttributes) {

--- a/lib/model.js
+++ b/lib/model.js
@@ -405,7 +405,18 @@ module.exports = (function() {
    }).then(function () {
      return self.QueryInterface.createTable(self.getTableName(), attributes, options);
    }).then(function () {
-      return Promise.map(self.options.indexes, function (index) {
+      return self.QueryInterface.showIndex(self.getTableName(), options);
+    }).then(function (indexes) {
+      // Assign an auto-generated name to indexes which are not named by the user
+      self.options.indexes = self.QueryInterface.nameIndexes(self.options.indexes, self.tableName);
+      indexes = Utils._.filter(self.options.indexes, function (item1) {
+        return !Utils._.any(indexes, function (item2) {
+          return item1.name === item2.name;
+        });
+      });
+      return Promise.map(indexes, function (index) {
        return self.QueryInterface.addIndex(self.getTableName(), index.fields, index, self.tableName);
      });
    }).return(this);

--- a/lib/query-interface.js
+++ b/lib/query-interface.js
@@ -388,13 +388,19 @@ module.exports = (function() {
      rawTablename = tableName;
    }
+    options = options || {};
    var sql = this.QueryGenerator.addIndexQuery(tableName, attributes, options, rawTablename);
-    return this.sequelize.query(sql, null, {logging: this.sequelize.options.logging});
+    return this.sequelize.query(sql, null, {logging: options.hasOwnProperty('logging') ? options.logging : this.sequelize.options.logging});
  };
  QueryInterface.prototype.showIndex = function(tableName, options) {
    var sql = this.QueryGenerator.showIndexQuery(tableName, options);
-    return this.sequelize.query(sql, null, {logging: this.sequelize.options.logging});
+    options = options || {};
+    return this.sequelize.query(sql, null, {logging: options.hasOwnProperty('logging') ? options.logging : this.sequelize.options.logging});
+  };
+  QueryInterface.prototype.nameIndexes = function(indexes, rawTablename) {
+    return this.QueryGenerator.nameIndexes(indexes, rawTablename);
  };
  QueryInterface.prototype.getForeignKeysForTables = function(tableNames) {

--- a/lib/sequelize.js
+++ b/lib/sequelize.js
@@ -63,6 +63,7 @@ module.exports = (function() {
   * @param {String}   [options.protocol='tcp'] The protocol of the relational database.
   * @param {Object}   [options.define={}] Default options for model definitions. See sequelize.define for options
   * @param {Object}   [options.query={}] Default options for sequelize.query
+   * @param {Object}   [options.set={}] Default options for sequelize.set
   * @param {Object}   [options.sync={}] Default options for sequelize.sync
   * @param {String}   [options.timezone='+00:00'] The timezone used when converting a date from the database into a javascript date. The timezone is also used to SET TIMEZONE when connecting to the server, to ensure that the result of NOW, CURRENT_TIMESTAMP and other time related functions have in the right timezone. For best cross platform performance use the format +/-HH:MM.
   * @param {Function} [options.logging=console.log] A function that gets executed everytime Sequelize would log something.
@@ -562,6 +563,45 @@ module.exports = (function() {
  };
  /**
+   * Execute a query which would set an environment or user variable. The variables are set per connection, so this function needs a transaction.
+   * Only works for MySQL.
+   *
+   * @method set
+   * @param {Object}        variables Object with multiple variables.
+   * @param {Object}        options Query options.
+   * @param {Transaction}   options.transaction The transaction that the query should be executed under
+   *
+   * @return {Promise}
+   */
+  Sequelize.prototype.set = function( variables, options ) {
+    var query;
+    // Prepare options
+    options = Utils._.extend({}, this.options.set, typeof options === 'object' && options || {});
+    if (['mysql', 'mariadb'].indexOf(this.options.dialect) === -1) {
+      throw new Error('sequelize.set is only supported for mysql');
+    }
+    if (!options.transaction || !(options.transaction instanceof Transaction) ) {
+      throw new TypeError("options.transaction is required");
+    }
+    // Override some options, since this isn't a SELECT
+    options.raw = true;
+    options.plain = true;
+    options.type = 'SET';
+    // Generate SQL Query
+    query =
+      'SET '+
+      Utils._.map( variables, function ( v, k ) {
+        return '@'+k +' := '+ ( typeof v === 'string' ? '"'+v+'"' : v );
+      }).join(', ');
+    return this.query(query, null, options);
+  };
+  /**
   * Create a new database schema.
   *
   * Note,that this is a schema in the [postgres sense of the word](http://www.postgresql.org/docs/9.1/static/ddl-schemas.html),

--- a/test/dao-factory.test.js
+++ b/test/dao-factory.test.js
@@ -304,11 +304,11 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        bCol: { type: Sequelize.STRING, unique: 'a_and_b' }
      })
-      User.sync({ force: true }).on('sql', _.after(2, function(sql) {
+      User.sync({ force: true }).on('sql', _.after(2, _.once(function(sql) {
        expect(sql).to.match(/UNIQUE\s*(user_and_email)?\s*\([`"]?username[`"]?, [`"]?email[`"]?\)/)
        expect(sql).to.match(/UNIQUE\s*(a_and_b)?\s*\([`"]?aCol[`"]?, [`"]?bCol[`"]?\)/)
        done()
-      }))
+      })))
    })
    it('allows us to customize the error message for unique constraint', function(done) {
@@ -353,6 +353,8 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
      })
      return this.sequelize.sync().bind(this).then(function () {
+        return this.sequelize.sync(); // The second call should not try to create the indices again
+      }).then(function () {
        return this.sequelize.queryInterface.showIndex(Model.tableName);
      }).spread(function () {
        var primary, idx1, idx2;
@@ -1759,14 +1761,14 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
      var run = function() {
        UserPub.sync({ force: true }).success(function() {
-          ItemPub.sync({ force: true }).on('sql', _.after(2, function(sql) {
+          ItemPub.sync({ force: true }).on('sql', _.after(2, _.once(function(sql) {
            if (dialect === "postgres") {
              expect(sql).to.match(/REFERENCES\s+"prefix"\."UserPubs" \("id"\)/)
            } else {
              expect(sql).to.match(/REFERENCES\s+`prefix\.UserPubs` \(`id`\)/)
            }
            done()
-          }))
+          })))
        })
      }
@@ -1853,7 +1855,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
      Post.belongsTo(this.Author)
      // The posts table gets dropped in the before filter.
-      Post.sync().on('sql', function(sql) {
+      Post.sync().on('sql', _.once(function(sql) {
        if (dialect === 'postgres') {
          expect(sql).to.match(/"authorId" INTEGER REFERENCES "authors" \("id"\)/)
        } else if (Support.dialectIsMySQL()) {
@@ -1865,7 +1867,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        }
        done()
-      })
+      }))
    })
    it('uses a table name as a string and references the author table', function(done) {
@@ -1883,7 +1885,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
      Post.belongsTo(this.Author)
      // The posts table gets dropped in the before filter.
-      Post.sync().on('sql', function(sql) {
+      Post.sync().on('sql', _.once(function(sql) {
        if (dialect === 'postgres') {
          expect(sql).to.match(/"authorId" INTEGER REFERENCES "authors" \("id"\)/)
        } else if (Support.dialectIsMySQL()) {
@@ -1895,7 +1897,7 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        }
        done()
-      })
+      }))
    })
    it("emits an error event as the referenced table name is invalid", function(done) {
@@ -2348,10 +2350,10 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        str: { type: Sequelize.STRING, unique: true }
      })
-      uniqueTrue.sync({force: true}).on('sql', _.after(2, function(s) {
+      uniqueTrue.sync({force: true}).on('sql', _.after(2, _.once(function(s) {
        expect(s).to.match(/UNIQUE/)
        done()
-      }))
+      })))
    })
    it("should not set unique when unique is false", function(done) {
@@ -2360,10 +2362,10 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        str: { type: Sequelize.STRING, unique: false }
      })
-      uniqueFalse.sync({force: true}).on('sql', _.after(2, function(s) {
+      uniqueFalse.sync({force: true}).on('sql', _.after(2, _.once(function(s) {
        expect(s).not.to.match(/UNIQUE/)
        done()
-      }))
+      })))
    })
    it("should not set unique when unique is unset", function(done) {
@@ -2372,10 +2374,10 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
        str: { type: Sequelize.STRING }
      })
-      uniqueUnset.sync({force: true}).on('sql', _.after(2, function(s) {
+      uniqueUnset.sync({force: true}).on('sql', _.after(2, _.once(function(s) {
        expect(s).not.to.match(/UNIQUE/)
        done()
-      }))
+      })))
    })
  })
@@ -2403,5 +2405,4 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
    return this.sequelize.sync({force: true});
  });
 })
--- a/test/dao-factory/create.test.js
+++ b/test/dao-factory/create.test.js
@@ -168,13 +168,17 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
            this.User.findOrCreate({ where: { uniqueName: 'winner' }}, { transaction: transaction }),
            this.User.findOrCreate({ where: { uniqueName: 'winner' }}, { transaction: transaction }),
            function (first, second) {
+               var firstInstance = first[0]
+              , firstCreated = first[1]
+              , secondInstance = second[0]
+              , secondCreated = second[1];
-              expect(first[0]).to.be.ok;
+              expect(firstInstance).to.be.ok;
-              expect(first[1]).to.be.ok;
+              expect(firstCreated).to.be.ok;
-              expect(second[0]).to.be.ok;
+              expect(secondInstance).to.be.ok;
-              expect(second[1]).not.to.be.ok;
+              expect(secondCreated).not.to.be.ok;
-              expect(first[0].id).to.equal(second[0].id);
+              expect(firstInstance.id).to.equal(secondInstance.id);
              return transaction.commit();
            }
@@ -188,12 +192,18 @@ describe(Support.getTestDialectTeaser("DAOFactory"), function () {
          this.User.findOrCreate({ where: { uniqueName: 'winner' }}),
          this.User.findOrCreate({ where: { uniqueName: 'winner' }}),
          function (first, second) {
-            expect(first[0]).to.be.ok;
+            var firstInstance = first[0]
-            expect(first[1]).to.be.ok;
+              , firstCreated = first[1]
-            expect(second[0]).to.be.ok;
+              , secondInstance = second[0]
-            expect(second[1]).not.to.be.ok;
+              , secondCreated = second[1];
-            expect(first[0].id).to.equal(second[0].id);
+            // Depending on execution order and MAGIC either the first OR the second call should return true
+            expect(firstCreated ? !secondCreated : secondCreated).to.be.ok // XOR
+            expect(firstInstance).to.be.ok;
+            expect(secondInstance).to.be.ok;
+            expect(firstInstance.id).to.equal(secondInstance.id);
          }
        );
      });

--- a/test/query-interface.test.js
+++ b/test/query-interface.test.js
--- a/test/sequelize.test.js
+++ b/test/sequelize.test.js
@@ -449,6 +449,46 @@ describe(Support.getTestDialectTeaser("Sequelize"), function () {
    }
  })
+  if (Support.dialectIsMySQL()) {
+    describe('set', function() {
+      it("should return an promised error if transaction isn't defined", function () {
+        expect(function () {
+          this.sequelize.set({ foo: 'bar' })
+        }.bind(this)).to.throw(TypeError, "options.transaction is required")
+      })
+      it("one value", function () {
+        return this.sequelize.transaction().bind(this).then(function (t) {
+          this.t = t;
+          return this.sequelize.set({ foo: 'bar' }, { transaction: t });
+        }).then(function () {
+          return this.sequelize.query( 'SELECT @foo as `foo`', null, { raw: true, plain: true, transaction: this.t });
+        }).then(function (data) {
+          expect(data).to.be.ok
+          expect(data.foo).to.be.equal('bar')
+          return this.t.commit();
+        });
+      });
+      it("multiple values", function () {
+        return this.sequelize.transaction().bind(this).then(function (t) {
+          this.t = t;
+          return this.sequelize.set({
+            foo: 'bar',
+            foos: 'bars',
+          }, { transaction: t });
+        }).then(function () {
+          return this.sequelize.query( 'SELECT @foo as `foo`, @foos as `foos`', null, { raw: true, plain: true, transaction: this.t });
+        }).then(function (data) {
+          expect(data).to.be.ok;
+          expect(data.foo).to.be.equal('bar');
+          expect(data.foos).to.be.equal('bars');
+          return this.t.commit();
+        });
+      });
+    });
+  }
  describe('define', function() {
    it("adds a new dao to the dao manager", function(done) {
      expect(this.sequelize.daoFactoryManager.all.length).to.equal(0)