db-migrations: use pg for new migrations

.github/workflows/db-partial-migrations.yml

@@ -0,0 +1,50 @@
+# This is a test of legacy migrations in various states.
+# This test workflow should not be required after knex has been removed.
+
+name: Partial Database Migrations
+
+on:
+  push:
+  pull_request:
+
+jobs:
+  db-partial-migration-tests:
+    strategy:
+      matrix:
+        previous-migrations:
+        - none
+        - legacy-first-1
+        - legacy-first-10
+        - legacy-first-100
+        - legacy-all
+        - new-first-1-as-legacy
+        - new-first-10-as-legacy
+        - all
+    timeout-minutes: 2
+    # TODO should we use the same container as circle & central?
+    runs-on: ubuntu-latest
+    services:
+      # see: https://docs.github.com/en/[email protected]/actions/using-containerized-services/creating-postgresql-service-containers
+      postgres:
+        image: postgres:14.10
+        env:
+          POSTGRES_PASSWORD: odktest
+        ports:
+          - 5432:5432
+        # Set health checks to wait until postgres has started
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+    steps:
+    - run: sudo apt-get install tree
+    - uses: actions/checkout@v4
+    - name: Set node version
+      uses: actions/setup-node@v4
+      with:
+        node-version: 22.14.0
+        cache: 'npm'
+    - run: npm ci
+    - run: node lib/bin/create-docker-databases.js
+    - run: test/db-partial-migrations/test.sh ${{ matrix.previous-migrations }}

Makefile

@@ -73,6 +73,9 @@ fake-s3-server-persistent:
 .PHONY: migrations
 migrations: node_version
 	node lib/bin/run-migrations.js
+.PHONY: migrations-legacy
+migrations-legacy: node_version
+	node lib/bin/run-migrations-legacy.js
 
 
 ################################################################################

lib/bin/run-migrations-legacy.js

@@ -0,0 +1,20 @@
+// Copyright 2022 ODK Central Developers
+// See the NOTICE file at the top-level directory of this distribution and at
+// https://github.com/getodk/central-backend/blob/master/NOTICE.
+// This file is part of ODK Central. It is subject to the license terms in
+// the LICENSE file found in the top-level directory of this distribution and at
+// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central,
+// including this file, may be copied, modified, propagated, or distributed
+// except according to the terms contained in the LICENSE file.
+
+const { withKnex, migrate } = require('../model/knex-migrator-legacy');
+
+(async () => {
+  try {
+    await withKnex(require('config').get('default.database'))(migrate);
+  } catch (err) {
+    if (err.detail) console.error('Error:', err.message, `(${err.detail})`);
+    else            console.error('Error:', err.message); // eslint-disable-line no-multi-spaces
+    process.exit(1);
+  }
+})();

lib/bin/run-migrations.js

@@ -7,11 +7,13 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const { withKnex, migrate } = require('../model/knex-migrator');
+const { withKnex, migrate: knexMigrate } = require('../model/knex-migrator');
+const { withPg, migrate: pgMigrate } = require('../model/pg-migrator');
 
 (async () => {
   try {
-    await withKnex(require('config').get('default.database'))(migrate);
+    await withKnex(require('config').get('default.database'))(knexMigrate);
+    await withPg(require('config').get('default.database'))(pgMigrate);
   } catch (err) {
     if (err.detail) console.error('Error:', err.message, `(${err.detail})`);
     else            console.error('Error:', err.message); // eslint-disable-line no-multi-spaces

lib/model/knex-migrator-legacy.js

@@ -0,0 +1,29 @@
+// Copyright 2017 ODK Central Developers
+// See the NOTICE file at the top-level directory of this distribution and at
+// https://github.com/getodk/central-backend/blob/master/NOTICE.
+// This file is part of ODK Central. It is subject to the license terms in
+// the LICENSE file found in the top-level directory of this distribution and at
+// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central,
+// including this file, may be copied, modified, propagated, or distributed
+// except according to the terms contained in the LICENSE file.
+//
+// This is a variety of functions helpful for connecting to and performing
+// top-level operations with a database, like migrations.
+
+const knex = require('knex');
+const { knexConnection } = require('../util/db');
+
+// Connects to the postgres database specified in configuration and returns it.
+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 withKnex = (config) => (mutator) => {
+  const db = knexConnect(config);
+  return mutator(db).finally(() => db.destroy());
+};
+
+// Given a database, initiates migrations on it.
+const migrate = (db) => db.migrate.latest({ directory: `${__dirname}/migrations` });
+
+module.exports = { knexConnect, withKnex, migrate };
+

lib/model/knex-migrator.js

@@ -23,7 +23,12 @@ const withKnex = (config) => (mutator) => {
 };
 
 // Given a database, initiates migrations on it.
-const migrate = (db) => db.migrate.latest({ directory: `${__dirname}/migrations` });
+// disableMigrationsListValidation: because knex-migrator now shares a DB table
+// with pg-migrator, knex's built-in check that all migrations found in the db
+// have corresponding migration files in `directory` will fail.
+// REVIEW note that we could maintain this check if we duplicate migration
+// tables and store pg migrations separately.
+const migrate = (db) => db.migrate.latest({ directory: `${__dirname}/migrations/legacy`, disableMigrationsListValidation: true });
 
 module.exports = { knexConnect, withKnex, migrate };
 

lib/model/migrations/20230802-01-delete-orphan-submissions.js

@@ -9,7 +9,7 @@
 
 // Delete draft Submissions that don't have any definition - cb#911
 const up = async (db) => {
-  await db.raw(`
+  await db.query(`
   DELETE FROM submissions s 
   WHERE draft AND id IN ( 
     SELECT s.id FROM submissions s 

lib/model/migrations/20230818-01-remove-schemaId-from-dsPropertyFields.js

@@ -8,7 +8,7 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw('ALTER TABLE ds_property_fields DROP COLUMN "schemaId"');
+  await db.query('ALTER TABLE ds_property_fields DROP COLUMN "schemaId"');
 };
 
 const down = () => {};

lib/model/migrations/20230824-01-add-entity-version.js

@@ -8,10 +8,10 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw('ALTER TABLE entity_defs ADD COLUMN version INT4 NOT NULL DEFAULT 1');
+  await db.query('ALTER TABLE entity_defs ADD COLUMN version INT4 NOT NULL DEFAULT 1');
 
   // Sets the correct version number for existing entities
-  await db.raw(`
+  await db.query(`
   UPDATE entity_defs SET "version" = vt.rownumber
   FROM (
     SELECT ROW_NUMBER() OVER(PARTITION BY "entityId" ORDER BY id ) rownumber, id FROM entity_defs
@@ -20,6 +20,6 @@ const up = async (db) => {
 `);
 };
 
-const down = (db) => db.raw('ALTER TABLE entity_defs DROP COLUMN version');
+const down = (db) => db.query('ALTER TABLE entity_defs DROP COLUMN version');
 
 module.exports = { up, down };

lib/model/migrations/20230830-01-remove-entity-label-from-audits.js

@@ -7,7 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const up = (db) => db.raw(`
+const up = (db) => db.query(`
 update audits set details=(details #- '{entity,label}')
   where action='entity.create'`);
 

lib/model/migrations/20230907-01-opened-form-verb.js

@@ -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 up = (db) => db.raw(`
+const up = (db) => db.query(`
 UPDATE roles SET verbs = REPLACE(verbs ::TEXT, 'form', 'open_form')::JSONB
 WHERE system IN ('app-user', 'formview', 'formfill', 'pub-link')`);
 
-const down = (db) => db.raw(`
+const down = (db) => db.query(`
 UPDATE roles SET verbs = REPLACE(verbs ::TEXT, 'open_form', 'form')::JSONB
 WHERE system IN ('app-user', 'formview', 'formfill', 'pub-link')`);
 

lib/model/migrations/20231002-01-add-conflict-details.js

@@ -8,11 +8,11 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw(`CREATE TYPE "conflictType" AS ENUM ('soft', 'hard')`);
+  await db.query(`CREATE TYPE "conflictType" AS ENUM ('soft', 'hard')`);
 
-  await db.raw('ALTER TABLE entities ADD COLUMN conflict "conflictType" NULL');
+  await db.query('ALTER TABLE entities ADD COLUMN conflict "conflictType" NULL');
 
-  await db.raw(`ALTER TABLE entity_defs 
+  await db.query(`ALTER TABLE entity_defs 
     ADD COLUMN "dataReceived" JSONB NOT NULL DEFAULT '{}'::jsonb,
 
     -- null means, it's a first version
@@ -25,13 +25,13 @@ const up = async (db) => {
   `);
 
   // Sets the value for "dataReceived" and "baseVersion" for existing row
-  await db.raw(`UPDATE entity_defs SET "dataReceived" = data || jsonb_build_object('label', "label"), "baseVersion" = CASE WHEN version > 1 THEN version - 1 ELSE NULL END`);
+  await db.query(`UPDATE entity_defs SET "dataReceived" = data || jsonb_build_object('label', "label"), "baseVersion" = CASE WHEN version > 1 THEN version - 1 ELSE NULL END`);
 
 };
 
 const down = async (db) => {
-  await db.raw('ALTER TABLE entities DROP COLUMN conflict');
-  await db.raw('ALTER TABLE entity_defs DROP COLUMN "dataReceived", DROP COLUMN "baseVersion"');
+  await db.query('ALTER TABLE entities DROP COLUMN conflict');
+  await db.query('ALTER TABLE entity_defs DROP COLUMN "dataReceived", DROP COLUMN "baseVersion"');
 };
 
 module.exports = { up, down };

lib/model/migrations/20231013-01-change-entity-error-action.js

@@ -8,12 +8,12 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw('UPDATE audits SET "action" = \'entity.error\' WHERE "action" = \'entity.create.error\'');
+  await db.query('UPDATE audits SET "action" = \'entity.error\' WHERE "action" = \'entity.create.error\'');
 };
 
 const down = async (db) => {
   // will set any error back to create error, which isn't necessarily right
-  await db.raw('UPDATE audits SET "action" = \'entity.create.error\' WHERE "action" = \'entity.error\'');
+  await db.query('UPDATE audits SET "action" = \'entity.create.error\' WHERE "action" = \'entity.error\'');
 };
 
 module.exports = { up, down };

lib/model/migrations/20231208-01-dataset-form-def-actions.js

@@ -8,12 +8,12 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw('ALTER TABLE dataset_form_defs ADD COLUMN actions jsonb');
-  await db.raw(`UPDATE dataset_form_defs SET actions = '["create"]'`);
-  await db.raw('ALTER TABLE dataset_form_defs ALTER COLUMN actions SET NOT NULL');
+  await db.query('ALTER TABLE dataset_form_defs ADD COLUMN actions jsonb');
+  await db.query(`UPDATE dataset_form_defs SET actions = '["create"]'`);
+  await db.query('ALTER TABLE dataset_form_defs ALTER COLUMN actions SET NOT NULL');
 };
 
 const down = (db) =>
-  db.raw('ALTER TABLE dataset_form_defs DROP COLUMN actions');
+  db.query('ALTER TABLE dataset_form_defs DROP COLUMN actions');
 
 module.exports = { up, down };

lib/model/migrations/20240215-01-entity-delete-verb.js

@@ -7,12 +7,12 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const up = (db) => db.raw(`
+const up = (db) => db.query(`
 UPDATE roles
 SET verbs = verbs || '["entity.delete"]'::jsonb
 WHERE system IN ('admin', 'manager')`);
 
-const down = (db) => db.raw(`
+const down = (db) => db.query(`
 UPDATE roles
 SET verbs = verbs - 'entity.delete'
 WHERE system IN ('admin', 'manager')`);

lib/model/migrations/20240215-02-dedupe-verbs.js

@@ -7,7 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const up = (db) => db.raw(`
+const up = (db) => db.query(`
 UPDATE roles
 SET verbs = (verbs - 'submission.update') || '["submission.update"]'::jsonb
 WHERE system IN ('admin', 'manager')`);

lib/model/migrations/20240312-01-add-dataset-create-verb.js

@@ -7,13 +7,13 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
-const up = (db) => db.raw(`
+const up = (db) => db.query(`
 UPDATE roles
 SET verbs = verbs || '["dataset.create"]'::jsonb
 WHERE system in ('admin', 'manager')
 `);
 
-const down = (db) => db.raw(`
+const down = (db) => db.query(`
 UPDATE roles
 SET verbs = (verbs - 'dataset.create')
 WHERE system in ('admin', 'manager')

lib/model/migrations/20240322-01-add-entity-source-index-to-audits.js

@@ -8,13 +8,13 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw("CREATE INDEX audits_details_entity_def_index ON audits USING HASH (((details ->> 'entityDefId')::INTEGER))");
-  await db.raw("CREATE INDEX audits_details_entity_source_index ON audits USING HASH (((details ->> 'sourceId')::INTEGER))");
+  await db.query("CREATE INDEX audits_details_entity_def_index ON audits USING HASH (((details ->> 'entityDefId')::INTEGER))");
+  await db.query("CREATE INDEX audits_details_entity_source_index ON audits USING HASH (((details ->> 'sourceId')::INTEGER))");
 };
 
 const down = async (db) => {
-  await db.raw('DROP INDEX audits_details_entity_def_index');
-  await db.raw('DROP INDEX audits_details_entity_source_index');
+  await db.query('DROP INDEX audits_details_entity_def_index');
+  await db.query('DROP INDEX audits_details_entity_source_index');
 };
 
 module.exports = { up, down };

lib/model/migrations/20240515-01-entity-tz-precision.js

@@ -8,11 +8,11 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw('ALTER TABLE entities ALTER COLUMN "deletedAt" TYPE timestamptz(3)');
+  await db.query('ALTER TABLE entities ALTER COLUMN "deletedAt" TYPE timestamptz(3)');
 };
 
 const down = async (db) => {
-  await db.raw('ALTER TABLE entities ALTER COLUMN "deletedAt" TYPE timestamp');
+  await db.query('ALTER TABLE entities ALTER COLUMN "deletedAt" TYPE timestamp');
 };
 
 module.exports = { up, down };

lib/model/migrations/20240607-01-add-offline-entity-branch-trunk-info.js

@@ -8,13 +8,13 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw(`ALTER TABLE entity_defs
+  await db.query(`ALTER TABLE entity_defs
     ADD COLUMN "branchId" UUID,
     ADD COLUMN "trunkVersion" INT4,
     ADD COLUMN "branchBaseVersion" INT4`);
 };
 
-const down = (db) => db.raw(`ALTER TABLE entity_defs
+const down = (db) => db.query(`ALTER TABLE entity_defs
   DROP COLUMN "branchId",
   DROP COLUMN "trunkVersion",
   DROP COLUMN "branchBaseVersion"

lib/model/migrations/20240607-02-add-submission-backlog.js

@@ -8,7 +8,7 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw(`CREATE TABLE entity_submission_backlog (
+  await db.query(`CREATE TABLE entity_submission_backlog (
     "submissionId" INT4 NOT NULL,
     "submissionDefId" INT4 NOT NULL,
     "branchId" UUID NOT NULL,
@@ -26,6 +26,6 @@ const up = async (db) => {
   )`);
 };
 
-const down = (db) => db.raw('DROP TABLE entity_submission_backlog');
+const down = (db) => db.query('DROP TABLE entity_submission_backlog');
 
 module.exports = { up, down };

lib/model/migrations/20240715-01-backlog-add-event-entityuuid.js

@@ -8,7 +8,7 @@
 // except according to the terms contained in the LICENSE file.
 
 const up = async (db) => {
-  await db.raw(`ALTER TABLE entity_submission_backlog
+  await db.query(`ALTER TABLE entity_submission_backlog
     ADD COLUMN "auditId" INT4 NOT NULL,
     ADD COLUMN "entityUuid" UUID NOT NULL,
     ADD CONSTRAINT fk_audit_id
@@ -17,7 +17,7 @@ const up = async (db) => {
       ON DELETE CASCADE`);
 };
 
-const down = (db) => db.raw(`ALTER TABLE entity_submission_backlog
+const down = (db) => db.query(`ALTER TABLE entity_submission_backlog
   DROP COLUMN "auditId",
   DROP COLUMN "entityUuid"
 `);