Manually initializing a destroyed connection pool did not change database schema name

[VeryCrazyDog]

Summary

I am setting up test script to create multiple database schema to utilize AVA parallel test run. As a result, I need to create the schema first, then switch the application connection pool to use the new created schema. However I encountered a bug in doing so, and would like to ask the official proper way to change the database schema name.

Environment

Knex version: 0.21.0
Database + version: MySQL 5.7
OS: Windows 10

Bug

According to the documentation:

If you ever need to explicitly teardown the connection pool, you may use knex.destroy([callback]). You may use knex.destroy by passing a callback, or by chaining as a promise, just not both. To manually initialize a destroyed connection pool, you may use knex.initialize([config]), if no config is passed, it will use the first knex configuration used.

I expect I can change the database name by knex.destroy and knex.initialize(newConfig) with new configuration. This does not work.

Reduced test case as follows:

const knex = require('knex')

function getConfig () {
  return {
    client: 'mysql',
    connection: {
      host: 'localhost',
      user: 'root',
      password: ''
    }
  }
}

const db = knex(getConfig()).on('query', function (data) {
  console.log(data.sql)
})

const method = 1

;(async () => {
  // Randomly (in my case it is based on server listening port) assigned a database name for AVA parallel testing
  const dbName = 'TMP_DB_' + (Math.floor(Math.random() * 999) + 9001)
  try {
    await db.raw(`CREATE DATABASE ${dbName}`)
    await db.raw(`USE ${dbName}`)
    await db.schema.createTable('tmp1', (table) => {
      table.increments()
      table.string('val1', 10).notNullable()
    })
    if (method === 1) {
      // I tried the documented way, but the `connection.database` is not changed with the following code
      await db.destroy()
      const newConfig = getConfig()
      newConfig.connection.database = dbName
      db.initialize(newConfig)
    } else if (method === 2) {
      // Is this the proper way to set the `connection.database`?
      db.client.connectionSettings.database = dbName
    }
    // This will result in `Error: ER_NO_DB_ERROR: No database selected` if database name is not set
    await Promise.all([
      db('tmp1').insert({ val1: 'a' }),
      db('tmp1').insert({ val1: 'b' }),
      db('tmp1').insert({ val1: 'c' })
    ])
  } finally {
    try {
      await db.raw(`DROP DATABASE ${dbName}`)
    } catch (ignore) {}
    await db.destroy()
  }
  console.log('End')
})().catch(e => console.log(e.stack))

Question

My additional question together with this bug, is how to change the database schema of the connection pool? Is the const method = 2 the proper way to do so? Or it can only be treated as undocumented way and may change in the future?

[elhigu]

You could just destroy and throw away old knex instance and create completely new instance with updated config. That would be at least working official way.

Anyways this might be also a real bug. I didn't find any tests for passing new configuration for initialize.

knex/test/integration/suite.js

Line 48 in e37aeaa

describe('knex.initialize', function () {

[VeryCrazyDog]

Thanks a lot. The main reason why I am avoiding throw away old knex instance is because I am abstracting my database implementation in separated files as shown below. Some complexity will be introduced if I need to handle throw away of knex instance for testing ☹️

// In file `db.js`
const knex = require('knex')

const db = knex({
  client: 'mysql',
  connection: {
    host: 'localhost',
    user: 'root',
    password: ''
  }
}).on('query', function (data) {
  console.log(data.sql)
})

module.exports = db

// In file `index.js`
const db = require('./db')
// ...

[elhigu]

hmm.. I don't see how that works for testing if you are trying to run multiple tests concurrently against multiple databases without having multiple knex instances there which are connected to different database 🤔

If you have single global knex instance, then if you try to destroy and reinitialize knex with correct DB before each test, all other concurrently running tests will fail.

[VeryCrazyDog]

Good catch. This is process isolation provided by AVA which run each test file in separated process. Therefore even I defined one single global knex instance, it only apply to one process and another separated process will have it's own global knex instance pointing to another database.

[MehdiSaffar]

I just hit the exact same problem and was disappointed to see that client.config does not get updated (basically the entire flow happening inside the knex constructor does not execute again for the new config), and the main reason I tried avoid recreating an instance is for the exact same reason @VeryCrazyDog mentioned :(

[kibertoad]

@MehdiSaffar Would you like to send a PR implementing support for this use-case?

[MehdiSaffar]

@kibertoad I will give it a try

[MehdiSaffar]

I opened a PR please take a look :) @VeryCrazyDog @kibertoad

[MehdiSaffar]

@VeryCrazyDog Here is a temporary approach with ES6 proxies you could use if you want, not sure how correct it is but it seems to work on my side. Example:

const defaultConfig = YOUR_KNEX_CONFIG

const makeKnex = require('knex')

const wrapper = { instance: makeKnex(defaultConfig) }

async function reconnect (config) {
  await wrapper.instance.destroy()
  wrapper.instance = makeKnex(config)
}

const knexInstance = new Proxy(wrapper, {
  get: (target, prop) => {
    if (prop === 'reconnect') {
      return reconnect
    }

    return target.instance[prop]
  }
})

module.exports = knexInstance

[VeryCrazyDog]

@MehdiSaffar I haven't thought of using Proxy at that time (maybe I didn't even know Proxy at that time). Thanks a lot for sharing, though I am no longer working with project using RDBMS now.