Expose function to validate migration files

CHANGELOG.md

@@ -1,10 +1,16 @@
 # Changelog
 
-## V5
+## 5.1.0
+
+- Validate migration ordering when loading files (instead of when applying migrations)
+- Expose `loadMigrationFiles` publicly, which can be used to validate files in e.g. a pre-push hook
+- Add `pg-validate-migrations` bin script
+
+## 5.0.0
 
 - [BREAKING] Update `pg` to version 8. See the [pg changelog](https://github.com/brianc/node-postgres/blob/master/CHANGELOG.md#pg800) for details.
 
-## V4
+## 4.0.0
 
 - [BREAKING] Updated whole project to TypeScript
   - some types might differ, no functional change

README.md

@@ -76,6 +76,16 @@ async function() {
 }
 ```
 
+### Validating migration files
+
+Occasionally, if two people are working on the same codebase independently, they might both create a migration at the same time. For example, `5_add-table.sql` and `5_add-column.sql`. If these both get pushed, there will be a conflict.
+
+While the migration system will notice this and refuse to apply the migrations, it can be useful to catch this as early as possible.
+
+The `loadMigrationFiles` function can be used to check if the migration files satisfy the rules.
+
+Alternatively, use the `pg-validate-migrations` bin script: `pg-validate-migrations "path/to/migration/files"`.
+
 ## Design decisions
 
 ### No down migrations

package.json

@@ -4,6 +4,9 @@
   "description": "Stack Overflow style database migrations for PostgreSQL",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
+  "bin": {
+    "pg-validate-migrations": "./dist/bin/validate.js"
+  },
   "author": "Thom Wright",
   "keywords": [
     "postgres",

src/__unit__/migration-file-validation/validate.ts

@@ -0,0 +1,15 @@
+// tslint:disable no-console
+import test from "ava"
+import {loadMigrationFiles} from "../.."
+process.on("uncaughtException", function (err) {
+  console.log(err)
+})
+
+test("two migrations with the same id", async (t) => {
+  const error = await t.throwsAsync(async () =>
+    loadMigrationFiles(
+      "src/__unit__/migration-file-validation/fixtures/conflict",
+    ),
+  )
+  t.regex(error.message, /non-consecutive/)
+})

src/__unit__/migration-file/index.ts

@@ -1,19 +1,19 @@
 import test from "ava"
-import {load} from "../../migration-file"
+import {loadMigrationFile} from "../../migration-file"
 
 test("Hashes of JS files should be the same when the SQL is the same", async (t) => {
   const [js1, js2] = await Promise.all([
-    load(__dirname + "/fixtures/different-js-same-sql-1/1_js.js"),
-    load(__dirname + "/fixtures/different-js-same-sql-2/1_js.js"),
+    loadMigrationFile(__dirname + "/fixtures/different-js-same-sql-1/1_js.js"),
+    loadMigrationFile(__dirname + "/fixtures/different-js-same-sql-2/1_js.js"),
   ])
 
   t.is(js1.hash, js2.hash)
 })
 
 test("Hashes of JS files should be different when the SQL is different", async (t) => {
   const [js1, js2] = await Promise.all([
-    load(__dirname + "/fixtures/same-js-different-sql-1/1_js.js"),
-    load(__dirname + "/fixtures/same-js-different-sql-2/1_js.js"),
+    loadMigrationFile(__dirname + "/fixtures/same-js-different-sql-1/1_js.js"),
+    loadMigrationFile(__dirname + "/fixtures/same-js-different-sql-2/1_js.js"),
   ])
 
   t.not(js1.hash, js2.hash)

src/bin/validate.ts

@@ -0,0 +1,16 @@
+#!/usr/bin/env node
+// tslint:disable no-console
+
+import {argv} from "process"
+import {loadMigrationFiles} from "../files-loader"
+
+async function main(args: Array<string>) {
+  const directory = args[0]
+
+  await loadMigrationFiles(directory, (x) => console.error(x))
+}
+
+main(argv.slice(2)).catch((e) => {
+  console.error(`ERROR: ${e.message}`)
+  process.exit(1)
+})

src/files-loader.ts

@@ -1,35 +1,49 @@
 import * as fs from "fs"
 import * as path from "path"
 import {promisify} from "util"
-import {load as loadMigrationFile} from "./migration-file"
+import {loadMigrationFile} from "./migration-file"
 import {Logger, Migration} from "./types"
+import {validateMigrationOrdering} from "./validation"
 
 const readDir = promisify(fs.readdir)
 
 const isValidFile = (fileName: string) => /\.(sql|js)$/gi.test(fileName)
 
-export const load = async (
+/**
+ * Load the migration files and assert they are reasonably valid.
+ *
+ * 'Reasonably valid' in this case means obeying the file name and
+ * consecutive ordering rules.
+ *
+ * No assertions are made about the validity of the SQL.
+ */
+export const loadMigrationFiles = async (
   directory: string,
-  log: Logger,
+  // tslint:disable-next-line no-empty
+  log: Logger = () => {},
 ): Promise<Array<Migration>> => {
   log(`Loading migrations from: ${directory}`)
 
   const fileNames = await readDir(directory)
   log(`Found migration files: ${fileNames}`)
 
-  if (fileNames != null) {
-    const migrationFiles = [
-      path.join(__dirname, "migrations/0_create-migrations-table.sql"),
-      ...fileNames.map((fileName) => path.resolve(directory, fileName)),
-    ].filter(isValidFile)
+  if (fileNames == null) {
+    return []
+  }
 
-    const unorderedMigrations = await Promise.all(
-      migrationFiles.map(loadMigrationFile),
-    )
+  const migrationFiles = [
+    path.join(__dirname, "migrations/0_create-migrations-table.sql"),
+    ...fileNames.map((fileName) => path.resolve(directory, fileName)),
+  ].filter(isValidFile)
 
-    // Arrange in ID order
-    return unorderedMigrations.sort((a, b) => a.id - b.id)
-  }
+  const unorderedMigrations = await Promise.all(
+    migrationFiles.map(loadMigrationFile),
+  )
+
+  // Arrange in ID order
+  const orderedMigrations = unorderedMigrations.sort((a, b) => a.id - b.id)
+
+  validateMigrationOrdering(orderedMigrations)
 
-  return []
+  return orderedMigrations
 }

src/index.ts

@@ -1,5 +1,6 @@
 export {createDb} from "./create"
 export {migrate} from "./migrate"
+export {loadMigrationFiles} from "./files-loader"
 
 export {
   ConnectionParams,

src/migrate.ts

@@ -1,6 +1,6 @@
 import * as pg from "pg"
 import SQL from "sql-template-strings"
-import {load} from "./files-loader"
+import {loadMigrationFiles} from "./files-loader"
 import {runMigration} from "./run-migration"
 import {
   BasicPgClient,
@@ -10,6 +10,7 @@ import {
   Migration,
   MigrationError,
 } from "./types"
+import {validateMigrationHashes} from "./validation"
 import {withConnection} from "./with-connection"
 import {withAdvisoryLock} from "./with-lock"
 
@@ -32,7 +33,7 @@ export async function migrate(
   if (typeof migrationsDirectory !== "string") {
     throw new Error("Must pass migrations directory as a string")
   }
-  const intendedMigrations = await load(migrationsDirectory, log)
+  const intendedMigrations = await loadMigrationFiles(migrationsDirectory, log)
 
   if ("client" in dbConfig) {
     // we have been given a client to use, it should already be connected
@@ -78,7 +79,7 @@ function runMigrations(intendedMigrations: Array<Migration>, log: Logger) {
         log,
       )
 
-      validateMigrations(intendedMigrations, appliedMigrations)
+      validateMigrationHashes(intendedMigrations, appliedMigrations)
 
       const migrationsToRun = filterMigrations(
         intendedMigrations,
@@ -135,36 +136,6 @@ so the database is new and we need to run all migrations.`)
   return appliedMigrations
 }
 
-/** Validates mutation order and hash */
-function validateMigrations(
-  migrations: Array<Migration>,
-  appliedMigrations: Record<number, Migration | undefined>,
-) {
-  const indexNotMatch = (migration: Migration, index: number) =>
-    migration.id !== index
-  const invalidHash = (migration: Migration) => {
-    const appliedMigration = appliedMigrations[migration.id]
-    return appliedMigration != null && appliedMigration.hash !== migration.hash
-  }
-
-  // Assert migration IDs are consecutive integers
-  const notMatchingId = migrations.find(indexNotMatch)
-  if (notMatchingId) {
-    throw new Error(
-      `Found a non-consecutive migration ID on file: '${notMatchingId.fileName}'`,
-    )
-  }
-
-  // Assert migration hashes are still same
-  const invalidHashes = migrations.filter(invalidHash)
-  if (invalidHashes.length > 0) {
-    // Someone has altered one or more migrations which has already run - gasp!
-    const invalidFiles = invalidHashes.map(({fileName}) => fileName)
-    throw new Error(`Hashes don't match for migrations '${invalidFiles}'.
-This means that the scripts have changed since it was applied.`)
-  }
-}
-
 /** Work out which migrations to apply */
 function filterMigrations(
   migrations: Array<Migration>,

src/migration-file.ts

@@ -31,7 +31,7 @@ const getSqlStringLiteral = (
   }
 }
 
-export const load = async (filePath: string) => {
+export const loadMigrationFile = async (filePath: string) => {
   const fileName = getFileName(filePath)
 
   try {
@@ -49,9 +49,6 @@ export const load = async (filePath: string) => {
       sql,
     }
   } catch (err) {
-    throw new Error(`${err.message}
-Offending file: '${fileName}'.`)
+    throw new Error(`${err.message} - Offending file: '${fileName}'.`)
   }
 }
-
-// module.exports._fileNameParser = fileNameParser

src/validation.ts

@@ -0,0 +1,34 @@
+import {Migration} from "./types"
+
+const indexNotMatch = (migration: Migration, index: number) =>
+  migration.id !== index
+
+/** Assert migration IDs are consecutive integers */
+export function validateMigrationOrdering(migrations: Array<Migration>) {
+  const notMatchingId = migrations.find(indexNotMatch)
+  if (notMatchingId) {
+    throw new Error(
+      `Found a non-consecutive migration ID on file: '${notMatchingId.fileName}'`,
+    )
+  }
+}
+
+/** Assert hashes match */
+export function validateMigrationHashes(
+  migrations: Array<Migration>,
+  appliedMigrations: Record<number, Migration | undefined>,
+) {
+  const invalidHash = (migration: Migration) => {
+    const appliedMigration = appliedMigrations[migration.id]
+    return appliedMigration != null && appliedMigration.hash !== migration.hash
+  }
+
+  // Assert migration hashes are still same
+  const invalidHashes = migrations.filter(invalidHash)
+  if (invalidHashes.length > 0) {
+    // Someone has altered one or more migrations which has already run - gasp!
+    const invalidFiles = invalidHashes.map(({fileName}) => fileName)
+    throw new Error(`Hashes don't match for migrations '${invalidFiles}'.
+This means that the scripts have changed since it was applied.`)
+  }
+}