Migrate to Node-MySQL2 (#6354)

* node-mysql2 install

* fixed geom, buffer and connection error handling

* disconnect connections gracefully

* switch work

* typeCast is global now, removed comment

* use mysql promise wrapper

* use sequelize's Promise instance

* used domains to catch errors properly

* parse named timezones for mysql

* return null when BLOB is empty

* use connection event emitter errors, why I didnt think about it earlier :(

* using connect fix introduced in mysql2@rc9

* handshake phase event cleanup

* need refer defined before using them

* use latedef flag with const

* [ci skip] Changelog entry

changelog.md

 # Future
+- [INTERNAL] Migrated to `node-mysql2` for prepared statements [#6354](https://github.com/sequelize/sequelize/issues/6354)
 - [ADDED] SQLCipher support via the SQLite connection manager
 - [CHANGED] Range type bounds now default to [postgres default](https://www.postgresql.org/docs/9.5/static/rangetypes.html#RANGETYPES-CONSTRUCT) `[)` (inclusive, exclusive) [#5990](https://github.com/sequelize/sequelize/issues/5990)
 - [ADDED] Support for range operators [#5990](https://github.com/sequelize/sequelize/issues/5990)

docs/articles/getting-started.md

@@ -33,7 +33,7 @@ Once done, you can install Sequelize and the connector for your database of choi
 ```bash
 $ npm install --save sequelize
 $ npm install --save pg       # for postgres
-$ npm install --save mysql    # for mysql
+$ npm install --save mysql2   # for mysql
 $ npm install --save sqlite3  # for sqlite
 ```
 

docs/docs/getting-started.md

@@ -7,7 +7,7 @@ $ npm install --save sequelize
 
 # And one of the following:
 $ npm install --save pg pg-hstore
-$ npm install --save mysql
+$ npm install --save mysql2
 $ npm install --save sqlite3
 $ npm install --save tedious // MSSQL
 ```

docs/docs/usage.md

@@ -52,7 +52,6 @@ var sequelize = new Sequelize('database', 'username', 'password', {
   logging: false,
  
   // the sql dialect of the database
-  // - default is 'mysql'
   // - currently supported: 'mysql', 'sqlite', 'postgres', 'mssql'
   dialect: 'mysql',
  
@@ -166,13 +165,10 @@ With the release of Sequelize`1.6.0`, the library got independent from specific
 
 ### MySQL
 
-In order to get Sequelize working nicely together with MySQL, you'll need to install`mysql@~2.5.0`or higher. Once that's done you can use it like this:
+In order to get Sequelize working nicely together with MySQL, you'll need to install`mysql2@^1.0.0-rc.10`or higher. Once that's done you can use it like this:
 
 ```js
 var sequelize = new Sequelize('database', 'username', 'password', {
-  // mysql is the default dialect, but you know...
-  // for demo purposes we are defining it nevertheless :)
-  // so: we want mysql!
   dialect: 'mysql'
 })
 ```

lib/dialects/mysql/connection-manager.js

 'use strict';
 
 const AbstractConnectionManager = require('../abstract/connection-manager');
+const SequelizeErrors = require('../../errors');
 const Utils = require('../../utils');
+const DataTypes = require('../../data-types').mysql;
+const momentTz = require('moment-timezone');
 const debug = Utils.getLogger().debugContext('connection:mysql');
-const Promise = require('../../promise');
-const sequelizeErrors = require('../../errors');
-const dataTypes = require('../../data-types').mysql;
 const parserMap = new Map();
 
+/**
+ * MySQL Connection Managger
+ *
+ * Get connections, validate and disconnect them.
+ * AbstractConnectionManager pooling use it to handle MySQL specific connections
+ * Use https://github.com/sidorares/node-mysql2 to connect with MySQL server
+ *
+ * @extends AbstractConnectionManager
+ * @return Class<ConnectionManager>
+ */
+
 class ConnectionManager extends AbstractConnectionManager {
   constructor(dialect, sequelize) {
     super(dialect, sequelize);
@@ -18,19 +29,19 @@ class ConnectionManager extends AbstractConnectionManager {
       if (sequelize.config.dialectModulePath) {
         this.lib = require(sequelize.config.dialectModulePath);
       } else {
-        this.lib = require('mysql');
+        this.lib = require('mysql2');
       }
     } catch (err) {
       if (err.code === 'MODULE_NOT_FOUND') {
-        throw new Error('Please install mysql package manually');
+        throw new Error('Please install mysql2 package manually');
       }
       throw err;
     }
 
-    this.refreshTypeParser(dataTypes);
+    this.refreshTypeParser(DataTypes);
   }
 
-  // Expose this as a method so that the parsing may be updated when the user has added additional, custom types
+  // Update parsing when the user has added additional, custom types
   _refreshTypeParser(dataType) {
     for (const type of dataType.types.mysql) {
       parserMap.set(type, dataType.parse);
@@ -43,14 +54,19 @@ class ConnectionManager extends AbstractConnectionManager {
 
   static _typecast(field, next) {
     if (parserMap.has(field.type)) {
-      return parserMap.get(field.type)(field, this.sequelize.options);
+      return parserMap.get(field.type)(field, this.sequelize.options, next);
     }
-
     return next();
   }
 
+  /**
+   * Connect with MySQL database based on config, Handle any errors in connection
+   * Set the pool handlers on connection.error
+   * Also set proper timezone once conection is connected
+   *
+   * @return Promise<Connection>
+   */
   connect(config) {
-    return new Promise((resolve, reject) => {
       const connectionConfig = {
         host: config.host,
         port: config.port,
@@ -69,46 +85,36 @@ class ConnectionManager extends AbstractConnectionManager {
         }
       }
 
+      return new Utils.Promise((resolve, reject) => {
         const connection = this.lib.createConnection(connectionConfig);
 
-      connection.connect(err => {
-        if (err) {
-          if (err.code) {
-            switch (err.code) {
-            case 'ECONNREFUSED':
-              reject(new sequelizeErrors.ConnectionRefusedError(err));
-              break;
-            case 'ER_ACCESS_DENIED_ERROR':
-              reject(new sequelizeErrors.AccessDeniedError(err));
-              break;
-            case 'ENOTFOUND':
-              reject(new sequelizeErrors.HostNotFoundError(err));
-              break;
-            case 'EHOSTUNREACH':
-              reject(new sequelizeErrors.HostNotReachableError(err));
-              break;
-            case 'EINVAL':
-              reject(new sequelizeErrors.InvalidConnectionError(err));
-              break;
-            default:
-              reject(new sequelizeErrors.ConnectionError(err));
-              break;
-            }
-          } else {
-            reject(new sequelizeErrors.ConnectionError(err));
-          }
+        /*jshint latedef:false*/
+        const errorHandler = (e) => {
+          // clean up connect event if there is error
+          connection.removeListener('connect', connectHandler);
+          reject(e);
+        };
 
-          return;
-        }
+        const connectHandler = () => {
+          // clean up error event if connected
+          connection.removeListener('error', errorHandler);
+          resolve(connection);
+        };
+        /*jshint latedef:true*/
+
+        connection.once('error', errorHandler);
+        connection.once('connect', connectHandler);
+      })
+      .then((connection) => {
 
         if (config.pool.handleDisconnects) {
           // Connection to the MySQL server is usually
           // lost due to either server restart, or a
-          // connnection idle timeout (the wait_timeout
+          // connection idle timeout (the wait_timeout
           // server variable configures this)
           //
           // See [stackoverflow answer](http://stackoverflow.com/questions/20210522/nodejs-mysql-error-connection-lost-the-server-closed-the-connection)
-          connection.on('error', err => {
+          connection.on('error', (err) => {
             if (err.code === 'PROTOCOL_CONNECTION_LOST') {
               // Remove it from read/write pool
               this.pool.destroy(connection);
@@ -116,35 +122,66 @@ class ConnectionManager extends AbstractConnectionManager {
             debug(`connection error ${err.code}`);
           });
         }
+
         debug(`connection acquired`);
-        resolve(connection);
+        return connection;
+      })
+      .then((connection) => {
+        return new Utils.Promise((resolve, reject) => {
+          // set timezone for this connection
+          // but named timezone are not directly supported in mysql, so get its offset first
+          let tzOffset = this.sequelize.options.timezone;
+          tzOffset = /\//.test(tzOffset) ? momentTz.tz(tzOffset).format('Z') : tzOffset;
+
+          connection.query(`SET time_zone = '${tzOffset}'`, (err) => {
+            if (err) { reject(err); } else { resolve(connection); }
           });
-
-    }).tap(connection => {
-      connection.query("SET time_zone = '" + this.sequelize.options.timezone + "'"); /* jshint ignore: line */
+        });
+      })
+      .catch((err) => {
+        if (err.code) {
+          switch (err.code) {
+            case 'ECONNREFUSED':
+              throw new SequelizeErrors.ConnectionRefusedError(err);
+            case 'ER_ACCESS_DENIED_ERROR':
+              throw new SequelizeErrors.AccessDeniedError(err);
+            case 'ENOTFOUND':
+              throw new SequelizeErrors.HostNotFoundError(err);
+            case 'EHOSTUNREACH':
+              throw new SequelizeErrors.HostNotReachableError(err);
+            case 'EINVAL':
+              throw new SequelizeErrors.InvalidConnectionError(err);
+            default:
+              throw new SequelizeErrors.ConnectionError(err);
+          }
+        } else {
+          throw new SequelizeErrors.ConnectionError(err);
+        }
       });
   }
 
   disconnect(connection) {
 
-    // Dont disconnect connections with an ended protocol
-    // That wil trigger a connection error
-    if (connection._protocol._ended) {
-      debug(`connection tried to disconnect but was already at ENDED state`);
-      return Promise.resolve();
+    // Dont disconnect connections with CLOSED state
+    if (connection._closing) {
+      debug(`connection tried to disconnect but was already at CLOSED state`);
+      return Utils.Promise.resolve();
     }
 
-    return new Promise((resolve, reject) => {
-      connection.end(err => {
-        if (err) return reject(new sequelizeErrors.ConnectionError(err));
+    return new Utils.Promise((resolve, reject) => {
+      connection.end((err) => {
+        if (err) {
+          reject(new SequelizeErrors.ConnectionError(err));
+        } else {
           debug(`connection disconnected`);
           resolve();
+        }
       });
     });
   }
 
   validate(connection) {
-    return connection && ['disconnected', 'protocol_error'].indexOf(connection.state) === -1;
+    return connection && connection._fatalError === null && connection._protocolError === null && !connection._closing;
   }
 }
 

lib/dialects/mysql/data-types.js

@@ -25,6 +25,22 @@ module.exports = BaseTypes => {
   BaseTypes.REAL.types.mysql = ['DOUBLE'];
   BaseTypes.DOUBLE.types.mysql = ['DOUBLE'];
 
+  function BLOB(length) {
+    if (!(this instanceof BLOB)) return new BLOB(length);
+    BaseTypes.BLOB.apply(this, arguments);
+  }
+  inherits(BLOB, BaseTypes.BLOB);
+
+  BLOB.parse = function (value, options, next) {
+    let data = next();
+
+    if (Buffer.isBuffer(data) && data.length === 0) {
+      return null;
+    }
+
+    return data;
+  };
+
   function DECIMAL(precision, scale) {
     if (!(this instanceof DECIMAL)) return new DECIMAL(precision, scale);
     BaseTypes.DECIMAL.apply(this, arguments);
@@ -91,6 +107,7 @@ module.exports = BaseTypes => {
     return 'CHAR(36) BINARY';
   };
 
+
   const SUPPORTED_GEOMETRY_TYPES = ['POINT', 'LINESTRING', 'POLYGON'];
 
   function GEOMETRY(type, srid) {
@@ -110,8 +127,9 @@ module.exports = BaseTypes => {
   GEOMETRY.parse = GEOMETRY.prototype.parse = function parse(value) {
     value = value.buffer();
 
-    //MySQL doesn't support POINT EMPTY, https://dev.mysql.com/worklog/task/?id=2381
-    if (value === null) {
+    // Empty buffer, MySQL doesn't support POINT EMPTY
+    // check, https://dev.mysql.com/worklog/task/?id=2381
+    if (value.length === 0) {
       return null;
     }
 
@@ -146,7 +164,8 @@ module.exports = BaseTypes => {
     DATE,
     UUID,
     GEOMETRY,
-    DECIMAL
+    DECIMAL,
+    BLOB
   };
 
   _.forIn(exports, (DataType, key) => {

lib/dialects/mysql/query.js

@@ -42,8 +42,7 @@ class Query extends AbstractQuery {
     debug(`executing(${this.connection.uuid || 'default'}) : ${this.sql}`);
 
     return new Utils.Promise((resolve, reject) => {
-      this.connection.query(this.sql, (err, results) => {
-
+      this.connection.query({ sql: this.sql }, (err, results) => {
         debug(`executed(${this.connection.uuid || 'default'}) : ${this.sql}`);
 
         if (benchmark) {
@@ -61,7 +60,7 @@ class Query extends AbstractQuery {
     })
     // Log warnings if we've got them.
     .then(results => {
-      if (showWarnings && results && results.warningCount > 0) {
+      if (showWarnings && results && results.warningStatus > 0) {
         return this.logWarnings(results);
       }
       return results;
@@ -136,7 +135,6 @@ class Query extends AbstractQuery {
     return this.run('SHOW WARNINGS').then(warningResults => {
       const warningMessage = 'MySQL Warnings (' + (this.connection.uuid||'default') + '): ';
       const messages = [];
-
       for (const _warningRow of warningResults) {
         for (const _warningResult of _warningRow) {
           if (_warningResult.hasOwnProperty('Message')) {

package.json

@@ -72,7 +72,7 @@
     "jshint": "^2.9.2",
     "lcov-result-merger": "^1.2.0",
     "mocha": "^3.0.2",
-    "mysql": "~2.11.1",
+    "mysql2": "^1.0.0-rc.10",
     "pg": "^6.0.0",
     "pg-hstore": "^2.3.1",
     "pg-native": "^1.8.0",

test/integration/data-types.test.js

@@ -396,7 +396,7 @@ describe(Support.getTestDialectTeaser('DataTypes'), function() {
       return Model.sync({ force: true }).then(function () {
         return Model.create(sampleData);
       }).then(function () {
-        return Model.find({id: 1});
+        return Model.findById(1);
       }).then(function (user) {
         expect(user.get('jewelPurity')).to.be.eql(sampleData.jewelPurity);
         expect(user.get('jewelPurity')).to.be.string;

test/integration/dialects/mysql/connector-manager.test.js

@@ -100,7 +100,7 @@ if (dialect === 'mysql') {
             conn = connection;
 
             // simulate a unexpected end
-            connection._protocol.end();
+            connection.close();
           })
           .then(function() {
             return cm.releaseConnection(conn);

test/integration/include/seperate.test.js

@@ -415,8 +415,7 @@ if (current.dialect.supports.groupedLimit) {
                 include: [{ model: Task, limit: 2, as: 'tasks', order:[['id', 'ASC']] }],
                 order: [
                   ['id', 'ASC']
-                ],
-                logging: console.log
+                ]
               }).then((result) => {
                 expect(result[0].tasks.length).to.equal(2);
                 expect(result[0].tasks[0].title).to.equal('b');

test/integration/model.test.js

@@ -2383,7 +2383,7 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         return;
       }).catch (function(err) {
         if (dialect === 'mysql') {
-          expect(err.message).to.match(/ER_CANNOT_ADD_FOREIGN|ER_CANT_CREATE_TABLE/);
+          expect(err.message).to.match(/Can\'t create table/);
         } else if (dialect === 'sqlite') {
           // the parser should not end up here ... see above
           expect(1).to.equal(2);