diff --git a/src/migrate/Migrator.js b/src/migrate/Migrator.js index 1e35ac50da..2c9e4ba14b 100644 --- a/src/migrate/Migrator.js +++ b/src/migrate/Migrator.js @@ -7,6 +7,7 @@ import { each, filter, get, + isFunction, isBoolean, isEmpty, isUndefined, @@ -46,10 +47,15 @@ const CONFIG_DEFAULT = Object.freeze({ // the migration. export default class Migrator { constructor(knex) { - this.knex = knex; - this.config = getMergedConfig(knex.client.config.migrations); - this.generator = new MigrationGenerator(knex.client.config.migrations); - + // 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; + + this.config = getMergedConfig(this.knex.client.config.migrations); + this.generator = new MigrationGenerator(this.knex.client.config.migrations); this._activeMigration = { fileName: null, }; diff --git a/src/migrate/table-creator.js b/src/migrate/table-creator.js index 98432191fb..c36ca87d61 100644 --- a/src/migrate/table-creator.js +++ b/src/migrate/table-creator.js @@ -10,21 +10,26 @@ export function ensureTable(tableName, schemaName, trxOrKnex) { const lockTableWithSchema = getLockTableNameWithSchema(tableName, schemaName); return getSchemaBuilder(trxOrKnex, schemaName) .hasTable(tableName) - .then( - (exists) => - !exists && _createMigrationTable(tableName, schemaName, trxOrKnex) - ) - .then(() => getSchemaBuilder(trxOrKnex, schemaName).hasTable(lockTable)) - .then( - (exists) => + .then((exists) => { + return !exists && _createMigrationTable(tableName, schemaName, trxOrKnex); + }) + .then(() => { + return getSchemaBuilder(trxOrKnex, schemaName).hasTable(lockTable); + }) + .then((exists) => { + return ( !exists && _createMigrationLockTable(lockTable, schemaName, trxOrKnex) - ) - .then(() => getTable(trxOrKnex, lockTable, schemaName).select('*')) - .then( - (data) => + ); + }) + .then(() => { + return getTable(trxOrKnex, lockTable, schemaName).select('*'); + }) + .then((data) => { + return ( !data.length && trxOrKnex.into(lockTableWithSchema).insert({ is_locked: 0 }) - ); + ); + }); } function _createMigrationTable(tableName, schemaName, trxOrKnex) { diff --git a/src/util/make-knex.js b/src/util/make-knex.js index 9dce63c433..86ba9bcd01 100644 --- a/src/util/make-knex.js +++ b/src/util/make-knex.js @@ -6,28 +6,26 @@ import FunctionHelper from '../functionhelper'; import QueryInterface from '../query/methods'; import { assign } from 'lodash'; import batchInsert from './batchInsert'; +import * as bluebird from 'bluebird'; export default function makeKnex(client) { // The object we're potentially using to kick off an initial chain. function knex(tableName, options) { - const qb = knex.queryBuilder(); - if (!tableName) - client.logger.warn( - 'calling knex without a tableName is deprecated. Use knex.queryBuilder() instead.' - ); - return tableName ? qb.table(tableName, options) : qb; + return createQueryBuilder(knex.context, tableName, options); } + redefineProperties(knex, client); + return knex; +} - assign(knex, { - Promise: require('bluebird'), - - // A new query builder instance. +function initContext(knexFn) { + const knexContext = knexFn.context || {}; + assign(knexContext, { queryBuilder() { - return client.queryBuilder(); + return this.client.queryBuilder(); }, raw() { - return client.raw.apply(client, arguments); + return this.client.raw.apply(this.client, arguments); }, batchInsert(table, batch, chunkSize = 1000) { @@ -37,41 +35,48 @@ export default function makeKnex(client) { // Runs a new transaction, taking a container and returning a promise // for when the transaction is resolved. transaction(container, config) { - const trx = client.transaction(container, config); + const trx = this.client.transaction(container, config); trx.userParams = this.userParams; return trx; }, // Typically never needed, initializes the pool for a knex client. initialize(config) { - return client.initializePool(config); + return this.client.initializePool(config); }, // Convenience method for tearing down the pool. destroy(callback) { - return client.destroy(callback); + return this.client.destroy(callback); }, ref(ref) { - return client.ref(ref); + return this.client.ref(ref); }, withUserParams(params) { - const knexClone = shallowCloneFunction(knex); // We need to include getters in our clone - if (knex.client) { - knexClone.client = Object.assign({}, knex.client); // Clone client to avoid leaking listeners that are set on it - const parentPrototype = Object.getPrototypeOf(knex.client); + const knexClone = shallowCloneFunction(knexFn); // We need to include getters in our clone + if (this.client) { + knexClone.client = Object.assign({}, this.client); // Clone client to avoid leaking listeners that are set on it + knexClone.client.config = Object.assign({}, this.client.config); // Clone client config to make sure they can be modified independently + const parentPrototype = Object.getPrototypeOf(this.client); if (parentPrototype) { Object.setPrototypeOf(knexClone.client, parentPrototype); } } - redefineProperties(knexClone); + redefineProperties(knexClone, knexClone.client); knexClone.userParams = params; return knexClone; }, }); + if (!knexFn.context) { + knexFn.context = knexContext; + } +} + +function redefineProperties(knex, client) { // Allow chaining methods from the root object, before // any other information is specified. QueryInterface.forEach(function(method) { @@ -81,17 +86,47 @@ export default function makeKnex(client) { }; }); - knex.client = client; - redefineProperties(knex); + Object.defineProperties(knex, { + context: { + get() { + return knex._context; + }, + set(context) { + knex._context = context; + + // Redefine public API for knex instance that would be proxying methods from correct context + knex.raw = context.raw; + knex.batchInsert = context.batchInsert; + knex.transaction = context.transaction; + knex.initialize = context.initialize; + knex.destroy = context.destroy; + knex.ref = context.ref; + knex.withUserParams = context.withUserParams; + knex.queryBuilder = context.queryBuilder; + }, + configurable: true, + }, - client.makeKnex = makeKnex; + client: { + get() { + return knex.context.client; + }, + set(client) { + knex.context.client = client; + }, + configurable: true, + }, - knex.userParams = {}; - return knex; -} + userParams: { + get() { + return knex.context.userParams; + }, + set(userParams) { + knex.context.userParams = userParams; + }, + configurable: true, + }, -function redefineProperties(knex) { - Object.defineProperties(knex, { schema: { get() { return knex.client.schemaBuilder(); @@ -121,6 +156,12 @@ function redefineProperties(knex) { }, }); + initContext(knex); + knex.Promise = bluebird; + knex.client = client; + knex.client.makeKnex = makeKnex; + knex.userParams = {}; + // Hook up the "knex" object as an EventEmitter. const ee = new EventEmitter(); for (const key in ee) { @@ -142,13 +183,28 @@ function redefineProperties(knex) { }); } +function createQueryBuilder(knexContext, tableName, options) { + const qb = knexContext.queryBuilder(); + if (!tableName) + knexContext.client.logger.warn( + 'calling knex without a tableName is deprecated. Use knex.queryBuilder() instead.' + ); + return tableName ? qb.table(tableName, options) : qb; +} + function shallowCloneFunction(originalFunction) { - const clonedFunction = originalFunction.bind( - Object.create( - Object.getPrototypeOf(originalFunction), - Object.getOwnPropertyDescriptors(originalFunction) - ) + const fnContext = Object.create( + Object.getPrototypeOf(originalFunction), + Object.getOwnPropertyDescriptors(originalFunction) ); + + const knexContext = {}; + const knexFnWrapper = (tableName, options) => { + return createQueryBuilder(knexContext, tableName, options); + }; + + const clonedFunction = knexFnWrapper.bind(fnContext); Object.assign(clonedFunction, originalFunction); + clonedFunction._context = knexContext; return clonedFunction; } diff --git a/test/integration/builder/additional.js b/test/integration/builder/additional.js index 838eb514aa..b9a3d900e5 100644 --- a/test/integration/builder/additional.js +++ b/test/integration/builder/additional.js @@ -2,9 +2,9 @@ /*eslint no-var:0, max-len:0 */ 'use strict'; -var Knex = require('../../../knex'); -var _ = require('lodash'); -var Promise = require('bluebird'); +const Knex = require('../../../knex'); +const _ = require('lodash'); +const Promise = require('bluebird'); module.exports = function(knex) { describe('Additional', function() { @@ -198,7 +198,7 @@ module.exports = function(knex) { .select() .limit(1) .then(function(accounts) { - var firstAccount = accounts[0]; + const firstAccount = accounts[0]; return knex('accounts') .select() .limit(1) @@ -210,9 +210,9 @@ module.exports = function(knex) { }); it('should forward the .mapSeries() function from bluebird', function() { - var asyncTask = function() { + const asyncTask = function() { return new Promise(function(resolve, reject) { - var output = asyncTask.num++; + const output = asyncTask.num++; setTimeout(function() { resolve(output); }, Math.random() * 200); @@ -220,7 +220,7 @@ module.exports = function(knex) { }; asyncTask.num = 1; - var returnedValues = []; + const returnedValues = []; return knex('accounts') .select() .limit(3) @@ -237,7 +237,7 @@ module.exports = function(knex) { }); it('should forward the .delay() function from bluebird', function() { - var startTime = new Date().valueOf(); + const startTime = new Date().valueOf(); return knex('accounts') .select() .limit(1) @@ -292,7 +292,7 @@ module.exports = function(knex) { }); it('should allow raw queries directly with `knex.raw`', function() { - var tables = { + const tables = { mysql: 'SHOW TABLES', mysql2: 'SHOW TABLES', pg: @@ -533,7 +533,7 @@ module.exports = function(knex) { }); it('should allow renaming a column', function() { - var countColumn; + let countColumn; switch (knex.client.driverName) { case 'oracledb': countColumn = 'COUNT(*)'; @@ -545,8 +545,8 @@ module.exports = function(knex) { countColumn = 'count(*)'; break; } - var count, - inserts = []; + let count; + const inserts = []; _.times(40, function(i) { inserts.push({ email: 'email' + i, @@ -603,7 +603,7 @@ module.exports = function(knex) { }); it('should allow dropping a column', function() { - var countColumn; + let countColumn; switch (knex.client.driverName) { case 'oracledb': countColumn = 'COUNT(*)'; @@ -615,7 +615,7 @@ module.exports = function(knex) { countColumn = 'count(*)'; break; } - var count; + let count; return knex .count('*') .from('accounts') @@ -671,14 +671,14 @@ module.exports = function(knex) { }); it('.timeout() should throw TimeoutError', function() { - var driverName = knex.client.driverName; + const driverName = knex.client.driverName; if (driverName === 'sqlite3') { return; } //TODO -- No built-in support for sleeps if (/redshift/.test(driverName)) { return; } - var testQueries = { + const testQueries = { pg: function() { return knex.raw('SELECT pg_sleep(1)'); }, @@ -700,7 +700,7 @@ module.exports = function(knex) { throw new Error('Missing test query for driver: ' + driverName); } - var query = testQueries[driverName](); + const query = testQueries[driverName](); return query .timeout(200) @@ -718,7 +718,7 @@ module.exports = function(knex) { }); it('.timeout(ms, {cancel: true}) should throw TimeoutError and cancel slow query', function() { - var driverName = knex.client.driverName; + const driverName = knex.client.driverName; if (driverName === 'sqlite3') { return; } //TODO -- No built-in support for sleeps @@ -731,7 +731,7 @@ module.exports = function(knex) { // A subsequent query will acquire the connection (still in-use) and hang // until the first query finishes. Setting a sleep time longer than the // mocha timeout exposes this behavior. - var testQueries = { + const testQueries = { pg: function() { return knex.raw('SELECT pg_sleep(10)'); }, @@ -753,7 +753,7 @@ module.exports = function(knex) { throw new Error('Missing test query for driverName: ' + driverName); } - var query = testQueries[driverName](); + const query = testQueries[driverName](); function addTimeout() { return query.timeout(200, { cancel: true }); @@ -770,7 +770,7 @@ module.exports = function(knex) { return; } - var getProcessesQueries = { + const getProcessesQueries = { pg: function() { return knex.raw('SELECT * from pg_stat_activity'); }, @@ -786,7 +786,7 @@ module.exports = function(knex) { throw new Error('Missing test query for driverName: ' + driverName); } - var getProcessesQuery = getProcessesQueries[driverName](); + const getProcessesQuery = getProcessesQueries[driverName](); return addTimeout() .then(function() { @@ -811,8 +811,8 @@ module.exports = function(knex) { return getProcessesQuery; }) .then(function(results) { - var processes; - var sleepProcess; + let processes; + let sleepProcess; if (_.startsWith(driverName, 'pg')) { processes = results.rows; @@ -830,7 +830,7 @@ module.exports = function(knex) { it('.timeout(ms, {cancel: true}) should throw error if cancellation cannot acquire connection', function() { // Only mysql/postgres query cancelling supported for now - var driverName = knex.client.driverName; + const driverName = knex.client.driverName; if ( !_.startsWith(driverName, 'mysql') && !_.startsWith(driverName, 'pg') @@ -840,14 +840,14 @@ module.exports = function(knex) { // To make this test easier, I'm changing the pool settings to max 1. // Also setting acquireTimeoutMillis to lower as not to wait the default time - var knexConfig = _.cloneDeep(knex.client.config); + const knexConfig = _.cloneDeep(knex.client.config); knexConfig.pool.min = 0; knexConfig.pool.max = 1; knexConfig.pool.acquireTimeoutMillis = 100; - var knexDb = new Knex(knexConfig); + const knexDb = new Knex(knexConfig); - var testQueries = { + const testQueries = { pg: function() { return knexDb.raw('SELECT pg_sleep(10)'); }, @@ -869,7 +869,7 @@ module.exports = function(knex) { throw new Error('Missing test query for dialect: ' + driverName); } - var query = testQueries[driverName](); + const query = testQueries[driverName](); return query .timeout(1, { cancel: true }) @@ -889,21 +889,21 @@ module.exports = function(knex) { it('.timeout(ms, {cancel: true}) should release connections after failing if connection cancellation throws an error', function() { // Only mysql/postgres query cancelling supported for now - var driverName = knex.client.driverName; + const driverName = knex.client.driverName; if (!_.startsWith(driverName, 'pg')) { return; } // To make this test easier, I'm changing the pool settings to max 1. // Also setting acquireTimeoutMillis to lower as not to wait the default time - var knexConfig = _.cloneDeep(knex.client.config); + const knexConfig = _.cloneDeep(knex.client.config); knexConfig.pool.min = 0; knexConfig.pool.max = 2; knexConfig.pool.acquireTimeoutMillis = 100; - var knexDb = new Knex(knexConfig); + const knexDb = new Knex(knexConfig); - var rawTestQueries = { + const rawTestQueries = { pg: (sleepSeconds) => `SELECT pg_sleep(${sleepSeconds})`, }; @@ -958,9 +958,9 @@ module.exports = function(knex) { }); it('Event: query-response', function() { - var queryCount = 0; + let queryCount = 0; - var onQueryResponse = function(response, obj, builder) { + const onQueryResponse = function(response, obj, builder) { queryCount++; expect(response).to.be.an('array'); expect(obj).to.be.an('object'); @@ -987,9 +987,9 @@ module.exports = function(knex) { }); it('Event: does not duplicate listeners on a copy with user params', function() { - var queryCount = 0; + let queryCount = 0; - var onQueryResponse = function(response, obj, builder) { + const onQueryResponse = function(response, obj, builder) { queryCount++; expect(response).to.be.an('array'); expect(obj).to.be.an('object'); @@ -1020,9 +1020,9 @@ module.exports = function(knex) { }); it('Event: query-error', function() { - var queryCountKnex = 0; - var queryCountBuilder = 0; - var onQueryErrorKnex = function(error, obj) { + let queryCountKnex = 0; + let queryCountBuilder = 0; + const onQueryErrorKnex = function(error, obj) { queryCountKnex++; expect(obj).to.be.an('object'); expect(obj.__knexUid).to.be.a('string'); @@ -1030,7 +1030,7 @@ module.exports = function(knex) { expect(error).to.be.an('error'); }; - var onQueryErrorBuilder = function(error, obj) { + const onQueryErrorBuilder = function(error, obj) { queryCountBuilder++; expect(obj).to.be.an('object'); expect(obj.__knexUid).to.be.a('string'); @@ -1058,7 +1058,7 @@ module.exports = function(knex) { return knex('accounts') .insert({ last_name: 'Start event test' }) .then(function() { - var queryBuilder = knex('accounts').select(); + const queryBuilder = knex('accounts').select(); queryBuilder.on('start', function(builder) { //Alter builder prior to compilation @@ -1075,13 +1075,13 @@ module.exports = function(knex) { }); it("Event 'query' should not emit native sql string", function() { - var builder = knex('accounts') + const builder = knex('accounts') .where('id', 1) .select(); builder.on('query', function(obj) { - var native = builder.toSQL().toNative().sql; - var sql = builder.toSQL().sql; + const native = builder.toSQL().toNative().sql; + const sql = builder.toSQL().sql; //Only assert if they diff to begin with. //IE Maria does not diff @@ -1101,16 +1101,6 @@ module.exports = function(knex) { after(() => { delete knex.client.config.asyncStackTraces; }); - it('should capture stack trace on query builder instantiation', () => { - return knex('some_nonexisten_table') - .select() - .catch((err) => { - expect(err.stack.split('\n')[1]).to.match( - /at Function\.queryBuilder \(/ - ); // the index 1 might need adjustment if the code is refactored - expect(typeof err.originalStack).to.equal('string'); - }); - }); it('should capture stack trace on raw query', () => { return knex.raw('select * from some_nonexisten_table').catch((err) => { expect(err.stack.split('\n')[2]).to.match(/at Function\.raw \(/); // the index 2 might need adjustment if the code is refactored @@ -1128,10 +1118,10 @@ module.exports = function(knex) { }); it('Overwrite knex.logger functions using config', () => { - var knexConfig = _.clone(knex.client.config); + const knexConfig = _.clone(knex.client.config); - var callCount = 0; - var assertCall = function(expectedMessage, message) { + let callCount = 0; + const assertCall = function(expectedMessage, message) { expect(message).to.equal(expectedMessage); callCount++; }; @@ -1149,7 +1139,7 @@ module.exports = function(knex) { //Sqlite warning message knexConfig.useNullAsDefault = true; - var knexDb = new Knex(knexConfig); + const knexDb = new Knex(knexConfig); knexDb.client.logger.warn('test'); knexDb.client.logger.error('test'); diff --git a/test/unit/knex.js b/test/unit/knex.js index 261bd52c74..af91410881 100644 --- a/test/unit/knex.js +++ b/test/unit/knex.js @@ -3,6 +3,7 @@ const { expect } = require('chai'); const Promise = require('bluebird'); const sqliteConfig = require('../knexfile').sqlite3; const sqlite3 = require('sqlite3'); +const { noop } = require('lodash'); describe('knex', () => { describe('supports passing existing connection', () => { @@ -92,6 +93,46 @@ describe('knex', () => { }); }); + it('sets correct postProcessResponse for builders instantiated from clone', () => { + const knex = Knex({ + client: 'sqlite', + postProcessResponse: noop, + }); + + const knexWithParams = knex.withUserParams(); + knexWithParams.client.config.postProcessResponse = null; + const builderForTable = knex('tableName'); + const builderWithParamsForTable = knexWithParams('tableName'); + + expect(knex.client.config.postProcessResponse).to.equal(noop); + expect(knexWithParams.client.config.postProcessResponse).to.equal(null); + expect(builderForTable.client.config.postProcessResponse).to.equal(noop); + expect( + builderWithParamsForTable.client.config.postProcessResponse + ).to.equal(null); + }); + + it('sets correct postProcessResponse for chained builders', () => { + const knex = Knex({ + client: 'sqlite', + postProcessResponse: noop, + }); + + const knexWithParams = knex.withUserParams(); + knexWithParams.client.config.postProcessResponse = null; + const builderForTable = knex('tableName').where('1 = 1'); + const builderWithParamsForTable = knexWithParams('tableName').where( + '1 = 1' + ); + + expect(knex.client.config.postProcessResponse).to.equal(noop); + expect(knexWithParams.client.config.postProcessResponse).to.equal(null); + expect(builderForTable.client.config.postProcessResponse).to.equal(noop); + expect( + builderWithParamsForTable.client.config.postProcessResponse + ).to.equal(null); + }); + it('transaction of a copy with userParams retains userparams', (done) => { const knex = Knex(sqliteConfig); @@ -125,4 +166,20 @@ describe('knex', () => { /Knex: run\n$ npm install oracle/ ); }); + + describe('async stack traces', () => { + it('should capture stack trace on query builder instantiation', () => { + const knex = Knex({ + ...sqliteConfig, + asyncStackTraces: true, + }); + + return knex('some_nonexisten_table') + .select() + .catch((err) => { + expect(err.stack.split('\n')[1]).to.match(/at createQueryBuilder \(/); // the index 1 might need adjustment if the code is refactored + expect(typeof err.originalStack).to.equal('string'); + }); + }); + }); }); diff --git a/test/unit/migrate/Migrator.js b/test/unit/migrate/Migrator.js new file mode 100644 index 0000000000..e887919b36 --- /dev/null +++ b/test/unit/migrate/Migrator.js @@ -0,0 +1,34 @@ +const Knex = require('../../../lib/index'); +const { expect } = require('chai'); +const sqliteConfig = require('../../knexfile').sqlite3; +const FsMigrations = require('../../../lib/migrate/sources/fs-migrations') + .default; + +describe('Migrator', () => { + describe('does not use postProcessResponse for internal queries', (done) => { + let migrationSource; + let knex; + before(() => { + migrationSource = new FsMigrations('test/unit/migrate/migrations/'); + knex = Knex({ + ...sqliteConfig, + migrationSource, + postProcessResponse: () => { + throw new Error('Response was processed'); + }, + }); + }); + + it('latest', (done) => { + expect(() => { + knex.migrate + .latest({ + directory: 'test/unit/migrate/migrations', + }) + .then(() => { + done(); + }); + }).not.to.throw(); + }); + }); +}); diff --git a/test/unit/migrate/migrations/blank_migration.js b/test/unit/migrate/migrations/blank_migration.js new file mode 100644 index 0000000000..dc8824ebf2 --- /dev/null +++ b/test/unit/migrate/migrations/blank_migration.js @@ -0,0 +1,3 @@ +exports.up = function(knex) {}; + +exports.down = function(knex) {}; diff --git a/test/unit/schema/postgres.js b/test/unit/schema/postgres.js index e305cce5e8..89412c1f4d 100644 --- a/test/unit/schema/postgres.js +++ b/test/unit/schema/postgres.js @@ -9,7 +9,7 @@ const PG_Client = require('../../../lib/dialects/postgres'); const client = new PG_Client({ client: 'pg' }); const knex = require('../../../knex'); -const equal = require('assert').equal; +const equal = require('chai').assert.equal; describe('PostgreSQL Config', function() { let knexInstance;