Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connection configuration provider - without changes to drivers #3497

Merged
merged 14 commits into from Oct 27, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 42 additions & 9 deletions lib/client.js
Expand Up @@ -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 }; // causes the provider to be called on first use
oranoran marked this conversation as resolved.
Show resolved Hide resolved
} else {
this.connectionSettings = cloneDeep(config.connection || {});
}
if (this.driverName && config.connection) {
this.initializeDriver();
if (!config.pool || (config.pool && config.pool.max !== 0)) {
Expand Down Expand Up @@ -254,16 +259,44 @@ Object.assign(Client.prototype, {
// choose the smallest, positive timeout setting and set on poolConfig
poolConfig.acquireTimeoutMillis = Math.min(...timeouts);

const updatePoolConnectionSettingsFromProvider = () => {
if (!this.connectionConfigProvider) {
return; // static configuration, nothing to update
}
if (
this.connectionSettings.expired === undefined ||
oranoran marked this conversation as resolved.
Show resolved Hide resolved
!this.connectionSettings.expired()
) {
return; // reuse existing connection
}
const providerResult = this.connectionConfigProvider();
if (!providerResult.then) {
oranoran marked this conversation as resolved.
Show resolved Hide resolved
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) => {
Expand Down
77 changes: 77 additions & 0 deletions test/tape/connection-config-provider.js
@@ -0,0 +1,77 @@
'use strict';

oranoran marked this conversation as resolved.
Show resolved Hide resolved
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), { 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(_.cloneDeep(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();
}
1 change: 1 addition & 0 deletions test/tape/index.js
Expand Up @@ -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]);
Expand Down