Apply postprocessing in migrations

package.json

@@ -32,9 +32,9 @@
     ]
   },
   "devDependencies": {
-    "@babel/cli": "^7.1.5",
-    "@babel/core": "^7.1.6",
-    "@babel/preset-env": "^7.1.6",
+    "@babel/cli": "^7.2.0",
+    "@babel/core": "^7.2.0",
+    "@babel/preset-env": "^7.2.0",
     "@types/node": "*",
     "JSONStream": "^1.3.5",
     "async": "^2.6.1",
@@ -57,12 +57,12 @@
     "mysql": "^2.16.0",
     "mysql2": "^1.6.4",
     "nyc": "^13.1.0",
-    "pg": "^7.6.1",
+    "pg": "^7.7.1",
     "pg-query-stream": "^1.1.2",
-    "prettier": "^1.15.2",
+    "prettier": "^1.15.3",
     "rimraf": "^2.6.2",
     "sinon": "^7.1.1",
-    "sinon-chai": "^3.2.0",
+    "sinon-chai": "^3.3.0",
     "source-map-support": "^0.5.9",
     "sqlite3": "^4.0.4",
     "tap-spec": "^5.0.0",

src/dialects/oracledb/query/compiler.js

@@ -296,10 +296,7 @@ _.assign(Oracledb_Compiler.prototype, {
     let returningClause = '';
     let intoClause = '';
 
-    if (
-      _.isEmpty(updates) &&
-      typeof this.single.update !== 'function'
-    ) {
+    if (_.isEmpty(updates) && typeof this.single.update !== 'function') {
       return '';
     }
 

src/migrate/Migrator.js

@@ -48,11 +48,14 @@ const CONFIG_DEFAULT = Object.freeze({
 export default class Migrator {
   constructor(knex) {
     // Clone knex instance and remove post-processing that is unnecessary for internal queries from a cloned config
-    this.knex = isFunction(knex)
-      ? knex.withUserParams(knex.userParams)
-      : Object.assign({}, knex);
-    this.knex.client.config.wrapIdentifier = null;
-    this.knex.client.config.postProcessResponse = null;
+    if (isFunction(knex)) {
+      this.knex = knex.withUserParams({
+        ...knex.userParams,
+      });
+      this.knex.disableProcessing();
+    } else {
+      this.knex = Object.assign({}, knex);
+    }
 
     this.config = getMergedConfig(this.knex.client.config.migrations);
     this.generator = new MigrationGenerator(this.knex.client.config.migrations);
@@ -346,15 +349,25 @@ export default class Migrator {
             !trx &&
             this._useTransaction(migrationContent, disableTransactions)
           ) {
-            return this._transaction(migrationContent, direction, name);
+            this.knex.enableProcessing();
+            return this._transaction(
+              this.knex,
+              migrationContent,
+              direction,
+              name
+            );
           }
-          return warnPromise(
-            this.knex,
+
+          trxOrKnex.enableProcessing();
+          return checkPromise(
+            this.knex.client.logger,
             migrationContent[direction](trxOrKnex, Promise),
             name
           );
         })
         .then(() => {
+          trxOrKnex.disableProcessing();
+          this.knex.disableProcessing();
           log.push(name);
           if (direction === 'up') {
             return trxOrKnex.into(getTableName(tableName, schemaName)).insert({
@@ -375,10 +388,10 @@ export default class Migrator {
     return current.thenReturn([batchNo, log]);
   }
 
-  _transaction(migrationContent, direction, name) {
-    return this.knex.transaction((trx) => {
-      return warnPromise(
-        this.knex,
+  _transaction(knex, migrationContent, direction, name) {
+    return knex.transaction((trx) => {
+      return checkPromise(
+        knex.client.logger,
         migrationContent[direction](trx, Promise),
         name,
         () => {
@@ -447,10 +460,12 @@ function getNewMigrations(migrationSource, all, completed) {
   });
 }
 
-function warnPromise(knex, value, name, fn) {
-  if (!value || typeof value.then !== 'function') {
-    knex.client.logger.warn(`migration ${name} did not return a promise`);
-    if (fn && typeof fn === 'function') fn();
+function checkPromise(logger, migrationPromise, name, commitFn) {
+  if (!migrationPromise || typeof migrationPromise.then !== 'function') {
+    logger.warn(`migration ${name} did not return a promise`);
+    if (commitFn) {
+      commitFn();
+    }
   }
-  return value;
+  return migrationPromise;
 }

src/util/make-knex.js

@@ -54,6 +54,30 @@ function initContext(knexFn) {
       return this.client.ref(ref);
     },
 
+    // Do not document this as public API until naming and API is improved for general consumption
+    // This method exists to disable processing of internal queries in migrations
+    disableProcessing() {
+      if (this.userParams.isProcessingDisabled) {
+        return;
+      }
+      this.userParams.wrapIdentifier = this.client.config.wrapIdentifier;
+      this.userParams.postProcessResponse = this.client.config.postProcessResponse;
+      this.client.config.wrapIdentifier = null;
+      this.client.config.postProcessResponse = null;
+      this.userParams.isProcessingDisabled = true;
+    },
+
+    // Do not document this as public API until naming and API is improved for general consumption
+    // This method exists to enable execution of non-internal queries with consistent identifier naming in migrations
+    enableProcessing() {
+      if (!this.userParams.isProcessingDisabled) {
+        return;
+      }
+      this.client.config.wrapIdentifier = this.userParams.wrapIdentifier;
+      this.client.config.postProcessResponse = this.userParams.postProcessResponse;
+      this.userParams.isProcessingDisabled = false;
+    },
+
     withUserParams(params) {
       const knexClone = shallowCloneFunction(knexFn); // We need to include getters in our clone
       if (this.client) {
@@ -103,6 +127,8 @@ function redefineProperties(knex, client) {
         knex.ref = context.ref;
         knex.withUserParams = context.withUserParams;
         knex.queryBuilder = context.queryBuilder;
+        knex.disableProcessing = context.disableProcessing;
+        knex.enableProcessing = context.enableProcessing;
       },
       configurable: true,
     },

test/unit/migrate/Migrator.js

@@ -8,17 +8,22 @@ describe('Migrator', () => {
   describe('does not use postProcessResponse for internal queries', (done) => {
     let migrationSource;
     let knex;
-    before(() => {
+    beforeEach(() => {
       migrationSource = new FsMigrations('test/unit/migrate/migrations/');
       knex = Knex({
         ...sqliteConfig,
+        connection: ':memory:',
         migrationSource,
         postProcessResponse: () => {
           throw new Error('Response was processed');
         },
       });
     });
 
+    afterEach(() => {
+      knex.destroy();
+    });
+
     it('latest', (done) => {
       expect(() => {
         knex.migrate
@@ -31,4 +36,39 @@ describe('Migrator', () => {
       }).not.to.throw();
     });
   });
+
+  describe('uses postProcessResponse for migrations', (done) => {
+    let migrationSource;
+    let knex;
+    beforeEach(() => {
+      migrationSource = new FsMigrations(
+        'test/unit/migrate/processed-migrations/'
+      );
+    });
+
+    afterEach(() => {
+      knex.destroy();
+    });
+
+    it('latest', (done) => {
+      let wasPostProcessed = false;
+      knex = Knex({
+        ...sqliteConfig,
+        connection: ':memory:',
+        migrationSource,
+        postProcessResponse: () => {
+          wasPostProcessed = true;
+        },
+      });
+
+      knex.migrate
+        .latest({
+          directory: 'test/unit/migrate/processed-migrations',
+        })
+        .then(() => {
+          expect(wasPostProcessed).to.equal(true);
+          done();
+        });
+    });
+  });
 });

test/unit/migrate/processed-migrations/simple_migration.js

@@ -0,0 +1,9 @@
+exports.up = (knex) => {
+  return knex.schema.createTable('old_users', (table) => {
+    table.string('name');
+  });
+};
+
+exports.down = (knex) => {
+  return knex.schema.dropTable('old_users');
+};