From 7459157e93f3d15f462df88e00c1c0ffa813927e Mon Sep 17 00:00:00 2001 From: korynunn Date: Thu, 24 Sep 2020 10:14:32 +1000 Subject: [PATCH 01/11] Override knexfile options with CLI options --- bin/cli.js | 13 +++++++-- bin/utils/cli-config-utils.js | 33 ++++++++++++++++++----- test/cli/migrate-make.spec.js | 10 +++---- test/cli/migrate.spec.js | 50 +++++++++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 13 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index ab7e603272..fea2dc576f 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -1,6 +1,7 @@ #!/usr/bin/env node /* eslint no-console:0, no-var:0 */ const Liftoff = require('liftoff'); +const merge = require('lodash/merge'); const interpret = require('interpret'); const path = require('path'); const tildify = require('tildify'); @@ -9,6 +10,7 @@ const color = require('colorette'); const argv = require('getopts')(process.argv.slice(2)); const cliPkg = require('../package'); const { + parseConfigObj, mkConfigObj, resolveEnvironmentConfig, exit, @@ -44,17 +46,24 @@ async function initKnex(env, opts) { ); } - env.configuration = env.configPath + const knexFileConfig = env.configPath ? await openKnexfile(env.configPath) : mkConfigObj(opts); + env.configuration = knexFileConfig; + const resolvedConfig = resolveEnvironmentConfig( opts, env.configuration, env.configPath ); + + const optionsConfig = parseConfigObj(opts); + + const config = merge(resolvedConfig, optionsConfig); + const knex = require(env.modulePath); - return knex(resolvedConfig); + return knex(config); } function invoke(env) { diff --git a/bin/utils/cli-config-utils.js b/bin/utils/cli-config-utils.js index 01800f8b17..4f5d9ebe94 100644 --- a/bin/utils/cli-config-utils.js +++ b/bin/utils/cli-config-utils.js @@ -6,6 +6,28 @@ const tildify = require('tildify'); const color = require('colorette'); const argv = require('getopts')(process.argv.slice(2)); +function parseConfigObj(opts) { + const config = { migrations: {} }; + + if (opts.client) { + config.client = opts.client; + } + + if (opts.connection) { + config.connection = opts.connection; + } + + if (opts.migrationsDirectory) { + config.migrations.directory = opts.migrationsDirectory; + } + + if (opts.migrationsTableName) { + config.migrations.tableName = opts.migrationsTableName; + } + + return config; +} + function mkConfigObj(opts) { if (!opts.client) { throw new Error( @@ -16,16 +38,14 @@ function mkConfigObj(opts) { const envName = opts.env || process.env.NODE_ENV || 'development'; const resolvedClientName = resolveClientNameWithAliases(opts.client); const useNullAsDefault = resolvedClientName === 'sqlite3'; + const parsedConfig = parseConfigObj(opts); + return { ext: DEFAULT_EXT, [envName]: { + ...parsedConfig, useNullAsDefault, - client: opts.client, - connection: opts.connection, - migrations: { - directory: opts.migrationsDirectory, - tableName: opts.migrationsTableName || DEFAULT_TABLE_NAME, - }, + tableName: parsedConfig.tableName || DEFAULT_TABLE_NAME, }, }; } @@ -140,6 +160,7 @@ function getStubPath(configKey, env, opts) { } module.exports = { + parseConfigObj, mkConfigObj, resolveEnvironmentConfig, exit, diff --git a/test/cli/migrate-make.spec.js b/test/cli/migrate-make.spec.js index 397c747b94..9f88a17827 100644 --- a/test/cli/migrate-make.spec.js +++ b/test/cli/migrate-make.spec.js @@ -124,7 +124,7 @@ module.exports = { migrations: { directory: __dirname + '/test/jake-util/knexfile_migrations', }, -}; +}; `, { isPathAbsolute: true } ); @@ -152,7 +152,7 @@ module.exports = { migrations: { directory: __dirname + '/test/jake-util/knexfile_migrations', }, -}; +}; `, { isPathAbsolute: true } ); @@ -187,7 +187,7 @@ module.exports = { directory: __dirname + '/test/jake-util/knexfile_migrations', } } -}; +}; `, { isPathAbsolute: true } ); @@ -237,7 +237,7 @@ module.exports = { extension: 'ts', directory: __dirname + '/test/jake-util/knexfile_migrations', }, -}; +}; `, { isPathAbsolute: true } ); @@ -272,7 +272,7 @@ development: { directory: __dirname + '/test/jake-util/knexfile_migrations', }, } -}; +}; `, { isPathAbsolute: true } ); diff --git a/test/cli/migrate.spec.js b/test/cli/migrate.spec.js index fbc5e9aa75..13847eba88 100644 --- a/test/cli/migrate.spec.js +++ b/test/cli/migrate.spec.js @@ -56,4 +56,54 @@ describe('migrate:latest', () => { expect(row.name).to.equal('000_create_rule_table.js'); db.close(); }); + + it('CLI Options override knexfile', async () => { + const path = process.cwd() + '/knexfile.js'; + fileHelper.createFile( + path, + ` +module.exports = { + client: 'sqlite3', + connection: { + filename: '${dbPath}', + }, + migrations: { + directory: __dirname + '/test//jake-util/knexfile_migrations', + }, +}; + `, + { isPathAbsolute: true } + ); + + fileHelper.createFile( + 'migrations/subdirectory/000_create_rule_table.js', + ` + exports.up = (knex)=> knex.schema.createTable('rules', (table)=> { + table.string('name'); + }); + exports.down = (knex)=> knex.schema.dropTable('rules'); + `, + { willBeCleanedUp: true, isPathAbsolute: false } + ); + + expect(fileHelper.fileExists(dbPath)).to.equal(false); + await execCommand(`node ${KNEX} migrate:latest \ + --knexpath=../knexfile.js \ + --migrations-directory=${rootDir}/migrations/subdirectory/ \ + create_rule_table`); + expect(fileHelper.fileExists(dbPath)).to.equal(true); + const db = await new sqlite3.Database(dbPath); + + const getPromise = new Promise((resolve, reject) => { + db.get('SELECT name FROM knex_migrations', {}, (err, row) => { + if (err) { + reject(err); + } + resolve(row); + }); + }); + const row = await getPromise; + expect(row.name).to.equal('000_create_rule_table.js'); + db.close(); + }); }); From 132d394ea6d34fe377fb449da854efaa19529783 Mon Sep 17 00:00:00 2001 From: korynunn Date: Tue, 29 Sep 2020 15:22:56 +1000 Subject: [PATCH 02/11] Null out default migrations directory so that it doesnt become defaulted --- bin/cli.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bin/cli.js b/bin/cli.js index fea2dc576f..7ef69b9c43 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -59,9 +59,13 @@ async function initKnex(env, opts) { ); const optionsConfig = parseConfigObj(opts); - const config = merge(resolvedConfig, optionsConfig); + // Migrations directory gets defaulted if it is undefined. + if (!config.migrations.directory) { + config.migrations.directory = null; + } + const knex = require(env.modulePath); return knex(config); } From 952043a1da6fa6994c76facf36f7a1406a79fbd2 Mon Sep 17 00:00:00 2001 From: korynunn Date: Wed, 30 Sep 2020 09:12:00 +1000 Subject: [PATCH 03/11] Only null out miration directory if no config file present --- bin/cli.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/cli.js b/bin/cli.js index 7ef69b9c43..a8c91ac61f 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -62,7 +62,7 @@ async function initKnex(env, opts) { const config = merge(resolvedConfig, optionsConfig); // Migrations directory gets defaulted if it is undefined. - if (!config.migrations.directory) { + if (!env.configPath && !config.migrations.directory) { config.migrations.directory = null; } From 9243790d35f4d7b5a08e0f4169ffbbd8210ca596 Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Sat, 26 Mar 2022 13:57:56 +0100 Subject: [PATCH 04/11] case insensitve override --- bin/utils/cli-config-utils.js | 33 ++++++++++++++++++++++++--------- test/cli/migrate.spec.js | 4 +++- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/bin/utils/cli-config-utils.js b/bin/utils/cli-config-utils.js index a8969d6dac..aa79f0e539 100644 --- a/bin/utils/cli-config-utils.js +++ b/bin/utils/cli-config-utils.js @@ -1,29 +1,44 @@ const { DEFAULT_EXT, DEFAULT_TABLE_NAME } = require('./constants'); const { resolveClientNameWithAliases } = require('../../lib/util/helpers'); -const fs = require('fs'); const path = require('path'); const escalade = require('escalade/sync'); const tildify = require('tildify'); const color = require('colorette'); const argv = require('getopts')(process.argv.slice(2)); +function findCaseInsensitiveProperty(propertyName, object) { + return Object.keys(object).find( + (key) => key.toLowerCase() === propertyName.toLowerCase() + ); +} + function parseConfigObj(opts) { const config = { migrations: {} }; - if (opts.client) { - config.client = opts.client; + const clientProperty = findCaseInsensitiveProperty('client', config); + if (clientProperty) { + config.client = opts[clientProperty]; } - if (opts.connection) { - config.connection = opts.connection; + const connectionProperty = findCaseInsensitiveProperty('connection', config); + if (connectionProperty) { + config.connection = opts[connectionProperty]; } - if (opts.migrationsDirectory) { - config.migrations.directory = opts.migrationsDirectory; + const migrationsDirectoryProperty = findCaseInsensitiveProperty( + 'migrationsDirectory', + config + ); + if (migrationsDirectoryProperty) { + config.migrations.directory = opts[migrationsDirectoryProperty]; } - if (opts.migrationsTableName) { - config.migrations.tableName = opts.migrationsTableName; + const migrationsTableNameProperty = findCaseInsensitiveProperty( + 'migrationsTableName', + config + ); + if (migrationsTableNameProperty) { + config.migrations.tableName = opts[migrationsTableNameProperty]; } return config; diff --git a/test/cli/migrate.spec.js b/test/cli/migrate.spec.js index e63126c960..ae4e379c83 100644 --- a/test/cli/migrate.spec.js +++ b/test/cli/migrate.spec.js @@ -87,9 +87,11 @@ module.exports = { ); expect(fileHelper.fileExists(dbPath)).to.equal(false); + + // We write --Migrations-Directory to test case insensitive. await execCommand(`node ${KNEX} migrate:latest \ --knexpath=../knexfile.js \ - --migrations-directory=${rootDir}/migrations/subdirectory/ \ + --Migrations-Directory=${rootDir}/migrations/subdirectory/ \ create_rule_table`); expect(fileHelper.fileExists(dbPath)).to.equal(true); const db = await new sqlite3.Database(dbPath); From 2468a3df795fdfc5e649ab371228ccf5e65959b5 Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Sat, 26 Mar 2022 14:06:54 +0100 Subject: [PATCH 05/11] bad object in parameter --- bin/utils/cli-config-utils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/utils/cli-config-utils.js b/bin/utils/cli-config-utils.js index aa79f0e539..364702c477 100644 --- a/bin/utils/cli-config-utils.js +++ b/bin/utils/cli-config-utils.js @@ -15,19 +15,19 @@ function findCaseInsensitiveProperty(propertyName, object) { function parseConfigObj(opts) { const config = { migrations: {} }; - const clientProperty = findCaseInsensitiveProperty('client', config); + const clientProperty = findCaseInsensitiveProperty('client', opts); if (clientProperty) { config.client = opts[clientProperty]; } - const connectionProperty = findCaseInsensitiveProperty('connection', config); + const connectionProperty = findCaseInsensitiveProperty('connection', opts); if (connectionProperty) { config.connection = opts[connectionProperty]; } const migrationsDirectoryProperty = findCaseInsensitiveProperty( 'migrationsDirectory', - config + opts ); if (migrationsDirectoryProperty) { config.migrations.directory = opts[migrationsDirectoryProperty]; @@ -35,7 +35,7 @@ function parseConfigObj(opts) { const migrationsTableNameProperty = findCaseInsensitiveProperty( 'migrationsTableName', - config + opts ); if (migrationsTableNameProperty) { config.migrations.tableName = opts[migrationsTableNameProperty]; From 08ef78f40cc3c8f17708663aee81a75f3f8faefe Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Sat, 26 Mar 2022 14:22:02 +0100 Subject: [PATCH 06/11] modify test --- test/cli/migrate.spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/cli/migrate.spec.js b/test/cli/migrate.spec.js index ae4e379c83..4cb39cd640 100644 --- a/test/cli/migrate.spec.js +++ b/test/cli/migrate.spec.js @@ -88,10 +88,9 @@ module.exports = { expect(fileHelper.fileExists(dbPath)).to.equal(false); - // We write --Migrations-Directory to test case insensitive. await execCommand(`node ${KNEX} migrate:latest \ --knexpath=../knexfile.js \ - --Migrations-Directory=${rootDir}/migrations/subdirectory/ \ + --migrations-directory=${rootDir}/migrations/subdirectory/ \ create_rule_table`); expect(fileHelper.fileExists(dbPath)).to.equal(true); const db = await new sqlite3.Database(dbPath); From 0da14f15cc0a83c4d89dcf0e851f51436531fc95 Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Tue, 29 Mar 2022 20:31:48 +0200 Subject: [PATCH 07/11] revert and add migration table name to test --- bin/utils/cli-config-utils.js | 26 ++++++++------------------ test/cli/migrate.spec.js | 1 + 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/bin/utils/cli-config-utils.js b/bin/utils/cli-config-utils.js index 364702c477..fb98ffcd76 100644 --- a/bin/utils/cli-config-utils.js +++ b/bin/utils/cli-config-utils.js @@ -15,30 +15,20 @@ function findCaseInsensitiveProperty(propertyName, object) { function parseConfigObj(opts) { const config = { migrations: {} }; - const clientProperty = findCaseInsensitiveProperty('client', opts); - if (clientProperty) { - config.client = opts[clientProperty]; + if (opts.client) { + config.client = opts.client; } - const connectionProperty = findCaseInsensitiveProperty('connection', opts); - if (connectionProperty) { - config.connection = opts[connectionProperty]; + if (opts.connection) { + config.connection = opts.connection; } - const migrationsDirectoryProperty = findCaseInsensitiveProperty( - 'migrationsDirectory', - opts - ); - if (migrationsDirectoryProperty) { - config.migrations.directory = opts[migrationsDirectoryProperty]; + if (opts.migrationsDirectory) { + config.migrations.directory = opts.migrationsDirectory; } - const migrationsTableNameProperty = findCaseInsensitiveProperty( - 'migrationsTableName', - opts - ); - if (migrationsTableNameProperty) { - config.migrations.tableName = opts[migrationsTableNameProperty]; + if (opts.migrationsTableName) { + config.migrations.tableName = opts.migrationsTableName; } return config; diff --git a/test/cli/migrate.spec.js b/test/cli/migrate.spec.js index 4cb39cd640..d0c7306706 100644 --- a/test/cli/migrate.spec.js +++ b/test/cli/migrate.spec.js @@ -91,6 +91,7 @@ module.exports = { await execCommand(`node ${KNEX} migrate:latest \ --knexpath=../knexfile.js \ --migrations-directory=${rootDir}/migrations/subdirectory/ \ + --migrations-table-name=migration-table \ create_rule_table`); expect(fileHelper.fileExists(dbPath)).to.equal(true); const db = await new sqlite3.Database(dbPath); From bc0b56ad085ba62032f02382ee294dd15d10415c Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Tue, 29 Mar 2022 20:45:02 +0200 Subject: [PATCH 08/11] modify test to createTable --- bin/cli.js | 4 +--- test/cli/cli-test-utils.js | 10 ++++++++++ test/cli/esm-interop.spec.js | 10 +--------- test/cli/migrate.spec.js | 13 +++++++++---- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 9a8424bf8d..63ac5ede74 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -52,12 +52,10 @@ async function initKnex(env, opts) { ); } - const knexFileConfig = env.configPath + env.configuration = env.configPath ? await openKnexfile(env.configPath) : mkConfigObj(opts); - env.configuration = knexFileConfig; - const resolvedConfig = resolveEnvironmentConfig( opts, env.configuration, diff --git a/test/cli/cli-test-utils.js b/test/cli/cli-test-utils.js index 39b69b0b5b..b45339529b 100644 --- a/test/cli/cli-test-utils.js +++ b/test/cli/cli-test-utils.js @@ -75,10 +75,20 @@ function setupFileHelper() { return fileHelper; } +function createTable(db, ddl) { + return new Promise((resolve, reject) => + db.exec(`create TABLE if not exists ${ddl};`, (err) => { + if (err) reject(err); + else resolve(); + }) + ); +} + module.exports = { expectContentMatchesStub, getRootDir, migrationStubOptionSetup, seedStubOptionSetup, setupFileHelper, + createTable, }; diff --git a/test/cli/esm-interop.spec.js b/test/cli/esm-interop.spec.js index f3b2480028..3607f74350 100644 --- a/test/cli/esm-interop.spec.js +++ b/test/cli/esm-interop.spec.js @@ -5,6 +5,7 @@ const fs = require('fs'); const { execCommand } = require('cli-testlab'); const sqlite3 = require('@vscode/sqlite3'); const semver = require('semver'); +const { createTable } = require('./cli-test-utils'); const KNEX = path.normalize(__dirname + '/../../bin/cli.js'); const TEST_BASE = '../test/jake-util'; @@ -734,15 +735,6 @@ describe('esm interop and mjs support', () => { } }); -function createTable(db, ddl) { - return new Promise((resolve, reject) => - db.exec(`create TABLE if not exists ${ddl};`, (err) => { - if (err) reject(err); - else resolve(); - }) - ); -} - function getSchema(db) { return new Promise((resolve, reject) => { db.all('SELECT name from SQLITE_MASTER', (err, rows) => { diff --git a/test/cli/migrate.spec.js b/test/cli/migrate.spec.js index d0c7306706..c253142c10 100644 --- a/test/cli/migrate.spec.js +++ b/test/cli/migrate.spec.js @@ -4,7 +4,11 @@ const path = require('path'); const sqlite3 = require('@vscode/sqlite3'); const { expect } = require('chai'); const { execCommand } = require('cli-testlab'); -const { getRootDir, setupFileHelper } = require('./cli-test-utils'); +const { + getRootDir, + setupFileHelper, + createTable, +} = require('./cli-test-utils'); const KNEX = path.normalize(__dirname + '/../../bin/cli.js'); @@ -88,15 +92,17 @@ module.exports = { expect(fileHelper.fileExists(dbPath)).to.equal(false); + const db = await new sqlite3.Database(dbPath); + await createTable(db, `migration-table`); + await execCommand(`node ${KNEX} migrate:latest \ --knexpath=../knexfile.js \ --migrations-directory=${rootDir}/migrations/subdirectory/ \ --migrations-table-name=migration-table \ create_rule_table`); expect(fileHelper.fileExists(dbPath)).to.equal(true); - const db = await new sqlite3.Database(dbPath); - const getPromise = new Promise((resolve, reject) => { + const row = await new Promise((resolve, reject) => { db.get('SELECT name FROM knex_migrations', {}, (err, row) => { if (err) { reject(err); @@ -104,7 +110,6 @@ module.exports = { resolve(row); }); }); - const row = await getPromise; expect(row.name).to.equal('000_create_rule_table.js'); db.close(); }); From 5a35c54c1fb479ec5b4721cfc946025c4b696dc4 Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Tue, 29 Mar 2022 21:08:16 +0200 Subject: [PATCH 09/11] modify test to createTable again --- test/cli/migrate.spec.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/cli/migrate.spec.js b/test/cli/migrate.spec.js index c253142c10..48cda39ce6 100644 --- a/test/cli/migrate.spec.js +++ b/test/cli/migrate.spec.js @@ -92,16 +92,14 @@ module.exports = { expect(fileHelper.fileExists(dbPath)).to.equal(false); - const db = await new sqlite3.Database(dbPath); - await createTable(db, `migration-table`); - await execCommand(`node ${KNEX} migrate:latest \ --knexpath=../knexfile.js \ --migrations-directory=${rootDir}/migrations/subdirectory/ \ - --migrations-table-name=migration-table \ + --migrations-table-name=migration_table \ create_rule_table`); expect(fileHelper.fileExists(dbPath)).to.equal(true); + const db = await new sqlite3.Database(dbPath); const row = await new Promise((resolve, reject) => { db.get('SELECT name FROM knex_migrations', {}, (err, row) => { if (err) { From 39bd21a6522021cdf34b7bdb4a4f27b2e140979d Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Tue, 29 Mar 2022 21:15:15 +0200 Subject: [PATCH 10/11] rename migration table --- test/cli/migrate.spec.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/cli/migrate.spec.js b/test/cli/migrate.spec.js index 48cda39ce6..ecd060c35a 100644 --- a/test/cli/migrate.spec.js +++ b/test/cli/migrate.spec.js @@ -4,11 +4,7 @@ const path = require('path'); const sqlite3 = require('@vscode/sqlite3'); const { expect } = require('chai'); const { execCommand } = require('cli-testlab'); -const { - getRootDir, - setupFileHelper, - createTable, -} = require('./cli-test-utils'); +const { getRootDir, setupFileHelper } = require('./cli-test-utils'); const KNEX = path.normalize(__dirname + '/../../bin/cli.js'); @@ -101,7 +97,7 @@ module.exports = { const db = await new sqlite3.Database(dbPath); const row = await new Promise((resolve, reject) => { - db.get('SELECT name FROM knex_migrations', {}, (err, row) => { + db.get('SELECT name FROM migration_table', {}, (err, row) => { if (err) { reject(err); } From f931ba347937695c695b7dbf0f88766ea167ee59 Mon Sep 17 00:00:00 2001 From: Olivier Cavadenti Date: Tue, 29 Mar 2022 21:58:06 +0200 Subject: [PATCH 11/11] update option texts --- bin/cli.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/bin/cli.js b/bin/cli.js index 63ac5ede74..c3e3268d25 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -149,16 +149,10 @@ function invoke() { .option('--knexfile [path]', 'Specify the knexfile path.') .option('--knexpath [path]', 'Specify the path to knex instance.') .option('--cwd [path]', 'Specify the working directory.') - .option('--client [name]', 'Set DB client without a knexfile.') - .option('--connection [address]', 'Set DB connection without a knexfile.') - .option( - '--migrations-directory [path]', - 'Set migrations directory without a knexfile.' - ) - .option( - '--migrations-table-name [path]', - 'Set migrations table name without a knexfile.' - ) + .option('--client [name]', 'Set DB client.') + .option('--connection [address]', 'Set DB connection.') + .option('--migrations-directory [path]', 'Set migrations directory.') + .option('--migrations-table-name [path]', 'Set migrations table name.') .option( '--env [name]', 'environment, default: process.env.NODE_ENV || development'