From 3ca01556ed1e8cf4a6aff9346f83c2ba39e45b79 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Fri, 25 Oct 2019 13:28:34 +0300 Subject: [PATCH 01/13] added support for connection configuration provider --- lib/client.js | 60 ++++++++++++++++++---- test/tape/connection-config-provider.js | 66 +++++++++++++++++++++++++ test/tape/index.js | 1 + 3 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 test/tape/connection-config-provider.js diff --git a/lib/client.js b/lib/client.js index 9f092c9e45..36e186085c 100644 --- a/lib/client.js +++ b/lib/client.js @@ -55,7 +55,12 @@ function Client(config = {}) { this.version = config.version; } - this.connectionSettings = cloneDeep(config.connection || {}); + if (config.connection && config.connection instanceof Function) { + this.connectionConfigProvider = config.connection; + this.connectionSettings = { expired: () => true }; + } else { + this.connectionSettings = cloneDeep(config.connection || {}); + } if (this.driverName && config.connection) { this.initializeDriver(); if (!config.pool || (config.pool && config.pool.max !== 0)) { @@ -254,16 +259,53 @@ Object.assign(Client.prototype, { // choose the smallest, positive timeout setting and set on poolConfig poolConfig.acquireTimeoutMillis = Math.min(...timeouts); + const updatePoolConnectionSettingsFromProvider = () => { + if (this.connectionConfigProvider === null) { + // static configuration, nothing to update + return; + } + if ( + this.connectionConfigProvider === undefined && + !(this.connectionSettings instanceof Function) + ) { + this.connectionConfigProvider = null; // uninitialized, mark as static for future calls + return; + } + // At this point we know a connectionConfigProvider is in use + if ( + this.connectionSettings.expired === undefined || + !this.connectionSettings.expired() + ) { + return; // reuse existing connection + } + const providerResult = this.connectionConfigProvider(); + if (!providerResult.then) { + this.connectionSettings = providerResult; + return; // provider is synchronous, no need to return a promise + } + return providerResult.then((connectionSettings) => { + this.connectionSettings = connectionSettings; + }); + }; + + const createPoolConnection = () => { + return this.acquireRawConnection().then(async (connection) => { + connection.__knexUid = uniqueId('__knexUid'); + + if (poolConfig.afterCreate) { + await promisify(poolConfig.afterCreate)(connection); + } + return connection; + }); + }; + return Object.assign(poolConfig, { create: () => { - return this.acquireRawConnection().then(async (connection) => { - connection.__knexUid = uniqueId('__knexUid'); - - if (poolConfig.afterCreate) { - await promisify(poolConfig.afterCreate)(connection); - } - return connection; - }); + const promiseOfUpdate = updatePoolConnectionSettingsFromProvider(); + if (promiseOfUpdate) { + return promiseOfUpdate.then(createPoolConnection.bind(this)); + } + return createPoolConnection.bind(this)(); }, destroy: (connection) => { diff --git a/test/tape/connection-config-provider.js b/test/tape/connection-config-provider.js new file mode 100644 index 0000000000..bd4543b397 --- /dev/null +++ b/test/tape/connection-config-provider.js @@ -0,0 +1,66 @@ +'use strict'; + +const knexfile = require('../knexfile'); +const makeKnex = require('../../knex'); +const test = require('tape'); +const _ = require('lodash'); + +const originalConfig = knexfile['sqlite3']; +const originalConnection = _.cloneDeep(originalConfig.connection); + +test('static config works without a provider', async function(t) { + await runTwoConcurrentTransactions(originalConnection); + t.pass('static config used successfully'); + t.end(); +}); + +test('by default, the same resolved config is used for all connections', async function(t) { + let providerCallCount = 0; + const connectionConfig = () => { + ++providerCallCount; + return Promise.resolve(originalConnection); + }; + await runTwoConcurrentTransactions(connectionConfig); + t.equal(providerCallCount, 1); + t.end(); +}); + +test('when not yet expired, a resolved config is reused', async function(t) { + let providerCallCount = 0; + const connectionConfig = () => { + ++providerCallCount; + return Promise.resolve( + Object.assign(originalConnection, { expired: () => false }) + ); + }; + await runTwoConcurrentTransactions(connectionConfig); + t.equal(providerCallCount, 1); + t.end(); +}); + +test('when expired, a resolved config is replaced', async function(t) { + let providerCallCount = 0; + const connectionConfig = () => { + ++providerCallCount; + return Promise.resolve( + Object.assign(originalConnection, { expired: () => true }) + ); + }; + await runTwoConcurrentTransactions(connectionConfig); + t.equal(providerCallCount, 2); + t.end(); +}); + +async function runTwoConcurrentTransactions(connectionConfig) { + const config = _.cloneDeep(originalConfig); + config.connection = connectionConfig; + config.pool.max = 2; + const knex = makeKnex(config); + await knex.transaction(async (trx) => { + await trx.select(1); + await knex.transaction(async (trx2) => { + await trx2.select(2); + }); + }); + await knex.destroy(); +} diff --git a/test/tape/index.js b/test/tape/index.js index aced900289..aac9ff479d 100644 --- a/test/tape/index.js +++ b/test/tape/index.js @@ -13,6 +13,7 @@ require('./migrate'); require('./pool'); require('./knex'); require('./invalid-db-setup')(knexfile); +require('./connection-config-provider'); Object.keys(knexfile).forEach(function(key) { var knex = makeKnex(knexfile[key]); From 46fc1c930b8754119b41ab2d942309a606a58556 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Fri, 25 Oct 2019 13:42:44 +0300 Subject: [PATCH 02/13] removed a few redundant lines and improved readibility --- lib/client.js | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/client.js b/lib/client.js index 36e186085c..39fdfeb079 100644 --- a/lib/client.js +++ b/lib/client.js @@ -57,7 +57,7 @@ function Client(config = {}) { if (config.connection && config.connection instanceof Function) { this.connectionConfigProvider = config.connection; - this.connectionSettings = { expired: () => true }; + this.connectionSettings = { expired: () => true }; // causes the provider to be called on first use } else { this.connectionSettings = cloneDeep(config.connection || {}); } @@ -260,18 +260,9 @@ Object.assign(Client.prototype, { poolConfig.acquireTimeoutMillis = Math.min(...timeouts); const updatePoolConnectionSettingsFromProvider = () => { - if (this.connectionConfigProvider === null) { - // static configuration, nothing to update - return; + if (!this.connectionConfigProvider) { + return; // static configuration, nothing to update } - if ( - this.connectionConfigProvider === undefined && - !(this.connectionSettings instanceof Function) - ) { - this.connectionConfigProvider = null; // uninitialized, mark as static for future calls - return; - } - // At this point we know a connectionConfigProvider is in use if ( this.connectionSettings.expired === undefined || !this.connectionSettings.expired() From 8aa3edc36816185cc94fc82881caa9089db01493 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Fri, 25 Oct 2019 14:03:24 +0300 Subject: [PATCH 03/13] added test for synchronouse provider, and fixed potential unwanted side effect between tests --- test/tape/connection-config-provider.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/tape/connection-config-provider.js b/test/tape/connection-config-provider.js index bd4543b397..c191e2b1bd 100644 --- a/test/tape/connection-config-provider.js +++ b/test/tape/connection-config-provider.js @@ -14,7 +14,7 @@ test('static config works without a provider', async function(t) { t.end(); }); -test('by default, the same resolved config is used for all connections', async function(t) { +test('by default, the same async-resolved config is used for all connections', async function(t) { let providerCallCount = 0; const connectionConfig = () => { ++providerCallCount; @@ -25,12 +25,23 @@ test('by default, the same resolved config is used for all connections', async f t.end(); }); +test('by default, the same sync-resolved config is used for all connections', async function(t) { + let providerCallCount = 0; + const connectionConfig = () => { + ++providerCallCount; + return originalConnection; + }; + await runTwoConcurrentTransactions(connectionConfig); + t.equal(providerCallCount, 1); + t.end(); +}); + test('when not yet expired, a resolved config is reused', async function(t) { let providerCallCount = 0; const connectionConfig = () => { ++providerCallCount; return Promise.resolve( - Object.assign(originalConnection, { expired: () => false }) + Object.assign(_.cloneDeep(originalConnection), { expired: () => false }) ); }; await runTwoConcurrentTransactions(connectionConfig); @@ -43,7 +54,7 @@ test('when expired, a resolved config is replaced', async function(t) { const connectionConfig = () => { ++providerCallCount; return Promise.resolve( - Object.assign(originalConnection, { expired: () => true }) + Object.assign(_.cloneDeep(originalConnection), { expired: () => true }) ); }; await runTwoConcurrentTransactions(connectionConfig); From 3bdf0bfa47a38f6060a22fe7c99c3a90f1906849 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Fri, 25 Oct 2019 14:25:57 +0300 Subject: [PATCH 04/13] renamed expired => expirationChecker, removed explicit comparison to "undefined" so "null" will also be supported. --- lib/client.js | 8 ++++---- test/tape/connection-config-provider.js | 8 ++++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/client.js b/lib/client.js index 39fdfeb079..12df97a1da 100644 --- a/lib/client.js +++ b/lib/client.js @@ -57,7 +57,7 @@ function Client(config = {}) { if (config.connection && config.connection instanceof Function) { this.connectionConfigProvider = config.connection; - this.connectionSettings = { expired: () => true }; // causes the provider to be called on first use + this.connectionSettings = { expirationChecker: () => true }; // causes the provider to be called on first use } else { this.connectionSettings = cloneDeep(config.connection || {}); } @@ -264,10 +264,10 @@ Object.assign(Client.prototype, { return; // static configuration, nothing to update } if ( - this.connectionSettings.expired === undefined || - !this.connectionSettings.expired() + !this.connectionSettings.expirationChecker || + !this.connectionSettings.expirationChecker() ) { - return; // reuse existing connection + return; // not expired, reuse existing connection } const providerResult = this.connectionConfigProvider(); if (!providerResult.then) { diff --git a/test/tape/connection-config-provider.js b/test/tape/connection-config-provider.js index c191e2b1bd..c0e0bf270e 100644 --- a/test/tape/connection-config-provider.js +++ b/test/tape/connection-config-provider.js @@ -41,7 +41,9 @@ test('when not yet expired, a resolved config is reused', async function(t) { const connectionConfig = () => { ++providerCallCount; return Promise.resolve( - Object.assign(_.cloneDeep(originalConnection), { expired: () => false }) + Object.assign(_.cloneDeep(originalConnection), { + expirationChecker: () => false, + }) ); }; await runTwoConcurrentTransactions(connectionConfig); @@ -54,7 +56,9 @@ test('when expired, a resolved config is replaced', async function(t) { const connectionConfig = () => { ++providerCallCount; return Promise.resolve( - Object.assign(_.cloneDeep(originalConnection), { expired: () => true }) + Object.assign(_.cloneDeep(originalConnection), { + expirationChecker: () => true, + }) ); }; await runTwoConcurrentTransactions(connectionConfig); From 6d0b78bbc3e184c240483867f81800cdf224c3e5 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sat, 26 Oct 2019 10:56:03 +0300 Subject: [PATCH 05/13] Moved connection config provider tests to integration so they cover all DBs + removing expiryChecker from config being passed to DB drivers --- lib/client.js | 20 +++-- .../integration/connection-config-provider.js | 74 +++++++++++++++++ test/integration/index.js | 1 + test/tape/connection-config-provider.js | 81 ------------------- 4 files changed, 90 insertions(+), 86 deletions(-) create mode 100644 test/integration/connection-config-provider.js delete mode 100644 test/tape/connection-config-provider.js diff --git a/lib/client.js b/lib/client.js index 12df97a1da..7d7fb885e3 100644 --- a/lib/client.js +++ b/lib/client.js @@ -57,7 +57,7 @@ function Client(config = {}) { if (config.connection && config.connection instanceof Function) { this.connectionConfigProvider = config.connection; - this.connectionSettings = { expirationChecker: () => true }; // causes the provider to be called on first use + this.connectionConfigExpirationChecker = () => true; // causes the provider to be called on first use } else { this.connectionSettings = cloneDeep(config.connection || {}); } @@ -259,23 +259,33 @@ Object.assign(Client.prototype, { // choose the smallest, positive timeout setting and set on poolConfig poolConfig.acquireTimeoutMillis = Math.min(...timeouts); + const handleConnectionConfigProviderResult = (result) => { + if (result.expirationChecker) { + this.connectionConfigExpirationChecker = result.expirationChecker; + delete result.expirationChecker; + } else { + delete this.connectionConfigExpirationChecker; + } + this.connectionSettings = result; + }; + const updatePoolConnectionSettingsFromProvider = () => { if (!this.connectionConfigProvider) { return; // static configuration, nothing to update } if ( - !this.connectionSettings.expirationChecker || - !this.connectionSettings.expirationChecker() + !this.connectionConfigExpirationChecker || + !this.connectionConfigExpirationChecker() ) { return; // not expired, reuse existing connection } const providerResult = this.connectionConfigProvider(); if (!providerResult.then) { - this.connectionSettings = providerResult; + handleConnectionConfigProviderResult(providerResult); return; // provider is synchronous, no need to return a promise } return providerResult.then((connectionSettings) => { - this.connectionSettings = connectionSettings; + handleConnectionConfigProviderResult(connectionSettings); }); }; diff --git a/test/integration/connection-config-provider.js b/test/integration/connection-config-provider.js new file mode 100644 index 0000000000..d5358c66ee --- /dev/null +++ b/test/integration/connection-config-provider.js @@ -0,0 +1,74 @@ +/*global expect*/ + +'use strict'; + +const _ = require('lodash'); +const makeKnex = require('../../knex'); + +module.exports = function(config) { + describe('Connection configuration provider', function() { + let configWorkingCopy; + let providerInvocationCount; + let connectionConfigWorkingCopy; + + this.beforeEach(() => { + configWorkingCopy = _.cloneDeep(config); + configWorkingCopy.pool.min = 1; + configWorkingCopy.pool.max = 2; + providerInvocationCount = 0; + connectionConfigWorkingCopy = configWorkingCopy.connection; + }); + + it('is not used when configuration is static', async function() { + return runTwoConcurrentTransactions(0); + }); + + it('can return a promise for a config object, which is reused when not given given an expiry checker', async () => { + configWorkingCopy.connection = () => { + ++providerInvocationCount; + return Promise.resolve(connectionConfigWorkingCopy); + }; + return runTwoConcurrentTransactions(1); + }); + + it('can return a config object, which is reused when not given given an expiry checker', async () => { + configWorkingCopy.connection = () => { + ++providerInvocationCount; + return connectionConfigWorkingCopy; + }; + return runTwoConcurrentTransactions(1); + }); + + it('reuses the same resolved config when not yet expired', async () => { + configWorkingCopy.connection = () => { + ++providerInvocationCount; + return Object.assign(connectionConfigWorkingCopy, { + expirationChecker: () => false, + }); + }; + return runTwoConcurrentTransactions(1); + }); + + it('replaces the resolved config when expired', async () => { + configWorkingCopy.connection = () => { + ++providerInvocationCount; + return Object.assign(connectionConfigWorkingCopy, { + expirationChecker: () => true, + }); + }; + return runTwoConcurrentTransactions(2); + }); + + async function runTwoConcurrentTransactions(expectedInvocationCount) { + const knex = makeKnex(configWorkingCopy); + await knex.transaction(async (trx) => { + await trx.select(1); + await knex.transaction(async (trx2) => { + await trx2.select(2); + }); + }); + await knex.destroy(); + expect(providerInvocationCount).equals(expectedInvocationCount); + } + }); +}; diff --git a/test/integration/index.js b/test/integration/index.js index ecb1ab45ca..1af217017b 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -6,6 +6,7 @@ const config = require('../knexfile'); const fs = require('fs'); Object.keys(config).forEach((dialectName) => { + require('./connection-config-provider')(config[dialectName]); return require('./suite')(logger(knex(config[dialectName]))); }); diff --git a/test/tape/connection-config-provider.js b/test/tape/connection-config-provider.js deleted file mode 100644 index c0e0bf270e..0000000000 --- a/test/tape/connection-config-provider.js +++ /dev/null @@ -1,81 +0,0 @@ -'use strict'; - -const knexfile = require('../knexfile'); -const makeKnex = require('../../knex'); -const test = require('tape'); -const _ = require('lodash'); - -const originalConfig = knexfile['sqlite3']; -const originalConnection = _.cloneDeep(originalConfig.connection); - -test('static config works without a provider', async function(t) { - await runTwoConcurrentTransactions(originalConnection); - t.pass('static config used successfully'); - t.end(); -}); - -test('by default, the same async-resolved config is used for all connections', async function(t) { - let providerCallCount = 0; - const connectionConfig = () => { - ++providerCallCount; - return Promise.resolve(originalConnection); - }; - await runTwoConcurrentTransactions(connectionConfig); - t.equal(providerCallCount, 1); - t.end(); -}); - -test('by default, the same sync-resolved config is used for all connections', async function(t) { - let providerCallCount = 0; - const connectionConfig = () => { - ++providerCallCount; - return originalConnection; - }; - await runTwoConcurrentTransactions(connectionConfig); - t.equal(providerCallCount, 1); - t.end(); -}); - -test('when not yet expired, a resolved config is reused', async function(t) { - let providerCallCount = 0; - const connectionConfig = () => { - ++providerCallCount; - return Promise.resolve( - Object.assign(_.cloneDeep(originalConnection), { - expirationChecker: () => false, - }) - ); - }; - await runTwoConcurrentTransactions(connectionConfig); - t.equal(providerCallCount, 1); - t.end(); -}); - -test('when expired, a resolved config is replaced', async function(t) { - let providerCallCount = 0; - const connectionConfig = () => { - ++providerCallCount; - return Promise.resolve( - Object.assign(_.cloneDeep(originalConnection), { - expirationChecker: () => true, - }) - ); - }; - await runTwoConcurrentTransactions(connectionConfig); - t.equal(providerCallCount, 2); - t.end(); -}); - -async function runTwoConcurrentTransactions(connectionConfig) { - const config = _.cloneDeep(originalConfig); - config.connection = connectionConfig; - config.pool.max = 2; - const knex = makeKnex(config); - await knex.transaction(async (trx) => { - await trx.select(1); - await knex.transaction(async (trx2) => { - await trx2.select(2); - }); - }); - await knex.destroy(); -} From 9d3c543f2a3c0b51bd529d1c0007b37ad8121b28 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sat, 26 Oct 2019 11:50:47 +0300 Subject: [PATCH 06/13] fixed removal of tests from tape --- test/tape/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/tape/index.js b/test/tape/index.js index aac9ff479d..aced900289 100644 --- a/test/tape/index.js +++ b/test/tape/index.js @@ -13,7 +13,6 @@ require('./migrate'); require('./pool'); require('./knex'); require('./invalid-db-setup')(knexfile); -require('./connection-config-provider'); Object.keys(knexfile).forEach(function(key) { var knex = makeKnex(knexfile[key]); From 826fbfd0af468c5b1fabd4239aca77c2d5052ddf Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sat, 26 Oct 2019 12:01:07 +0300 Subject: [PATCH 07/13] Simplified use of async/await and promises for connection config provider --- lib/client.js | 44 +++++++++++++------------------------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/lib/client.js b/lib/client.js index 7d7fb885e3..339f0177e2 100644 --- a/lib/client.js +++ b/lib/client.js @@ -259,17 +259,7 @@ Object.assign(Client.prototype, { // choose the smallest, positive timeout setting and set on poolConfig poolConfig.acquireTimeoutMillis = Math.min(...timeouts); - const handleConnectionConfigProviderResult = (result) => { - if (result.expirationChecker) { - this.connectionConfigExpirationChecker = result.expirationChecker; - delete result.expirationChecker; - } else { - delete this.connectionConfigExpirationChecker; - } - this.connectionSettings = result; - }; - - const updatePoolConnectionSettingsFromProvider = () => { + const updatePoolConnectionSettingsFromProvider = async () => { if (!this.connectionConfigProvider) { return; // static configuration, nothing to update } @@ -279,34 +269,26 @@ Object.assign(Client.prototype, { ) { return; // not expired, reuse existing connection } - const providerResult = this.connectionConfigProvider(); - if (!providerResult.then) { - handleConnectionConfigProviderResult(providerResult); - return; // provider is synchronous, no need to return a promise + const providerResult = await this.connectionConfigProvider(); + if (providerResult.expirationChecker) { + this.connectionConfigExpirationChecker = + providerResult.expirationChecker; + delete providerResult.expirationChecker; + } else { + delete this.connectionConfigExpirationChecker; } - return providerResult.then((connectionSettings) => { - handleConnectionConfigProviderResult(connectionSettings); - }); + this.connectionSettings = providerResult; }; - const createPoolConnection = () => { - return this.acquireRawConnection().then(async (connection) => { + return Object.assign(poolConfig, { + create: async () => { + await updatePoolConnectionSettingsFromProvider(); + const connection = await this.acquireRawConnection(); connection.__knexUid = uniqueId('__knexUid'); - if (poolConfig.afterCreate) { await promisify(poolConfig.afterCreate)(connection); } return connection; - }); - }; - - return Object.assign(poolConfig, { - create: () => { - const promiseOfUpdate = updatePoolConnectionSettingsFromProvider(); - if (promiseOfUpdate) { - return promiseOfUpdate.then(createPoolConnection.bind(this)); - } - return createPoolConnection.bind(this)(); }, destroy: (connection) => { From 01b6d287cef4df59adeac3fe8c8de25483308893 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sat, 26 Oct 2019 17:27:49 +0300 Subject: [PATCH 08/13] Setting this.connectionConfigExpirationChecker to null instead of removing it (and also initializing it to null if needed) - in order to maintain a stable set of properties in the client. --- lib/client.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 339f0177e2..58f3144c24 100644 --- a/lib/client.js +++ b/lib/client.js @@ -60,6 +60,7 @@ function Client(config = {}) { this.connectionConfigExpirationChecker = () => true; // causes the provider to be called on first use } else { this.connectionSettings = cloneDeep(config.connection || {}); + this.connectionConfigExpirationChecker = null; } if (this.driverName && config.connection) { this.initializeDriver(); @@ -275,7 +276,7 @@ Object.assign(Client.prototype, { providerResult.expirationChecker; delete providerResult.expirationChecker; } else { - delete this.connectionConfigExpirationChecker; + this.connectionConfigExpirationChecker = null; } this.connectionSettings = providerResult; }; From b0c2c397c7a8042465c13d5d72cb39afd35bf1ec Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sat, 26 Oct 2019 17:38:04 +0300 Subject: [PATCH 09/13] removed the actual dummy queries from the test, as they need to be different for different databases, and the tests seem to be just as valid wihtout them. --- test/integration/connection-config-provider.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/integration/connection-config-provider.js b/test/integration/connection-config-provider.js index d5358c66ee..ac8a910122 100644 --- a/test/integration/connection-config-provider.js +++ b/test/integration/connection-config-provider.js @@ -62,10 +62,7 @@ module.exports = function(config) { async function runTwoConcurrentTransactions(expectedInvocationCount) { const knex = makeKnex(configWorkingCopy); await knex.transaction(async (trx) => { - await trx.select(1); - await knex.transaction(async (trx2) => { - await trx2.select(2); - }); + await knex.transaction(async (trx2) => {}); }); await knex.destroy(); expect(providerInvocationCount).equals(expectedInvocationCount); From 673703326057d747fc4a593e4b0e19f242f0289a Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sun, 27 Oct 2019 07:44:49 +0200 Subject: [PATCH 10/13] added comment explaining delete --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 58f3144c24..da12c65b47 100644 --- a/lib/client.js +++ b/lib/client.js @@ -274,7 +274,7 @@ Object.assign(Client.prototype, { if (providerResult.expirationChecker) { this.connectionConfigExpirationChecker = providerResult.expirationChecker; - delete providerResult.expirationChecker; + delete providerResult.expirationChecker; // MySQL2 driver warns on receiving extra properties } else { this.connectionConfigExpirationChecker = null; } From 16de28fcf18feefc942ca195966974d4fd17ebf8 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sun, 27 Oct 2019 14:08:26 +0200 Subject: [PATCH 11/13] Added typescript definition for optional Boolean expirationChecker function in connection objects. --- types/index.d.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/types/index.d.ts b/types/index.d.ts index c671d99649..ded7d1d029 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -1643,6 +1643,7 @@ declare namespace Knex { instanceName?: string; debug?: boolean; requestTimeout?: number; + expirationChecker?(): boolean; } // Config object for mssql: see https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/mssql/index.d.ts @@ -1658,6 +1659,7 @@ declare namespace Knex { requestTimeout?: number; stream?: boolean; parseJSON?: boolean; + expirationChecker?(): boolean; options?: { encrypt?: boolean; instanceName?: string; @@ -1705,6 +1707,7 @@ declare namespace Knex { read_default_group?: string; charset?: string; streamHWM?: number; + expirationChecker?(): boolean; } interface MariaSslConfiguration { @@ -1714,6 +1717,7 @@ declare namespace Knex { capath?: string; cipher?: string; rejectUnauthorized?: boolean; + expirationChecker?(): boolean; } // Config object for mysql: https://github.com/mysqljs/mysql#connection-options @@ -1741,6 +1745,7 @@ declare namespace Knex { flags?: string; ssl?: string | MariaSslConfiguration; decimalNumbers?: boolean; + expirationChecker?(): boolean; } interface OracleDbConnectionConfig { @@ -1753,12 +1758,14 @@ declare namespace Knex { debug?: boolean; requestTimeout?: number; connectString?: string; + expirationChecker?(): boolean; } /** Used with SQLite3 adapter */ interface Sqlite3ConnectionConfig { filename: string; debug?: boolean; + expirationChecker?(): boolean; } interface SocketConnectionConfig { @@ -1767,6 +1774,7 @@ declare namespace Knex { password: string; database: string; debug?: boolean; + expirationChecker?(): boolean; } interface PoolConfig { From 5b5b49e8abe2b8927bbd7add85f88fbdd1a278d7 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sun, 27 Oct 2019 15:44:34 +0200 Subject: [PATCH 12/13] Added typescript definitions for connection config provider --- types/index.d.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/types/index.d.ts b/types/index.d.ts index ded7d1d029..93ee220b54 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -1609,15 +1609,7 @@ declare namespace Knex { client?: string | typeof Client; dialect?: string; version?: string; - connection?: - | string - | ConnectionConfig - | MariaSqlConnectionConfig - | MySqlConnectionConfig - | MsSqlConnectionConfig - | OracleDbConnectionConfig - | Sqlite3ConnectionConfig - | SocketConnectionConfig; + connection?: string | StaticConnectionConfig | ConnectionConfigProvider; pool?: PoolConfig; migrations?: MigratorConfig; postProcessResponse?: (result: any, queryContext: any) => any; @@ -1634,6 +1626,18 @@ declare namespace Knex { log?: Logger; } + type StaticConnectionConfig = ConnectionConfig + | MariaSqlConnectionConfig + | MySqlConnectionConfig + | MsSqlConnectionConfig + | OracleDbConnectionConfig + | Sqlite3ConnectionConfig + | SocketConnectionConfig; + + type ConnectionConfigProvider = SyncConnectionConfigProvider | AsyncConnectionConfigProvider; + type SyncConnectionConfigProvider = () => StaticConnectionConfig; + type AsyncConnectionConfigProvider = () => Promise; + interface ConnectionConfig { host: string; user: string; @@ -1643,7 +1647,6 @@ declare namespace Knex { instanceName?: string; debug?: boolean; requestTimeout?: number; - expirationChecker?(): boolean; } // Config object for mssql: see https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/mssql/index.d.ts From 63984403e11600d1934b16bb2df42abb83dead55 Mon Sep 17 00:00:00 2001 From: Oran Epelbaum Date: Sun, 27 Oct 2019 16:06:58 +0200 Subject: [PATCH 13/13] Fixed latest merge to not allow string as connection config returned by provider. This is currently not supported since parsing a config string is done in knex.js while the connection config provider is implemented in client.js --- types/index.d.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/types/index.d.ts b/types/index.d.ts index 719de0668a..e667b5dd3c 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -1627,7 +1627,6 @@ declare namespace Knex { } type StaticConnectionConfig = - | string | ConnectionConfig | MariaSqlConnectionConfig | MySqlConnectionConfig