Skip to content

Commit

Permalink
db-migrate: rename knex-specific functions (#1372)
Browse files Browse the repository at this point in the history
Renames `withDatabase()` and `connect()` functions in the database migrator to make it clearer they are directly using knex.

In line with #1364, this renaming will:

* make intention/use of these functions clearer
* ease finding of knex-related code across the repo
* aid deprecating and finally removing knex dependency
  • Loading branch information
alxndrsn authored Jan 18, 2025
1 parent 6324ebe commit c7b2af2
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 15 deletions.
4 changes: 2 additions & 2 deletions lib/bin/check-migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
// including this file, may be copied, modified, propagated, or distributed
// except according to the terms contained in the LICENSE file.

const { withDatabase, checkMigrations } = require('../model/migrate');
const { withKnex, checkMigrations } = require('../model/migrate');

(async () => {
try {
await withDatabase(require('config').get('default.database'))(checkMigrations);
await withKnex(require('config').get('default.database'))(checkMigrations);
} catch (err) {
console.error('Error:', err.message);
process.exit(1);
Expand Down
4 changes: 2 additions & 2 deletions lib/bin/check-open-db-queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
// including this file, may be copied, modified, propagated, or distributed
// except according to the terms contained in the LICENSE file.

const { withDatabase } = require('../model/migrate');
const { withKnex } = require('../model/migrate');

(async () => {
try {
const { rows } = await withDatabase(require('config').get('default.database'))((db) =>
const { rows } = await withKnex(require('config').get('default.database'))((db) =>
db.raw('SELECT COUNT(*) FROM pg_stat_activity WHERE usename=CURRENT_USER'));
const queryCount = rows[0].count - 1; // the count query will appear as one of the open queries

Expand Down
4 changes: 2 additions & 2 deletions lib/bin/run-migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
// including this file, may be copied, modified, propagated, or distributed
// except according to the terms contained in the LICENSE file.

const { withDatabase, migrate } = require('../model/migrate');
const { withKnex, migrate } = require('../model/migrate');

(async () => {
try {
await withDatabase(require('config').get('default.database'))(migrate);
await withKnex(require('config').get('default.database'))(migrate);
} catch (err) {
console.error('Error:', err.message);
process.exit(1);
Expand Down
8 changes: 4 additions & 4 deletions lib/model/migrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ const knex = require('knex');
const { knexConnection } = require('../util/db');

// Connects to the postgres database specified in configuration and returns it.
const connect = (config) => knex({ client: 'pg', connection: knexConnection(config) });
const knexConnect = (config) => knex({ client: 'pg', connection: knexConnection(config) });

// Connects to a database, passes it to a function for operations, then ensures its closure.
const withDatabase = (config) => (mutator) => {
const db = connect(config);
const withKnex = (config) => (mutator) => {
const db = knexConnect(config);
return mutator(db).finally(() => db.destroy());
};

Expand All @@ -33,5 +33,5 @@ const checkMigrations = (db) => db.migrate.list({ directory: `${__dirname}/migra
process.exitCode = 1;
});

module.exports = { checkMigrations, connect, withDatabase, migrate };
module.exports = { checkMigrations, knexConnect, withKnex, migrate };

2 changes: 1 addition & 1 deletion lib/model/migrations/20191231-02-add-schema-storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const up = async (db) => {
// this config hardcoding would be dangerous with tests except that
// tests will never invoke this path.
const config = require('config').get('default.database');
const db2 = require('../migrate').connect(config);
const db2 = require('../migrate').knexConnect(config);
return db2.select('projectId', 'xmlFormId').from('forms').where({ currentDefId: formDefId })
.then(([{ projectId, xmlFormId }]) => {
process.stderr.write(`\n!!!!\nThe database upgrade to v0.8 has failed because the Form '${xmlFormId}' in Project ${projectId} has an invalid schema. It tries to bind multiple instance nodes at the path ${path}.\n!!!!\n\n`);
Expand Down
4 changes: 2 additions & 2 deletions test/integration/other/migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ const { testContainerFullTrx, testServiceFullTrx } = require('../setup');
const { sql } = require('slonik');
const { createReadStream } = require('fs');
const { Actor, Config } = require(appRoot + '/lib/model/frames');
const { withDatabase } = require(appRoot + '/lib/model/migrate');
const { withKnex } = require(appRoot + '/lib/model/migrate');
const { exhaust } = require(appRoot + '/lib/worker/worker');

const testData = require('../../data/xml');
const populateUsers = require('../fixtures/01-users');
const populateForms = require('../fixtures/02-forms');
const { getFormFields } = require('../../../lib/data/schema');

const withTestDatabase = withDatabase(config.get('test.database'));
const withTestDatabase = withKnex(config.get('test.database'));
const migrationsDir = appRoot + '/lib/model/migrations';
const upToMigration = (toName, inclusive = true) => withTestDatabase(async (migrator) => {
await migrator.raw('drop owned by current_user');
Expand Down
4 changes: 2 additions & 2 deletions test/integration/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const testData = require('../data/xml');

// knex things.
const config = require('config');
const { connect } = require(appRoot + '/lib/model/migrate');
const { knexConnect } = require(appRoot + '/lib/model/migrate');

// slonik connection pool
const { slonikPool } = require(appRoot + '/lib/external/slonik');
Expand Down Expand Up @@ -72,7 +72,7 @@ const populate = (container, [ head, ...tail ] = fixtures) =>
// this hook won't run if `test-unit` is called, as this directory is skipped
// in that case.
const initialize = async () => {
const migrator = connect(config.get('test.database'));
const migrator = knexConnect(config.get('test.database'));
const { log } = console;
try {
await migrator.raw('drop owned by current_user');
Expand Down

0 comments on commit c7b2af2

Please sign in to comment.