From 9321490c8a4b7f47f202acfdb773c7eaee3cd2c3 Mon Sep 17 00:00:00 2001 From: maxdml Date: Thu, 16 Jul 2026 15:49:07 -0700 Subject: [PATCH 1/4] Add --print-only flag to dbos migrate Prints the full system database migration SQL (schema creation, all migrations, version bookkeeping, and optional role grants) to stdout without connecting to the database, so users can run 'dbos migrate --print-only > migration.sql' and apply it with psql. --- .../dev/dbos/transact/cli/MigrateCommand.java | 41 +++++++--- .../transact/cli/MigratePrintOnlyTest.java | 79 +++++++++++++++++++ .../transact/migrations/MigrationManager.java | 42 ++++++++++ 3 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java diff --git a/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java b/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java index 631c2eef..5bd5991a 100644 --- a/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java +++ b/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java @@ -1,5 +1,6 @@ package dev.dbos.transact.cli; +import dev.dbos.transact.database.SystemDatabase; import dev.dbos.transact.migrations.MigrationManager; import java.io.PrintWriter; @@ -29,6 +30,11 @@ public class MigrateCommand implements Callable { "Use LISTEN/NOTIFY on the DBOS system database [default: ${DEFAULT-VALUE}]. Use --no-listen-notify to disable.") boolean useListenNotify; + @Option( + names = {"--print-only"}, + description = "Print the migration SQL to stdout without connecting to the database") + boolean printOnly; + @Mixin DatabaseOptions dbOptions; @Option( @@ -39,9 +45,33 @@ public class MigrateCommand implements Callable { @Spec CommandSpec spec; + static final String[] GRANT_QUERIES = { + "GRANT USAGE ON SCHEMA %s TO %s", + "GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA %s TO %s", + "GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA %s TO %s", + "GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA %s TO %s", + "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT ALL ON TABLES TO %s", + "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT ALL ON SEQUENCES TO %s", + "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT EXECUTE ON FUNCTIONS TO %s" + }; + @Override public Integer call() throws Exception { var out = spec.commandLine().getOut(); + + if (printOnly) { + out.print(MigrationManager.generateMigrationScript(dbOptions.schema(), useListenNotify)); + if (appRole != null && !appRole.isEmpty()) { + var schema = SystemDatabase.sanitizeSchema(dbOptions.schema()); + out.format("%n-- Grant %s schema permissions to %s%n", schema, appRole); + for (var query : GRANT_QUERIES) { + out.println(query.formatted(schema, appRole) + ";"); + } + } + out.flush(); + return 0; + } + out.println("Starting DBOS migrations"); out.format(" System Database: %s\n", dbOptions.url()); out.format(" System Database User: %s\n", dbOptions.user()); @@ -66,19 +96,10 @@ void grantDBOSSchemaPermissions(PrintWriter out, String schema) throws SQLExcept "Granting permissions for the %s schema to %s in database %s\n", schema, appRole, dbOptions.url()); - String[] queries = { - "GRANT USAGE ON SCHEMA %s TO %s", - "GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA %s TO %s", - "GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA %s TO %s", - "GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA %s TO %s", - "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT ALL ON TABLES TO %s", - "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT ALL ON SEQUENCES TO %s", - "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT EXECUTE ON FUNCTIONS TO %s" - }; try (var conn = DriverManager.getConnection(dbOptions.url(), dbOptions.user(), dbOptions.password()); var stmt = conn.createStatement()) { - for (var query : queries) { + for (var query : GRANT_QUERIES) { query = query.formatted(schema, appRole); stmt.execute(query); } diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java new file mode 100644 index 00000000..29408587 --- /dev/null +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java @@ -0,0 +1,79 @@ +package dev.dbos.transact.cli; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dev.dbos.transact.migrations.MigrationManager; + +import java.io.PrintWriter; +import java.io.StringWriter; + +import org.junit.jupiter.api.Test; +import picocli.CommandLine; + +// Runs without a database; --print-only must not connect. +public class MigratePrintOnlyTest { + + @Test + public void printOnly() { + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + + var exitCode = cmd.execute("migrate", "--print-only"); + assertEquals(0, exitCode); + + var sql = sw.toString(); + assertFalse(sql.contains("Starting DBOS migrations")); + assertTrue(sql.startsWith("CREATE SCHEMA IF NOT EXISTS \"dbos\";")); + assertTrue( + sql.contains( + "CREATE TABLE IF NOT EXISTS \"dbos\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);")); + assertTrue(sql.contains("CREATE TABLE \"dbos\".workflow_status")); + assertTrue(sql.contains("INSERT INTO \"dbos\".dbos_migrations (version) VALUES (1);")); + + var latest = MigrationManager.getMigrations("dbos", true, false).size(); + assertTrue(sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(latest))); + assertFalse(sql.contains("GRANT")); + + // Every non-blank line is SQL or a comment; every statement block ends with ';'. + for (var line : sql.split("\n")) { + if (line.isBlank() || line.startsWith("--")) { + continue; + } + assertFalse(line.startsWith("Starting"), "unexpected non-SQL output: " + line); + } + } + + @Test + public void printOnlyWithAppRoleAndSchema() { + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + + var exitCode = + cmd.execute("migrate", "--print-only", "--schema", "custom", "--app-role", "app_user"); + assertEquals(0, exitCode); + + var sql = sw.toString(); + assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"custom\";")); + assertTrue(sql.contains("CREATE TABLE \"custom\".workflow_status")); + assertTrue(sql.contains("GRANT USAGE ON SCHEMA custom TO app_user;")); + assertTrue( + sql.contains( + "ALTER DEFAULT PRIVILEGES IN SCHEMA custom GRANT EXECUTE ON FUNCTIONS TO app_user;")); + } + + @Test + public void printOnlyInvalidSchemaFails() { + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + cmd.setErr(new PrintWriter(new StringWriter())); + + var exitCode = cmd.execute("migrate", "--print-only", "--schema", "bad\"schema"); + assertEquals(1, exitCode); + assertEquals("", sw.toString()); + } +} diff --git a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java index 10f7bb28..031a398c 100644 --- a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java +++ b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java @@ -54,6 +54,48 @@ public static void runMigrations( } } + /** + * Generates the full SQL script that migrations would execute on a fresh Postgres database, + * including schema creation and dbos_migrations version bookkeeping. Requires no connection. + */ + public static String generateMigrationScript(String schema, boolean useListenNotify) { + schema = SystemDatabase.sanitizeSchema(schema); + if (schema.contains("'") || schema.contains("\"")) { + throw new IllegalArgumentException("Schema name must not contain single or double quotes"); + } + + var sb = new StringBuilder(); + sb.append("CREATE SCHEMA IF NOT EXISTS \"%s\";\n".formatted(schema)); + sb.append( + "CREATE TABLE IF NOT EXISTS \"%s\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);\n" + .formatted(schema)); + + var migrations = getMigrations(schema, useListenNotify, false); + for (var i = 0; i < migrations.size(); i++) { + var version = i + 1; + sb.append("\n-- DBOS system database migration %d\n".formatted(version)); + var sql = migrations.get(i).strip(); + if (version == 10) { + // Migration 10 adds the notifications primary key; on a fresh database + // migration 1 already created it, so only the version is recorded. + sql = ""; + } + if (!sql.isEmpty()) { + sb.append(sql); + if (!sql.endsWith(";")) { + sb.append(';'); + } + sb.append('\n'); + } + if (version == 1) { + sb.append("INSERT INTO \"%s\".dbos_migrations (version) VALUES (1);\n".formatted(schema)); + } else { + sb.append("UPDATE \"%s\".dbos_migrations SET version = %d;\n".formatted(schema, version)); + } + } + return sb.toString(); + } + private static boolean shouldMigrate( Connection conn, String schema, boolean useListenNotify, boolean isCockroach) throws SQLException { From e627c8b67796b6469e79e2e162df3e33ebc9a68b Mon Sep 17 00:00:00 2001 From: maxdml Date: Thu, 16 Jul 2026 16:50:07 -0700 Subject: [PATCH 2/4] print-only: fresh-db fail-fast guard, migration 10 runner conditional, funny-schema tests --- .../dbos/transact/cli/MigrateCommandTest.java | 40 ++++++++++++++++ .../transact/cli/MigratePrintOnlyTest.java | 46 +++++++++++++++++-- .../dev/dbos/transact/cli/PgContainer.java | 10 ++++ .../transact/migrations/MigrationManager.java | 43 +++++++++++++++-- 4 files changed, 132 insertions(+), 7 deletions(-) diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java index 13aa042c..c8d04a51 100644 --- a/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java @@ -1,10 +1,13 @@ package dev.dbos.transact.cli; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import dev.dbos.transact.Constants; import dev.dbos.transact.database.SystemDatabase; +import dev.dbos.transact.migrations.MigrationManager; import java.io.PrintWriter; import java.io.StringWriter; @@ -105,6 +108,43 @@ public void migrate_custom_schema(String schema) throws Exception { assertTrue(checkTable(schema, "workflow_status")); } + @Test + public void migrate_print_only_apply_funny_schema() throws Exception { + var schema = "F8nny_sCHem@-n@m3"; + + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + var exitCode = cmd.execute("migrate", "--print-only", "--schema", schema); + assertEquals(0, exitCode); + var script = sw.toString(); + + // Apply the printed script to a fresh database with psql ON_ERROR_STOP. + var applied = pgContainer.execPsql(script); + assertEquals(0, applied.getExitCode(), applied.getStderr()); + + assertTrue(checkTable(schema, "dbos_migrations")); + assertTrue(checkTable(schema, "workflow_status")); + assertTrue(checkTable(schema, "notifications")); + + var latest = MigrationManager.getMigrations(schema, true, false).size(); + try (var conn = pgContainer.connection(); + var stmt = conn.createStatement(); + var rs = + stmt.executeQuery("SELECT version FROM \"%s\".dbos_migrations".formatted(schema))) { + assertTrue(rs.next()); + assertEquals(latest, rs.getInt(1)); + assertFalse(rs.next()); + } + + // Re-applying must abort immediately: the script is for fresh databases only. + var reapplied = pgContainer.execPsql(script); + assertNotEquals(0, reapplied.getExitCode()); + assertTrue( + reapplied.getStderr().contains("this script is for fresh databases only"), + reapplied.getStderr()); + } + boolean checkTable(String schema, String table) throws SQLException { var sql = "SELECT EXISTS(SELECT 1 FROM information_schema.tables WHERE table_schema = ? AND table_name = ?)"; diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java index 29408587..4956592a 100644 --- a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java @@ -10,6 +10,8 @@ import java.io.StringWriter; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import picocli.CommandLine; // Runs without a database; --print-only must not connect. @@ -26,12 +28,21 @@ public void printOnly() { var sql = sw.toString(); assertFalse(sql.contains("Starting DBOS migrations")); - assertTrue(sql.startsWith("CREATE SCHEMA IF NOT EXISTS \"dbos\";")); + assertTrue(sql.startsWith("-- DBOS system database migration script for schema \"dbos\".")); + assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"dbos\";")); assertTrue( sql.contains( "CREATE TABLE IF NOT EXISTS \"dbos\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);")); + // Fresh-database guard immediately after the migrations table is created. + assertTrue( + sql.contains( + "RAISE EXCEPTION 'DBOS schema dbos is already at version %; this script is for fresh databases only. Use dbos migrate instead.', existing_version;")); assertTrue(sql.contains("CREATE TABLE \"dbos\".workflow_status")); assertTrue(sql.contains("INSERT INTO \"dbos\".dbos_migrations (version) VALUES (1);")); + // Migration 10 is emitted with the runner's conditional, not omitted. + assertTrue(sql.contains("WHERE table_schema = 'dbos' AND table_name = 'notifications'")); + assertTrue(sql.contains("AND constraint_type = 'PRIMARY KEY'")); + assertTrue(sql.contains("ALTER TABLE \"dbos\".notifications ADD PRIMARY KEY (message_uuid);")); var latest = MigrationManager.getMigrations("dbos", true, false).size(); assertTrue(sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(latest))); @@ -66,13 +77,42 @@ public void printOnlyWithAppRoleAndSchema() { } @Test - public void printOnlyInvalidSchemaFails() { + public void printOnlyFunnySchema() { + var schema = "F8nny_sCHem@-n@m3"; + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + + var exitCode = cmd.execute("migrate", "--print-only", "--schema", schema); + assertEquals(0, exitCode); + + var sql = sw.toString(); + assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"%s\";".formatted(schema))); + assertTrue(sql.contains("CREATE TABLE \"%s\".workflow_status".formatted(schema))); + assertTrue( + sql.contains( + "CREATE TABLE IF NOT EXISTS \"%s\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);" + .formatted(schema))); + // Migration 10 guard uses the schema as a string literal. + assertTrue(sql.contains("WHERE table_schema = '%s' AND table_name".formatted(schema))); + // The schema never appears unquoted in an identifier position (schema immediately + // followed by a dot only happens without the closing double quote). + assertFalse(sql.contains(schema + ".")); + + var latest = MigrationManager.getMigrations(schema, true, false).size(); + assertTrue( + sql.contains("UPDATE \"%s\".dbos_migrations SET version = %d;".formatted(schema, latest))); + } + + @ParameterizedTest + @ValueSource(strings = {"bad\"schema", "bad'schema"}) + public void printOnlyInvalidSchemaFails(String schema) { var cmd = new CommandLine(new DBOSCommand()); var sw = new StringWriter(); cmd.setOut(new PrintWriter(sw)); cmd.setErr(new PrintWriter(new StringWriter())); - var exitCode = cmd.execute("migrate", "--print-only", "--schema", "bad\"schema"); + var exitCode = cmd.execute("migrate", "--print-only", "--schema", schema); assertEquals(1, exitCode); assertEquals("", sw.toString()); } diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/PgContainer.java b/transact-cli/src/test/java/dev/dbos/transact/cli/PgContainer.java index 3db061be..0a016d99 100644 --- a/transact-cli/src/test/java/dev/dbos/transact/cli/PgContainer.java +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/PgContainer.java @@ -10,6 +10,8 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.Semaphore; +import org.testcontainers.containers.Container; +import org.testcontainers.images.builder.Transferable; import org.testcontainers.postgresql.PostgreSQLContainer; public class PgContainer implements AutoCloseable { @@ -96,6 +98,14 @@ public Connection connection() throws SQLException { return DriverManager.getConnection(jdbcUrl(), username(), password()); } + /** Applies a SQL script with psql -v ON_ERROR_STOP=1 inside the container. */ + public Container.ExecResult execPsql(String script) throws Exception { + var path = "/tmp/" + UUID.randomUUID() + ".sql"; + pgContainer.copyFileToContainer(Transferable.of(script), path); + return pgContainer.execInContainer( + "psql", "-v", "ON_ERROR_STOP=1", "-U", username(), "-d", dbName, "-f", path); + } + public List options() { return List.of(urlOption(), userOption(), passwordOption()); } diff --git a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java index 031a398c..987eb968 100644 --- a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java +++ b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java @@ -56,7 +56,9 @@ public static void runMigrations( /** * Generates the full SQL script that migrations would execute on a fresh Postgres database, - * including schema creation and dbos_migrations version bookkeeping. Requires no connection. + * including schema creation and dbos_migrations version bookkeeping. Requires no connection. The + * script is for fresh databases only and fails fast (under psql ON_ERROR_STOP) if dbos_migrations + * already contains a row. */ public static String generateMigrationScript(String schema, boolean useListenNotify) { schema = SystemDatabase.sanitizeSchema(schema); @@ -65,10 +67,28 @@ public static String generateMigrationScript(String schema, boolean useListenNot } var sb = new StringBuilder(); + sb.append("-- DBOS system database migration script for schema \"%s\".\n".formatted(schema)); + sb.append("-- For FRESH databases only; aborts if DBOS migrations were already applied.\n"); + sb.append("-- Apply with: psql -v ON_ERROR_STOP=1 -f \n"); sb.append("CREATE SCHEMA IF NOT EXISTS \"%s\";\n".formatted(schema)); sb.append( "CREATE TABLE IF NOT EXISTS \"%s\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);\n" .formatted(schema)); + sb.append( + """ + + -- Fail fast if this is not a fresh database. + DO $$ + DECLARE + existing_version BIGINT; + BEGIN + SELECT version INTO existing_version FROM "%1$s".dbos_migrations LIMIT 1; + IF existing_version IS NOT NULL THEN + RAISE EXCEPTION 'DBOS schema %1$s is already at version %%; this script is for fresh databases only. Use dbos migrate instead.', existing_version; + END IF; + END $$; + """ + .formatted(schema)); var migrations = getMigrations(schema, useListenNotify, false); for (var i = 0; i < migrations.size(); i++) { @@ -76,9 +96,24 @@ public static String generateMigrationScript(String schema, boolean useListenNot sb.append("\n-- DBOS system database migration %d\n".formatted(version)); var sql = migrations.get(i).strip(); if (version == 10) { - // Migration 10 adds the notifications primary key; on a fresh database - // migration 1 already created it, so only the version is recorded. - sql = ""; + // Same conditional the migration runner applies at execution time. + sql = + """ + -- Mirrors the runner's guard: add the notifications primary key only if one + -- does not already exist (migration 1 creates it on fresh databases). + DO $$ + BEGIN + IF NOT EXISTS ( + SELECT 1 FROM information_schema.table_constraints + WHERE table_schema = '%1$s' AND table_name = 'notifications' + AND constraint_type = 'PRIMARY KEY' + ) THEN + %2$s + END IF; + END $$; + """ + .formatted(schema, sql) + .strip(); } if (!sql.isEmpty()) { sb.append(sql); From 41f9d9b5403007bee9ed73974814c69b1d92c017 Mon Sep 17 00:00:00 2001 From: maxdml Date: Thu, 16 Jul 2026 16:59:54 -0700 Subject: [PATCH 3/4] print-only: version-aware delta scripts (connect read-only, print N+1..latest) --- .../dev/dbos/transact/cli/MigrateCommand.java | 31 +++++- .../dbos/transact/cli/MigrateCommandTest.java | 79 +++++++++++++++ .../transact/cli/MigratePrintOnlyTest.java | 98 +++++++++++++++++++ .../transact/migrations/MigrationManager.java | 89 ++++++++++++----- 4 files changed, 273 insertions(+), 24 deletions(-) diff --git a/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java b/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java index 5bd5991a..0cad62f2 100644 --- a/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java +++ b/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java @@ -60,7 +60,9 @@ public Integer call() throws Exception { var out = spec.commandLine().getOut(); if (printOnly) { - out.print(MigrationManager.generateMigrationScript(dbOptions.schema(), useListenNotify)); + out.print( + MigrationManager.generateMigrationScript( + dbOptions.schema(), useListenNotify, resolveCurrentVersion())); if (appRole != null && !appRole.isEmpty()) { var schema = SystemDatabase.sanitizeSchema(dbOptions.schema()); out.format("%n-- Grant %s schema permissions to %s%n", schema, appRole); @@ -86,6 +88,33 @@ public Integer call() throws Exception { return 0; } + // Best-effort, read-only. Returns the current dbos_migrations version, or 0 (fresh script) + // when no URL is configured, the connection fails, or the migrations table does not exist. + // Deliberately silent so --print-only output stays pure SQL and pipeable. + int resolveCurrentVersion() { + if (dbOptions.url() == null) { + return 0; + } + var schema = SystemDatabase.sanitizeSchema(dbOptions.schema()); + try (var conn = + DriverManager.getConnection(dbOptions.url(), dbOptions.user(), dbOptions.password())) { + var sql = + "SELECT 1 FROM information_schema.tables" + + " WHERE table_schema = ? AND table_name = 'dbos_migrations'"; + try (var stmt = conn.prepareStatement(sql)) { + stmt.setString(1, schema); + try (var rs = stmt.executeQuery()) { + if (!rs.next()) { + return 0; + } + } + } + return MigrationManager.getCurrentSysDbVersion(conn, schema); + } catch (SQLException e) { + return 0; + } + } + void grantDBOSSchemaPermissions(PrintWriter out, String schema) throws SQLException { if (appRole == null || appRole.isEmpty()) { diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java index c8d04a51..a83cf120 100644 --- a/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java @@ -145,6 +145,85 @@ public void migrate_print_only_apply_funny_schema() throws Exception { reapplied.getStderr()); } + @ParameterizedTest + @ValueSource(strings = {Constants.DB_SCHEMA, "F8nny_sCHem@-n@m3"}) + public void migrate_print_only_delta(String schema) throws Exception { + var latest = MigrationManager.getMigrations(schema, true, false).size(); + var from = latest - 3; + + // Build a genuinely partial database at version `from` by applying the fresh + // script only up through migration `from`'s bookkeeping. + var fresh = MigrationManager.generateMigrationScript(schema, true); + var marker = "\n-- DBOS system database migration %d\n".formatted(from + 1); + var idx = fresh.indexOf(marker); + assertTrue(idx > 0); + var partial = pgContainer.execPsql(fresh.substring(0, idx)); + assertEquals(0, partial.getExitCode(), partial.getStderr()); + assertEquals(from, currentVersion(schema)); + + // The CLI connects, reads the version, and prints a delta. + var delta = runPrintOnly(schema); + assertTrue(delta.contains("IF existing_version IS DISTINCT FROM %d THEN".formatted(from))); + assertFalse(delta.contains("CREATE SCHEMA")); + assertFalse(delta.contains("INSERT INTO \"%s\".dbos_migrations".formatted(schema))); + assertFalse(delta.contains("-- DBOS system database migration %d\n".formatted(from))); + assertTrue(delta.contains("-- DBOS system database migration %d\n".formatted(from + 1))); + + var applied = pgContainer.execPsql(delta); + assertEquals(0, applied.getExitCode(), applied.getStderr()); + assertEquals(latest, currentVersion(schema)); + + // Re-applying the delta fails the exact-version guard under ON_ERROR_STOP. + var reapplied = pgContainer.execPsql(delta); + assertNotEquals(0, reapplied.getExitCode()); + assertTrue( + reapplied.getStderr().contains("upgrades from version %d".formatted(from)), + reapplied.getStderr()); + assertEquals(latest, currentVersion(schema)); + + // An up-to-date database prints only the nothing-to-do comment. + assertEquals( + "-- Database is already at the latest DBOS schema version (%d); nothing to do.\n" + .formatted(latest), + runPrintOnly(schema)); + + // The fresh script against an already-migrated database fails fast. + var freshOnMigrated = pgContainer.execPsql(fresh); + assertNotEquals(0, freshOnMigrated.getExitCode()); + assertTrue( + freshOnMigrated.getStderr().contains("this script is for fresh databases only"), + freshOnMigrated.getStderr()); + } + + String runPrintOnly(String schema) { + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + var ew = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + cmd.setErr(new PrintWriter(ew)); + + var args = + Stream.of(List.of("migrate", "--print-only", "--schema", schema), pgContainer.options()) + .flatMap(Collection::stream) + .toArray(String[]::new); + + assertEquals(0, cmd.execute(args)); + assertEquals("", ew.toString()); + return sw.toString(); + } + + int currentVersion(String schema) throws SQLException { + try (var conn = pgContainer.connection(); + var stmt = conn.createStatement(); + var rs = + stmt.executeQuery("SELECT version FROM \"%s\".dbos_migrations".formatted(schema))) { + assertTrue(rs.next()); + var version = rs.getInt(1); + assertFalse(rs.next()); + return version; + } + } + boolean checkTable(String schema, String table) throws SQLException { var sql = "SELECT EXISTS(SELECT 1 FROM information_schema.tables WHERE table_schema = ? AND table_name = ?)"; diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java index 4956592a..94fbfcda 100644 --- a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import dev.dbos.transact.migrations.MigrationManager; @@ -21,10 +22,14 @@ public class MigratePrintOnlyTest { public void printOnly() { var cmd = new CommandLine(new DBOSCommand()); var sw = new StringWriter(); + var ew = new StringWriter(); cmd.setOut(new PrintWriter(sw)); + cmd.setErr(new PrintWriter(ew)); var exitCode = cmd.execute("migrate", "--print-only"); assertEquals(0, exitCode); + // No credentials: full fresh script, silently (stdout must be pipeable to a .sql file). + assertEquals("", ew.toString()); var sql = sw.toString(); assertFalse(sql.contains("Starting DBOS migrations")); @@ -104,6 +109,99 @@ public void printOnlyFunnySchema() { sql.contains("UPDATE \"%s\".dbos_migrations SET version = %d;".formatted(schema, latest))); } + @Test + public void printOnlyConnectionFailureFallsBackToFreshScript() { + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + var ew = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + cmd.setErr(new PrintWriter(ew)); + + var exitCode = + cmd.execute( + "migrate", + "--print-only", + "-D=jdbc:postgresql://127.0.0.1:1/nosuchdb?connectTimeout=2", + "-U=nobody", + "-P=nothing"); + assertEquals(0, exitCode); + assertEquals("", ew.toString()); + + var sql = sw.toString(); + assertTrue(sql.startsWith("-- DBOS system database migration script for schema \"dbos\".")); + assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"dbos\";")); + assertTrue(sql.contains("INSERT INTO \"dbos\".dbos_migrations (version) VALUES (1);")); + } + + @Test + public void printOnlyDelta() { + var latest = MigrationManager.getMigrations("dbos", true, false).size(); + var from = latest - 2; + var sql = MigrationManager.generateMigrationScript("dbos", true, from); + + // Delta prelude: no schema/table creation, no fresh guard; exact-version guard instead. + assertTrue( + sql.startsWith( + "-- DBOS system database delta migration script for schema \"dbos\" (version %d to %d)." + .formatted(from, latest))); + assertFalse(sql.contains("CREATE SCHEMA")); + assertFalse(sql.contains("CREATE TABLE IF NOT EXISTS \"dbos\".dbos_migrations")); + assertFalse(sql.contains("fresh databases only")); + assertTrue(sql.contains("IF existing_version IS DISTINCT FROM %d THEN".formatted(from))); + + // Only migrations from+1..latest appear. + for (var v = 1; v <= from; v++) { + assertFalse(sql.contains("-- DBOS system database migration %d\n".formatted(v))); + } + for (var v = from + 1; v <= latest; v++) { + assertTrue(sql.contains("-- DBOS system database migration %d\n".formatted(v))); + } + // Migration 10 is out of range, so its conditional block is absent. + assertFalse(sql.contains("table_constraints")); + + // Bookkeeping: UPDATE only, never INSERT. + assertFalse(sql.contains("INSERT INTO \"dbos\".dbos_migrations")); + assertTrue( + sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(from + 1))); + assertTrue(sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(latest))); + } + + @Test + public void printOnlyDeltaFunnySchema() { + var schema = "F8nny_sCHem@-n@m3"; + var latest = MigrationManager.getMigrations(schema, true, false).size(); + var from = 9; // puts migration 10 in the delta range + var sql = MigrationManager.generateMigrationScript(schema, true, from); + + assertTrue(sql.contains("SELECT version INTO existing_version FROM \"%s\"".formatted(schema))); + assertTrue(sql.contains("WHERE table_schema = '%s' AND table_name".formatted(schema))); + assertTrue( + sql.contains( + "ALTER TABLE \"%s\".notifications ADD PRIMARY KEY (message_uuid);".formatted(schema))); + assertTrue( + sql.contains("UPDATE \"%s\".dbos_migrations SET version = %d;".formatted(schema, latest))); + assertFalse(sql.contains(schema + ".")); // never unquoted in identifier position + // Bookkeeping starts with an UPDATE, never an INSERT (function bodies in + // migrations 14/38 legitimately INSERT INTO workflow_status). + assertFalse(sql.contains("INSERT INTO \"%s\".dbos_migrations".formatted(schema))); + } + + @Test + public void printOnlyUpToDateAndInvalidFromVersion() { + var latest = MigrationManager.getMigrations("dbos", true, false).size(); + assertEquals( + "-- Database is already at the latest DBOS schema version (%d); nothing to do.\n" + .formatted(latest), + MigrationManager.generateMigrationScript("dbos", true, latest)); + + assertThrows( + IllegalArgumentException.class, + () -> MigrationManager.generateMigrationScript("dbos", true, -1)); + assertThrows( + IllegalArgumentException.class, + () -> MigrationManager.generateMigrationScript("dbos", true, latest + 1)); + } + @ParameterizedTest @ValueSource(strings = {"bad\"schema", "bad'schema"}) public void printOnlyInvalidSchemaFails(String schema) { diff --git a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java index 987eb968..9ada4e3a 100644 --- a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java +++ b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java @@ -61,37 +61,80 @@ public static void runMigrations( * already contains a row. */ public static String generateMigrationScript(String schema, boolean useListenNotify) { + return generateMigrationScript(schema, useListenNotify, 0); + } + + /** + * Generates the SQL script that upgrades a database from {@code fromVersion} (0 = fresh) to the + * latest DBOS schema version. Requires no connection. Fresh scripts abort (under psql + * ON_ERROR_STOP) if migrations were already applied; delta scripts abort unless the database is + * exactly at {@code fromVersion}. + */ + public static String generateMigrationScript( + String schema, boolean useListenNotify, int fromVersion) { schema = SystemDatabase.sanitizeSchema(schema); if (schema.contains("'") || schema.contains("\"")) { throw new IllegalArgumentException("Schema name must not contain single or double quotes"); } + var migrations = getMigrations(schema, useListenNotify, false); + var latest = migrations.size(); + if (fromVersion < 0 || fromVersion > latest) { + throw new IllegalArgumentException( + "fromVersion must be between 0 and %d, got %d".formatted(latest, fromVersion)); + } + if (fromVersion == latest) { + return "-- Database is already at the latest DBOS schema version (%d); nothing to do.\n" + .formatted(latest); + } + var sb = new StringBuilder(); - sb.append("-- DBOS system database migration script for schema \"%s\".\n".formatted(schema)); - sb.append("-- For FRESH databases only; aborts if DBOS migrations were already applied.\n"); - sb.append("-- Apply with: psql -v ON_ERROR_STOP=1 -f \n"); - sb.append("CREATE SCHEMA IF NOT EXISTS \"%s\";\n".formatted(schema)); - sb.append( - "CREATE TABLE IF NOT EXISTS \"%s\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);\n" - .formatted(schema)); - sb.append( - """ + if (fromVersion == 0) { + sb.append("-- DBOS system database migration script for schema \"%s\".\n".formatted(schema)); + sb.append("-- For FRESH databases only; aborts if DBOS migrations were already applied.\n"); + sb.append("-- Apply with: psql -v ON_ERROR_STOP=1 -f \n"); + sb.append("CREATE SCHEMA IF NOT EXISTS \"%s\";\n".formatted(schema)); + sb.append( + "CREATE TABLE IF NOT EXISTS \"%s\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);\n" + .formatted(schema)); + sb.append( + """ - -- Fail fast if this is not a fresh database. - DO $$ - DECLARE - existing_version BIGINT; - BEGIN - SELECT version INTO existing_version FROM "%1$s".dbos_migrations LIMIT 1; - IF existing_version IS NOT NULL THEN - RAISE EXCEPTION 'DBOS schema %1$s is already at version %%; this script is for fresh databases only. Use dbos migrate instead.', existing_version; - END IF; - END $$; - """ - .formatted(schema)); + -- Fail fast if this is not a fresh database. + DO $$ + DECLARE + existing_version BIGINT; + BEGIN + SELECT version INTO existing_version FROM "%1$s".dbos_migrations LIMIT 1; + IF existing_version IS NOT NULL THEN + RAISE EXCEPTION 'DBOS schema %1$s is already at version %%; this script is for fresh databases only. Use dbos migrate instead.', existing_version; + END IF; + END $$; + """ + .formatted(schema)); + } else { + sb.append( + "-- DBOS system database delta migration script for schema \"%s\" (version %d to %d).\n" + .formatted(schema, fromVersion, latest)); + sb.append("-- Apply with: psql -v ON_ERROR_STOP=1 -f \n"); + sb.append( + """ - var migrations = getMigrations(schema, useListenNotify, false); - for (var i = 0; i < migrations.size(); i++) { + -- Fail fast unless the database is exactly at the expected version. + DO $$ + DECLARE + existing_version BIGINT; + BEGIN + SELECT version INTO existing_version FROM "%1$s".dbos_migrations LIMIT 1; + IF existing_version IS DISTINCT FROM %2$d THEN + RAISE EXCEPTION 'DBOS schema %1$s is at version %% but this script upgrades from version %2$d; regenerate it with dbos migrate --print-only.', existing_version; + END IF; + END $$; + """ + .formatted(schema, fromVersion)); + } + + for (var i = fromVersion; i < migrations.size(); i++) { var version = i + 1; sb.append("\n-- DBOS system database migration %d\n".formatted(version)); var sql = migrations.get(i).strip(); From a359aea198e10c8594ec71d858514067bcbd3909 Mon Sep 17 00:00:00 2001 From: maxdml Date: Tue, 21 Jul 2026 19:01:08 -0700 Subject: [PATCH 4/4] Replace --print-only with --print-migrations and --print-user-role Match the final design of dbos-transact-py#780: - dbos migrate --print-migrations [all|N] prints migrations N..latest to stdout without ever connecting to a database; no DO $$ guard blocks, migration 10 always skipped with a comment, per-migration version bookkeeping mirroring the runner. - dbos migrate --print-user-role --app-role R prints the GRANT statements; mutually exclusive with --print-migrations. - GRANT_QUERIES now double-quote schema and role identifiers on both the print and execute paths. --- .../dev/dbos/transact/cli/MigrateCommand.java | 119 ++++++---- .../dbos/transact/cli/MigrateCommandTest.java | 140 +++++------ .../transact/cli/MigratePrintOnlyTest.java | 217 ------------------ .../dbos/transact/cli/MigratePrintTest.java | 196 ++++++++++++++++ .../transact/migrations/MigrationManager.java | 112 +++------ 5 files changed, 376 insertions(+), 408 deletions(-) delete mode 100644 transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java create mode 100644 transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintTest.java diff --git a/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java b/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java index 0cad62f2..7bbd4742 100644 --- a/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java +++ b/transact-cli/src/main/java/dev/dbos/transact/cli/MigrateCommand.java @@ -31,9 +31,17 @@ public class MigrateCommand implements Callable { boolean useListenNotify; @Option( - names = {"--print-only"}, - description = "Print the migration SQL to stdout without connecting to the database") - boolean printOnly; + names = {"--print-migrations"}, + paramLabel = "[all|NUMBER]", + description = + "Print the SQL of all migrations ('--print-migrations all') or of migrations from a number onward ('--print-migrations 3') instead of running them") + String printMigrations; + + @Option( + names = {"--print-user-role"}, + description = + "Print the SQL granting the application role (--app-role) access to DBOS system tables instead of executing it") + boolean printUserRole; @Mixin DatabaseOptions dbOptions; @@ -46,32 +54,23 @@ public class MigrateCommand implements Callable { @Spec CommandSpec spec; static final String[] GRANT_QUERIES = { - "GRANT USAGE ON SCHEMA %s TO %s", - "GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA %s TO %s", - "GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA %s TO %s", - "GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA %s TO %s", - "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT ALL ON TABLES TO %s", - "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT ALL ON SEQUENCES TO %s", - "ALTER DEFAULT PRIVILEGES IN SCHEMA %s GRANT EXECUTE ON FUNCTIONS TO %s" + "GRANT USAGE ON SCHEMA \"%1$s\" TO \"%2$s\"", + "GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA \"%1$s\" TO \"%2$s\"", + "GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA \"%1$s\" TO \"%2$s\"", + "GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA \"%1$s\" TO \"%2$s\"", + "ALTER DEFAULT PRIVILEGES IN SCHEMA \"%1$s\" GRANT ALL ON TABLES TO \"%2$s\"", + "ALTER DEFAULT PRIVILEGES IN SCHEMA \"%1$s\" GRANT ALL ON SEQUENCES TO \"%2$s\"", + "ALTER DEFAULT PRIVILEGES IN SCHEMA \"%1$s\" GRANT EXECUTE ON FUNCTIONS TO \"%2$s\"" }; @Override public Integer call() throws Exception { var out = spec.commandLine().getOut(); - if (printOnly) { - out.print( - MigrationManager.generateMigrationScript( - dbOptions.schema(), useListenNotify, resolveCurrentVersion())); - if (appRole != null && !appRole.isEmpty()) { - var schema = SystemDatabase.sanitizeSchema(dbOptions.schema()); - out.format("%n-- Grant %s schema permissions to %s%n", schema, appRole); - for (var query : GRANT_QUERIES) { - out.println(query.formatted(schema, appRole) + ";"); - } - } + if (printMigrations != null || printUserRole) { + var exitCode = printSql(out, spec.commandLine().getErr()); out.flush(); - return 0; + return exitCode; } out.println("Starting DBOS migrations"); @@ -88,31 +87,66 @@ public Integer call() throws Exception { return 0; } - // Best-effort, read-only. Returns the current dbos_migrations version, or 0 (fresh script) - // when no URL is configured, the connection fails, or the migrations table does not exist. - // Deliberately silent so --print-only output stays pure SQL and pipeable. - int resolveCurrentVersion() { - if (dbOptions.url() == null) { - return 0; + // Stdout stays pure SQL and comments (pipeable to a .sql file); never connects. + int printSql(PrintWriter out, PrintWriter err) { + if (printMigrations != null && printUserRole) { + err.println("--print-user-role cannot be combined with --print-migrations"); + return 1; } var schema = SystemDatabase.sanitizeSchema(dbOptions.schema()); - try (var conn = - DriverManager.getConnection(dbOptions.url(), dbOptions.user(), dbOptions.password())) { - var sql = - "SELECT 1 FROM information_schema.tables" - + " WHERE table_schema = ? AND table_name = 'dbos_migrations'"; - try (var stmt = conn.prepareStatement(sql)) { - stmt.setString(1, schema); - try (var rs = stmt.executeQuery()) { - if (!rs.next()) { - return 0; - } - } + if (schema.contains("'") || schema.contains("\"")) { + err.println("Schema names containing quotes are not supported"); + return 1; + } + + if (printUserRole) { + if (appRole == null || appRole.isEmpty()) { + err.println("--print-user-role requires --app-role"); + return 1; + } + if (appRole.contains("'") || appRole.contains("\"")) { + err.println("Role names containing quotes are not supported"); + return 1; + } + out.format("-- Permissions on DBOS schema %s for role %s%n", schema, appRole); + for (var query : GRANT_QUERIES) { + out.println(query.formatted(schema, appRole) + ";"); } - return MigrationManager.getCurrentSysDbVersion(conn, schema); - } catch (SQLException e) { return 0; } + + var latest = MigrationManager.getMigrations(schema, useListenNotify, false).size(); + int start; + if (printMigrations.equals("all")) { + start = 1; + } else { + try { + start = Integer.parseInt(printMigrations); + } catch (NumberFormatException e) { + err.format( + "Invalid --print-migrations value '%s': expected 'all' or a migration number%n", + printMigrations); + return 1; + } + if (start < 1 || start > latest) { + err.format( + "Migration %d does not exist: valid migrations are 1 through %d%n", start, latest); + return 1; + } + } + + out.format("-- DBOS system database migrations for %s%n", maskPassword(dbOptions.url())); + out.println( + "-- Contains CREATE/DROP INDEX CONCURRENTLY: run outside a transaction block (e.g. plain psql, not psql --single-transaction)."); + out.print(MigrationManager.generateMigrationScript(schema, useListenNotify, start)); + return 0; + } + + static String maskPassword(String url) { + if (url == null) { + return "the system database"; + } + return url.replaceAll("(?i)(password=)[^&]*", "$1***"); } void grantDBOSSchemaPermissions(PrintWriter out, String schema) throws SQLException { @@ -120,6 +154,7 @@ void grantDBOSSchemaPermissions(PrintWriter out, String schema) throws SQLExcept if (appRole == null || appRole.isEmpty()) { return; } + schema = SystemDatabase.sanitizeSchema(schema); out.format( "Granting permissions for the %s schema to %s in database %s\n", diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java index a83cf120..7963c05c 100644 --- a/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/MigrateCommandTest.java @@ -2,7 +2,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import dev.dbos.transact.Constants; @@ -11,6 +10,7 @@ import java.io.PrintWriter; import java.io.StringWriter; +import java.sql.DriverManager; import java.sql.SQLException; import java.util.Collection; import java.util.List; @@ -109,15 +109,13 @@ public void migrate_custom_schema(String schema) throws Exception { } @Test - public void migrate_print_only_apply_funny_schema() throws Exception { + public void migrate_print_migrations_apply_funny_schema() throws Exception { var schema = "F8nny_sCHem@-n@m3"; - var cmd = new CommandLine(new DBOSCommand()); - var sw = new StringWriter(); - cmd.setOut(new PrintWriter(sw)); - var exitCode = cmd.execute("migrate", "--print-only", "--schema", schema); - assertEquals(0, exitCode); - var script = sw.toString(); + var script = runPrint("--schema", schema, "--print-migrations", "all"); + assertTrue(script.contains("-- Migration 10 skipped: not applicable on fresh databases")); + assertFalse(script.contains("ADD PRIMARY KEY (message_uuid)")); + assertFalse(script.contains("DO $$")); // Apply the printed script to a fresh database with psql ON_ERROR_STOP. var applied = pgContainer.execPsql(script); @@ -128,74 +126,88 @@ public void migrate_print_only_apply_funny_schema() throws Exception { assertTrue(checkTable(schema, "notifications")); var latest = MigrationManager.getMigrations(schema, true, false).size(); - try (var conn = pgContainer.connection(); - var stmt = conn.createStatement(); - var rs = - stmt.executeQuery("SELECT version FROM \"%s\".dbos_migrations".formatted(schema))) { - assertTrue(rs.next()); - assertEquals(latest, rs.getInt(1)); - assertFalse(rs.next()); - } + assertEquals(latest, currentVersion(schema)); - // Re-applying must abort immediately: the script is for fresh databases only. - var reapplied = pgContainer.execPsql(script); - assertNotEquals(0, reapplied.getExitCode()); - assertTrue( - reapplied.getStderr().contains("this script is for fresh databases only"), - reapplied.getStderr()); + // A real migration run now considers the database up to date. + var cmd = new CommandLine(new DBOSCommand()); + cmd.setOut(new PrintWriter(new StringWriter())); + var args = + Stream.of(List.of("migrate", "--schema", schema), pgContainer.options()) + .flatMap(Collection::stream) + .toArray(String[]::new); + assertEquals(0, cmd.execute(args)); + assertEquals(latest, currentVersion(schema)); } - @ParameterizedTest - @ValueSource(strings = {Constants.DB_SCHEMA, "F8nny_sCHem@-n@m3"}) - public void migrate_print_only_delta(String schema) throws Exception { + @Test + public void migrate_print_from_migration() throws Exception { + var schema = Constants.DB_SCHEMA; var latest = MigrationManager.getMigrations(schema, true, false).size(); - var from = latest - 3; - // Build a genuinely partial database at version `from` by applying the fresh - // script only up through migration `from`'s bookkeeping. - var fresh = MigrationManager.generateMigrationScript(schema, true); - var marker = "\n-- DBOS system database migration %d\n".formatted(from + 1); - var idx = fresh.indexOf(marker); + // Bring a fresh database to version latest-1 by truncating the full script. + var full = runPrint("--print-migrations", "all"); + var marker = "UPDATE \"%s\".dbos_migrations SET version = %d;".formatted(schema, latest - 1); + var idx = full.indexOf(marker); assertTrue(idx > 0); - var partial = pgContainer.execPsql(fresh.substring(0, idx)); - assertEquals(0, partial.getExitCode(), partial.getStderr()); - assertEquals(from, currentVersion(schema)); - - // The CLI connects, reads the version, and prints a delta. - var delta = runPrintOnly(schema); - assertTrue(delta.contains("IF existing_version IS DISTINCT FROM %d THEN".formatted(from))); - assertFalse(delta.contains("CREATE SCHEMA")); - assertFalse(delta.contains("INSERT INTO \"%s\".dbos_migrations".formatted(schema))); - assertFalse(delta.contains("-- DBOS system database migration %d\n".formatted(from))); - assertTrue(delta.contains("-- DBOS system database migration %d\n".formatted(from + 1))); - - var applied = pgContainer.execPsql(delta); + var appliedPartial = pgContainer.execPsql(full.substring(0, idx + marker.length()) + "\n"); + assertEquals(0, appliedPartial.getExitCode(), appliedPartial.getStderr()); + assertEquals(latest - 1, currentVersion(schema)); + + // The last migration printed alone applies on top of version latest-1. + var single = runPrint("--print-migrations", String.valueOf(latest)); + assertFalse(single.contains("CREATE SCHEMA")); + assertFalse(single.contains("DO $$")); + assertFalse(single.contains("INSERT INTO \"%s\".dbos_migrations".formatted(schema))); + var applied = pgContainer.execPsql(single); assertEquals(0, applied.getExitCode(), applied.getStderr()); assertEquals(latest, currentVersion(schema)); + } - // Re-applying the delta fails the exact-version guard under ON_ERROR_STOP. - var reapplied = pgContainer.execPsql(delta); - assertNotEquals(0, reapplied.getExitCode()); + @Test + public void migrate_print_user_role_grants_access() throws Exception { + var schema = "F8nny_sCHem@-n@m3"; + var role = "my-app-role"; + + var script = runPrint("--schema", schema, "--print-migrations", "all"); + var roleScript = runPrint("--schema", schema, "--print-user-role", "--app-role", role); assertTrue( - reapplied.getStderr().contains("upgrades from version %d".formatted(from)), - reapplied.getStderr()); - assertEquals(latest, currentVersion(schema)); + roleScript.contains("GRANT USAGE ON SCHEMA \"%s\" TO \"%s\";".formatted(schema, role))); + for (var line : roleScript.split("\n")) { + assertTrue( + line.startsWith("--") || line.startsWith("GRANT") || line.startsWith("ALTER"), + "unexpected output: " + line); + } - // An up-to-date database prints only the nothing-to-do comment. - assertEquals( - "-- Database is already at the latest DBOS schema version (%d); nothing to do.\n" - .formatted(latest), - runPrintOnly(schema)); + try (var conn = pgContainer.connection(); + var stmt = conn.createStatement()) { + stmt.execute("DROP ROLE IF EXISTS \"%s\"".formatted(role)); + stmt.execute("CREATE ROLE \"%s\" LOGIN PASSWORD 'app_role_pwd'".formatted(role)); + } + try { + for (var s : List.of(script, roleScript)) { + var applied = pgContainer.execPsql(s); + assertEquals(0, applied.getExitCode(), applied.getStderr()); + } - // The fresh script against an already-migrated database fails fast. - var freshOnMigrated = pgContainer.execPsql(fresh); - assertNotEquals(0, freshOnMigrated.getExitCode()); - assertTrue( - freshOnMigrated.getStderr().contains("this script is for fresh databases only"), - freshOnMigrated.getStderr()); + // The app role can query the DBOS schema. + var latest = MigrationManager.getMigrations(schema, true, false).size(); + try (var conn = DriverManager.getConnection(pgContainer.jdbcUrl(), role, "app_role_pwd"); + var stmt = conn.createStatement(); + var rs = + stmt.executeQuery("SELECT version FROM \"%s\".dbos_migrations".formatted(schema))) { + assertTrue(rs.next()); + assertEquals(latest, rs.getInt(1)); + } + } finally { + try (var conn = pgContainer.connection(); + var stmt = conn.createStatement()) { + stmt.execute("DROP OWNED BY \"%s\"".formatted(role)); + stmt.execute("DROP ROLE \"%s\"".formatted(role)); + } + } } - String runPrintOnly(String schema) { + String runPrint(String... printArgs) { var cmd = new CommandLine(new DBOSCommand()); var sw = new StringWriter(); var ew = new StringWriter(); @@ -203,11 +215,11 @@ String runPrintOnly(String schema) { cmd.setErr(new PrintWriter(ew)); var args = - Stream.of(List.of("migrate", "--print-only", "--schema", schema), pgContainer.options()) + Stream.of(List.of("migrate"), List.of(printArgs), pgContainer.options()) .flatMap(Collection::stream) .toArray(String[]::new); - assertEquals(0, cmd.execute(args)); + assertEquals(0, cmd.execute(args), ew.toString()); assertEquals("", ew.toString()); return sw.toString(); } diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java deleted file mode 100644 index 94fbfcda..00000000 --- a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintOnlyTest.java +++ /dev/null @@ -1,217 +0,0 @@ -package dev.dbos.transact.cli; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import dev.dbos.transact.migrations.MigrationManager; - -import java.io.PrintWriter; -import java.io.StringWriter; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; -import picocli.CommandLine; - -// Runs without a database; --print-only must not connect. -public class MigratePrintOnlyTest { - - @Test - public void printOnly() { - var cmd = new CommandLine(new DBOSCommand()); - var sw = new StringWriter(); - var ew = new StringWriter(); - cmd.setOut(new PrintWriter(sw)); - cmd.setErr(new PrintWriter(ew)); - - var exitCode = cmd.execute("migrate", "--print-only"); - assertEquals(0, exitCode); - // No credentials: full fresh script, silently (stdout must be pipeable to a .sql file). - assertEquals("", ew.toString()); - - var sql = sw.toString(); - assertFalse(sql.contains("Starting DBOS migrations")); - assertTrue(sql.startsWith("-- DBOS system database migration script for schema \"dbos\".")); - assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"dbos\";")); - assertTrue( - sql.contains( - "CREATE TABLE IF NOT EXISTS \"dbos\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);")); - // Fresh-database guard immediately after the migrations table is created. - assertTrue( - sql.contains( - "RAISE EXCEPTION 'DBOS schema dbos is already at version %; this script is for fresh databases only. Use dbos migrate instead.', existing_version;")); - assertTrue(sql.contains("CREATE TABLE \"dbos\".workflow_status")); - assertTrue(sql.contains("INSERT INTO \"dbos\".dbos_migrations (version) VALUES (1);")); - // Migration 10 is emitted with the runner's conditional, not omitted. - assertTrue(sql.contains("WHERE table_schema = 'dbos' AND table_name = 'notifications'")); - assertTrue(sql.contains("AND constraint_type = 'PRIMARY KEY'")); - assertTrue(sql.contains("ALTER TABLE \"dbos\".notifications ADD PRIMARY KEY (message_uuid);")); - - var latest = MigrationManager.getMigrations("dbos", true, false).size(); - assertTrue(sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(latest))); - assertFalse(sql.contains("GRANT")); - - // Every non-blank line is SQL or a comment; every statement block ends with ';'. - for (var line : sql.split("\n")) { - if (line.isBlank() || line.startsWith("--")) { - continue; - } - assertFalse(line.startsWith("Starting"), "unexpected non-SQL output: " + line); - } - } - - @Test - public void printOnlyWithAppRoleAndSchema() { - var cmd = new CommandLine(new DBOSCommand()); - var sw = new StringWriter(); - cmd.setOut(new PrintWriter(sw)); - - var exitCode = - cmd.execute("migrate", "--print-only", "--schema", "custom", "--app-role", "app_user"); - assertEquals(0, exitCode); - - var sql = sw.toString(); - assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"custom\";")); - assertTrue(sql.contains("CREATE TABLE \"custom\".workflow_status")); - assertTrue(sql.contains("GRANT USAGE ON SCHEMA custom TO app_user;")); - assertTrue( - sql.contains( - "ALTER DEFAULT PRIVILEGES IN SCHEMA custom GRANT EXECUTE ON FUNCTIONS TO app_user;")); - } - - @Test - public void printOnlyFunnySchema() { - var schema = "F8nny_sCHem@-n@m3"; - var cmd = new CommandLine(new DBOSCommand()); - var sw = new StringWriter(); - cmd.setOut(new PrintWriter(sw)); - - var exitCode = cmd.execute("migrate", "--print-only", "--schema", schema); - assertEquals(0, exitCode); - - var sql = sw.toString(); - assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"%s\";".formatted(schema))); - assertTrue(sql.contains("CREATE TABLE \"%s\".workflow_status".formatted(schema))); - assertTrue( - sql.contains( - "CREATE TABLE IF NOT EXISTS \"%s\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);" - .formatted(schema))); - // Migration 10 guard uses the schema as a string literal. - assertTrue(sql.contains("WHERE table_schema = '%s' AND table_name".formatted(schema))); - // The schema never appears unquoted in an identifier position (schema immediately - // followed by a dot only happens without the closing double quote). - assertFalse(sql.contains(schema + ".")); - - var latest = MigrationManager.getMigrations(schema, true, false).size(); - assertTrue( - sql.contains("UPDATE \"%s\".dbos_migrations SET version = %d;".formatted(schema, latest))); - } - - @Test - public void printOnlyConnectionFailureFallsBackToFreshScript() { - var cmd = new CommandLine(new DBOSCommand()); - var sw = new StringWriter(); - var ew = new StringWriter(); - cmd.setOut(new PrintWriter(sw)); - cmd.setErr(new PrintWriter(ew)); - - var exitCode = - cmd.execute( - "migrate", - "--print-only", - "-D=jdbc:postgresql://127.0.0.1:1/nosuchdb?connectTimeout=2", - "-U=nobody", - "-P=nothing"); - assertEquals(0, exitCode); - assertEquals("", ew.toString()); - - var sql = sw.toString(); - assertTrue(sql.startsWith("-- DBOS system database migration script for schema \"dbos\".")); - assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"dbos\";")); - assertTrue(sql.contains("INSERT INTO \"dbos\".dbos_migrations (version) VALUES (1);")); - } - - @Test - public void printOnlyDelta() { - var latest = MigrationManager.getMigrations("dbos", true, false).size(); - var from = latest - 2; - var sql = MigrationManager.generateMigrationScript("dbos", true, from); - - // Delta prelude: no schema/table creation, no fresh guard; exact-version guard instead. - assertTrue( - sql.startsWith( - "-- DBOS system database delta migration script for schema \"dbos\" (version %d to %d)." - .formatted(from, latest))); - assertFalse(sql.contains("CREATE SCHEMA")); - assertFalse(sql.contains("CREATE TABLE IF NOT EXISTS \"dbos\".dbos_migrations")); - assertFalse(sql.contains("fresh databases only")); - assertTrue(sql.contains("IF existing_version IS DISTINCT FROM %d THEN".formatted(from))); - - // Only migrations from+1..latest appear. - for (var v = 1; v <= from; v++) { - assertFalse(sql.contains("-- DBOS system database migration %d\n".formatted(v))); - } - for (var v = from + 1; v <= latest; v++) { - assertTrue(sql.contains("-- DBOS system database migration %d\n".formatted(v))); - } - // Migration 10 is out of range, so its conditional block is absent. - assertFalse(sql.contains("table_constraints")); - - // Bookkeeping: UPDATE only, never INSERT. - assertFalse(sql.contains("INSERT INTO \"dbos\".dbos_migrations")); - assertTrue( - sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(from + 1))); - assertTrue(sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(latest))); - } - - @Test - public void printOnlyDeltaFunnySchema() { - var schema = "F8nny_sCHem@-n@m3"; - var latest = MigrationManager.getMigrations(schema, true, false).size(); - var from = 9; // puts migration 10 in the delta range - var sql = MigrationManager.generateMigrationScript(schema, true, from); - - assertTrue(sql.contains("SELECT version INTO existing_version FROM \"%s\"".formatted(schema))); - assertTrue(sql.contains("WHERE table_schema = '%s' AND table_name".formatted(schema))); - assertTrue( - sql.contains( - "ALTER TABLE \"%s\".notifications ADD PRIMARY KEY (message_uuid);".formatted(schema))); - assertTrue( - sql.contains("UPDATE \"%s\".dbos_migrations SET version = %d;".formatted(schema, latest))); - assertFalse(sql.contains(schema + ".")); // never unquoted in identifier position - // Bookkeeping starts with an UPDATE, never an INSERT (function bodies in - // migrations 14/38 legitimately INSERT INTO workflow_status). - assertFalse(sql.contains("INSERT INTO \"%s\".dbos_migrations".formatted(schema))); - } - - @Test - public void printOnlyUpToDateAndInvalidFromVersion() { - var latest = MigrationManager.getMigrations("dbos", true, false).size(); - assertEquals( - "-- Database is already at the latest DBOS schema version (%d); nothing to do.\n" - .formatted(latest), - MigrationManager.generateMigrationScript("dbos", true, latest)); - - assertThrows( - IllegalArgumentException.class, - () -> MigrationManager.generateMigrationScript("dbos", true, -1)); - assertThrows( - IllegalArgumentException.class, - () -> MigrationManager.generateMigrationScript("dbos", true, latest + 1)); - } - - @ParameterizedTest - @ValueSource(strings = {"bad\"schema", "bad'schema"}) - public void printOnlyInvalidSchemaFails(String schema) { - var cmd = new CommandLine(new DBOSCommand()); - var sw = new StringWriter(); - cmd.setOut(new PrintWriter(sw)); - cmd.setErr(new PrintWriter(new StringWriter())); - - var exitCode = cmd.execute("migrate", "--print-only", "--schema", schema); - assertEquals(1, exitCode); - assertEquals("", sw.toString()); - } -} diff --git a/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintTest.java b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintTest.java new file mode 100644 index 00000000..940b2eab --- /dev/null +++ b/transact-cli/src/test/java/dev/dbos/transact/cli/MigratePrintTest.java @@ -0,0 +1,196 @@ +package dev.dbos.transact.cli; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dev.dbos.transact.migrations.MigrationManager; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import picocli.CommandLine; + +// Runs without a database; the print flags must never connect. +public class MigratePrintTest { + + record Result(int exitCode, String out, String err) {} + + static Result run(String... args) { + var cmd = new CommandLine(new DBOSCommand()); + var sw = new StringWriter(); + var ew = new StringWriter(); + cmd.setOut(new PrintWriter(sw)); + cmd.setErr(new PrintWriter(ew)); + var exitCode = cmd.execute(args); + return new Result(exitCode, sw.toString(), ew.toString()); + } + + @Test + public void printMigrationsAll() { + var r = run("migrate", "--print-migrations", "all"); + assertEquals(0, r.exitCode()); + // Stdout is pure SQL and comments, pipeable to a .sql file. + assertEquals("", r.err()); + + var sql = r.out(); + var latest = MigrationManager.getMigrations("dbos", true, false).size(); + assertTrue(sql.startsWith("-- DBOS system database migrations for ")); + assertTrue( + sql.contains( + "-- Contains CREATE/DROP INDEX CONCURRENTLY: run outside a transaction block (e.g. plain psql, not psql --single-transaction).")); + assertTrue(sql.contains("-- This script is for FRESH databases only.")); + assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"dbos\";")); + assertTrue( + sql.contains( + "CREATE TABLE IF NOT EXISTS \"dbos\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);")); + assertTrue(sql.contains("CREATE TABLE \"dbos\".workflow_status")); + assertTrue(sql.contains("INSERT INTO \"dbos\".dbos_migrations (version) VALUES (1);")); + assertTrue(sql.contains("-- Migration 10 skipped: not applicable on fresh databases")); + assertFalse(sql.contains("ADD PRIMARY KEY (message_uuid)")); + assertTrue(sql.contains("UPDATE \"dbos\".dbos_migrations SET version = 10;")); + assertTrue(sql.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(latest))); + assertFalse(sql.contains("DO $$")); + // Role grants are only printed by --print-user-role + assertFalse(sql.contains("GRANT")); + for (var line : sql.split("\n")) { + assertFalse(line.startsWith("Starting"), "unexpected non-SQL output: " + line); + assertFalse(line.startsWith("Granting"), "unexpected non-SQL output: " + line); + } + } + + @Test + public void printMigrationsFromNumber() { + // Starting from 1 is identical to "all". + assertEquals( + run("migrate", "--print-migrations", "all").out(), + run("migrate", "--print-migrations", "1").out()); + + // Starting mid-way omits the prelude and earlier migrations. + var latest = MigrationManager.getMigrations("dbos", true, false).size(); + var r = run("migrate", "--print-migrations", "10"); + assertEquals(0, r.exitCode()); + assertEquals("", r.err()); + var out = r.out(); + assertFalse(out.contains("CREATE SCHEMA")); + assertFalse(out.contains("-- Migration 9\n")); + assertTrue(out.contains("-- Migration 10 skipped: not applicable on fresh databases")); + assertTrue(out.contains("UPDATE \"dbos\".dbos_migrations SET version = 10;")); + assertTrue(List.of(out.split("\n")).contains("-- Migration 11")); + assertTrue(out.contains("UPDATE \"dbos\".dbos_migrations SET version = %d;".formatted(latest))); + assertFalse(out.contains("INSERT INTO \"dbos\".dbos_migrations")); + assertFalse(out.contains("DO $$")); + } + + @Test + public void printMigrationsInvalidValues() { + var latest = MigrationManager.getMigrations("dbos", true, false).size(); + for (var bad : List.of("0", "-1", String.valueOf(latest + 1))) { + var r = run("migrate", "--print-migrations", bad); + assertEquals(1, r.exitCode()); + assertEquals("", r.out()); + assertTrue( + r.err().contains("does not exist: valid migrations are 1 through %d".formatted(latest)), + r.err()); + } + + var r = run("migrate", "--print-migrations", "nope"); + assertEquals(1, r.exitCode()); + assertEquals("", r.out()); + assertTrue( + r.err() + .contains( + "Invalid --print-migrations value 'nope': expected 'all' or a migration number"), + r.err()); + + assertThrows( + IllegalArgumentException.class, + () -> MigrationManager.generateMigrationScript("dbos", true, 0)); + assertThrows( + IllegalArgumentException.class, + () -> MigrationManager.generateMigrationScript("dbos", true, latest + 1)); + } + + @Test + public void printMigrationsFunnySchema() { + var schema = "F8nny_sCHem@-n@m3"; + var r = run("migrate", "--print-migrations", "all", "--schema", schema); + assertEquals(0, r.exitCode()); + assertEquals("", r.err()); + + var sql = r.out(); + assertTrue(sql.contains("CREATE SCHEMA IF NOT EXISTS \"%s\";".formatted(schema))); + assertTrue(sql.contains("CREATE TABLE \"%s\".workflow_status".formatted(schema))); + // The schema never appears unquoted in an identifier position (schema immediately + // followed by a dot only happens without the closing double quote). + assertFalse(sql.contains(schema + ".")); + + var latest = MigrationManager.getMigrations(schema, true, false).size(); + assertTrue( + sql.contains("UPDATE \"%s\".dbos_migrations SET version = %d;".formatted(schema, latest))); + } + + @Test + public void printUserRole() { + var r = run("migrate", "--print-user-role", "--schema", "custom", "--app-role", "app_user"); + assertEquals(0, r.exitCode()); + assertEquals("", r.err()); + + var sql = r.out(); + assertTrue(sql.startsWith("-- Permissions on DBOS schema custom for role app_user")); + assertTrue(sql.contains("GRANT USAGE ON SCHEMA \"custom\" TO \"app_user\";")); + assertTrue( + sql.contains( + "ALTER DEFAULT PRIVILEGES IN SCHEMA \"custom\" GRANT EXECUTE ON FUNCTIONS TO \"app_user\";")); + for (var line : sql.split("\n")) { + assertTrue( + line.startsWith("--") || line.startsWith("GRANT") || line.startsWith("ALTER"), + "unexpected output: " + line); + } + } + + @Test + public void printUserRoleRequiresAppRole() { + var r = run("migrate", "--print-user-role"); + assertEquals(1, r.exitCode()); + assertEquals("", r.out()); + assertTrue(r.err().contains("--print-user-role requires --app-role"), r.err()); + } + + @Test + public void printFlagsAreMutuallyExclusive() { + var r = run("migrate", "--print-migrations", "all", "--print-user-role", "-r", "app_user"); + assertEquals(1, r.exitCode()); + assertEquals("", r.out()); + assertTrue( + r.err().contains("--print-user-role cannot be combined with --print-migrations"), r.err()); + } + + @ParameterizedTest + @ValueSource(strings = {"bad\"schema", "bad'schema"}) + public void printInvalidSchemaFails(String schema) { + var r = run("migrate", "--print-migrations", "all", "--schema", schema); + assertEquals(1, r.exitCode()); + assertEquals("", r.out()); + assertTrue(r.err().contains("Schema names containing quotes are not supported"), r.err()); + + r = run("migrate", "--print-user-role", "-r", "app_user", "--schema", schema); + assertEquals(1, r.exitCode()); + assertEquals("", r.out()); + assertTrue(r.err().contains("Schema names containing quotes are not supported"), r.err()); + } + + @ParameterizedTest + @ValueSource(strings = {"bad\"role", "bad'role"}) + public void printInvalidRoleFails(String role) { + var r = run("migrate", "--print-user-role", "-r", role); + assertEquals(1, r.exitCode()); + assertEquals("", r.out()); + assertTrue(r.err().contains("Role names containing quotes are not supported"), r.err()); + } +} diff --git a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java index 9ada4e3a..197983fb 100644 --- a/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java +++ b/transact/src/main/java/dev/dbos/transact/migrations/MigrationManager.java @@ -54,24 +54,19 @@ public static void runMigrations( } } - /** - * Generates the full SQL script that migrations would execute on a fresh Postgres database, - * including schema creation and dbos_migrations version bookkeeping. Requires no connection. The - * script is for fresh databases only and fails fast (under psql ON_ERROR_STOP) if dbos_migrations - * already contains a row. - */ + /** Generates the SQL script of all migrations, for fresh Postgres databases only. */ public static String generateMigrationScript(String schema, boolean useListenNotify) { - return generateMigrationScript(schema, useListenNotify, 0); + return generateMigrationScript(schema, useListenNotify, 1); } /** - * Generates the SQL script that upgrades a database from {@code fromVersion} (0 = fresh) to the - * latest DBOS schema version. Requires no connection. Fresh scripts abort (under psql - * ON_ERROR_STOP) if migrations were already applied; delta scripts abort unless the database is - * exactly at {@code fromVersion}. + * Generates the SQL script of migrations {@code startMigration} (1-based, inclusive) through + * latest, with dbos_migrations version bookkeeping mirroring the runner. When {@code + * startMigration} is 1 the script includes the schema and dbos_migrations prelude and is for + * fresh databases only. Requires no connection. */ public static String generateMigrationScript( - String schema, boolean useListenNotify, int fromVersion) { + String schema, boolean useListenNotify, int startMigration) { schema = SystemDatabase.sanitizeSchema(schema); if (schema.contains("'") || schema.contains("\"")) { throw new IllegalArgumentException("Schema name must not contain single or double quotes"); @@ -79,96 +74,43 @@ public static String generateMigrationScript( var migrations = getMigrations(schema, useListenNotify, false); var latest = migrations.size(); - if (fromVersion < 0 || fromVersion > latest) { + if (startMigration < 1 || startMigration > latest) { throw new IllegalArgumentException( - "fromVersion must be between 0 and %d, got %d".formatted(latest, fromVersion)); - } - if (fromVersion == latest) { - return "-- Database is already at the latest DBOS schema version (%d); nothing to do.\n" - .formatted(latest); + "startMigration must be between 1 and %d, got %d".formatted(latest, startMigration)); } var sb = new StringBuilder(); - if (fromVersion == 0) { - sb.append("-- DBOS system database migration script for schema \"%s\".\n".formatted(schema)); - sb.append("-- For FRESH databases only; aborts if DBOS migrations were already applied.\n"); - sb.append("-- Apply with: psql -v ON_ERROR_STOP=1 -f \n"); + if (startMigration == 1) { + sb.append("-- This script is for FRESH databases only.\n"); sb.append("CREATE SCHEMA IF NOT EXISTS \"%s\";\n".formatted(schema)); sb.append( "CREATE TABLE IF NOT EXISTS \"%s\".dbos_migrations (version BIGINT NOT NULL PRIMARY KEY);\n" .formatted(schema)); - sb.append( - """ - - -- Fail fast if this is not a fresh database. - DO $$ - DECLARE - existing_version BIGINT; - BEGIN - SELECT version INTO existing_version FROM "%1$s".dbos_migrations LIMIT 1; - IF existing_version IS NOT NULL THEN - RAISE EXCEPTION 'DBOS schema %1$s is already at version %%; this script is for fresh databases only. Use dbos migrate instead.', existing_version; - END IF; - END $$; - """ - .formatted(schema)); - } else { - sb.append( - "-- DBOS system database delta migration script for schema \"%s\" (version %d to %d).\n" - .formatted(schema, fromVersion, latest)); - sb.append("-- Apply with: psql -v ON_ERROR_STOP=1 -f \n"); - sb.append( - """ - - -- Fail fast unless the database is exactly at the expected version. - DO $$ - DECLARE - existing_version BIGINT; - BEGIN - SELECT version INTO existing_version FROM "%1$s".dbos_migrations LIMIT 1; - IF existing_version IS DISTINCT FROM %2$d THEN - RAISE EXCEPTION 'DBOS schema %1$s is at version %% but this script upgrades from version %2$d; regenerate it with dbos migrate --print-only.', existing_version; - END IF; - END $$; - """ - .formatted(schema, fromVersion)); } - for (var i = fromVersion; i < migrations.size(); i++) { - var version = i + 1; - sb.append("\n-- DBOS system database migration %d\n".formatted(version)); - var sql = migrations.get(i).strip(); - if (version == 10) { - // Same conditional the migration runner applies at execution time. - sql = - """ - -- Mirrors the runner's guard: add the notifications primary key only if one - -- does not already exist (migration 1 creates it on fresh databases). - DO $$ - BEGIN - IF NOT EXISTS ( - SELECT 1 FROM information_schema.table_constraints - WHERE table_schema = '%1$s' AND table_name = 'notifications' - AND constraint_type = 'PRIMARY KEY' - ) THEN - %2$s - END IF; - END $$; - """ - .formatted(schema, sql) - .strip(); - } - if (!sql.isEmpty()) { + var versionRowExists = startMigration > 1; + for (var i = startMigration; i <= latest; i++) { + var sql = migrations.get(i - 1).strip(); + if (i == 10) { + // Migration 10 backfills the notifications primary key, which + // migration 1 already creates on a fresh database. + sb.append("-- Migration 10 skipped: not applicable on fresh databases\n"); + } else if (!sql.isEmpty()) { + sb.append("-- Migration %d\n".formatted(i)); sb.append(sql); if (!sql.endsWith(";")) { sb.append(';'); } sb.append('\n'); } - if (version == 1) { - sb.append("INSERT INTO \"%s\".dbos_migrations (version) VALUES (1);\n".formatted(schema)); + // Per-migration version bookkeeping, mirroring the runner: an + // interrupted apply can be resumed from the next migration number. + if (versionRowExists) { + sb.append("UPDATE \"%s\".dbos_migrations SET version = %d;\n".formatted(schema, i)); } else { - sb.append("UPDATE \"%s\".dbos_migrations SET version = %d;\n".formatted(schema, version)); + sb.append( + "INSERT INTO \"%s\".dbos_migrations (version) VALUES (%d);\n".formatted(schema, i)); + versionRowExists = true; } } return sb.toString();