From 2f75c046e3af1fa6f674edbe3f136a791dd86ed7 Mon Sep 17 00:00:00 2001
From: Nathan Woltman <nwoltman@outlook.com>
Date: Wed, 7 Dec 2016 16:41:30 -0500
Subject: [PATCH] PoolPlus: Use a transaction when syncing defined tables

---
 README.md                  |  9 +++++---
 lib/PoolPlus.js            | 47 ++++++++++++++++++--------------------
 package.json               |  2 +-
 test/unit/PoolPlus.test.js |  5 +++-
 4 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/README.md b/README.md
index cbd593f..3650fd8 100644
--- a/README.md
+++ b/README.md
@@ -329,8 +329,11 @@ const userTable = pool.defineTable('user', {
 
 ### poolPlus.sync(cb) ⇒ <code>void</code>
 Syncs the defined tables to the database by creating new tables and dropping
-or migrating existing tables (depending on the migration setting). Generally
-should only be called once when starting up a server.
+or migrating existing tables (depending on the migration setting). If an
+error occurs while syncing the tables, all changes are rolled back so the
+database will be in the same state as it was before the attempted sync.
+
+Generally, this should only be called once when starting up a server.
 
 
 | Param | Type | Description |
@@ -339,7 +342,7 @@ should only be called once when starting up a server.
 
 **Example**:
 ```js
-pool.sync(function(err) {
+pool.sync((err) => {
   if (err) throw err;
   // Now do something such as start an HTTP server
 });
diff --git a/lib/PoolPlus.js b/lib/PoolPlus.js
index 9f5d7cf..30886a0 100644
--- a/lib/PoolPlus.js
+++ b/lib/PoolPlus.js
@@ -4,7 +4,6 @@
 
 'use strict';
 
-const CallbackManager = require('es6-callback-manager');
 const ColumnDefinitions = require('./ColumnDefinitions');
 const MySQLTable = require('./MySQLTable');
 const Pool = require('mysql/lib/Pool');
@@ -91,15 +90,18 @@ class PoolPlus extends Pool {
 
  /**
   * Syncs the defined tables to the database by creating new tables and dropping
-  * or migrating existing tables (depending on the migration setting). Generally
-  * should only be called once when starting up a server.
+  * or migrating existing tables (depending on the migration setting). If an
+  * error occurs while syncing the tables, all changes are rolled back so the
+  * database will be in the same state as it was before the attempted sync.
+  *
+  * Generally, this should only be called once when starting up a server.
   *
   * @param {function} cb - A callback that is called once all defined table schemas have been synced to the
   *     database. If an error occured, the first argument passed to the callback will be the error object.
   * @returns {void}
   *
   * @example
-  * pool.sync(function(err) {
+  * pool.sync((err) => {
   *   if (err) throw err;
   *   // Now do something such as start an HTTP server
   * });
@@ -119,13 +121,17 @@ class PoolPlus extends Pool {
       } else if (operations.length) {
         allOperations.push.apply(allOperations, operations);
       }
-      if (--tablesRemaining === 0) {
-        if (error) {
-          cb(error);
-          return;
-        }
-        this._runOperations(allOperations, cb);
+
+      if (--tablesRemaining > 0) {
+        return;
+      }
+
+      if (error) {
+        cb(error);
+        return;
       }
+
+      this._runOperations(allOperations, cb);
     };
 
     for (const tableDefintion of this._tables.values()) {
@@ -321,22 +327,13 @@ class PoolPlus extends Pool {
       return;
     }
 
-    this.getConnection((err, connection) => {
-      if (err) {
-        cb(err);
-        return;
-      }
-
-      const cbManager = new CallbackManager(err => {
-        connection.release();
-        cb(err);
-      });
-      for (var i = 0; i < operations.length; i++) {
-        connection.query(operations[i].sql, cbManager.registerCallback());
-      }
-    });
+    this.transaction(trxn => {
+      return Promise.all(
+        operations.map(operation => trxn.pquery(operation.sql))
+      );
+    }).then(() => cb(), cb);
 
-    // Sort the operations while the connection is being fetched
+    // Sort the operations while the transaction connection is being fetched
     for (var i = 0; i < operations.length; i++) {
       operations[i].position = i; // Setup for stable sorting
     }
diff --git a/package.json b/package.json
index f771cd3..645cff6 100644
--- a/package.json
+++ b/package.json
@@ -18,12 +18,12 @@
   "homepage": "https://github.com/nwoltman/node-mysql-plus#readme",
   "dependencies": {
     "array-equal": "^1.0.0",
-    "es6-callback-manager": "^2.0.0",
     "lodash": "^4.15.0",
     "mysql": "^2.11.1"
   },
   "devDependencies": {
     "coveralls": "^2.11.6",
+    "es6-callback-manager": "^2.0.0",
     "grunt": "^1.0.1",
     "grunt-env": "^0.4.4",
     "grunt-eslint": "^19.0.0",
diff --git a/test/unit/PoolPlus.test.js b/test/unit/PoolPlus.test.js
index af7246d..c06e76c 100644
--- a/test/unit/PoolPlus.test.js
+++ b/test/unit/PoolPlus.test.js
@@ -115,7 +115,10 @@ describe('PoolPlus', () => {
 
     it('should sync the defined tables to the database', done => {
       pool.sync(err => {
-        if (err) throw err;
+        if (err) {
+          done(err);
+          return;
+        }
         pool.query(`SHOW CREATE TABLE ${TEST_TABLE_NAME}`, (err, result) => {
           if (err) throw err;
           result[0]['Create Table'].should.equal(