diff --git a/packages/database/dev/src/lib/migrations.test.ts b/packages/database/dev/src/lib/migrations.test.ts index 61a3fa5c8..7eac8d9f0 100644 --- a/packages/database/dev/src/lib/migrations.test.ts +++ b/packages/database/dev/src/lib/migrations.test.ts @@ -177,7 +177,7 @@ test('Throws MigrationFileNotFoundError for directory missing migration.sql', as db = await PGlite.create() await expect(applyMigrations(db, tmpDir.path, '0001_missing_file')).rejects.toThrow( - /Migration SQL file not found: .*0001_missing_file[/\\]migration.sql/, + /Migration "0001_missing_file" is missing its SQL file at .*0001_missing_file[/\\]migration\.sql/, ) }) @@ -280,7 +280,20 @@ test('Throws when a directory and flat file share the same name', async () => { db = await PGlite.create() - await expect(applyMigrations(db, tmpDir.path)).rejects.toThrow(/Duplicate migration name "0001_create_users"/) + await expect(applyMigrations(db, tmpDir.path)).rejects.toThrow( + /Two or more migrations share the name "0001_create_users"/, + ) +}) + +test('Throws when distinct migrations share a version', async () => { + tmpDir = await tmp.dir() + + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + db = await PGlite.create() + + await expect(applyMigrations(db, tmpDir.path)).rejects.toThrow(/Version 1 is used by multiple migrations/) }) test('Ignores flat .sql files whose name does not match the migration pattern', async () => { @@ -297,3 +310,18 @@ test('Ignores flat .sql files whose name does not match the migration pattern', expect(applied).toEqual(['0001_create_users']) }) + +test('Skips entries with disallowed characters (uppercase, spaces, special) in the slug', async () => { + tmpDir = await tmp.dir() + + await createFlatMigration(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0002_CAPS_NOT_OK', 'CREATE TABLE caps (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0003_special!chars', 'CREATE TABLE bad (id SERIAL PRIMARY KEY)') + await fs.writeFile(join(tmpDir.path, '0004_with space.sql'), 'CREATE TABLE bad (id SERIAL PRIMARY KEY)') + + db = await PGlite.create() + + const applied = await applyMigrations(db, tmpDir.path) + + expect(applied).toEqual(['0001_create_users']) +}) diff --git a/packages/database/dev/src/lib/migrations.ts b/packages/database/dev/src/lib/migrations.ts index 9ae68af22..77c8d6e56 100644 --- a/packages/database/dev/src/lib/migrations.ts +++ b/packages/database/dev/src/lib/migrations.ts @@ -5,44 +5,215 @@ import type { Dirent } from 'node:fs' import type { SQLExecutor } from './sql-executor.js' -const MIGRATION_NAME_PATTERN = /^\d+_.+$/ +const MIGRATION_NAME_PATTERN = /^(\d+)_[a-z0-9_-]+$/ const MIGRATION_FILE = 'migration.sql' const SQL_EXTENSION = '.sql' const TRACKING_SCHEMA = 'netlify' const TRACKING_TABLE = `${TRACKING_SCHEMA}.migrations` -interface Migration { +export interface Migration { name: string + version: number sqlPath: string } -// Maps a directory entry to a Migration, or null if it isn't one. Supports -// both directory-style migrations (`_/migration.sql`) and flat -// file migrations (`_.sql`). -function toMigration(entry: Dirent, migrationsDirectory: string): Migration | null { - if (entry.isDirectory()) { - if (!MIGRATION_NAME_PATTERN.test(entry.name)) { - return null - } +export interface PgErrorDetails { + message: string + code?: string + position?: number + hint?: string + detail?: string + severity?: string +} - return { - name: entry.name, - sqlPath: join(migrationsDirectory, entry.name, MIGRATION_FILE), +export type MigrationIssue = + | { + kind: 'duplicate-name' + version: number + name: string + files: Migration[] + summary: string + remediation: string + } + | { + kind: 'duplicate-version' + version: number + files: Migration[] + summary: string + remediation: string + } + | { + kind: 'apply-failure' + migration: Migration + appliedBefore: Migration[] + remaining: Migration[] + pgError: PgErrorDetails + summary: string + remediation: string } + | { + kind: 'migration-file-error' + reason: 'missing' | 'unreadable' + migrationName: string + sqlPath: string + summary: string + remediation: string + cause?: unknown + } + | { + kind: 'unknown' + summary: string + remediation: string + cause?: unknown + } + +const optionalString = (value: unknown): string | undefined => + typeof value === 'string' && value !== '' ? value : undefined + +const parseOptionalInt = (value: unknown): number | undefined => { + if (typeof value === 'number' && Number.isFinite(value)) return value + if (typeof value === 'string') { + const n = parseInt(value, 10) + return Number.isFinite(n) ? n : undefined } + return undefined +} - if (entry.isFile() && entry.name.endsWith(SQL_EXTENSION)) { - const name = entry.name.slice(0, -SQL_EXTENSION.length) +export function normalizePgError(error: unknown): PgErrorDetails { + if (!(error instanceof Error)) { + return { message: String(error) } + } + const e = error as Error & Record + return { + message: e.message, + code: optionalString(e.code), + position: parseOptionalInt(e.position), + hint: optionalString(e.hint), + detail: optionalString(e.detail), + severity: optionalString(e.severity), + } +} - if (!MIGRATION_NAME_PATTERN.test(name)) { - return null - } +const buildDuplicateNameIssue = (version: number, name: string, files: Migration[]): MigrationIssue => ({ + kind: 'duplicate-name', + version, + name, + files, + summary: `Two or more migrations share the name "${name}".`, + remediation: `Delete one of the duplicate files before applying.`, +}) + +const buildDuplicateVersionIssue = (version: number, files: Migration[]): MigrationIssue => { + const fileList = files.map((m) => `"${m.name}"`).join(', ') + return { + kind: 'duplicate-version', + version, + files, + summary: `Version ${String(version)} is used by multiple migrations: ${fileList}.`, + remediation: `Increment one of the prefixes so ordering is unambiguous, or delete a file if it was an unintended duplicate.`, + } +} +const buildApplyFailureIssue = (options: { + migration: Migration + appliedBefore: Migration[] + remaining: Migration[] + cause: unknown +}): MigrationIssue & { kind: 'apply-failure' } => { + const pgError = normalizePgError(options.cause) + const codeNote = pgError.code ? ` Postgres returned SQLSTATE ${pgError.code}; look that up for common causes.` : '' + return { + kind: 'apply-failure', + migration: options.migration, + appliedBefore: options.appliedBefore, + remaining: options.remaining, + pgError, + summary: `Migration "${options.migration.name}" failed to apply: ${pgError.message}`, + remediation: + `This may be a problem in the SQL of "${options.migration.name}" itself (for example, a syntax error), ` + + `or in the cumulative database state left by previously applied migrations ` + + `(for example, the migration tries to create an object that an earlier migration already created, ` + + `or references one that was never created).${codeNote} ` + + `If the failing migration (or a prior one whose state is implicated) has not yet been applied to any database, ` + + `fix its SQL. If it has already been applied, you likely edited the file after the fact — applied migrations ` + + `are immutable, so restore it to its applied version (for example via version control or ` + + `\`netlify database migrations pull --force\`). If neither situation matches, this may indicate a divergence ` + + `between Postgres and the embedded engine used for this check — please file a bug.`, + } +} + +const buildMigrationFileIssue = (options: { + migrationName: string + sqlPath: string + reason: 'missing' | 'unreadable' + cause?: unknown +}): MigrationIssue & { kind: 'migration-file-error' } => { + if (options.reason === 'missing') { return { - name, - sqlPath: join(migrationsDirectory, entry.name), + kind: 'migration-file-error', + reason: 'missing', + migrationName: options.migrationName, + sqlPath: options.sqlPath, + summary: `Migration "${options.migrationName}" is missing its SQL file at ${options.sqlPath}.`, + remediation: + `Create the file at ${options.sqlPath}, or remove the migration's directory if it isn't intended. ` + + `Only remove the directory if the migration has not yet been applied to any database — if it was applied, ` + + `restore the file from version control instead.`, + cause: options.cause, } } + const causeMessage = options.cause instanceof Error ? options.cause.message : String(options.cause) + return { + kind: 'migration-file-error', + reason: 'unreadable', + migrationName: options.migrationName, + sqlPath: options.sqlPath, + summary: `Could not read migration "${options.migrationName}" at ${options.sqlPath}: ${causeMessage}`, + remediation: `Check filesystem permissions on ${options.sqlPath} and that it isn't held open by another process.`, + cause: options.cause, + } +} + +const formatIssueAsErrorLine = (issue: MigrationIssue): string => { + if (issue.kind === 'duplicate-name') { + const paths = issue.files.map((m) => ` - ${m.sqlPath}`).join('\n') + return ` - ${issue.summary} ${issue.remediation}\n${paths}` + } + if (issue.kind === 'duplicate-version') { + const paths = issue.files.map((m) => ` - ${m.sqlPath} (${m.name})`).join('\n') + return ` - ${issue.summary} ${issue.remediation}\n${paths}` + } + if (issue.kind === 'apply-failure' || issue.kind === 'migration-file-error') { + return ` - ${issue.summary}\n ${issue.remediation}` + } + return ` - ${issue.summary} ${issue.remediation}` +} + +function nameAndFileToMigration(name: string, sqlPath: string): Migration | null { + const match = MIGRATION_NAME_PATTERN.exec(name) + if (!match) { + return null + } + + return { + name, + version: parseInt(match[1], 10), + sqlPath, + } +} + +// Maps a directory entry to a Migration, or null if it isn't one. Supports +// both directory-style migrations (`_/migration.sql`) and flat +// file migrations (`_.sql`). +function direntToMigration(entry: Dirent, migrationsDirectory: string): Migration | null { + if (entry.isDirectory()) { + return nameAndFileToMigration(entry.name, join(migrationsDirectory, entry.name, MIGRATION_FILE)) + } + + if (entry.isFile() && entry.name.endsWith(SQL_EXTENSION)) { + const name = entry.name.slice(0, -SQL_EXTENSION.length) + return nameAndFileToMigration(name, join(migrationsDirectory, entry.name)) + } return null } @@ -59,11 +230,79 @@ export async function initializeTrackingTable(db: SQLExecutor): Promise { `) } -export async function applyMigrations( +export class MissingMigrationDirectoryError extends Error { + public readonly migrationsDirectory: string + constructor(options: ErrorOptions & { migrationsDirectory: string }) { + super(`Migration directory not found: ${options.migrationsDirectory}`, { cause: options.cause }) + this.name = 'MissingMigrationDirectoryError' + this.migrationsDirectory = options.migrationsDirectory + } +} + +export class DuplicateMigrationVersionsError extends Error { + public readonly issues: MigrationIssue[] + public readonly migrationsDirectory: string + constructor(options: { issues: MigrationIssue[]; migrationsDirectory: string }) { + const detail = options.issues.map(formatIssueAsErrorLine).join('\n') + super(`Duplicate migrations found in ${options.migrationsDirectory}:\n${detail}`) + this.name = 'DuplicateMigrationVersionsError' + this.issues = options.issues + this.migrationsDirectory = options.migrationsDirectory + } +} + +export class MigrationsApplyError extends Error { + public readonly issue: MigrationIssue & { kind: 'apply-failure' } + public readonly cause: Error + constructor(options: { issue: MigrationIssue & { kind: 'apply-failure' } }) { + super(`${options.issue.summary}\n${options.issue.remediation}`, { cause: options.issue.pgError }) + this.name = 'MigrationsApplyError' + this.issue = options.issue + this.cause = new Error(options.issue.pgError.message) + } +} + +export class MigrationFileError extends Error { + public readonly issue: MigrationIssue & { kind: 'migration-file-error' } + constructor(options: { issue: MigrationIssue & { kind: 'migration-file-error' } }) { + super(`${options.issue.summary}\n${options.issue.remediation}`, { cause: options.issue.cause }) + this.name = 'MigrationFileError' + this.issue = options.issue + } +} + +export function errorToIssues(error: unknown): MigrationIssue[] { + if (error instanceof MissingMigrationDirectoryError) { + return [] + } + if (error instanceof DuplicateMigrationVersionsError) { + return error.issues + } + if (error instanceof MigrationsApplyError || error instanceof MigrationFileError) { + return [error.issue] + } + + const message = error instanceof Error ? error.message : String(error) + return [ + { + kind: 'unknown', + summary: `An unexpected error occurred while validating migrations: ${message}`, + remediation: `Re-run the command. If the problem persists, file a bug report with the error details above.`, + cause: error, + }, + ] +} + +export async function applyMigrations(...args: Parameters): Promise { + const migrations = await applyMigrationsWithDetails(...args) + return migrations.map((m) => m.name) +} + +export async function applyMigrationsWithDetails( db: SQLExecutor, migrationsDirectory: string, target?: string, -): Promise { +): Promise { await initializeTrackingTable(db) let migrations: Migration[] @@ -71,25 +310,53 @@ export async function applyMigrations( try { const dirents = await readdir(migrationsDirectory, { withFileTypes: true }) migrations = dirents - .map((entry) => toMigration(entry, migrationsDirectory)) + .map((entry) => direntToMigration(entry, migrationsDirectory)) .filter((m): m is Migration => m !== null) - } catch { - throw new Error(`Migration directory not found: ${migrationsDirectory}`) + } catch (error) { + throw new MissingMigrationDirectoryError({ migrationsDirectory, cause: error }) } - const seen = new Map() + const migrationsByVersion = new Map() for (const migration of migrations) { - const existing = seen.get(migration.name) + const existing = migrationsByVersion.get(migration.version) if (existing) { - throw new Error( - `Duplicate migration name "${migration.name}" in ${migrationsDirectory}: ` + - `found both "${existing}" and "${migration.sqlPath}". Remove one before applying migrations.`, - ) + existing.push(migration) + } else { + migrationsByVersion.set(migration.version, [migration]) + } + } + + const issues: MigrationIssue[] = [] + for (const [version, migrationsForVersion] of migrationsByVersion) { + if (migrationsForVersion.length < 2) continue + + const byName = new Map() + for (const m of migrationsForVersion) { + const list = byName.get(m.name) + if (list) { + list.push(m) + } else { + byName.set(m.name, [m]) + } + } + + for (const [name, migrationsWithName] of byName) { + if (migrationsWithName.length >= 2) { + issues.push(buildDuplicateNameIssue(version, name, migrationsWithName)) + } + } + + if (byName.size >= 2) { + const representatives = Array.from(byName.values()).map((migs) => migs[0]) + issues.push(buildDuplicateVersionIssue(version, representatives)) } - seen.set(migration.name, migration.sqlPath) } - migrations.sort((a, b) => a.name.localeCompare(b.name)) + if (issues.length > 0) { + throw new DuplicateMigrationVersionsError({ issues, migrationsDirectory }) + } + + migrations.sort((a, b) => a.version - b.version) let migrationsToConsider: Migration[] @@ -114,32 +381,47 @@ export async function applyMigrations( const result = await db.query<{ name: string }>(`SELECT name FROM ${TRACKING_TABLE} WHERE name = ANY($1)`, [names]) const alreadyApplied = new Set(result.rows.map((row) => row.name)) - const applied: string[] = [] - + const migrationsToApplyWithContent: { migration: Migration; sql: string }[] = [] for (const migration of migrationsToConsider) { if (alreadyApplied.has(migration.name)) { continue } - let sql: string - try { - sql = await readFile(migration.sqlPath, 'utf-8') + const sql = await readFile(migration.sqlPath, 'utf-8') + migrationsToApplyWithContent.push({ migration, sql }) } catch (error) { const err = error as NodeJS.ErrnoException - if (err.code === 'ENOENT') { - throw new Error(`Migration SQL file not found: ${migration.sqlPath}`) - } - - throw new Error(`Failed to read migration "${migration.name}" at "${migration.sqlPath}": ${err.message}`) + throw new MigrationFileError({ + issue: buildMigrationFileIssue({ + migrationName: migration.name, + sqlPath: migration.sqlPath, + reason: err.code === 'ENOENT' ? 'missing' : 'unreadable', + cause: err, + }), + }) } + } - await db.transaction(async (tx) => { - await tx.exec(sql) - await tx.query(`INSERT INTO ${TRACKING_TABLE} (name) VALUES ($1)`, [migration.name]) - }) + const applied: Migration[] = [] + for (const { migration, sql } of migrationsToApplyWithContent) { + try { + await db.transaction(async (tx) => { + await tx.exec(sql) + await tx.query(`INSERT INTO ${TRACKING_TABLE} (name) VALUES ($1)`, [migration.name]) + }) + } catch (error) { + throw new MigrationsApplyError({ + issue: buildApplyFailureIssue({ + migration, + appliedBefore: applied, + remaining: migrationsToConsider.slice(applied.length + 1), + cause: error, + }), + }) + } - applied.push(migration.name) + applied.push(migration) } return applied diff --git a/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts new file mode 100644 index 000000000..73b8a4b1c --- /dev/null +++ b/packages/database/dev/src/lib/testMigrationsInEphemeralDatabase.test.ts @@ -0,0 +1,209 @@ +import { promises as fs } from 'node:fs' +import { join } from 'node:path' + +import tmp from 'tmp-promise' +import { test, expect, afterEach } from 'vitest' + +import { testMigrationsInEphemeralDatabase } from '../main.js' + +let tmpDir: tmp.DirectoryResult | undefined + +afterEach(async () => { + if (tmpDir) { + await tmpDir.cleanup() + tmpDir = undefined + } +}) + +async function createMigrationDir(basePath: string, name: string, sql: string) { + const dir = join(basePath, name) + await fs.mkdir(dir, { recursive: true }) + await fs.writeFile(join(dir, 'migration.sql'), sql) +} + +async function createFlatMigration(basePath: string, name: string, sql: string) { + await fs.writeFile(join(basePath, `${name}.sql`), sql) +} + +test('Returns success with applied migrations when all apply cleanly', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY, name TEXT)') + await createMigrationDir(tmpDir.path, '0002_add_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY, title TEXT)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('success') + if (result.status !== 'success') return + expect(result.applied).toHaveLength(2) + expect(result.applied.map((m) => m.name)).toEqual(['0001_create_users', '0002_add_posts']) +}) + +test('Returns success with empty applied when migrations directory is missing', async () => { + const result = await testMigrationsInEphemeralDatabase('/nonexistent/path/to/migrations') + + expect(result.status).toBe('success') + if (result.status !== 'success') return + expect(result.applied).toEqual([]) +}) + +test('Returns success with empty applied when migrations directory is empty', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('success') + if (result.status !== 'success') return + expect(result.applied).toEqual([]) +}) + +test('Returns failure with duplicate-name issue when a directory and flat file share a name', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('duplicate-name') + if (issue.kind !== 'duplicate-name') return + expect(issue.name).toBe('0001_create_users') + expect(issue.version).toBe(1) + expect(issue.files).toHaveLength(2) + expect(issue.summary).toBe('Two or more migrations share the name "0001_create_users".') + expect(issue.remediation).toBe('Delete one of the duplicate files before applying.') +}) + +test('Returns failure with duplicate-version issue when distinct migrations share a version', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('duplicate-version') + if (issue.kind !== 'duplicate-version') return + expect(issue.version).toBe(1) + expect(issue.files.map((f) => f.name).sort()).toEqual(['0001_create_posts', '0001_create_users']) + // File order in the summary depends on readdir order, so allow either. + expect(issue.summary).toMatch( + /^Version 1 is used by multiple migrations: ("0001_create_users", "0001_create_posts"|"0001_create_posts", "0001_create_users")\.$/, + ) + expect(issue.remediation).toBe( + 'Increment one of the prefixes so ordering is unambiguous, or delete a file if it was an unintended duplicate.', + ) +}) + +test('Reports both duplicate-name and duplicate-version issues at once', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + // Two with the same name + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createFlatMigration(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + // A third with a different name but the same version — triggers duplicate-version too + await createFlatMigration(tmpDir.path, '0001_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + const kinds = result.issues.map((i) => i.kind).sort() + expect(kinds).toEqual(['duplicate-name', 'duplicate-version']) +}) + +test('Returns failure with migration-file-error (missing) when a migration directory has no migration.sql', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await fs.mkdir(join(tmpDir.path, '0001_create_users')) + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('migration-file-error') + if (issue.kind !== 'migration-file-error') return + expect(issue.reason).toBe('missing') + expect(issue.migrationName).toBe('0001_create_users') + expect(issue.sqlPath).toMatch(/0001_create_users[/\\]migration\.sql$/) + // The path is absolute and varies per run, so anchor the rest of the wording around it. + expect(issue.summary).toMatch( + /^Migration "0001_create_users" is missing its SQL file at .+0001_create_users[/\\]migration\.sql\.$/, + ) + expect(issue.remediation).toMatch( + /^Create the file at .+0001_create_users[/\\]migration\.sql, or remove the migration's directory if it isn't intended\. Only remove the directory if the migration has not yet been applied to any database — if it was applied, restore the file from version control instead\.$/, + ) +}) + +test('Returns failure with apply-failure issue when a migration has a syntax error', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0002_bad', 'INVALID SQL SYNTAX HERE') + await createMigrationDir(tmpDir.path, '0003_create_posts', 'CREATE TABLE posts (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + expect(result.issues).toHaveLength(1) + + const issue = result.issues[0] + expect(issue.kind).toBe('apply-failure') + if (issue.kind !== 'apply-failure') return + expect(issue.migration.name).toBe('0002_bad') + expect(issue.appliedBefore.map((m) => m.name)).toEqual(['0001_create_users']) + expect(issue.remaining.map((m) => m.name)).toEqual(['0003_create_posts']) + expect(issue.pgError.code).toBe('42601') + expect(issue.pgError.message).toBe('syntax error at or near "INVALID"') + expect(issue.summary).toBe('Migration "0002_bad" failed to apply: syntax error at or near "INVALID"') + expect(issue.remediation).toBe( + 'This may be a problem in the SQL of "0002_bad" itself (for example, a syntax error), ' + + 'or in the cumulative database state left by previously applied migrations ' + + '(for example, the migration tries to create an object that an earlier migration already created, ' + + 'or references one that was never created). Postgres returned SQLSTATE 42601; look that up for common causes. ' + + 'If the failing migration (or a prior one whose state is implicated) has not yet been applied to any database, ' + + 'fix its SQL. If it has already been applied, you likely edited the file after the fact — applied migrations ' + + 'are immutable, so restore it to its applied version (for example via version control or ' + + '`netlify database migrations pull --force`). If neither situation matches, this may indicate a divergence ' + + 'between Postgres and the embedded engine used for this check — please file a bug.', + ) +}) + +test('Returns failure with apply-failure issue when a migration creates a relation that already exists', async () => { + tmpDir = await tmp.dir({ unsafeCleanup: true }) + await createMigrationDir(tmpDir.path, '0001_create_users', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + await createMigrationDir(tmpDir.path, '0002_create_users_again', 'CREATE TABLE users (id SERIAL PRIMARY KEY)') + + const result = await testMigrationsInEphemeralDatabase(tmpDir.path) + + expect(result.status).toBe('failure') + if (result.status !== 'failure') return + + const issue = result.issues[0] + expect(issue.kind).toBe('apply-failure') + if (issue.kind !== 'apply-failure') return + expect(issue.appliedBefore.map((m) => m.name)).toEqual(['0001_create_users']) + expect(issue.migration.name).toBe('0002_create_users_again') + expect(issue.pgError.code).toBe('42P07') + expect(issue.pgError.message).toBe('relation "users" already exists') + expect(issue.summary).toBe('Migration "0002_create_users_again" failed to apply: relation "users" already exists') + expect(issue.remediation).toBe( + 'This may be a problem in the SQL of "0002_create_users_again" itself (for example, a syntax error), ' + + 'or in the cumulative database state left by previously applied migrations ' + + '(for example, the migration tries to create an object that an earlier migration already created, ' + + 'or references one that was never created). Postgres returned SQLSTATE 42P07; look that up for common causes. ' + + 'If the failing migration (or a prior one whose state is implicated) has not yet been applied to any database, ' + + 'fix its SQL. If it has already been applied, you likely edited the file after the fact — applied migrations ' + + 'are immutable, so restore it to its applied version (for example via version control or ' + + '`netlify database migrations pull --force`). If neither situation matches, this may indicate a divergence ' + + 'between Postgres and the embedded engine used for this check — please file a bug.', + ) +}) diff --git a/packages/database/dev/src/main.ts b/packages/database/dev/src/main.ts index e33f379af..237211c8e 100644 --- a/packages/database/dev/src/main.ts +++ b/packages/database/dev/src/main.ts @@ -6,13 +6,25 @@ import type { ConnectionState, MessageResponse } from 'pg-gateway' import { fromNodeSocket } from 'pg-gateway/node' import { broadcastNotifications } from './lib/notifications.js' -import { applyMigrations, initializeTrackingTable } from './lib/migrations.js' +import { + applyMigrations, + applyMigrationsWithDetails, + errorToIssues, + initializeTrackingTable, + type Migration, + type MigrationIssue, +} from './lib/migrations.js' import type { SQLExecutor } from './lib/sql-executor.js' export { applyMigrations, initializeTrackingTable } from './lib/migrations.js' +export type { Migration, MigrationIssue, PgErrorDetails } from './lib/migrations.js' export type { SQLExecutor } from './lib/sql-executor.js' +export type TestMigrationsInEphemeralDatabaseResult = + | { status: 'success'; applied: Migration[] } + | { status: 'failure'; issues: MigrationIssue[] } const DEFAULT_HOST = 'localhost' +const IN_MEMORY_DIRECTORY = 'memory://' type Logger = (...message: unknown[]) => void @@ -71,7 +83,7 @@ export class NetlifyDB implements SQLExecutor { } async start(): Promise { - if (this.directory) { + if (this.directory && this.directory !== IN_MEMORY_DIRECTORY) { await mkdir(this.directory, { recursive: true }) } @@ -220,3 +232,23 @@ export class NetlifyDB implements SQLExecutor { }) } } + +export async function testMigrationsInEphemeralDatabase( + migrationsDirectory: string, +): Promise { + const inMemoryNetlifyDB = new NetlifyDB({ directory: IN_MEMORY_DIRECTORY }) + await inMemoryNetlifyDB.start() + + try { + const applied = await applyMigrationsWithDetails(inMemoryNetlifyDB, migrationsDirectory) + return { status: 'success', applied } + } catch (error) { + const issues = errorToIssues(error) + if (issues.length === 0) { + return { status: 'success', applied: [] } + } + return { status: 'failure', issues } + } finally { + await inMemoryNetlifyDB.stop() + } +} diff --git a/packages/dev/src/main.ts b/packages/dev/src/main.ts index 23999cf7c..31c143ae9 100644 --- a/packages/dev/src/main.ts +++ b/packages/dev/src/main.ts @@ -24,8 +24,19 @@ import { RedirectsHandler } from '@netlify/redirects' import { StaticHandler } from '@netlify/static' import { NetlifyDB } from '@netlify/database-dev' -export { applyMigrations, initializeTrackingTable, resetDatabase } from '@netlify/database-dev' -export type { SQLExecutor } from '@netlify/database-dev' +export { + applyMigrations, + initializeTrackingTable, + resetDatabase, + testMigrationsInEphemeralDatabase, +} from '@netlify/database-dev' +export type { + Migration, + MigrationIssue, + PgErrorDetails, + SQLExecutor, + TestMigrationsInEphemeralDatabaseResult, +} from '@netlify/database-dev' import { InjectedEnvironmentVariable, injectEnvVariables } from './lib/env.js' import { isDirectory, isFile } from './lib/fs.js'