From 5a73373160b4e95200b1578c42101d67b9d28a6f Mon Sep 17 00:00:00 2001 From: Thom Wright Date: Tue, 2 Mar 2021 19:50:15 +0000 Subject: [PATCH] Change strategy for creating database Use 'ensureDatabaseExists' flag on the migrate function. I _think_ this makes the API smaller/simpler. This implementation also checks whether the database exists before trying to create it, which means it _should_ work on readonly replicas. Fixes #57 --- CHANGELOG.md | 5 ++ README.md | 48 ++++++-------- .../fixtures/ensure-exists/1_success.sql | 3 + src/__tests__/migrate.ts | 63 ++++++++++++++++++- src/create.ts | 9 ++- src/migrate.ts | 58 ++++++++++++++--- src/types.ts | 22 ++++++- 7 files changed, 164 insertions(+), 44 deletions(-) create mode 100644 src/__tests__/fixtures/ensure-exists/1_success.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index faacf28..8c9476b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 5.3.0 + +- [DEPRECATION] Deprecate `createDb` +- Add `ensureDatabaseExists` to check/create database in `migrate` + ## 5.1.0 - Validate migration ordering when loading files (instead of when applying migrations) diff --git a/README.md b/README.md index 90526c6..5ff1947 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ There are two ways to use the API. Either, pass a database connection config object: ```typescript -import {createDb, migrate} from "postgres-migrations" +import {migrate} from "postgres-migrations" async function() { const dbConfig = { @@ -36,12 +36,16 @@ async function() { password: "password", host: "localhost", port: 5432, + + // Default: false for backwards-compatibility + // This might change! + ensureDatabaseExists: true + + // Default: "postgres" + // Used when checking/creating "database-name" + defaultDatabase: "postgres" } - await createDb(databaseName, { - ...dbConfig, - defaultDatabase: "postgres", // defaults to "postgres" - }) await migrate(dbConfig, "path/to/migration/files") } ``` @@ -49,7 +53,7 @@ async function() { Or, pass a `pg` client: ```typescript -import {createDb, migrate} from "postgres-migrations" +import {migrate} from "postgres-migrations" async function() { const dbConfig = { @@ -60,27 +64,13 @@ async function() { port: 5432, } - { - const client = new pg.Client({ - ...dbConfig, - database: "postgres", - }) - await client.connect() - try { - await createDb(databaseName, {client}) - } finally { - await client.end() - } - } - - { - const client = new pg.Client(dbConfig) // or a Pool, or a PoolClient - await client.connect() - try { - await migrate({client}, "path/to/migration/files") - } finally { - await client.end() - } + // Note: when passing a client, it is assumed that the database already exists + const client = new pg.Client(dbConfig) // or a Pool, or a PoolClient + await client.connect() + try { + await migrate({client}, "path/to/migration/files") + } finally { + await client.end() } } ``` @@ -251,10 +241,10 @@ If you want sane date handling, it is recommended you use the following code sni ```js const pg = require("pg") -const parseDate = val => +const parseDate = (val) => val === null ? null : moment(val).format("YYYY-MM-DD") const DATATYPE_DATE = 1082 -pg.types.setTypeParser(DATATYPE_DATE, val => { +pg.types.setTypeParser(DATATYPE_DATE, (val) => { return val === null ? null : parseDate(val) }) ``` diff --git a/src/__tests__/fixtures/ensure-exists/1_success.sql b/src/__tests__/fixtures/ensure-exists/1_success.sql new file mode 100644 index 0000000..598172f --- /dev/null +++ b/src/__tests__/fixtures/ensure-exists/1_success.sql @@ -0,0 +1,3 @@ +CREATE TABLE success ( + id integer +); diff --git a/src/__tests__/migrate.ts b/src/__tests__/migrate.ts index 30357c4..0503395 100644 --- a/src/__tests__/migrate.ts +++ b/src/__tests__/migrate.ts @@ -2,7 +2,7 @@ import test from "ava" import * as pg from "pg" import SQL from "sql-template-strings" -import {createDb, migrate} from "../" +import {createDb, migrate, MigrateDBConfig} from "../" import {PASSWORD, startPostgres, stopPostgres} from "./fixtures/docker-postgres" const CONTAINER_NAME = "pg-migrations-test-migrate" @@ -384,7 +384,7 @@ test("bad arguments - incorrect port", (t) => { }) }) -test("no database", (t) => { +test("no database - ensureDatabaseExists = undefined", (t) => { return t .throwsAsync( migrate( @@ -406,6 +406,65 @@ test("no database", (t) => { }) }) +test("no database - ensureDatabaseExists = true", (t) => { + const databaseName = "migration-test-no-db-ensure-exists" + const dbConfig: MigrateDBConfig = { + database: databaseName, + user: "postgres", + password: PASSWORD, + host: "localhost", + port, + + ensureDatabaseExists: true, + } + + return migrate(dbConfig, "src/__tests__/fixtures/ensure-exists") + .then(() => doesTableExist(dbConfig, "success")) + .then((exists) => { + t.truthy(exists) + }) +}) + +test("existing database - ensureDatabaseExists = true", (t) => { + const databaseName = "migration-test-existing-db-ensure-exists" + const dbConfig: MigrateDBConfig = { + database: databaseName, + user: "postgres", + password: PASSWORD, + host: "localhost", + port, + + ensureDatabaseExists: true, + } + + return createDb(databaseName, dbConfig) + .then(() => migrate(dbConfig, "src/__tests__/fixtures/ensure-exists")) + .then(() => doesTableExist(dbConfig, "success")) + .then((exists) => { + t.truthy(exists) + }) +}) + +test("no database - ensureDatabaseExists = true, bad default database", (t) => { + const databaseName = "migration-test-ensure-exists-nope" + const dbConfig: MigrateDBConfig = { + database: databaseName, + user: "postgres", + password: PASSWORD, + host: "localhost", + port, + + ensureDatabaseExists: true, + defaultDatabase: "nopenopenope", + } + + return t + .throwsAsync(migrate(dbConfig, "src/__tests__/fixtures/ensure-exists")) + .then((err) => { + t.regex(err.message, /database "nopenopenope" does not exist/) + }) +}) + test("no migrations dir", (t) => { const databaseName = "migration-test-no-dir" const dbConfig = { diff --git a/src/create.ts b/src/create.ts index 29de664..9b39c36 100644 --- a/src/create.ts +++ b/src/create.ts @@ -4,6 +4,9 @@ import {withConnection} from "./with-connection" const DUPLICATE_DATABASE = "42P04" +/** + * @deprecated Use `migrate` instead with `ensureDatabaseExists: true`. + */ export async function createDb( dbName: string, dbConfig: CreateDBConfig, @@ -25,7 +28,7 @@ export async function createDb( } if ("client" in dbConfig) { - return betterCreate(dbName, log)(dbConfig.client) + return runCreateQuery(dbName, log)(dbConfig.client) } if ( @@ -50,12 +53,12 @@ export async function createDb( log(`pg client emitted an error: ${err.message}`) }) - const runWith = withConnection(log, betterCreate(dbName, log)) + const runWith = withConnection(log, runCreateQuery(dbName, log)) return runWith(client) } -function betterCreate(dbName: string, log: Logger) { +export function runCreateQuery(dbName: string, log: Logger) { return async (client: BasicPgClient): Promise => { await client .query(`CREATE DATABASE "${dbName.replace(/\"/g, '""')}"`) diff --git a/src/migrate.ts b/src/migrate.ts index 59846de..e00807b 100644 --- a/src/migrate.ts +++ b/src/migrate.ts @@ -1,5 +1,6 @@ import * as pg from "pg" import SQL from "sql-template-strings" +import {runCreateQuery} from "./create" import {loadMigrationFiles} from "./files-loader" import {runMigration} from "./run-migration" import { @@ -14,6 +15,17 @@ import {validateMigrationHashes} from "./validation" import {withConnection} from "./with-connection" import {withAdvisoryLock} from "./with-lock" +/** + * Run the migrations. + * + * If `dbConfig.ensureDatabaseExists` is true then `dbConfig.database` will be created if it + * does not exist. + * + * @param dbConfig Details about how to connect to the database + * @param migrationsDirectory Directory containing the SQL migration files + * @param config Extra configuration + * @returns Details about the migrations which were run + */ export async function migrate( dbConfig: MigrateDBConfig, migrationsDirectory: string, @@ -53,17 +65,45 @@ export async function migrate( throw new Error("Database config problem") } - const client = new pg.Client(dbConfig) - client.on("error", (err) => { - log(`pg client emitted an error: ${err.message}`) - }) + if (dbConfig.ensureDatabaseExists === true) { + // Check whether database exists + const {user, password, host, port} = dbConfig + const client = new pg.Client({ + database: + dbConfig.defaultDatabase != null + ? dbConfig.defaultDatabase + : "postgres", + user, + password, + host, + port, + }) + + const runWith = withConnection(log, async (connectedClient) => { + const result = await connectedClient.query({ + text: "SELECT 1 FROM pg_database WHERE datname=$1", + values: [dbConfig.database], + }) + if (result.rowCount !== 1) { + await runCreateQuery(dbConfig.database, log)(connectedClient) + } + }) + + await runWith(client) + } + { + const client = new pg.Client(dbConfig) + client.on("error", (err) => { + log(`pg client emitted an error: ${err.message}`) + }) - const runWith = withConnection( - log, - withAdvisoryLock(log, runMigrations(intendedMigrations, log)), - ) + const runWith = withConnection( + log, + withAdvisoryLock(log, runMigrations(intendedMigrations, log)), + ) - return runWith(client) + return runWith(client) + } } function runMigrations(intendedMigrations: Array, log: Logger) { diff --git a/src/types.ts b/src/types.ts index f496bc4..5e16f9a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -21,6 +21,26 @@ export interface ClientParams { readonly client: pg.Client | pg.PoolClient | pg.Pool } +export type EnsureDatabase = + | { + /** + * Might default to `true` in future versions + * @default false + */ + readonly ensureDatabaseExists: true + /** + * The database to connect to when creating a database (if necessary). + * @default postgres + */ + readonly defaultDatabase?: string + } + | { + readonly ensureDatabaseExists?: false + } + +/** + * @deprecated Use `migrate` instead with `ensureDatabaseExists: true`. + */ export type CreateDBConfig = | (ConnectionParams & { /** The database to connect to when creating the new database. */ @@ -31,7 +51,7 @@ export type CreateDBConfig = export type MigrateDBConfig = | (ConnectionParams & { readonly database: string - }) + } & EnsureDatabase) | ClientParams export type Logger = (msg: string) => void