Repository: rickbergfalk/sqlpad (js)

Issue:
Add --migration command
Add a command line flag to run migrations then exit the process, returning proper error code. 

Today migrations are run on startup, which may not be preferred if launching multiple SQLPad instances with a new version, which could create a race condition.

Instead, one SQLPad instance should be launched, with others starting once complete.

Pull request:
Add --migration command and option to disable automigrate
Adds `--migration` command to run migrations then exit, as well as configuration `SQLPAD_DB_AUTOMIGRATE` to disable automigration (turned on by default).

These options allow greater control over when migrations are run, which is particularly important when multiple instances of SQLPad are running.

Closes #749 

Patch:
diff --git a/config-example.ini b/config-example.ini
index 9c818e25..f8cbbc2e 100644
--- a/config-example.ini
+++ b/config-example.ini
@@ -25,6 +25,9 @@ cookieName="sqlpad.sid"
 ; Secret used to sign cookies
 cookieSecret="secret-used-to-sign-cookies-please-set-and-make-strong"
 
+; Enable/disable automigration on SQLPad process start. Disable by setting to `false`
+dbAutomigrate="true"
+
 ; Directory to store SQLPad embedded database content. This includes queries, users, query result cache files, etc.
 dbPath=""
 
diff --git a/config-example.json b/config-example.json
index f279197d..444becc5 100644
--- a/config-example.json
+++ b/config-example.json
@@ -8,6 +8,7 @@
   "certPath": "",
   "cookieName": "sqlpad.sid",
   "cookieSecret": "secret-used-to-sign-cookies-please-set-and-make-strong",
+  "dbAutomigrate": true,
   "dbPath": "",
   "defaultConnectionId": "",
   "disableAuth": false,
diff --git a/docs/configuration.md b/docs/configuration.md
index e67a47b6..12a2218a 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -76,6 +76,29 @@ Secret used to sign cookies
 - Env: `SQLPAD_COOKIE_SECRET`
 - Default: `secret-used-to-sign-cookies-please-set-and-make-strong`
 
+## migrate
+
+Run migrations on SQLPad process start, then exits. Use to control when migrations are run, particularly of use when running multiple instances of SQLPad.
+
+This option is most likely useful as a cli flag, but it can be specified via file as well.
+
+Example:
+
+```sh
+node server.js --config path/to/file.ext --migrate
+```
+
+- Key: `migrate`
+- Env: `SQLPAD_MIGRATE`
+
+## dbAutomigrate
+
+Enable/disable automigration on SQLPad process start. Disable by setting to `false`
+
+- Key: `dbAutomigrate`
+- Env: `SQLPAD_DB_AUTOMIGRATE`
+- Default: `true`
+
 ## dbPath
 
 Directory to store SQLPad embedded database content. This includes queries, users, query result cache files, etc.
diff --git a/server/lib/cli-flow.js b/server/lib/cli-flow.js
deleted file mode 100644
index 23d396ab..00000000
--- a/server/lib/cli-flow.js
+++ /dev/null
@@ -1,38 +0,0 @@
-const minimist = require('minimist');
-const argv = minimist(process.argv.slice(2));
-const appLog = require('./app-log');
-const packageJson = require('../package.json');
-const configItems = require('./config/config-items');
-
-const keyLengths = configItems.map((item) => item.key.length);
-const keyPadding = Math.max(...keyLengths) + 2;
-
-const helpText = `
-SQLPad version:  ${packageJson.version}
-
-CLI examples: 
-
-  sqlpad --dbPath ../db --port 3010 --baseUrl /sqlpad
-  node server.js --dbPath ../db --port 3010 --baseUrl /sqlpad
-  node server.js --config path/to/file.json
-  node server.js --config path/to/file.ini
-
-Options:
-
-${configItems
-  .map((option) => {
-    return `  --${option.key.padEnd(keyPadding)}${option.description}\n`;
-  })
-  .join('')}`;
-
-// If version is requested show version then exit
-if (argv.v || argv.version) {
-  appLog.info('SQLPad version %s', packageJson.version);
-  process.exit();
-}
-
-// If help is requested show help
-if (argv.h || argv.help) {
-  appLog.info(helpText);
-  process.exit();
-}
diff --git a/server/lib/config/config-items.js b/server/lib/config/config-items.js
index d9aa1cef..8dceff09 100644
--- a/server/lib/config/config-items.js
+++ b/server/lib/config/config-items.js
@@ -9,6 +9,11 @@ const configItems = [
     envVar: 'SQLPAD_CONFIG',
     default: '',
   },
+  {
+    key: 'migrate',
+    envVar: 'SQLPAD_MIGRATE',
+    default: '',
+  },
   {
     key: 'cookieName',
     envVar: 'SQLPAD_COOKIE_NAME',
@@ -54,6 +59,11 @@ const configItems = [
     envVar: 'SQLPAD_DB_PATH',
     default: '',
   },
+  {
+    key: 'dbAutomigrate',
+    envVar: 'SQLPAD_DB_AUTOMIGRATE',
+    default: true,
+  },
   {
     key: 'baseUrl',
     envVar: 'SQLPAD_BASE_URL',
diff --git a/server/lib/migrate.js b/server/lib/make-migrator.js
similarity index 66%
rename from server/lib/migrate.js
rename to server/lib/make-migrator.js
index 3a330637..a8bf9b38 100644
--- a/server/lib/migrate.js
+++ b/server/lib/make-migrator.js
@@ -1,8 +1,7 @@
 const path = require('path');
 const Umzug = require('umzug');
 
-function runMigrations(config, appLog, nedb, sequelizeInstance) {
-  appLog.info('Running migrations');
+function makeMigrator(config, appLog, nedb, sequelizeInstance) {
   const umzug = new Umzug({
     storage: 'sequelize',
     storageOptions: {
@@ -26,7 +25,16 @@ function runMigrations(config, appLog, nedb, sequelizeInstance) {
     },
   });
 
-  return umzug.up();
+  return {
+    migrate() {
+      return umzug.up();
+    },
+    async schemaUpToDate() {
+      const pending = await umzug.pending();
+      const upToDate = pending.length === 0;
+      return upToDate;
+    },
+  };
 }
 
-module.exports = runMigrations;
+module.exports = makeMigrator;
diff --git a/server/server.js b/server/server.js
index 5034081e..c436b9db 100755
--- a/server/server.js
+++ b/server/server.js
@@ -9,17 +9,48 @@ const makeApp = require('./app');
 const appLog = require('./lib/app-log');
 const Config = require('./lib/config');
 const { makeDb, getDb } = require('./lib/db');
-const migrate = require('./lib/migrate');
+const makeMigrator = require('./lib/make-migrator');
 const loadSeedData = require('./lib/load-seed-data');
 const ensureAdmin = require('./lib/ensure-admin');
 const ensureConnectionAccess = require('./lib/ensure-connection-access');
-
-// Parse command line flags to see if anything special needs to happen
-require('./lib/cli-flow.js');
+const packageJson = require('./package.json');
 
 const argv = minimist(process.argv.slice(2));
+
+const helpText = `
+  SQLPad version ${packageJson.version}
+
+  # Print this help text
+  node server.js --help
+
+  # Apply database migrations then exit:
+  node server.js --migrate --config path/to/file.format
+
+  # Start SQLPad using config file:
+  node server.js --config path/to/file.format
+`;
+
+function cliHas(value) {
+  const lowered = argv._.map((v) => v.toLowerCase().trim());
+  return lowered.includes(value);
+}
+
+if (argv.version || cliHas('version')) {
+  // eslint-disable-next-line no-console
+  console.log('SQLPad version %s', packageJson.version);
+  process.exit(0);
+}
+
+if (argv.help || cliHas('help')) {
+  // eslint-disable-next-line no-console
+  console.log(helpText);
+  process.exit(0);
+}
+
 const config = new Config(argv, process.env);
 
+const migrateOnly = config.get('migrate') || cliHas('migrate');
+
 appLog.setLevel(config.get('appLogLevel'));
 appLog.debug(config.get(), 'Final config values');
 appLog.debug(config.getConnections(), 'Connections from config');
@@ -88,8 +119,30 @@ let server;
 async function startServer() {
   const { models, nedb, sequelizeDb } = await getDb();
 
-  // Before application starts up migrate data models
-  await migrate(config, appLog, nedb, sequelizeDb.sequelize);
+  // Before application starts up apply any backend database migrations needed
+  // If --migrate / migrate was specified, the process exits afterwards
+  // Automatically running migrations may be disabled via config.
+  const migrator = makeMigrator(config, appLog, nedb, sequelizeDb.sequelize);
+  const isUpToDate = await migrator.schemaUpToDate();
+
+  const runMigrations = migrateOnly || config.get('dbAutomigrate');
+
+  if (!isUpToDate && !runMigrations) {
+    appLog.error(
+      'SQLPad schema not up to date. Turn on automigration or use --migrate'
+    );
+    process.exit(1);
+  }
+
+  if (runMigrations) {
+    appLog.info('Running migrations');
+    await migrator.migrate();
+    appLog.info('Migration finished');
+  }
+
+  if (migrateOnly) {
+    process.exit(0);
+  }
 
   // Load seed data after migrations
   await loadSeedData(appLog, config, models);


Test patch:
diff --git a/server/test/migrations/make-migrator.js b/server/test/migrations/make-migrator.js
new file mode 100644
index 00000000..5c1c712c
--- /dev/null
+++ b/server/test/migrations/make-migrator.js
@@ -0,0 +1,21 @@
+const assert = require('assert');
+const TestUtils = require('../utils');
+
+describe('lib/make-migrator', function () {
+  const utils = new TestUtils();
+
+  before(async function () {
+    await utils.initDbs();
+  });
+
+  it('Not up-to-date without running mirations', async function () {
+    const upToDate = await utils.migrator.schemaUpToDate();
+    assert(!upToDate);
+  });
+
+  it('Up to date after running migrations', async function () {
+    await utils.migrator.migrate();
+    const upToDate = await utils.migrator.schemaUpToDate();
+    assert(upToDate);
+  });
+});
diff --git a/server/test/utils.js b/server/test/utils.js
index 0d362029..1c1750f0 100644
--- a/server/test/utils.js
+++ b/server/test/utils.js
@@ -9,7 +9,7 @@ const { Sequelize } = require('sequelize');
 const appLog = require('../lib/app-log');
 const db = require('../lib/db');
 const makeApp = require('../app');
-const migrate = require('../lib/migrate');
+const makeMigrator = require('../lib/make-migrator');
 const loadSeedData = require('../lib/load-seed-data');
 const ensureConnectionAccess = require('../lib/ensure-connection-access');
 
@@ -116,10 +116,8 @@ class TestUtils {
     this.models = models;
     this.nedb = nedb;
     this.sequelizeDb = sequelizeDb;
-  }
 
-  async migrate() {
-    await migrate(
+    this.migrator = makeMigrator(
       this.config,
       this.appLog,
       this.nedb,
@@ -127,6 +125,10 @@ class TestUtils {
     );
   }
 
+  async migrate() {
+    await this.migrator.migrate();
+  }
+
   static validateErrorBody(body) {
     assert(body.title, 'Error response has title');
   }