diff --git a/CHANGELOG.md b/CHANGELOG.md index 078127553..0814381dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -470,6 +470,30 @@ until you regenerate. ### Fixed +- **`meta verify --db` no longer reports a matching view as drift, and says what differs when a + view does not.** An adopter on 1.1.0-rc.2 rebuilt a SQLite database from the committed chain: + every report and projection view's stored SQL was byte-identical to the metadata's, yet + `meta verify --db` and `meta migrate --from-db` listed each one as `- view` / `+ view` and + `verify` exited 1, with nothing in the text or `--format json` saying what differed. The cause + was not the view comparison. When a migration alters a table (a column on every dialect, and an + FK or CHECK on SQLite and D1, which rebuild the table, #243), the diff drops and recreates every + view that reads it, and that pair looked the same as a view whose definition changed. Every + view change now carries a `reason` (`ViewChangeReason`, exported from + `@metaobjectsdev/migrate-ts` with `isViewRecreateOnly` and `withoutViewRecreates`), and a + recreate whose definition matches is `unchanged`. `verify --db`, the D1 and committed-snapshot + drift gates, `computeDriftFromActual` and `classifyDrift` leave those out, so the table change + is reported alone. `meta migrate` still emits the pair, because the SQL needs it, and adds a + note naming the views that match ("recreated only because the migration alters a table they + read") to its text output and to a new `notes` array in `--format json`. A view that does + differ is reported with why: SQLite and D1 show the first differing excerpt of the definition + text (`definition text differs: metadata «…» vs database «…»`), Postgres says the fingerprint + does not match, and an unstamped Postgres view says it cannot be compared. `verify --format + json` gains a `schemaDrift` section listing each schema difference and its explanation. The + emitted migration SQL does not change. One gate is tightened on the way: an unstamped Postgres + view over an altered table used to be recreated without `--allow adopt-view`; it now needs the + flag, as it does when no table changes. Gated in `migrate-ts` unit and drift tests, the CLI's + `verify-db-view-recreate` test, and new `integration-tests` lanes on a real SQLite (with the D1 + diff) and a real Postgres. - **TypeScript: a report's decimal fields reach the wire as strings on SQLite, as on Postgres (FR-044).** A ratio is typed `decimal`, the TypeScript read schema types a decimal as `string`, and SQLite has no decimal: the view computes a `REAL`, which the driver hands the diff --git a/docs/features/cli.md b/docs/features/cli.md index 626b935bd..07fdcf5e6 100644 --- a/docs/features/cli.md +++ b/docs/features/cli.md @@ -550,14 +550,19 @@ meta gen --format json → { gen[], summary, help[], antiPatterns: { status meta verify --format json → { verify[], exitCode, summary, help[], antiPatterns: { status, total, rows[] }, requirements: { status, total, rows[] }, - requirementCounts?, notRepresented[] } + requirementCounts?, schemaDrift?, notRepresented[] } ``` A pass that did **not** run says so (`status: "skipped"` with a `note` giving the reason) rather than reporting an empty list — "found nothing" and "never looked" are different answers. `meta verify`'s payload carries each gate's pass/fail -verdict; the per-gate drift **detail** stays on stderr as text, and the payload's -own `notRepresented[]` says so. +verdict. The schema gate's differences are in `schemaDrift` (`changes[]`, one +`{ kind, object, detail }` per difference with the same explanation the text prints, +plus the ledger `findings[]`); every other gate's drift **detail** stays on stderr as +text, and the payload's own `notRepresented[]` says so. A view that matches the +metadata and is recreated only because the migration alters a table it reads is not +drift, so it appears in neither. `meta migrate` still emits that drop/create pair and +names the views in a note (`notes[]` in its structured output). In a structured run every narration line moves to **stderr**, so stdout is one parseable document. `--format` is honored by `gen`, `verify` and `migrate`; any diff --git a/docs/features/migrations-and-drift.md b/docs/features/migrations-and-drift.md index 3133170c2..c4fdbe164 100644 --- a/docs/features/migrations-and-drift.md +++ b/docs/features/migrations-and-drift.md @@ -13,7 +13,7 @@ There are **7 drift sources**, and the toolchain has a guard for each. |---|---|---| | **Code-vs-DB** | Codegen — the generated SQL DDL is emitted from the same metadata as the entity / table code. | Build time | | **Code-vs-API-doc** | Cross-port codegen from the same metadata. | Build time | -| **DB-vs-metadata** | `meta verify --db` (TS CLI) — introspects the live DB and fails if it has drifted from metadata. Includes modeled projection **view bodies** (a changed `CREATE VIEW` is `replace-view` drift); a hand-authored *unmodeled* view is unmanaged and never flagged. A schema concern owned by the Node toolchain regardless of server language; on the JVM ports the runtime auto-create/validator path was removed (ADR-0015) and the `metaobjects:verify` Maven goal is not available. Cloudflare D1 has no client wire protocol, so it can't go through `--db`'s Kysely-driver introspection — use `meta verify --dialect d1 [--d1 ] [--remote]` instead (the same wrangler-shelled-out path `meta migrate --dialect d1` uses); `--remote` is required to check the *deployed* database, not the local `wrangler dev` shadow copy. Pointing `--db file:` at wrangler's local D1 state directory (`.wrangler/state/**/d1/**`) still runs, but only verifies that local copy — `verify` warns when it detects this. | CI on every PR | +| **DB-vs-metadata** | `meta verify --db` (TS CLI) — introspects the live DB and fails if it has drifted from metadata. Includes modeled projection **view bodies** (a changed `CREATE VIEW` is `replace-view` drift, reported with what differs: the first differing excerpt of the text on SQLite/D1, a fingerprint mismatch on Postgres). A view `meta migrate` drops and recreates only because the migration alters a table it reads, with its definition unchanged, is not drift and is not reported; a hand-authored *unmodeled* view is unmanaged and never flagged. A schema concern owned by the Node toolchain regardless of server language; on the JVM ports the runtime auto-create/validator path was removed (ADR-0015) and the `metaobjects:verify` Maven goal is not available. Cloudflare D1 has no client wire protocol, so it can't go through `--db`'s Kysely-driver introspection — use `meta verify --dialect d1 [--d1 ] [--remote]` instead (the same wrangler-shelled-out path `meta migrate --dialect d1` uses); `--remote` is required to check the *deployed* database, not the local `wrangler dev` shadow copy. Pointing `--db file:` at wrangler's local D1 state directory (`.wrangler/state/**/d1/**`) still runs, but only verifies that local copy — `verify` warns when it detects this. | CI on every PR | | **Migration-vs-metadata** | The Node `meta migrate` emits migrations FROM metadata diffs — they cannot drift from metadata by construction. Schema migrations for **every** port are owned by this Node toolchain (`@metaobjectsdev/cli migrate`, ADR-0015); the C# and Python migrate surfaces were removed. | Build time | | **Generated-edited** | `@generated` headers in emitted code + three-way merge that preserves hand-edits inside non-generated regions. | Code review | | **Prompt-vs-payload** | FR-004 `Renderer.verify` parses `{{...}}` references in templates and checks each one exists on the payload VO. | Build time + runtime | diff --git a/server/typescript/packages/cli/src/commands/migrate.ts b/server/typescript/packages/cli/src/commands/migrate.ts index a4a7b6a25..d1ec403b0 100644 --- a/server/typescript/packages/cli/src/commands/migrate.ts +++ b/server/typescript/packages/cli/src/commands/migrate.ts @@ -61,7 +61,7 @@ import { type WranglerRunner, } from "../lib/wrangler.js"; import { buildProjectionViews } from "@metaobjectsdev/codegen-ts"; -import { tokensToAllowOptions, blockedEntriesFor, blockedHintLines } from "../lib/allow.js"; +import { tokensToAllowOptions, blockedEntriesFor, blockedHintLines, viewRecreateNote } from "../lib/allow.js"; import { reportLoadError } from "../lib/load-error.js"; import { scanForReferentialActionConflicts } from "../lib/referential-action-advisory.js"; import type { MetaData } from "@metaobjectsdev/metadata"; @@ -253,6 +253,17 @@ function migrateResultDefaults(dryRun: boolean): Pick `migrate: ${n}`); +} + /** The `-- UP -- / -- DOWN --` preview a dry run prints in text format. */ function sqlPreview(sql: { up: string; down: string }): string { return `-- UP --\n${sql.up}\n\n-- DOWN --\n${sql.down}`; @@ -739,6 +750,7 @@ export async function migrateCommand( let writtenPaths: string[] = []; /** A dry run's SQL: printed in text format, carried in the document otherwise. */ let dryRunSql: { up: string; down: string } | undefined; + const notes: string[] = []; let appliedNames: string[] = []; let applyFailed = false; let blocked: BlockedEntry[] = []; @@ -896,6 +908,7 @@ export async function migrateCommand( if (diffResult.changes.length === 0) { // no-op — output will say "No schema changes" } else { + notes.push(...viewRecreateNotes(diffResult.changes)); let emitted: EmitResult | undefined; try { emitted = emit(diffResult.changes, { @@ -1051,6 +1064,7 @@ export async function migrateCommand( applied: appliedNames, applyFailed, warnings: hazardWarnings, + notes, ...(dryRunSql !== undefined ? { sql: dryRunSql } : {}), }; const output = @@ -1425,12 +1439,14 @@ export async function runOfflineGenerate( const { diff: diffResult, nextSnapshot, expected: governedExpected } = plan; logOutOfScope(plan.outOfScope, plan.importedOutOfScope ?? [], fmt); + const offlineNotes = viewRecreateNotes(diffResult.changes); const offlineResult = (extra: Partial): MigrateResultShape => ({ dialect: offlineDialect, displayUrl: "", changeCounts: summarizeChanges(diffResult.changes), ...migrateResultDefaults(config.dryRun), format: config.format, + notes: offlineNotes, ...extra, }); if (diffResult.blocked.length > 0) { @@ -1459,7 +1475,11 @@ export async function runOfflineGenerate( if (config.dryRun) { const sql = { up: emitResult.up, down: emitResult.down }; - finishMigrate(fmt, [sqlPreview(sql)], migrateResultToData(offlineResult({ sql, warnings: offlineWarnings }))); + finishMigrate( + fmt, + [...narratedNotes(offlineNotes), sqlPreview(sql)], + migrateResultToData(offlineResult({ sql, warnings: offlineWarnings })), + ); return 0; } @@ -1482,7 +1502,7 @@ export async function runOfflineGenerate( await writeSnapshot(path, nextSnapshot); finishMigrate( fmt, - [`migrate: wrote ${res.upPath}`, `migrate: wrote ${res.downPath}`], + [`migrate: wrote ${res.upPath}`, `migrate: wrote ${res.downPath}`, ...narratedNotes(offlineNotes)], migrateResultToData(offlineResult({ writtenPaths: [res.upPath, res.downPath], warnings: offlineWarnings })), ); return 0; @@ -1700,6 +1720,7 @@ async function runD1Migrate( } const changeCounts = summarizeChanges(diffResult.changes); + const d1Notes = viewRecreateNotes(diffResult.changes); warnDataHazards(diffResult.hazards); // Views are emitted by the one schema-diff path: renderD1 = renderSqlite (which @@ -1748,7 +1769,7 @@ async function runD1Migrate( if (config.dryRun) { const sql = { up: combinedUp, down: combinedDown }; - finishMigrate(fmt, [sqlPreview(sql)], migrateResultToData(d1Result({ sql }))); + finishMigrate(fmt, [...narratedNotes(d1Notes), sqlPreview(sql)], migrateResultToData(d1Result({ sql, notes: d1Notes }))); return 0; } @@ -1762,8 +1783,9 @@ async function runD1Migrate( `migrate: wrote ${writeResult.upPath}`, `migrate: wrote ${writeResult.downPath}`, ...Object.entries(changeCounts).map(([kind, count]) => ` ${kind}: ${count}`), + ...narratedNotes(d1Notes), ], - migrateResultToData(d1Result({ writtenPaths: [writeResult.upPath, writeResult.downPath] })), + migrateResultToData(d1Result({ writtenPaths: [writeResult.upPath, writeResult.downPath], notes: d1Notes })), ); // 7. Optional --apply: run `wrangler d1 migrations apply`. diff --git a/server/typescript/packages/cli/src/commands/verify.ts b/server/typescript/packages/cli/src/commands/verify.ts index 9002b9f4e..600e2e551 100644 --- a/server/typescript/packages/cli/src/commands/verify.ts +++ b/server/typescript/packages/cli/src/commands/verify.ts @@ -70,6 +70,7 @@ import { verifyReplay, introspect, diff, + withoutViewRecreates, readSnapshot, snapshotPath, type SchemaSnapshot, @@ -382,6 +383,9 @@ export async function verifyCommand( // Set when the schema gate could not reach or read the database: a failure, but not // drift, and the payload must not report it as drift. let schemaRunError: string | undefined; + // The schema gate's findings, for the structured payload: what differs, not only that + // something does. Left undefined when the gate did not compare anything. + let schemaDrift: SchemaDriftSection | undefined; const schemaExit = await runSchemaVerify(); const codegenExit = runCodegen ? await runCodegenVerify() : 0; const docsExit = runDocs ? await runDocsVerify() : 0; @@ -467,6 +471,7 @@ export async function verifyCommand( names: nameSection, deprecations: deprecationSection, fields: fieldSection, + ...(schemaDrift !== undefined ? { schemaDrift } : {}), errors: schemaRunError !== undefined ? [{ gate: "schema", error: schemaRunError }] : [], }), fmt, @@ -1478,7 +1483,9 @@ export async function verifyCommand( // MERGED with the out-of-scope set, and a second key would silently drop that half. dialect, }); - if (result.changes.length === 0) return []; + // A view recreated only around a table change matches the snapshot (D3) — not a difference. + const changes = withoutViewRecreates(result.changes); + if (changes.length === 0) return []; return [ // `meta migrate --from-db` is NOT the repair: it writes a snapshot only when it has @@ -1488,10 +1495,10 @@ export async function verifyCommand( // been told everything is in sync. `baseline --from-db` rewrites it unconditionally, // which is the whole point of the subcommand. `the committed schema snapshot disagrees with ${displayUrl} ` + - `(${result.changes.length} difference(s)) — the next 'meta migrate' would emit DDL from it ` + + `(${changes.length} difference(s)) — the next 'meta migrate' would emit DDL from it ` + `and fail at apply. Re-derive it with ` + `'meta migrate baseline --from-db --db --dialect ${dialect}'.`, - ...summarizeDrift(result.changes), + ...summarizeDrift(changes), ]; } @@ -1515,6 +1522,7 @@ export async function verifyCommand( } const changes = driftResult.changes; + schemaDrift = { changes: changes.map(toSchemaDriftRow), findings: ledgerDrift }; if (changes.length === 0 && ledgerDrift.length === 0) { say(`meta verify — schema in sync with ${displayUrl}.`); return 0; @@ -1822,6 +1830,28 @@ function summarizeDrift(changes: Change[]): string[] { }); } +/** One schema difference in the structured payload — the same text the stderr summary prints. */ +interface SchemaDriftRow { + kind: Change["kind"]; + object: string; + detail: string; +} + +/** + * The schema gate's findings in the structured payload. An adopter whose `verify --db` failed + * on views could not tell from `--format json` what differed (D3): the payload carried the + * verdict only. `changes` is the metadata↔database comparison; `findings` are the + * migration-ledger and committed-snapshot lines, which are text by construction. + */ +interface SchemaDriftSection { + changes: SchemaDriftRow[]; + findings: string[]; +} + +function toSchemaDriftRow(c: Change): SchemaDriftRow { + return { kind: c.kind, object: DRIFT_PRESENTATION[c.kind].noun, detail: describeChange(c) }; +} + // --------------------------------------------------------------------------- // structured output (--format toon|json) // --------------------------------------------------------------------------- @@ -1948,6 +1978,8 @@ function buildVerifyPayload(input: { names: AdvisorySection; deprecations: AdvisorySection; fields: AdvisorySection; + /** What the schema gate found, when it compared anything. */ + schemaDrift?: SchemaDriftSection; /** Gates that could not run at all (an unreachable database) — failures, not drift. */ errors?: readonly { gate: string; error: string }[]; }): Record { @@ -1988,7 +2020,12 @@ function buildVerifyPayload(input: { const help: string[] = errors.map((e) => `the ${e.gate} gate could not run, which is not drift: ${e.error} — fix the connection and re-run`); - if (drifted.length > 0) { + if (drifted.some((g) => g.gate === "schema") && input.schemaDrift !== undefined) { + help.push( + `the schema gate's differences are in schemaDrift — changes[] (metadata vs database, one row per change) and findings[] (migration ledger and committed snapshot)`, + ); + } + if (drifted.some((g) => g.gate !== "schema")) { help.push( `the failing gate's drift DETAIL is printed as text on stderr — this payload carries the verdict only`, ); @@ -2041,11 +2078,12 @@ function buildVerifyPayload(input: { deprecations: input.deprecations, fields: input.fields, ...(input.requirementCounts !== undefined ? { requirementCounts: input.requirementCounts } : {}), + ...(input.schemaDrift !== undefined ? { schemaDrift: input.schemaDrift } : {}), // The honest boundary. Everything named here is REACHABLE — it is printed as // text on stderr — but it is not in this document, and a reader must not have // to discover that by its absence. notRepresented: [ - "per-gate drift detail (which template variable drifted, which schema change, which generated file differs, which migration failed to replay) — printed as text on stderr; this payload carries each gate's pass/fail verdict", + "per-gate drift detail for every gate but schema (which template variable drifted, which generated file differs, which migration failed to replay) — printed as text on stderr; this payload carries each gate's pass/fail verdict, and the schema gate's differences in schemaDrift", "the loader's own warnings and the agent-context/manifest advisories — printed as text on stderr", ], }; diff --git a/server/typescript/packages/cli/src/lib/allow.ts b/server/typescript/packages/cli/src/lib/allow.ts index 9adfec18a..67acc9961 100644 --- a/server/typescript/packages/cli/src/lib/allow.ts +++ b/server/typescript/packages/cli/src/lib/allow.ts @@ -2,8 +2,8 @@ // both `meta migrate` and `meta verify --db`. Keeping a single copy avoids the // two commands drifting on which `--allow` tokens exist or how a change reads. -import { allowOptionFor, suggestColumnRenames } from "@metaobjectsdev/migrate-ts"; -import type { AllowOptions, Change, ColumnRenameSuggestion } from "@metaobjectsdev/migrate-ts"; +import { allowOptionFor, isViewRecreateOnly, suggestColumnRenames } from "@metaobjectsdev/migrate-ts"; +import type { AllowOptions, Change, ColumnRenameSuggestion, ViewChangeReason } from "@metaobjectsdev/migrate-ts"; import type { BlockedEntry } from "./output.js"; // Map CLI allow tokens → migrate-ts AllowOptions field names. @@ -84,9 +84,9 @@ export function describeChange(c: Change): string { return c.restore !== undefined ? `${c.table} check ${c.check} (${c.restore.expression})` : `${c.table} check ${c.check}`; - case "create-view": return c.view.name; - case "replace-view": return c.view.name; - case "drop-view": return c.view; + case "create-view": return withViewReason(c.view.name, c.reason); + case "replace-view": return withViewReason(c.view.name, c.reason); + case "drop-view": return withViewReason(c.view, c.reason); } // Exhaustive: a new Change kind fails to compile here rather than printing a JSON dump // of the change object to a person (which is what a blocked drop-check used to do). @@ -94,6 +94,57 @@ export function describeChange(c: Change): string { return unhandled; } +/** + * A view's name plus WHY the diff planned its change. A bare name was all a view change + * ever printed, so a view whose definition matched the metadata — dropped and recreated only + * because the migration alters a table it reads — read exactly like one that differed (D3). + */ +function withViewReason(name: string, reason: ViewChangeReason | undefined): string { + return reason === undefined ? name : `${name} (${viewReasonText(reason)})`; +} + +function viewReasonText(r: ViewChangeReason): string { + switch (r.kind) { + case "missing": return "declared by the metadata, not in the database"; + case "undeclared": return "in the database, declared by no metadata object"; + case "definition": { + const what = r.compared === "fingerprint" + ? "definition differs: the database view's fingerprint does not match the metadata's" + : r.firstDifference !== undefined + ? `definition text differs: metadata «${r.firstDifference.expected}» vs database «${r.firstDifference.actual}»` + : "definition text differs"; + return recreatedAround(what, r.tables); + } + case "unfingerprinted": + return recreatedAround( + "the database view carries no MetaObjects fingerprint, so its definition cannot be compared", + r.tables, + ); + case "unchanged": + return `definition matches the metadata; recreated because the migration alters ${tableList(r.tables)}`; + } +} + +function recreatedAround(what: string, tables: readonly string[] | undefined): string { + return tables === undefined ? what : `${what}; recreated around the change to ${tableList(tables)}`; +} + +function tableList(tables: readonly string[]): string { + return `${tables.length === 1 ? "table" : "tables"} ${tables.join(", ")}`; +} + +/** + * One line naming the views a migration drops and recreates only because it alters a table + * they read. Their definitions match the metadata, and the migration needs the pair, but + * without this line the DROP VIEW / CREATE VIEW in the SQL reads like a view change (D3). + */ +export function viewRecreateNote(changes: readonly Change[]): string | undefined { + const views = changes.flatMap((c) => c.kind === "create-view" && isViewRecreateOnly(c) ? [c.view.name] : []); + if (views.length === 0) return undefined; + return `${views.length} view(s) match the metadata and are recreated only because the migration ` + + `alters a table they read: ${views.join(", ")}`; +} + /** The `--allow` token that unblocks `c`. migrate-ts picks the permission by what blocked the * change (a type change can be blocked by the auto-sequence default it carries), so this only * maps that permission back to its CLI token. */ diff --git a/server/typescript/packages/cli/src/lib/output.ts b/server/typescript/packages/cli/src/lib/output.ts index cbc41fddb..b5e00e30f 100644 --- a/server/typescript/packages/cli/src/lib/output.ts +++ b/server/typescript/packages/cli/src/lib/output.ts @@ -203,6 +203,11 @@ export interface MigrateResultShape { * caller sees the risk before applying. */ warnings?: string[]; + /** + * Explanations of planned changes that are not what they look like — today, views the + * migration drops and recreates only because it alters a table they read (D3). + */ + notes?: string[]; /** A dry run's SQL. Text format prints it as a preview; JSON/toon carry it here. */ sql?: { up: string; down: string }; } @@ -221,6 +226,12 @@ export function formatMigrateResult(result: MigrateResultShape, _opts: FormatOpt lines.push(` Changes: ${summary}`, ""); } + const notes = result.notes ?? []; + if (notes.length > 0) { + for (const n of notes) lines.push(` Note: ${n}`); + lines.push(""); + } + if (result.blocked.length > 0) { const anyRename = result.blocked.some((b) => b.renameFlag !== undefined); lines.push(anyRename ? " Blocked (re-run with the flag shown):" : " Blocked (re-run with --allow):"); @@ -331,6 +342,7 @@ export function migrateResultToData(result: MigrateResultShape): { summary: string; help: string[]; warnings?: string[]; + notes?: string[]; sql?: { up: string; down: string }; } { const changeEntries = Object.entries(result.changeCounts).filter(([, v]) => v > 0); @@ -398,6 +410,7 @@ export function migrateResultToData(result: MigrateResultShape): { changes, written: result.writtenPaths, summary, help, // Only when present, so a run with no hazard keeps its existing shape. ...(warnings.length > 0 ? { warnings } : {}), + ...((result.notes ?? []).length > 0 ? { notes: result.notes } : {}), ...(result.sql !== undefined ? { sql: result.sql } : {}), }; } diff --git a/server/typescript/packages/cli/test/integration/verify-db-view-recreate.test.ts b/server/typescript/packages/cli/test/integration/verify-db-view-recreate.test.ts new file mode 100644 index 000000000..719435ab5 --- /dev/null +++ b/server/typescript/packages/cli/test/integration/verify-db-view-recreate.test.ts @@ -0,0 +1,153 @@ +/** + * D3 (real engine, whole CLI pipeline, real SQLite): `meta verify --db` and `meta migrate` + * reported every managed view over a drifted table as a `- view` / `+ view` pair, even with + * the view's SQL byte-identical to the metadata's. An adopter could not tell whether any view + * matched, and neither the text nor `--format json` said what differed. + * + * The pair is real migration SQL: SQLite rebuilds a table to change a column's NOT NULL, and + * that strands the views reading it (#243). But a view recreated only for that reason is not + * drift. So `verify --db` reports the table change alone, `migrate` still emits the pair and + * says why, and a view whose definition really differs is reported with what differs. + */ +import { describe, test, expect, afterAll } from "bun:test"; +import { mkdtempSync, rmSync, mkdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { run } from "../../src/index.js"; + +const dirs: string[] = []; +afterAll(() => { for (const d of dirs) rmSync(d, { recursive: true, force: true }); }); + +function model(opts: { titleRequired: boolean; viewHasTitle?: boolean }): string { + return JSON.stringify({ + "metadata.root": { + package: "acme", + children: [ + { + "object.entity": { + name: "Program", + children: [ + { "source.rdb": { name: "src", "@table": "programs" } }, + { "field.long": { name: "id" } }, + { "field.string": { name: "title", ...(opts.titleRequired ? { "@required": true } : {}) } }, + { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } }, + ], + }, + }, + { + "object.projection": { + name: "ProgramView", + children: [ + { "source.rdb": { name: "src", "@kind": "view", "@view": "v_programs" } }, + { "field.long": { name: "id", extends: "acme::Program.id" } }, + ...(opts.viewHasTitle === false ? [] : [{ "field.string": { name: "title", extends: "acme::Program.title" } }]), + { "identity.primary": { name: "pk", extends: "acme::Program.pk" } }, + ], + }, + }, + ], + }, + }); +} + +function writeModel(repo: string, src: string): void { + writeFileSync(join(repo, "metaobjects", "meta.programs.json"), src, "utf8"); +} + +async function capture(argv: string[]): Promise<{ code: number; stdout: string; stderr: string }> { + const out: string[] = []; + const err: string[] = []; + const log = console.log; + const error = console.error; + console.log = (...a: unknown[]) => { out.push(a.map(String).join(" ")); }; + console.error = (...a: unknown[]) => { err.push(a.map(String).join(" ")); }; + try { + return { code: await run(argv), stdout: out.join("\n"), stderr: err.join("\n") }; + } finally { + console.log = log; + console.error = error; + } +} + +/** A database migrated from the model with `title` nullable, then the model makes it NOT NULL. */ +async function driftedRepo(): Promise<{ repo: string; dbUrl: string }> { + const repo = mkdtempSync(join(tmpdir(), "verify-db-view-recreate-")); + dirs.push(repo); + mkdirSync(join(repo, "metaobjects"), { recursive: true }); + writeModel(repo, model({ titleRequired: false })); + const dbUrl = `file:${join(repo, "local.db")}`; + const init = await capture([ + "migrate", "--cwd", repo, "--db", dbUrl, "--dialect", "sqlite", "--slug", "initial", "--apply", + ]); + expect(init.code).toBe(0); + writeModel(repo, model({ titleRequired: true })); + return { repo, dbUrl }; +} + +describe("D3 — a view recreated only around a table change", () => { + test("verify --db reports the table change and not the view", async () => { + const { repo, dbUrl } = await driftedRepo(); + const { code, stderr } = await capture(["verify", "--cwd", repo, "--db", dbUrl, "--dialect", "sqlite"]); + expect(code).toBe(1); + expect(stderr).toContain("~ column programs.title"); + expect(stderr).not.toContain("v_programs"); + }); + + test("verify --db --format json carries the difference, and no view row", async () => { + const { repo, dbUrl } = await driftedRepo(); + const { code, stdout } = await capture([ + "verify", "--cwd", repo, "--db", dbUrl, "--dialect", "sqlite", "--format", "json", + ]); + expect(code).toBe(1); + const payload = JSON.parse(stdout) as { + schemaDrift: { changes: { kind: string; object: string; detail: string }[]; findings: string[] }; + }; + expect(payload.schemaDrift.changes).toEqual([ + expect.objectContaining({ kind: "change-column-nullable", object: "column" }), + ]); + }); + + const NOTE = + "1 view(s) match the metadata and are recreated only because the migration alters a table they read: v_programs"; + const MIGRATE = ["--dialect", "sqlite", "--slug", "title-required", "--allow", "nullable-to-not-null", "--dry-run"]; + + // Both planners: the default diffs the metadata against the committed snapshot; --from-db + // (the adopter's command) diffs it against the live database. + for (const planner of [[], ["--from-db"]]) { + const label = planner.length === 0 ? "migrate" : "migrate --from-db"; + + test(`${label} still emits the DROP/CREATE VIEW pair, and says the view matches`, async () => { + const { repo, dbUrl } = await driftedRepo(); + const { code, stdout } = await capture([ + "migrate", "--cwd", repo, ...planner, "--db", dbUrl, ...MIGRATE, "--format", "text", + ]); + expect(code).toBe(0); + expect(stdout).toContain(`DROP VIEW IF EXISTS "v_programs"`); + expect(stdout).toContain(`CREATE VIEW "v_programs"`); + expect(stdout).toContain(NOTE); + }); + + test(`${label} --format json carries the same note`, async () => { + const { repo, dbUrl } = await driftedRepo(); + const { code, stdout } = await capture([ + "migrate", "--cwd", repo, ...planner, "--db", dbUrl, ...MIGRATE, "--format", "json", + ]); + expect(code).toBe(0); + const payload = JSON.parse(stdout) as { notes?: string[]; sql: { up: string } }; + expect(payload.notes).toEqual([NOTE]); + expect(payload.sql.up).toContain(`CREATE VIEW "v_programs"`); + }); + } +}); + +describe("D3 — a view whose definition differs", () => { + test("verify --db names the view and the first point its text differs", async () => { + const { repo, dbUrl } = await driftedRepo(); + // The table matches the database again; only the view's columns change. + writeModel(repo, model({ titleRequired: false, viewHasTitle: false })); + const { code, stderr } = await capture(["verify", "--cwd", repo, "--db", dbUrl, "--dialect", "sqlite"]); + expect(code).toBe(1); + expect(stderr).toMatch(/~ view v_programs \(definition text differs: metadata «.+» vs database «.+»\)/); + expect(stderr).not.toContain("column programs.title"); + }); +}); diff --git a/server/typescript/packages/cli/test/unit/describe-change.test.ts b/server/typescript/packages/cli/test/unit/describe-change.test.ts index c67085ac0..27e2cbe3e 100644 --- a/server/typescript/packages/cli/test/unit/describe-change.test.ts +++ b/server/typescript/packages/cli/test/unit/describe-change.test.ts @@ -52,3 +52,45 @@ test("every change kind reads as text, never JSON", () => { ]; for (const c of kinds) expect(describeChange(c)).not.toMatch(/^\{/); }); + +// D3: a view change printed only the view's name, so a view recreated around a table change +// (definition identical) read exactly like one whose definition differed. +test("a view change says why it was planned", () => { + const allowed = { state: "allowed" } as const; + const view = { name: "v_report", sql: "SELECT 1" }; + const cases: Array<[Change, string]> = [ + [ + { kind: "create-view", view, reason: { kind: "missing" }, status: allowed }, + "v_report (declared by the metadata, not in the database)", + ], + [ + { kind: "drop-view", view: "v_old", reason: { kind: "undeclared" }, status: allowed }, + "v_old (in the database, declared by no metadata object)", + ], + [ + { + kind: "replace-view", view, + reason: { kind: "definition", compared: "text", firstDifference: { expected: "select a as b", actual: "select a" } }, + status: allowed, + }, + "v_report (definition text differs: metadata «select a as b» vs database «select a»)", + ], + [ + { kind: "replace-view", view, reason: { kind: "definition", compared: "fingerprint" }, status: allowed }, + "v_report (definition differs: the database view's fingerprint does not match the metadata's)", + ], + [ + { kind: "drop-view", view: "v_report", reason: { kind: "definition", compared: "text", tables: ["weeks"] }, status: allowed }, + "v_report (definition text differs; recreated around the change to table weeks)", + ], + [ + { kind: "drop-view", view: "v_report", reason: { kind: "unfingerprinted" }, status: allowed }, + "v_report (the database view carries no MetaObjects fingerprint, so its definition cannot be compared)", + ], + [ + { kind: "create-view", view, reason: { kind: "unchanged", tables: ["weeks", "programs"] }, status: allowed }, + "v_report (definition matches the metadata; recreated because the migration alters tables weeks, programs)", + ], + ]; + for (const [c, text] of cases) expect(describeChange(c)).toBe(text); +}); diff --git a/server/typescript/packages/integration-tests/test/view-recreate-drift-pg.test.ts b/server/typescript/packages/integration-tests/test/view-recreate-drift-pg.test.ts new file mode 100644 index 000000000..9c5d56e5c --- /dev/null +++ b/server/typescript/packages/integration-tests/test/view-recreate-drift-pg.test.ts @@ -0,0 +1,105 @@ +/** + * D3 on a REAL Postgres: a managed view recreated only around a table change is not drift. + * + * Postgres refuses to ALTER a column a view reads, so the diff drops and recreates every view + * over an altered table. Postgres also deparses the stored view body, so equality comes from + * the fingerprint in the view's COMMENT, not from the text. The pair stays in the migration + * SQL; it says the view is unchanged, a drift report leaves it out, and it converges. + */ +import { describe, test, expect, beforeAll, afterAll, beforeEach } from "bun:test"; +import { + buildExpectedSchema, collectUnmanagedNames, computeDriftFromActual, diff, emit, introspectPostgres, + isViewRecreateOnly, type Change, type SchemaSnapshot, +} from "@metaobjectsdev/migrate-ts"; +import { buildProjectionViews } from "@metaobjectsdev/codegen-ts"; +import type { MetaRoot } from "@metaobjectsdev/metadata"; +import { Kysely, PostgresDialect, sql } from "kysely"; +import { Pool } from "pg"; +import { startPostgres, type RunningPg } from "../src/postgres-container.ts"; +import { loadMetadataDir } from "../src/load-metadata.ts"; +import { CANONICAL_DIR } from "../src/paths.ts"; + +let pg: RunningPg; +let k: Kysely>; +let canonical: MetaRoot; + +beforeAll(async () => { + pg = await startPostgres(); + k = new Kysely>({ + dialect: new PostgresDialect({ pool: new Pool({ connectionString: pg.connectionUri }) }), + }); + canonical = await loadMetadataDir(CANONICAL_DIR); +}, 120_000); + +afterAll(async () => { + await k.destroy(); + await pg.stop(); +}, 60_000); + +beforeEach(async () => { + await sql.raw("DROP SCHEMA public CASCADE").execute(k); + await sql.raw("CREATE SCHEMA public").execute(k); +}); + +const STRATEGY = "literal" as const; +const views = () => buildProjectionViews(canonical, { dialect: "postgres", columnNamingStrategy: STRATEGY }); +const expectedSchema = (): SchemaSnapshot => + buildExpectedSchema(canonical, { columnNamingStrategy: STRATEGY, views: views() }); + +async function applyRaw(text: string): Promise { + for (const stmt of text.split(/;\s*\n/).map((s) => s.trim()).filter(Boolean)) { + await sql.raw(stmt.endsWith(";") ? stmt : `${stmt};`).execute(k); + } +} + +function isViewChange(c: Change): boolean { + return c.kind === "create-view" || c.kind === "drop-view" || c.kind === "replace-view"; +} + +async function migrate(allow = {}) { + const expected = expectedSchema(); + const unmanagedNames = collectUnmanagedNames(canonical); + const result = await diff({ expected, actual: await introspectPostgres(k), dialect: "postgres", allow, unmanagedNames }); + expect(result.blocked).toEqual([]); + const { up } = result.changes.length === 0 ? { up: "" } : emit(result.changes, { dialect: "postgres" }); + if (up.trim().length > 0) await applyRaw(up); + return { result, up }; +} + +/** The canonical model migrated from empty, then `weeks.durationMinutes` made nullable by hand. */ +async function migratedWithNullableWeeks(): Promise { + await migrate(); + await sql.raw(`ALTER TABLE "weeks" ALTER COLUMN "durationMinutes" DROP NOT NULL`).execute(k); +} + +describe("D3 — a view recreated only around a column change is not drift (postgres)", () => { + test("drift reports the column and no view", async () => { + await migratedWithNullableWeeks(); + const drift = await computeDriftFromActual(await introspectPostgres(k), "postgres", canonical, { + columnNamingStrategy: STRATEGY, + views: views(), + }); + expect(drift.changes.map((c) => c.kind)).toEqual(["change-column-nullable"]); + expect(drift.blocked.filter(isViewChange)).toEqual([]); + }, 60_000); + + test("the diff plans each pair marked unchanged; it applies and converges", async () => { + await migratedWithNullableWeeks(); + const { result, up } = await migrate({ nullableToNotNull: true }); + const pairs = result.changes.filter(isViewChange); + expect(pairs.length).toBeGreaterThan(0); + for (const c of pairs) { + expect(isViewRecreateOnly(c)).toBe(true); + expect("reason" in c ? c.reason : undefined).toEqual({ kind: "unchanged", tables: ["weeks"] }); + } + expect(up).toContain(`CREATE VIEW "v_program_minutes"`); + + const followup = await diff({ + expected: expectedSchema(), + actual: await introspectPostgres(k), + dialect: "postgres", + unmanagedNames: collectUnmanagedNames(canonical), + }); + expect(followup.changes).toEqual([]); + }, 60_000); +}); diff --git a/server/typescript/packages/integration-tests/test/view-recreate-drift-sqlite.test.ts b/server/typescript/packages/integration-tests/test/view-recreate-drift-sqlite.test.ts new file mode 100644 index 000000000..69e7fd938 --- /dev/null +++ b/server/typescript/packages/integration-tests/test/view-recreate-drift-sqlite.test.ts @@ -0,0 +1,128 @@ +/** + * D3 — managed views over a drifted table, against a REAL SQLite (and the D1 diff over it). + * + * An adopter replayed `meta migrate`'s own CREATE VIEW DDL, so every report and projection + * view's stored SQL was byte-identical to the metadata's. The tables under those views had + * unrelated drift (an FK), and SQLite rebuilds a table to change one, which strands the views + * reading it (#243). So the diff drops and recreates those views, and `verify --db` printed + * each as `- view` / `+ view` with no word on what differed. It could not tell the adopter + * whether any view matched the metadata. + * + * The recreate pair is real migration SQL and stays. What changes: the pair says the view is + * unchanged, a drift report leaves it out, and the pair still applies and converges. + */ +import { describe, test, expect, beforeEach, afterEach, beforeAll } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Kysely, sql } from "kysely"; +import { LibsqlDialect } from "@libsql/kysely-libsql"; +import { + buildExpectedSchema, computeDriftFromActual, diff, emit, introspectSqlite, isViewRecreateOnly, + type Change, type SchemaSnapshot, +} from "@metaobjectsdev/migrate-ts"; +import { buildProjectionViews } from "@metaobjectsdev/codegen-ts"; +import type { MetaRoot } from "@metaobjectsdev/metadata"; +import { loadMetadataDir } from "../src/load-metadata.ts"; +import { CANONICAL_DIR } from "../src/paths.ts"; + +let tmpDir: string; +let k: Kysely>; +let canonical: MetaRoot; + +beforeAll(async () => { canonical = await loadMetadataDir(CANONICAL_DIR); }); +beforeEach(() => { + tmpDir = mkdtempSync(join(tmpdir(), "view-recreate-drift-")); + k = new Kysely({ dialect: new LibsqlDialect({ url: `file:${join(tmpDir, "test.db")}` }) }); +}); +afterEach(async () => { + await k.destroy(); + rmSync(tmpDir, { recursive: true, force: true }); +}); + +const STRATEGY = "literal" as const; + +function viewsFor(dialect: "sqlite" | "d1") { + return buildProjectionViews(canonical, { dialect, columnNamingStrategy: STRATEGY }); +} + +function expectedFor(dialect: "sqlite" | "d1"): SchemaSnapshot { + return buildExpectedSchema(canonical, { dialect, columnNamingStrategy: STRATEGY, views: viewsFor(dialect) }); +} + +async function applyRaw(text: string): Promise { + for (const stmt of text.trim().split(";").map((s) => s.trim()).filter(Boolean)) await sql.raw(stmt).execute(k); +} + +function isViewChange(c: Change): boolean { + return c.kind === "create-view" || c.kind === "drop-view" || c.kind === "replace-view"; +} + +/** + * `field.inet` has no SQLite storage class, so the canonical `all_types` table reports a + * blocked `text -> inet` change on every run (see report-views-sqlite.test.ts). No view + * reads `all_types`. It is named here, not swallowed: any other residue still fails. + */ +const INET_COLUMNS: ReadonlySet = new Set(["inetVal", "inet6Val"]); +const isInetResidue = (c: Change): boolean => + c.kind === "change-column-type" && c.table === "all_types" && INET_COLUMNS.has(c.column) && c.to.kind === "inet"; + +/** + * The canonical model migrated from empty, except that `weeks` is created without its + * foreign keys: table drift on a table most report views read, with every view as declared. + */ +async function migrateWithWeeksFkMissing(): Promise { + const expected = expectedFor("sqlite"); + const initial = await diff({ expected, actual: await introspectSqlite(k), dialect: "sqlite" }); + const { up } = emit(initial.changes, { dialect: "sqlite", expectedSchema: expected }); + const withoutFk = up.replace(/CREATE TABLE "weeks" \([\s\S]*?\n\);/, (table) => + table.replace(/,\s*(CONSTRAINT "[^"]+" )?FOREIGN KEY[^\n]*/g, "")); + expect(withoutFk).not.toBe(up); + await applyRaw(withoutFk); +} + +describe("D3 — a view recreated only around a table rebuild is not drift", () => { + for (const dialect of ["sqlite", "d1"] as const) { + test(`${dialect}: drift reports the FK and no view`, async () => { + await migrateWithWeeksFkMissing(); + const drift = await computeDriftFromActual(await introspectSqlite(k), dialect, canonical, { + columnNamingStrategy: STRATEGY, + views: viewsFor(dialect), + }); + expect(drift.changes.some((c) => c.kind === "add-fk" && c.table === "weeks")).toBe(true); + expect(drift.changes.filter(isViewChange)).toEqual([]); + expect(drift.blocked.filter(isViewChange)).toEqual([]); + }); + + test(`${dialect}: the diff still plans each pair, marked unchanged`, async () => { + await migrateWithWeeksFkMissing(); + const r = await diff({ expected: expectedFor(dialect), actual: await introspectSqlite(k), dialect }); + const views = r.changes.filter(isViewChange); + expect(views.length).toBeGreaterThan(0); + for (const c of views) { + expect(isViewRecreateOnly(c)).toBe(true); + expect("reason" in c ? c.reason : undefined).toEqual({ kind: "unchanged", tables: ["weeks"] }); + } + const recreated = views.flatMap((c) => c.kind === "create-view" ? [c.view.name] : []); + expect(recreated).toContain("v_program_minutes"); + expect(recreated).toContain("v_program_roster"); + }); + } + + test("sqlite: the pair applies, and the database then matches the metadata", async () => { + await migrateWithWeeksFkMissing(); + const expected = expectedFor("sqlite"); + const actual = await introspectSqlite(k); + const r = await diff({ expected, actual, dialect: "sqlite" }); + expect(r.blocked.filter((c) => !isInetResidue(c))).toEqual([]); + const { up } = emit(r.changes.filter((c) => !isInetResidue(c)), { + dialect: "sqlite", + expectedSchema: expected, + ...(actual.meta !== undefined && { actualMeta: actual.meta }), + }); + expect(up).toContain(`DROP VIEW IF EXISTS "v_program_minutes"`); + await applyRaw(up); + const followup = await diff({ expected, actual: await introspectSqlite(k), dialect: "sqlite" }); + expect(followup.changes.filter((c) => !isInetResidue(c))).toEqual([]); + }); +}); diff --git a/server/typescript/packages/migrate-ts/src/diff/index.ts b/server/typescript/packages/migrate-ts/src/diff/index.ts index 2e5d9e0f6..3d34d7781 100644 --- a/server/typescript/packages/migrate-ts/src/diff/index.ts +++ b/server/typescript/packages/migrate-ts/src/diff/index.ts @@ -1,6 +1,6 @@ import type { SchemaSnapshot, TableDescriptor, ColumnDescriptor, IndexDescriptor, FkDescriptor, - ViewDescriptor, + ViewDescriptor, ViewChangeReason, DependentRelation, Change, ChangeStatus, DiffResult, AllowOptions, AmbiguousCallback, Dialect, CheckDescriptor, NameChange, DeclaredRename, @@ -10,7 +10,7 @@ import { sqlTypeEquals } from "../sql-type.js"; import { applyStatus } from "./status.js"; import { PrimaryKeyChangeError } from "../errors.js"; import { detectColumnRenames, detectTableRenames } from "./rename-heuristic.js"; -import { viewSqlEquals } from "../view-sql-compare.js"; +import { firstViewSqlDifference, viewSqlEquals } from "../view-sql-compare.js"; import { viewReplaceIsLegal } from "../view-column-types.js"; import { checkExprEquals, checkExprRespelledNullSafe, normalizeCheckExpr, renameExprIdentifiers } from "../check-expr-compare.js"; import { isPgAutoSequenceDefault } from "../pg-identity-default.js"; @@ -886,7 +886,9 @@ function diffViews( for (const [id, v] of exp) { const a = act.get(id); if (a === undefined) { - changes.push({ kind: "create-view", view: v, ...schemaSpread(v.schema), status: ALLOWED }); + changes.push({ + kind: "create-view", view: v, ...schemaSpread(v.schema), reason: { kind: "missing" }, status: ALLOWED, + }); continue; } @@ -922,8 +924,11 @@ function diffViews( // SQLite/D1 (and unknown dialects): verbatim body comparison. if (v.sql !== undefined && a.sql !== undefined && !viewSqlEquals(v.sql, a.sql)) { + const firstDifference = firstViewSqlDifference(v.sql, a.sql); changes.push({ - kind: "replace-view", view: v, ...schemaSpread(v.schema), restore: a, status: ALLOWED, + kind: "replace-view", view: v, ...schemaSpread(v.schema), restore: a, + reason: { kind: "definition", compared: "text", ...(firstDifference !== undefined ? { firstDifference } : {}) }, + status: ALLOWED, }); } } @@ -931,7 +936,8 @@ function diffViews( for (const [id, v] of act) { if (!exp.has(id)) { changes.push({ - kind: "drop-view", view: v.name, ...schemaSpread(v.schema), restore: v, status: ALLOWED, + kind: "drop-view", view: v.name, ...schemaSpread(v.schema), restore: v, + reason: { kind: "undeclared" }, status: ALLOWED, }); } } @@ -954,6 +960,10 @@ function pushViewUpdate( ): void { const sx = schemaSpread(expected.schema); const adopt = unmanaged ? { unmanagedActual: true as const } : {}; + // Postgres compares fingerprints, never text: an unstamped view cannot be compared at all. + const reason: ViewChangeReason = unmanaged + ? { kind: "unfingerprinted" } + : { kind: "definition", compared: "fingerprint" }; // #239/#240: emit a replace only when it is a legal CREATE OR REPLACE. The decision // keys on the EXPECTED (desired) view's column knowledge, NOT on both sides: // - EXPECTED columns KNOWN (a projection): run viewReplaceIsLegal. It fails safe to @@ -971,14 +981,14 @@ function pushViewUpdate( ? viewReplaceIsLegal(expected.columns, actual.columns) : unmanaged; if (legal) { - changes.push({ kind: "replace-view", view: expected, ...sx, restore: actual, ...adopt, status: ALLOWED }); + changes.push({ kind: "replace-view", view: expected, ...sx, restore: actual, ...adopt, reason, status: ALLOWED }); return; } // The column list changed shape (removed / renamed / reordered / retyped), so // Postgres refuses OR REPLACE. The view must be dropped and rebuilt — which is // destructive to anything depending on it (annotateViewDropDependents makes that loud). - changes.push({ kind: "drop-view", view: expected.name, ...sx, restore: actual, ...adopt, status: ALLOWED }); - changes.push({ kind: "create-view", view: expected, ...sx, status: ALLOWED }); + changes.push({ kind: "drop-view", view: expected.name, ...sx, restore: actual, ...adopt, reason, status: ALLOWED }); + changes.push({ kind: "create-view", view: expected, ...sx, reason, status: ALLOWED }); } /** @@ -1067,7 +1077,8 @@ function recreateViewsDependingOnChangedTables( if (alteredTables.size === 0) return; for (const v of expectedViews) { - if (!(v.dependsOn ?? []).some((t) => alteredTables.has(t))) continue; + const tables = (v.dependsOn ?? []).filter((t) => alteredTables.has(t)); + if (tables.length === 0) continue; const id = viewIdentity(v); // Brand-new view (create-view already queued): the DB has no prior view to @@ -1077,20 +1088,58 @@ function recreateViewsDependingOnChangedTables( // Supersede any replace-view (CREATE OR REPLACE can't run mid-ALTER) with an // explicit drop(before)/create(after) pair — the emit STAGE_ORDER sequences // drop-view ahead of the column change and create-view after it. + let superseded: Extract | undefined; for (let i = changes.length - 1; i >= 0; i--) { const c = changes[i]!; - if (c.kind === "replace-view" && viewIdentity(c.view) === id) changes.splice(i, 1); + if (c.kind === "replace-view" && viewIdentity(c.view) === id) { + superseded = c; + changes.splice(i, 1); + } } const prior = actualById.get(id); + const reason = recreateReason(v, prior, superseded, tables, dialect); changes.push({ kind: "drop-view", view: v.name, ...schemaSpread(v.schema), ...(prior !== undefined ? { restore: prior } : {}), + // A superseded ADOPTION (#239) keeps its gate: the pair would otherwise auto-allow + // overwriting a view that may be hand-written. + ...(superseded?.unmanagedActual === true ? { unmanagedActual: true } : {}), + ...(reason !== undefined ? { reason } : {}), + status: ALLOWED, + }); + changes.push({ + kind: "create-view", view: v, ...schemaSpread(v.schema), + ...(reason !== undefined ? { reason } : {}), status: ALLOWED, }); - changes.push({ kind: "create-view", view: v, ...schemaSpread(v.schema), status: ALLOWED }); } } +/** + * Why Pass 2c recreates `v` (D3). A superseded replace-view keeps its own reason, now + * naming the altered tables. Otherwise the definition is `unchanged` — but only when the + * comparison Pass 2b makes on this dialect actually PROVED it equal. When it could not (no + * expected fingerprint, or a body missing on either side) the change gets no reason, which + * reports it as drift: the honest answer to a comparison that never ran. + */ +function recreateReason( + v: ViewDescriptor, + prior: ViewDescriptor | undefined, + superseded: Extract | undefined, + tables: readonly string[], + dialect: Dialect | undefined, +): ViewChangeReason | undefined { + if (superseded !== undefined) { + const r = superseded.reason; + return r?.kind === "definition" || r?.kind === "unfingerprinted" ? { ...r, tables } : undefined; + } + if (prior === undefined) return undefined; + const proven = dialect === "postgres" + ? v.fingerprint !== undefined && prior.fingerprint === v.fingerprint + : viewSqlEquals(v.sql, prior.sql); + return proven ? { kind: "unchanged", tables } : undefined; +} + function indexEquals(a: IndexDescriptor, b: IndexDescriptor): boolean { if (a.unique !== b.unique) return false; // Access method: absent = "btree" (the default). diff --git a/server/typescript/packages/migrate-ts/src/drift/classify.ts b/server/typescript/packages/migrate-ts/src/drift/classify.ts index 1c1e7824a..824b4511b 100644 --- a/server/typescript/packages/migrate-ts/src/drift/classify.ts +++ b/server/typescript/packages/migrate-ts/src/drift/classify.ts @@ -1,6 +1,7 @@ // src/drift/classify.ts import { diff } from "../diff/index.js"; import type { Change, Dialect, DiffResult, SchemaSnapshot } from "../types.js"; +import { isViewRecreateOnly } from "../view-change-reason.js"; /** * Change kinds that represent an object present in the live DB but absent from @@ -30,6 +31,9 @@ export function classifyDrift(changes: Change[]): DriftClassification { const drift: Change[] = []; const unmanaged: Change[] = []; for (const c of changes) { + // A view recreated only around a table change matches the snapshot (D3). Its drop half + // is not a DB-only object and its create half is not drift; the table change is. + if (isViewRecreateOnly(c)) continue; if (UNMANAGED_KINDS.has(c.kind)) unmanaged.push(c); else drift.push(c); } diff --git a/server/typescript/packages/migrate-ts/src/drift/drift.ts b/server/typescript/packages/migrate-ts/src/drift/drift.ts index f515283bf..ffab2090a 100644 --- a/server/typescript/packages/migrate-ts/src/drift/drift.ts +++ b/server/typescript/packages/migrate-ts/src/drift/drift.ts @@ -20,6 +20,7 @@ import { buildExpectedSchemaWithProvenance } from "../expected-schema.js"; import { introspect } from "../introspect/index.js"; import { diff } from "../diff/index.js"; import { collectUnmanagedNames } from "../unmanaged.js"; +import { withoutViewRecreates } from "../view-change-reason.js"; import { scopeExpectedSchema, scopedDiffInputs, type ObjectScopePredicate } from "../scope.js"; import type { AllowOptions, Dialect, DiffResult, SchemaSnapshot } from "../types.js"; @@ -107,7 +108,8 @@ export interface DriftResult extends DiffResult { * some other way (e.g. `introspectD1` over a wrangler-shelled-out runner). * * Returns a `DiffResult` whose `changes` is empty iff `actual` matches the - * metadata. The caller decides exit behavior (the schema-drift gate fails when + * metadata. A view the migration would recreate only because it alters a table the + * view reads is left out (`isViewRecreateOnly`): its definition matches. The caller decides exit behavior (the schema-drift gate fails when * `changes` is non-empty). */ export async function computeDriftFromActual( @@ -140,6 +142,10 @@ export async function computeDriftFromActual( }); return { ...result, + // A view recreated only because the migration alters a table it reads matches the + // metadata (D3): `diff` plans the pair for the migration SQL, but it is not drift. + changes: withoutViewRecreates(result.changes), + blocked: withoutViewRecreates(result.blocked), outOfScope: scoped.outOfScope, declaredSchemas: scoped.declaredSchemas, importedOutOfScope: scoped.importedOutOfScope, diff --git a/server/typescript/packages/migrate-ts/src/index.ts b/server/typescript/packages/migrate-ts/src/index.ts index 81ca218ab..704b5103f 100644 --- a/server/typescript/packages/migrate-ts/src/index.ts +++ b/server/typescript/packages/migrate-ts/src/index.ts @@ -73,7 +73,7 @@ export type { SqlType } from "./sql-type.js"; export type { SchemaSnapshot, SnapshotMeta, TableDescriptor, ColumnDescriptor, IndexDescriptor, FkDescriptor, ColumnDefault, - ViewDescriptor, FkAction, NameChange, + ViewDescriptor, ViewChangeReason, FkAction, NameChange, Change, ChangeKind, ChangeStatus, AllowOptions, AmbiguousChange, AmbiguousResolution, AmbiguousCallback, DiffResult, DataHazard, DeclaredRename, EmitResult, Dialect, @@ -89,6 +89,7 @@ export type { WriteMigrationFlywayOptions, WriteMigrationFlywayResult } from "./ // there is no standalone view-migration emitter (the source-aware-diff / view-diff // / view-ddl-{postgres,sqlite} stack was folded into the schema-diff path). export { normalizeViewSql, viewSqlEquals } from "./view-sql-compare.js"; +export { isViewRecreateOnly, withoutViewRecreates } from "./view-change-reason.js"; // D1 dialect emitter + safety pass. // renderD1 is exported directly (unlike renderSqlite/renderPostgres) so diff --git a/server/typescript/packages/migrate-ts/src/types.ts b/server/typescript/packages/migrate-ts/src/types.ts index f5d2606b1..48bbc4e1e 100644 --- a/server/typescript/packages/migrate-ts/src/types.ts +++ b/server/typescript/packages/migrate-ts/src/types.ts @@ -313,13 +313,21 @@ export type Change = respelled?: true; status: ChangeStatus; } - // Declared for v0.3, never produced in v0.1: - | { kind: "create-view"; view: ViewDescriptor; schema?: string; status: ChangeStatus } + | { + kind: "create-view"; + view: ViewDescriptor; + schema?: string; + status: ChangeStatus; + /** Why the view is (re)created — see {@link ViewChangeReason}. */ + reason?: ViewChangeReason; + } | { kind: "drop-view"; view: string; schema?: string; status: ChangeStatus; + /** Why the view is dropped — see {@link ViewChangeReason}. */ + reason?: ViewChangeReason; /** The view as it exists in the DB — lets the down migration recreate it. */ restore?: ViewDescriptor; /** @@ -344,6 +352,8 @@ export type Change = view: ViewDescriptor; schema?: string; status: ChangeStatus; + /** Why the view is replaced — see {@link ViewChangeReason}. */ + reason?: ViewChangeReason; /** The view as it exists in the DB — lets the down migration restore the old body. */ restore?: ViewDescriptor; /** @@ -357,6 +367,50 @@ export type Change = export type ChangeKind = Change["kind"]; +/** + * Why `diff` planned a view change, so a report can say what differs instead of printing a + * bare drop/create pair. + * + * The distinction that matters most is `unchanged`. A migration that alters a table drops + * and recreates every view reading it — Postgres refuses to ALTER a column a view reads, + * and SQLite/D1 rebuild the table, which strands the view (#243) — so the migration SQL + * needs the pair even when the view's definition already matches the metadata. Such a pair + * is not drift, and `meta verify --db` must not report it as drift (`isViewRecreateOnly`). + * + * `diff` sets a reason on every view change it plans. It is optional so a hand-built + * `Change` stays valid; a change without one is reported as drift, with no detail. + */ +export type ViewChangeReason = + /** Declared by the metadata, absent from the database. */ + | { kind: "missing" } + /** In the database, declared by no metadata object. */ + | { kind: "undeclared" } + /** + * The definition differs from the metadata's. `compared` names what was compared: the + * normalized body TEXT on SQLite/D1, which store view SQL verbatim, or the FINGERPRINT + * stamped into the view's comment on Postgres, which deparses view SQL so the text can + * never match. `firstDifference` (text comparisons only) holds each side's normalized + * text from a little before the first character that differs. `tables` is set when the + * change became a drop/create pair because the migration also alters those tables. + */ + | { + kind: "definition"; + compared: "text" | "fingerprint"; + firstDifference?: { expected: string; actual: string }; + tables?: readonly string[]; + } + /** + * Postgres: the database view carries no MetaObjects fingerprint (hand-written, or created + * before fingerprinting), so its definition cannot be compared. `tables` as for + * `definition`. + */ + | { kind: "unfingerprinted"; tables?: readonly string[] } + /** + * The definition matches the metadata. The view is dropped and recreated only because + * the migration alters `tables`, which it reads. + */ + | { kind: "unchanged"; tables: readonly string[] }; + export interface ChangeStatus { state: "allowed" | "blocked"; blockedReason?: string; diff --git a/server/typescript/packages/migrate-ts/src/view-change-reason.ts b/server/typescript/packages/migrate-ts/src/view-change-reason.ts new file mode 100644 index 000000000..936a8391d --- /dev/null +++ b/server/typescript/packages/migrate-ts/src/view-change-reason.ts @@ -0,0 +1,21 @@ +// view-change-reason.ts — reading a view change's `reason` (D3). +// +// `diff` attaches a `ViewChangeReason` to every view change it plans. This module answers +// the one question every drift report needs from it: is this change drift at all? + +import type { Change } from "./types.js"; + +/** + * True for a view change whose definition matches the metadata: half of the drop/create + * pair a migration needs only because it alters a table the view reads. The pair belongs + * in the migration SQL; it does not belong in a drift report, where it reads as a view + * that differs. A change with no reason is not recreate-only, so it stays reported. + */ +export function isViewRecreateOnly(c: Change): boolean { + return (c.kind === "create-view" || c.kind === "drop-view") && c.reason?.kind === "unchanged"; +} + +/** `changes` without the recreate-only view changes — what a drift report lists. */ +export function withoutViewRecreates(changes: readonly C[]): C[] { + return changes.filter((c) => !isViewRecreateOnly(c)); +} diff --git a/server/typescript/packages/migrate-ts/src/view-sql-compare.ts b/server/typescript/packages/migrate-ts/src/view-sql-compare.ts index f4ea06e79..82c7642dc 100644 --- a/server/typescript/packages/migrate-ts/src/view-sql-compare.ts +++ b/server/typescript/packages/migrate-ts/src/view-sql-compare.ts @@ -47,3 +47,31 @@ export function viewSqlEquals(a: string | undefined, b: string | undefined): boo if (a === undefined || b === undefined) return false; return normalizeViewSql(a) === normalizeViewSql(b); } + +/** Characters of shared text kept before the first difference, so the excerpt has context. */ +const DIFFERENCE_LEAD = 20; +/** Characters kept from each side, starting at the excerpt's first character. */ +const DIFFERENCE_SPAN = 60; + +/** + * Where two view definitions first differ, as each side's NORMALIZED text (see + * `normalizeViewSql`) from a little before the first differing character. `undefined` when + * they are equal. A reader sees the difference itself rather than two whole bodies to + * compare by eye: a quoting or alias change in a long report view is otherwise invisible. + */ +export function firstViewSqlDifference( + expected: string, + actual: string, +): { expected: string; actual: string } | undefined { + const e = normalizeViewSql(expected); + const a = normalizeViewSql(actual); + if (e === a) return undefined; + let i = 0; + while (i < e.length && i < a.length && e[i] === a[i]) i++; + const start = Math.max(0, i - DIFFERENCE_LEAD); + const excerpt = (s: string): string => { + const end = start + DIFFERENCE_SPAN; + return `${start > 0 ? "…" : ""}${s.slice(start, end)}${end < s.length ? "…" : ""}`; + }; + return { expected: excerpt(e), actual: excerpt(a) }; +} diff --git a/server/typescript/packages/migrate-ts/test/drift/view-recreate-not-drift.test.ts b/server/typescript/packages/migrate-ts/test/drift/view-recreate-not-drift.test.ts new file mode 100644 index 000000000..b30c34cae --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/drift/view-recreate-not-drift.test.ts @@ -0,0 +1,80 @@ +/** + * D3 — a view the migration recreates only around a table change is not drift. + * + * `diff` still plans the drop/create pair (the migration SQL needs it), but a drift report + * answers "does the database match the metadata?", and a view whose definition matches + * does. An adopter's `verify --db` listed every report and projection view as + * `- view` / `+ view` because the tables under them had unrelated drift. + */ +import { test, expect, describe } from "bun:test"; +import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata"; +import { buildExpectedSchema } from "../../src/expected-schema.js"; +import { computeDriftFromActual } from "../../src/drift/drift.js"; +import { classifyDrift } from "../../src/drift/classify.js"; +import type { Change, Dialect, SchemaSnapshot } from "../../src/types.js"; + +const META = JSON.stringify({ + "metadata.root": { + package: "acme::d3", + children: [ + { + "object.entity": { + name: "Gadget", + children: [ + { "source.rdb": {} }, + { "field.long": { name: "id" } }, + { "field.string": { name: "label", "@required": true } }, + { "identity.primary": { name: "pk", "@fields": ["id"] } }, + ], + }, + }, + ], + }, +}); + +async function loadMeta() { + return (await new MetaDataLoader().load([new InMemoryStringSource(META)])).root; +} + +describe("D3 — computeDriftFromActual leaves recreate-only views out of the drift", () => { + for (const dialect of ["sqlite", "d1", "postgres"] as const satisfies readonly Dialect[]) { + test(`${dialect}: a table difference is drift; the matching view over it is not`, async () => { + const root = await loadMeta(); + const probe = buildExpectedSchema(root, { dialect }); + const table = probe.tables[0]!; + const body = `SELECT id, label FROM ${table.name}`; + const views = [{ name: "v_gadget", sql: body, dependsOn: [table.name] }]; + const expected = buildExpectedSchema(root, { dialect, views }); + const view = expected.views[0]!; + + // The database: the view exactly as the metadata declares it, and `label` nullable. + const actual: SchemaSnapshot = structuredClone(expected); + actual.tables[0]!.columns.find((c) => c.name === "label")!.nullable = true; + actual.views = [ + dialect === "postgres" + ? { name: view.name, sql: " SELECT gadget.id, gadget.label FROM gadget;", fingerprint: view.fingerprint! } + : { name: view.name, sql: `CREATE VIEW "${view.name}" AS ${body}` }, + ]; + + const result = await computeDriftFromActual(actual, dialect, root, { views }); + expect(result.changes.map((c) => c.kind)).toEqual(["change-column-nullable"]); + expect(result.blocked.some((c) => c.kind === "drop-view" || c.kind === "create-view")).toBe(false); + }); + } +}); + +describe("D3 — classifyDrift leaves recreate-only views out of both buckets", () => { + test("an unchanged recreate pair is neither drift nor unmanaged", () => { + const allowed = { state: "allowed" as const }; + const reason = { kind: "unchanged" as const, tables: ["t"] }; + const changes: Change[] = [ + { kind: "change-column-nullable", table: "t", column: "c", from: true, to: false, status: allowed }, + { kind: "drop-view", view: "v", reason, status: allowed }, + { kind: "create-view", view: { name: "v", sql: "SELECT c FROM t" }, reason, status: allowed }, + { kind: "drop-view", view: "legacy", reason: { kind: "undeclared" }, status: allowed }, + ]; + const { drift, unmanaged } = classifyDrift(changes); + expect(drift.map((c) => c.kind)).toEqual(["change-column-nullable"]); + expect(unmanaged).toMatchObject([{ kind: "drop-view", view: "legacy" }]); + }); +}); diff --git a/server/typescript/packages/migrate-ts/test/unit/diff-view-change-reason.test.ts b/server/typescript/packages/migrate-ts/test/unit/diff-view-change-reason.test.ts new file mode 100644 index 000000000..b4b4ba716 --- /dev/null +++ b/server/typescript/packages/migrate-ts/test/unit/diff-view-change-reason.test.ts @@ -0,0 +1,232 @@ +/** + * D3 — a view change says WHY it was planned. + * + * Pass 2c (the dependency recreate) drops and recreates every view that reads a table the + * migration alters: Postgres refuses to ALTER a column a view reads, and SQLite/D1 rebuild + * the table, which strands the view (#243). That pair is needed in the migration SQL, but + * it carried nothing that set it apart from a view whose definition really differs. So an + * adopter whose view SQL was byte-identical to the metadata's saw `- view v` / `+ view v` + * for every report and projection view, and `verify --db` could not say whether any view + * matched. Every view change now carries a `reason`, and a recreate whose definition + * matches is `{ kind: "unchanged" }`, which `isViewRecreateOnly` reports as not drift. + */ +import { test, expect, describe } from "bun:test"; +import { diff } from "../../src/diff/index.js"; +import { isViewRecreateOnly } from "../../src/view-change-reason.js"; +import { viewFingerprint } from "../../src/view-fingerprint.js"; +import type { + Change, ColumnDescriptor, Dialect, FkDescriptor, SchemaSnapshot, ViewDescriptor, +} from "../../src/types.js"; + +const BODY = "SELECT id, label FROM t"; + +function col(name: string, nullable: boolean): ColumnDescriptor { + return { name, sqlType: name === "id" ? { kind: "integer", bits: 64 } : { kind: "text" }, nullable }; +} + +const PARENT_FK: FkDescriptor = { + name: "t_parentId_fk", columns: ["parentId"], refTable: "p", refColumns: ["id"], +}; + +/** Table `t` (read by the view) plus table `p` (which `t`'s FK points at). */ +function snap( + opts: { labelNullable?: boolean; fk?: boolean; views: ViewDescriptor[] }, +): SchemaSnapshot { + return { + tables: [ + { + name: "p", columns: [col("id", false)], indexes: [], foreignKeys: [], primaryKey: ["id"], checks: [], + }, + { + name: "t", + columns: [col("id", false), col("label", opts.labelNullable ?? true), col("parentId", true)], + indexes: [], + foreignKeys: opts.fk === true ? [PARENT_FK] : [], + primaryKey: ["id"], + checks: [], + }, + ], + views: opts.views, + }; +} + +/** The view as the metadata declares it: a body, its fingerprint, and the table it reads. */ +function expectedView(sql = BODY): ViewDescriptor { + return { name: "v", sql, fingerprint: viewFingerprint(sql), dependsOn: ["t"] }; +} + +/** + * The view as introspection reads it back. SQLite/D1 store the statement verbatim; Postgres + * deparses the body (so the text never matches) and the fingerprint comes from the comment. + */ +function actualView(dialect: Dialect, sql = BODY): ViewDescriptor { + if (dialect === "postgres") { + return { name: "v", sql: ` SELECT t.id,\n t.label\n FROM t;`, fingerprint: viewFingerprint(sql) }; + } + return { name: "v", sql: `CREATE VIEW "v" AS ${sql}` }; +} + +function viewChanges(changes: readonly Change[]): Change[] { + return changes.filter((c) => c.kind === "create-view" || c.kind === "drop-view" || c.kind === "replace-view"); +} + +describe("D3 — a view recreated only around a table change is marked unchanged", () => { + for (const dialect of ["sqlite", "d1", "postgres"] as const) { + test(`${dialect}: NOT NULL change on a table the view reads`, async () => { + const r = await diff({ + expected: snap({ labelNullable: false, views: [expectedView()] }), + actual: snap({ labelNullable: true, views: [actualView(dialect)] }), + dialect, + }); + expect(r.changes.some((c) => c.kind === "change-column-nullable")).toBe(true); + + // The pair is still planned: the migration SQL needs it. + const views = viewChanges(r.changes); + expect(views.map((c) => c.kind).sort()).toEqual(["create-view", "drop-view"]); + for (const c of views) { + expect("reason" in c ? c.reason : undefined).toEqual({ kind: "unchanged", tables: ["t"] }); + expect(isViewRecreateOnly(c)).toBe(true); + } + }); + } + + // The adopter's case: an FK change rebuilds the table on SQLite/D1 (#243). Postgres adds + // and drops constraints in place, so it plans no view change at all. + for (const dialect of ["sqlite", "d1"] as const) { + test(`${dialect}: FK change on a table the view reads`, async () => { + const r = await diff({ + expected: snap({ fk: true, views: [expectedView()] }), + actual: snap({ fk: false, views: [actualView(dialect)] }), + dialect, + }); + expect(r.changes.some((c) => c.kind === "add-fk")).toBe(true); + const views = viewChanges(r.changes); + expect(views).toHaveLength(2); + expect(views.every(isViewRecreateOnly)).toBe(true); + }); + } + + test("postgres: FK change plans no view change", async () => { + const r = await diff({ + expected: snap({ fk: true, views: [expectedView()] }), + actual: snap({ fk: false, views: [actualView("postgres")] }), + dialect: "postgres", + }); + expect(viewChanges(r.changes)).toEqual([]); + }); + + test("no table change and a matching view → no view change on any dialect", async () => { + for (const dialect of ["sqlite", "d1", "postgres"] as const) { + const r = await diff({ + expected: snap({ views: [expectedView()] }), + actual: snap({ views: [actualView(dialect)] }), + dialect, + }); + expect(r.changes).toEqual([]); + } + }); +}); + +describe("D3 — a view whose definition differs says so", () => { + const CHANGED = "SELECT id, label AS title FROM t"; + + for (const dialect of ["sqlite", "d1"] as const) { + test(`${dialect}: replace-view names a text difference and where it starts`, async () => { + const r = await diff({ + expected: snap({ views: [expectedView(CHANGED)] }), + actual: snap({ views: [actualView(dialect)] }), + dialect, + }); + const [c] = viewChanges(r.changes); + expect(c?.kind).toBe("replace-view"); + expect(c !== undefined && isViewRecreateOnly(c)).toBe(false); + expect(c !== undefined && "reason" in c ? c.reason : undefined).toEqual({ + kind: "definition", + compared: "text", + firstDifference: { expected: "select id, label as title from t", actual: "select id, label from t" }, + }); + }); + + test(`${dialect}: a changed view over a rebuilt table keeps its difference`, async () => { + const r = await diff({ + expected: snap({ labelNullable: false, views: [expectedView(CHANGED)] }), + actual: snap({ labelNullable: true, views: [actualView(dialect)] }), + dialect, + }); + const views = viewChanges(r.changes); + expect(views.map((c) => c.kind).sort()).toEqual(["create-view", "drop-view"]); + for (const c of views) { + expect(isViewRecreateOnly(c)).toBe(false); + expect("reason" in c ? c.reason : undefined).toMatchObject({ + kind: "definition", compared: "text", tables: ["t"], + }); + } + }); + } + + test("postgres: a changed fingerprint is a definition difference", async () => { + const r = await diff({ + expected: snap({ views: [expectedView(CHANGED)] }), + actual: snap({ views: [actualView("postgres")] }), + dialect: "postgres", + }); + const views = viewChanges(r.changes); + expect(views.length).toBeGreaterThan(0); + for (const c of views) { + expect(isViewRecreateOnly(c)).toBe(false); + expect("reason" in c ? c.reason : undefined).toEqual({ kind: "definition", compared: "fingerprint" }); + } + }); + + test("postgres: an unstamped database view cannot be compared", async () => { + const unstamped: ViewDescriptor = { name: "v", sql: " SELECT t.id, t.label FROM t;" }; + const r = await diff({ + expected: snap({ views: [expectedView()] }), + actual: snap({ views: [unstamped] }), + dialect: "postgres", + }); + const views = viewChanges(r.changes); + expect(views.length).toBeGreaterThan(0); + for (const c of views) expect("reason" in c ? c.reason : undefined).toEqual({ kind: "unfingerprinted" }); + }); +}); + +describe("D3 — a view only one side has", () => { + for (const dialect of ["sqlite", "d1", "postgres"] as const) { + test(`${dialect}: missing from the database → create-view, reason missing`, async () => { + const r = await diff({ expected: snap({ views: [expectedView()] }), actual: snap({ views: [] }), dialect }); + expect(viewChanges(r.changes)).toMatchObject([{ kind: "create-view", reason: { kind: "missing" } }]); + }); + + test(`${dialect}: declared by nothing → drop-view, reason undeclared`, async () => { + const r = await diff({ + expected: snap({ views: [] }), + actual: snap({ views: [actualView(dialect)] }), + dialect, + allow: { dropView: true }, + }); + expect(viewChanges(r.changes)).toMatchObject([{ kind: "drop-view", reason: { kind: "undeclared" } }]); + }); + } +}); + +describe("D3 — an adoption Pass 2c supersedes keeps its gate", () => { + test("postgres: an unstamped view over an altered table still needs allow.adoptView", async () => { + const unstamped: ViewDescriptor = { name: "v", sql: " SELECT t.id, t.label FROM t;" }; + const args = { + expected: snap({ labelNullable: false, views: [expectedView()] }), + actual: snap({ labelNullable: true, views: [unstamped] }), + dialect: "postgres" as const, + }; + const r = await diff(args); + const drop = r.changes.find((c) => c.kind === "drop-view"); + expect(drop).toMatchObject({ + unmanagedActual: true, + reason: { kind: "unfingerprinted", tables: ["t"] }, + status: { state: "blocked" }, + }); + + const allowed = await diff({ ...args, allow: { adoptView: true, nullableToNotNull: true } }); + expect(allowed.blocked).toEqual([]); + }); +});