Skip to content

Commit 4271a75

Browse files
committed
no-mistakes(test): Make plain migrate say it skips the database and refuse --shadow-db
1 parent 320c7a5 commit 4271a75

4 files changed

Lines changed: 149 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,9 @@ here.**
318318
cannot be opened or applied is an error naming the cause. When a `CREATE`d one is not available,
319319
the rolled-back temp-object probe on the target answers (it needs write privileges and takes
320320
brief locks), and SQLite and offline plans keep the string normaliser. Every Postgres run says
321-
which path ran and, on a fallback, why. Clearing a live comment the metadata does not declare
321+
which path ran and, on a fallback, why: a plain `meta migrate` (no `--from-db` or `--apply`)
322+
says it compared the committed snapshot and read no database, and refuses `--shadow-db`, which
323+
it would otherwise have ignored. Clearing a live comment the metadata does not declare
322324
needs the new `--allow drop-comment`; without it `meta migrate` keeps the comment, plans the
323325
rest and says so, and `meta verify --db` still reports it. **Upgrade note:** the committed
324326
snapshot moves to `formatVersion` 4, which records comments and GENERATED identity read off the

‎docs/features/migrations-and-drift.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ the database. A real change of any of them is still reported. The scratch databa
411411

412412
| Source | How |
413413
|---|---|
414-
| An explicit URL | `--shadow-db <url>` on `verify` and `migrate`, else `SHADOW_DATABASE_URL`, else `migrate.shadowDatabaseUrl` in `.metaobjects/config.json`. Used as given and never dropped; nothing is left in it because the schema is applied inside a rolled-back transaction. |
414+
| An explicit URL | `--shadow-db <url>` on `verify` and on `migrate --from-db` / `--apply` (a plain `meta migrate` reads no database, says so, and refuses the flag), else `SHADOW_DATABASE_URL`, else `migrate.shadowDatabaseUrl` in `.metaobjects/config.json`. Used as given and never dropped; nothing is left in it because the schema is applied inside a rolled-back transaction. |
415415
| `CREATE DATABASE` | With none of the above, a database named `mo_shadow_<random>` is created on the `--db` server when the role may, and **always dropped** afterwards, including on error. |
416416

417417
A run opens its scratch database once, and every comparison it makes uses it: `meta verify --db`

‎server/typescript/packages/cli/src/commands/migrate.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,13 @@ export async function migrateCommand(
705705
// when explicitly requested via --from-db, when --apply needs a connection, or
706706
// for --rollback (which runs hand-authored down.sql against the live DB).
707707
if (!config.fromDb && !config.apply && config.rollback === undefined) {
708+
// The scratch database is one side of a comparison with a live database, which this
709+
// path never reads; a --shadow-db it silently ignored would read as a clean comparison.
710+
if (flags.shadowDb !== undefined) {
711+
log.error(`migrate: --shadow-db needs --from-db (or --apply): without it migrate compares the metadata with the committed snapshot and reads no database`);
712+
emitStructuredError("migrate: --shadow-db needs --from-db", "re-run with --from-db to compare the metadata with the database", fmt);
713+
return 2;
714+
}
708715
return await runOfflineGenerate(config, metaRoot, fmt, genRoot);
709716
}
710717

@@ -1562,6 +1569,12 @@ export async function runOfflineGenerate(
15621569
logOutOfScope(plan.outOfScope, plan.importedOutOfScope ?? [], fmt);
15631570

15641571
const offlineNotes = viewRecreateNotes(diffResult.changes);
1572+
// Which comparison ran (every Postgres run says so): this one never reads the database, so
1573+
// a change somebody made there by hand is not seen here.
1574+
if (config.dialect === "postgres") {
1575+
offlineNotes.unshift("compared the metadata with the committed snapshot; no database was read, so a change "
1576+
+ "made directly in the database is not seen here (meta migrate --from-db or meta verify --db compares with it).");
1577+
}
15651578
const offlineResult = (extra: Partial<MigrateResultShape>): MigrateResultShape => ({
15661579
dialect: offlineDialect,
15671580
displayUrl: "",
@@ -1578,7 +1591,7 @@ export async function runOfflineGenerate(
15781591
return 1;
15791592
}
15801593
if (diffResult.changes.length === 0) {
1581-
finishMigrate(fmt, [`migrate: no changes`], migrateResultToData(offlineResult({})));
1594+
finishMigrate(fmt, [`migrate: no changes`, ...narratedNotes(offlineNotes)], migrateResultToData(offlineResult({})));
15821595
return 0;
15831596
}
15841597
const offlineWarnings = warnDataHazards(diffResult.hazards);
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* Real column-default drift on a live Postgres database reaches the same verdict through
3+
* `meta verify --db` and `meta migrate --from-db --dry-run`, both on the shadow path: a
4+
* default somebody changed by hand (an integer 7 made 8, `now()` made `now() + 1 day`) is
5+
* reported by verify and planned by migrate, while the defaults exactly as the metadata
6+
* declares them read clean. The oracle is the ALTER TABLE the test runs itself.
7+
*
8+
* An explicitly configured scratch database that cannot be reached is a hard error naming
9+
* the cause in both commands, never a silent fall-back to another comparison. Plain
10+
* `meta migrate --dry-run` (no --from-db) reads no database at all: it says so, and it
11+
* refuses a --shadow-db it would otherwise have ignored.
12+
*
13+
* Skips gracefully when MIGRATE_TS_PG_URL is not set.
14+
*/
15+
import { describe, test, expect, beforeAll, afterAll } from "bun:test";
16+
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
17+
import { tmpdir } from "node:os";
18+
import { join } from "node:path";
19+
import { Pool } from "pg";
20+
import { run } from "../../src/index.js";
21+
22+
const PG_URL = process.env.MIGRATE_TS_PG_URL;
23+
const TABLE = "shadow_default_drift_events";
24+
const UNREACHABLE_SHADOW = "postgresql://nobody:nothing@127.0.0.1:1/nope";
25+
26+
const MODEL = JSON.stringify({
27+
"metadata.root": {
28+
children: [{
29+
"object.entity": {
30+
name: "Event",
31+
children: [
32+
{ "source.rdb": { name: "src", "@table": TABLE } },
33+
{ "field.long": { name: "id" } },
34+
{ "field.int": { name: "seq", "@default": 7, "@required": true } },
35+
{ "field.timestamp": { name: "startsAt", "@column": "starts_at", "@default": "NOW()", "@required": true } },
36+
{ "identity.primary": { name: "pk", "@fields": ["id"] } },
37+
],
38+
},
39+
}],
40+
},
41+
});
42+
43+
describe("shadow path: live default drift and an unreachable explicit shadow (env-gated)", () => {
44+
if (PG_URL === undefined || PG_URL.length === 0) {
45+
test.skip("skipped — MIGRATE_TS_PG_URL not set", () => {});
46+
return;
47+
}
48+
let pool: Pool;
49+
let root: string;
50+
const args = (...rest: string[]): string[] => [...rest, "--cwd", root, "--db", PG_URL, "--dialect", "postgres"];
51+
52+
/** Run the CLI, collecting everything it prints. */
53+
const captured = async (argv: string[]): Promise<{ exit: number; out: string }> => {
54+
const lines: string[] = [];
55+
const origLog = console.log;
56+
const origErr = console.error;
57+
const origWarn = console.warn;
58+
const keep = (...a: unknown[]): void => { lines.push(a.map(String).join(" ")); };
59+
console.log = keep; console.error = keep; console.warn = keep;
60+
try {
61+
return { exit: await run(argv), out: lines.join("\n") };
62+
} finally {
63+
console.log = origLog; console.error = origErr; console.warn = origWarn;
64+
}
65+
};
66+
67+
beforeAll(async () => {
68+
pool = new Pool({ connectionString: PG_URL });
69+
await pool.query(`DROP TABLE IF EXISTS ${TABLE} CASCADE`);
70+
root = mkdtempSync(join(tmpdir(), "shadow-default-drift-pg-"));
71+
mkdirSync(join(root, "metaobjects"), { recursive: true });
72+
writeFileSync(join(root, "metaobjects", "meta.events.json"), MODEL, "utf8");
73+
expect((await captured(args("migrate", "--from-db", "--slug", "init", "--apply"))).exit).toBe(0);
74+
});
75+
afterAll(async () => {
76+
await pool.query(`DROP TABLE IF EXISTS ${TABLE} CASCADE`);
77+
await pool.end();
78+
rmSync(root, { recursive: true, force: true });
79+
});
80+
81+
test("the defaults as declared read clean through both commands", async () => {
82+
expect((await captured(args("verify"))).exit).toBe(0);
83+
const planned = await captured(args("migrate", "--from-db", "--dry-run"));
84+
expect(planned.exit).toBe(0);
85+
expect(planned.out).not.toContain("SET DEFAULT");
86+
});
87+
88+
test("seq 8 vs 7 and starts_at now() + 1 day vs now() are drift for verify and planned by migrate", async () => {
89+
await pool.query(`ALTER TABLE ${TABLE} ALTER COLUMN seq SET DEFAULT 8`);
90+
await pool.query(`ALTER TABLE ${TABLE} ALTER COLUMN starts_at SET DEFAULT now() + interval '1 day'`);
91+
try {
92+
const verified = await captured(args("verify"));
93+
expect(verified.exit).toBe(1);
94+
expect(verified.out).toContain("seq");
95+
expect(verified.out).toContain("starts_at");
96+
97+
const planned = await captured(args("migrate", "--from-db", "--dry-run"));
98+
expect(planned.exit).toBe(0);
99+
expect(planned.out).toContain("compared against a scratch (shadow) database");
100+
expect(planned.out).toMatch(/ALTER COLUMN \\?"seq\\?" SET DEFAULT '?7'?;/);
101+
expect(planned.out).toMatch(/ALTER COLUMN \\?"starts_at\\?" SET DEFAULT now\(\);/);
102+
103+
// Without --from-db, migrate diffs the metadata against the committed snapshot and
104+
// never reads the database: it plans nothing here, and says so rather than reading
105+
// as a clean comparison with the database.
106+
const offline = await captured(args("migrate", "--dry-run"));
107+
expect(offline.exit).toBe(0);
108+
expect(offline.out).toContain("no database was read");
109+
expect(offline.out).toContain("--from-db");
110+
} finally {
111+
await pool.query(`ALTER TABLE ${TABLE} ALTER COLUMN seq SET DEFAULT 7`);
112+
await pool.query(`ALTER TABLE ${TABLE} ALTER COLUMN starts_at SET DEFAULT now()`);
113+
}
114+
});
115+
116+
test("an explicit --shadow-db that cannot be reached is a hard error in migrate and verify", async () => {
117+
// A run that reads no database refuses a --shadow-db it would otherwise ignore.
118+
const offline = await captured(args("migrate", "--dry-run", "--shadow-db", UNREACHABLE_SHADOW));
119+
expect(offline.exit).toBe(2);
120+
expect(offline.out).toContain("--shadow-db needs --from-db");
121+
for (const argv of [
122+
args("migrate", "--from-db", "--dry-run", "--shadow-db", UNREACHABLE_SHADOW),
123+
args("verify", "--shadow-db", UNREACHABLE_SHADOW),
124+
]) {
125+
const r = await captured(argv);
126+
expect(r.exit).not.toBe(0);
127+
expect(r.out).toContain("shadow database");
128+
expect(r.out).toContain("127.0.0.1:1");
129+
}
130+
});
131+
});

0 commit comments

Comments
 (0)