diff --git a/.changeset/23558-ceremony-dialect-resolver.md b/.changeset/23558-ceremony-dialect-resolver.md new file mode 100644 index 00000000000..452e3d06075 --- /dev/null +++ b/.changeset/23558-ceremony-dialect-resolver.md @@ -0,0 +1,31 @@ +--- +'@objectstack/cli': patch +'@objectstack/metadata-core': minor +--- + +fix(cli): `os migrate organization-ownership` reads its SQL dialect once, through the driver resolver it shares with metadata-core, and refuses an unresolved dialect before any statement, naming the driver, with no SQLite default left + +Clause-②: yes (widening) + +**Who this is for.** An operator who runs the organization-ownership ceremony (plan or `--apply`) on a PostgreSQL or MySQL database whose driver's knex `config.client` is a client CLASS (knex's bring-your-own-dialect hatch) or a client string the ceremony does not recognise. + +**FROM.** Both arms took the dialect from `String(config.client)` alone, with a reader of their own. + +- Such a run was refused (`driver-unsupported`, exit 1) by the plan step, which runs first. A client class was stringified, so the refusal printed the class's source text in place of the driver's name. +- The apply step and its template lookup each carried a `'sqlite'` default for an unresolved dialect. Neither was reachable, only because the plan step had already refused. + +**TO.** The dialect is resolved once, from the stack's driver, before either arm runs a statement. The resolver is the one metadata-core's legacy-row reader uses: the driver's `dialectName` first, then a recognised `config.client` string. + +- Refused: a client class with no recognised `dialectName`, an unrecognised client string, `dialectName: 'unknown'` (what `SqlDriver` answers for either), and a driver with no dialect at all. Each gets `driver-unsupported`, exit 1, with no statement run and no plan file written. The refusal names the driver: its name, its `dialectName`, and its client string or client class name. +- No `'sqlite'` default remains. The plan and the apply take the resolved dialect and never re-derive it. +- The plan file's `client` field records the client only when it is a string. + +Measured on each in-repo SQL driver: `SqlDriver` on better-sqlite3, pg and mysql2, and `TursoDriver`, resolve to the dialect they did before. + +**metadata-core.** `dialectOfDriver(driver)` is exported. It is the one resolver behind `readLegacyOrganizationRows`, and answers: + +- `'sqlite' | 'postgres' | 'mysql'`; +- `'none'` for a driver that is not a SQL driver; +- `'unresolved'` otherwise. + +A `dialectName` string is the whole answer, recognised or not. A `config.client` is read only when there is no `dialectName`, and only as a string. diff --git a/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts b/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts new file mode 100644 index 00000000000..7e0f8266b1b --- /dev/null +++ b/packages/cli/src/commands/migrate/organization-ownership.dialect.test.ts @@ -0,0 +1,230 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os migrate organization-ownership` reads its SQL dialect off the stack's + * driver ONCE, before the first statement, through the resolver it shares + * with `@objectstack/metadata-core` (`ceremonyDialectOf` → `dialectOfDriver`): + * the driver's `dialectName` first, then a recognised `config.client` string. + * On anything else both arms refuse, naming the driver, with exit 1 and no + * statement run; neither arm reads an unresolved dialect as SQLite. + * + * The boot is replaced by a stack whose driver records every statement and + * answers each with no rows; the apply step is replaced by a recorder (its own + * suite pins what it does with the executor it is handed). The driver shapes + * are the ones each in-repo driver presents, measured on the drivers + * themselves: `SqliteWasmDriver`'s knex `client` is a dialect CLASS and its + * `dialectName` is `'sqlite'`; `SqlDriver` answers `dialectName: 'unknown'` + * for a client it does not recognise, a client class included. + * + * The `SqliteWasmDriver` row pins the resolver's answer for that SHAPE. The + * command's boot does not hand that driver over today (the schema boot looks + * a SQL driver up by service names that do not include it, so a + * `wasm-sqlite://` run is refused earlier, as having no SQL driver at all); + * the row keeps the answer right for the day it does. + */ + +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const boot = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/schema-migrate.js', () => ({ bootSchemaStack: boot })); +// The siblings' occupancy stub: no case here passes --database-url, so the real +// probe would read whatever database the package's own directory resolves to. +vi.mock('../../utils/migrate-occupancy-gate.js', () => ({ + OCCUPANCY_STOP_HINT: 'occupancy hint', + probeMigrationTarget: vi.fn(async () => ({ status: 'free' })), +})); + +const apply = vi.hoisted(() => vi.fn()); +vi.mock('../../utils/organization-ownership-apply.js', async (importOriginal) => ({ + ...(await importOriginal()), + applyOrganizationOwnership: apply, +})); + +type Command = (typeof import('./organization-ownership.js'))['default']; +let MigrateOrganizationOwnership: Command; + +// The command's module graph is large, and oclif reads the package's config on +// the first run: both are paid outside the 5 s default (CASE_TIMEOUT_MS). +beforeAll(async () => { + ({ default: MigrateOrganizationOwnership } = await import('./organization-ownership.js')); +}, 60_000); + +const CASE_TIMEOUT_MS = 60_000; +const CLI_ROOT = fileURLToPath(new URL('../../..', import.meta.url)); + +/** knex's bring-your-own-dialect hatch: a Client constructor as `config.client`. */ +class Client_PG {} +class Client_WasmSqlite {} + +interface DriverShape { + name?: string; + dialectName?: string; + config?: { client?: unknown }; +} + +/** Refused: no dialect the ceremony can write statements for. */ +const REFUSED: Array<[string, DriverShape]> = [ + ['a client class with no dialectName', { name: 'com.example.driver', config: { client: Client_PG } }], + ['a client class, as SqlDriver answers it (dialectName unknown)', { name: 'com.objectstack.driver.sql', dialectName: 'unknown', config: { client: Client_PG } }], + ['an unrecognised client string', { name: 'com.objectstack.driver.sql', config: { client: 'oracledb' } }], + ['dialectName unknown beside a recognised client string', { name: 'com.objectstack.driver.sql', dialectName: 'unknown', config: { client: 'pg' } }], + ['a driver with neither a dialectName nor a client', { name: 'com.example.driver' }], +]; + +/** Proceeds: each in-repo driver's measured shape, and the first statement its dialect's catalog read. */ +const PROCEEDS: Array<[string, DriverShape, RegExp]> = [ + ['SqlDriver on better-sqlite3', { name: 'com.objectstack.driver.sql', dialectName: 'sqlite', config: { client: 'better-sqlite3' } }, /FROM sqlite_master/], + ['SqliteWasmDriver (a client class, dialectName sqlite)', { name: 'com.objectstack.driver.sqlite-wasm', dialectName: 'sqlite', config: { client: Client_WasmSqlite } }, /FROM sqlite_master/], + ['SqlDriver on pg', { name: 'com.objectstack.driver.sql', dialectName: 'postgres', config: { client: 'pg' } }, /current_schemas\(false\)/], + ['SqlDriver on mysql2', { name: 'com.objectstack.driver.sql', dialectName: 'mysql', config: { client: 'mysql2' } }, /table_schema = DATABASE\(\)/], + ['TursoDriver', { name: 'com.objectstack.driver.turso', dialectName: 'sqlite', config: { client: 'better-sqlite3' } }, /FROM sqlite_master/], + ['a driver with no dialectName and a recognised client string', { name: 'com.example.driver', config: { client: 'postgresql' } }, /current_schemas\(false\)/], +]; + +let dir: string; +/** Every statement the driver was handed, in order. */ +let statements: string[]; + +function stackOver(shape: DriverShape) { + return { + driver: { + ...shape, + execute: async (sql: string) => { + statements.push(sql); + return []; + }, + }, + dbLabel: 'fixture.db', + kernel: { + getService: (name: string) => + name === 'objectql' + ? { registry: { getItem: () => undefined, listItems: () => [], isPackageDisabled: () => false } } + : name === 'metadata' + ? { get: async () => undefined, list: async () => [] } + : undefined, + }, + tableAbsent: () => false, + composition: { securityPlugin: { composed: false, reason: 'no-secret' } }, + shutdown: async () => {}, + }; +} + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'os-ownership-dialect-')); + statements = []; + apply.mockImplementation(async () => ({ + kind: 'organization-ownership-apply', + generatedAt: '2026-10-15T00:00:00.000Z', + steps: [], + tables: [], + complete: true, + blocking: 0, + advisory: 0, + })); +}); + +afterEach(() => { + boot.mockReset(); + apply.mockReset(); + rmSync(dir, { recursive: true, force: true }); +}); + +/** A plan file as a plan run writes it (the fields the apply reads). */ +function writePlan(): string { + const file = join(dir, 'reviewed-plan.json'); + writeFileSync(file, JSON.stringify({ + kind: 'organization-ownership-plan', + ceremony: 'ADR-0131 D10', + generatedAt: '2026-10-15T00:00:00.000Z', + database: 'fixture.db', + inventoryDigest: 'f'.repeat(64), + posture: 'single', + })); + return file; +} + +async function runJson(argv: string[]): Promise<{ payload: Record; exitCode: unknown }> { + const chunks: string[] = []; + const savedExit = process.exitCode; + vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, ...rest: unknown[]) => { + chunks.push(String(chunk)); + const cb = rest.find((a) => typeof a === 'function') as (() => void) | undefined; + if (cb) cb(); + return true; + }) as typeof process.stdout.write); + vi.spyOn(process.stderr, 'write').mockImplementation((() => true) as typeof process.stderr.write); + try { + await MigrateOrganizationOwnership.run(argv, { root: CLI_ROOT }); + } catch (error) { + if ((error as { oclif?: unknown }).oclif === undefined) throw error; + } + const exitCode = process.exitCode; + process.exitCode = savedExit; + vi.restoreAllMocks(); + return { payload: JSON.parse(chunks.join('').trim()) as Record, exitCode }; +} + +describe('an unresolved dialect is refused, naming the driver, before any statement runs', () => { + for (const [label, shape] of REFUSED) { + it(`plan: ${label}`, async () => { + boot.mockImplementation(async () => stackOver(shape)); + const target = join(dir, 'plan.json'); + const { payload, exitCode } = await runJson(['--out', target, '--json']); + expect(payload).toMatchObject({ error: 'plan_refused', reason: 'driver-unsupported' }); + expect(String(payload.detail)).toContain(shape.name!); + expect(exitCode).toBe(1); + expect(statements).toEqual([]); + expect(existsSync(target)).toBe(false); + }, CASE_TIMEOUT_MS); + + it(`--apply: ${label}`, async () => { + boot.mockImplementation(async () => stackOver(shape)); + const { payload, exitCode } = await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), '--out', join(dir, 'apply.json'), '--json']); + expect(payload).toMatchObject({ error: 'apply_refused', reason: 'driver-unsupported' }); + expect(String(payload.detail)).toContain(shape.name!); + expect(exitCode).toBe(1); + expect(statements).toEqual([]); + expect(apply).not.toHaveBeenCalled(); + }, CASE_TIMEOUT_MS); + } + + it('a client class is named, never printed as its source', async () => { + boot.mockImplementation(async () => stackOver({ name: 'com.example.driver', config: { client: Client_PG } })); + const { payload } = await runJson(['--out', join(dir, 'plan.json'), '--json']); + expect(String(payload.detail)).toContain('client class Client_PG'); + expect(String(payload.detail)).not.toContain('class Client_PG {'); + }, CASE_TIMEOUT_MS); +}); + +describe('a recognised dialect proceeds, in that dialect', () => { + for (const [label, shape, catalog] of PROCEEDS) { + it(`plan: ${label}`, async () => { + boot.mockImplementation(async () => stackOver(shape)); + const { payload } = await runJson(['--out', join(dir, 'plan.json'), '--json']); + // The driver answers no tables, so the plan stops at its catalog read, + // past the dialect, which the catalog statement it issued names. + expect(payload).toMatchObject({ error: 'plan_refused', reason: 'not-an-objectstack-database' }); + expect(statements[0]).toMatch(catalog); + }, CASE_TIMEOUT_MS); + } + + it('--apply hands the apply step the dialect it resolved, and the client string only as a record', async () => { + boot.mockImplementation(async () => stackOver({ name: 'com.objectstack.driver.sqlite-wasm', dialectName: 'sqlite', config: { client: Client_WasmSqlite } })); + await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), '--out', join(dir, 'apply.json'), '--json']); + expect(apply).toHaveBeenCalledTimes(1); + const { executor } = apply.mock.calls[0]![0] as { executor: Record }; + expect(executor.dialect).toBe('sqlite'); + expect(executor).not.toHaveProperty('client'); + }, CASE_TIMEOUT_MS); + + it('--apply on pg: the executor speaks postgres', async () => { + boot.mockImplementation(async () => stackOver({ name: 'com.objectstack.driver.sql', dialectName: 'postgres', config: { client: 'pg' } })); + await runJson(['--apply', '--backup-taken', '--plan-file', writePlan(), '--out', join(dir, 'apply.json'), '--json']); + expect(apply).toHaveBeenCalledTimes(1); + expect(apply.mock.calls[0]![0]).toMatchObject({ executor: { dialect: 'postgres', client: 'pg' } }); + }, CASE_TIMEOUT_MS); +}); diff --git a/packages/cli/src/commands/migrate/organization-ownership.ts b/packages/cli/src/commands/migrate/organization-ownership.ts index 2c19326067e..1750f1b56aa 100644 --- a/packages/cli/src/commands/migrate/organization-ownership.ts +++ b/packages/cli/src/commands/migrate/organization-ownership.ts @@ -28,7 +28,7 @@ import { OrganizationOwnershipPlanRefusal, columnListSql, columnNameOf, - dialectOf, + ceremonyDialectOf, type OrganizationOwnershipPlan, } from '../../utils/organization-ownership-plan.js'; import { @@ -87,6 +87,12 @@ async function confirm(question: string): Promise { * ObjectStack one) refuses the whole plan, naming it — exit 1 and no file. ⛔ * Never a table reported as empty. * + * The SQL dialect is read once, off the stack's driver, before either arm + * runs a statement (`ceremonyDialectOf`, the resolver `@objectstack/metadata-core` + * shares): `dialectName` first, then a recognised `config.client` string. A + * driver whose dialect that cannot name is refused, naming the driver, exit 1 + * — ⛔ never read as SQLite. + * * ## Why a subcommand, not a flag on `os migrate` * * Bare `os migrate` is the schema-drift plan (#2186) and stays exactly that. @@ -346,11 +352,15 @@ export default class MigrateOrganizationOwnership extends Command { ); return; } + // The dialect, once, before the first statement: an unresolved one is + // refused here, naming the driver, and never read as SQLite. + const dialect = ceremonyDialectOf(stack.driver); const { normalizeRows } = await import('@objectstack/metadata-protocol'); const client = (stack.driver.config as { client?: unknown } | undefined)?.client; plan = await buildOrganizationOwnershipPlan({ reader: { - client: client === undefined ? undefined : String(client), + dialect, + ...(typeof client === 'string' ? { client } : {}), query: async (sql, params) => normalizeRows(await exec(sql, params ? [...params] : [])), }, posture, @@ -517,6 +527,9 @@ export default class MigrateOrganizationOwnership extends Command { ); return; } + // The dialect, once, before the first statement: an unresolved one is + // refused here, naming the driver, and never read as SQLite. + const dialect = ceremonyDialectOf(stack.driver); if (stack.tableAbsent('sys_migration')) { await refuse( { reason: 'ledger-absent', table: 'sys_migration', detail: 'the deployment ledger the ceremony checkpoints into does not exist' }, @@ -542,7 +555,8 @@ export default class MigrateOrganizationOwnership extends Command { }); const client = (stack.driver.config as { client?: unknown } | undefined)?.client; const executor: ApplyExecutor = { - client: client === undefined ? undefined : String(client), + dialect, + ...(typeof client === 'string' ? { client } : {}), query: async (sql, params) => normalizeRows(await exec(sql, params ? [...params] : [])), execute: async (sql, params) => { await exec(sql, params ? [...params] : []); @@ -619,7 +633,7 @@ export default class MigrateOrganizationOwnership extends Command { // overwrite), then what the registry serves for the bundle member. find: async (name, locale) => { // Once column-drops has run, every row is the environment's (C7b). - const scoped = (await executor.query(columnListSql(dialectOf(executor.client) ?? 'sqlite'), ['sys_metadata'])) + const scoped = (await executor.query(columnListSql(executor.dialect), ['sys_metadata'])) .some((row) => columnNameOf(row) === 'organization_id'); const stored = await executor.query( `SELECT metadata FROM sys_metadata WHERE type = ? AND name = ? AND state = ?${scoped ? ' AND organization_id IS NULL' : ''} ` + diff --git a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts index 578ff41b425..cdbce401a72 100644 --- a/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.served-catalog.integration.test.ts @@ -136,6 +136,7 @@ async function ceremony(posture: string, db: Database.Database): Promise db.prepare(sql).all(...(params as unknown[])) as Record[], execute: async (sql, params = []) => { db.prepare(sql).run(...(params as unknown[])); }, diff --git a/packages/cli/src/utils/organization-ownership-apply.test.ts b/packages/cli/src/utils/organization-ownership-apply.test.ts index 2fb0ea98131..a768930c8d0 100644 --- a/packages/cli/src/utils/organization-ownership-apply.test.ts +++ b/packages/cli/src/utils/organization-ownership-apply.test.ts @@ -57,6 +57,7 @@ let writes: string[]; function executor(opts: { failOnceOn?: RegExp } = {}): ApplyExecutor { let failed = false; return { + dialect: 'sqlite', client: 'better-sqlite3', async query(sql, params = []) { return db.prepare(sql).all(...(params as unknown[])) as Record[]; diff --git a/packages/cli/src/utils/organization-ownership-apply.ts b/packages/cli/src/utils/organization-ownership-apply.ts index 2f84f96aa8b..8d08e2fd8d7 100644 --- a/packages/cli/src/utils/organization-ownership-apply.ts +++ b/packages/cli/src/utils/organization-ownership-apply.ts @@ -24,7 +24,6 @@ import { SEEDED_BY, TEMPLATE_OBJECT, columnNameOf, - dialectOf, predicateSql, Sql, type AttributionSink, @@ -2559,11 +2558,12 @@ function refuseUnreviewedChoices(plan: OrganizationOwnershipPlan, opts: ApplyOpt * the report. */ export async function applyOrganizationOwnership(opts: ApplyOptions): Promise { - const dialect = dialectOf(opts.executor.client); const now = opts.now ?? (() => new Date()); const run: Run = { opts, - sql: new Sql(opts.executor, dialect ?? 'sqlite'), + // The executor's dialect was resolved from the driver before this call + // (`ceremonyDialectOf`, which refuses an unresolved one): never a default. + sql: new Sql(opts.executor, opts.executor.dialect), plan: undefined as unknown as OrganizationOwnershipPlan, columns: new Map(), attributed: new Map(), diff --git a/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts b/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts index 5f62929516d..16dc039b7ac 100644 --- a/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-host-supplement.integration.test.ts @@ -75,7 +75,7 @@ function open(name: string, ddl: string): { db: Database.Database; file: string; return { db, file, - reader: { client: 'better-sqlite3', query: async (sql, params = []) => db.prepare(sql).all(...(params as unknown[])) as Record[] }, + reader: { dialect: 'sqlite', client: 'better-sqlite3', query: async (sql, params = []) => db.prepare(sql).all(...(params as unknown[])) as Record[] }, }; } diff --git a/packages/cli/src/utils/organization-ownership-plan.integration.test.ts b/packages/cli/src/utils/organization-ownership-plan.integration.test.ts index 906fce74fd0..5cc5b1e5024 100644 --- a/packages/cli/src/utils/organization-ownership-plan.integration.test.ts +++ b/packages/cli/src/utils/organization-ownership-plan.integration.test.ts @@ -15,6 +15,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { buildOrganizationOwnershipPlan, + ceremonyDialectOf, OrganizationOwnershipPlanRefusal, type OrganizationOwnershipPlan, type PlanReader, @@ -28,9 +29,10 @@ let db: Database.Database; /** Every statement the plan issued, in order. */ let statements: string[]; -function reader(opts: { client?: string; failOn?: RegExp } = {}): PlanReader { +function reader(opts: { failOn?: RegExp } = {}): PlanReader { return { - client: opts.client ?? 'better-sqlite3', + dialect: 'sqlite', + client: 'better-sqlite3', async query(sql, params = []) { statements.push(sql); if (opts.failOn?.test(sql)) throw new Error('SQLITE_IOERR: disk I/O error'); @@ -249,9 +251,10 @@ describe('ADR-0131 D10 preflight — it refuses a table it cannot enumerate, nam expect(r.message).toContain('managed_by'); }); - it('a driver with no catalog statement', async () => { - const r = await refusal(plan('isolated', reader({ client: 'oracledb' }))); + it('a driver with no catalog statement: its dialect is refused before the plan issues one', async () => { + const r = await refusal((async () => plan('isolated', { ...reader(), dialect: ceremonyDialectOf({ config: { client: 'oracledb' } }) }))()); expect(r.reason).toBe('driver-unsupported'); + expect(statements).toEqual([]); }); it('a database that is not an ObjectStack one (the wrong --database-url)', async () => { diff --git a/packages/cli/src/utils/organization-ownership-plan.ts b/packages/cli/src/utils/organization-ownership-plan.ts index 75d6ad71271..25cf4a26b48 100644 --- a/packages/cli/src/utils/organization-ownership-plan.ts +++ b/packages/cli/src/utils/organization-ownership-plan.ts @@ -2,6 +2,7 @@ import { createHash } from 'node:crypto'; import { emailTemplateDefinitionFromRow } from '@objectstack/core'; +import { dialectOfDriver } from '@objectstack/metadata-core'; import { hasPlatformObjectPrefix } from '@objectstack/spec/system'; import { APPLICATION_OBJECT_FATE, @@ -54,8 +55,14 @@ export type PlanDialect = 'sqlite' | 'postgres' | 'mysql'; /** The one seam the plan reads through. Rows already normalized across dialects. */ export interface PlanReader { - /** The knex client spelling of the connected driver. */ - client: string | undefined; + /** + * The dialect every statement is written in: resolved ONCE from the stack's + * driver by {@link ceremonyDialectOf}, before the first statement, and never + * re-derived or defaulted here. + */ + dialect: PlanDialect; + /** The knex client spelling, when the driver's `config.client` is a string: recorded in the plan file, never read for the dialect. */ + client?: string; query(sql: string, params?: readonly unknown[]): Promise[]>; } @@ -413,12 +420,42 @@ const DEFAULT_ORGANIZATION_SLUG = 'default'; /** A name the plan may interpolate as a quoted identifier. */ export const SAFE_IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_]*$/; -export function dialectOf(client: string | undefined): PlanDialect | null { - const c = String(client ?? '').toLowerCase(); - if (['sqlite3', 'sqlite', 'better-sqlite3'].includes(c)) return 'sqlite'; - if (['postgres', 'pg', 'postgresql', 'pgnative'].includes(c)) return 'postgres'; - if (['mysql', 'mysql2'].includes(c)) return 'mysql'; - return null; +/** + * The ceremony's SQL dialect, read off the stack's driver once, before the + * first statement, or a `driver-unsupported` refusal that names the driver. + * + * The one resolver `@objectstack/metadata-core`'s legacy-row reader uses + * (`dialectOfDriver`): the driver's `dialectName` first (the only answer that + * names `SqliteWasmDriver`, whose knex `client` is a dialect CLASS), then a + * recognised `config.client` string. Everything else is refused, never read + * as SQLite: a client class with no `dialectName`, an unrecognised spelling, + * `dialectName: 'unknown'`, or a driver with no SQL dialect at all. A + * statement written for the wrong dialect either fails mid-ceremony or runs + * against the database with the wrong quoting. + */ +export function ceremonyDialectOf(driver: unknown): PlanDialect { + const named = driver as { dialectName?: unknown; config?: { client?: unknown } } | null | undefined; + const dialect = dialectOfDriver(named); + if (dialect === 'sqlite' || dialect === 'postgres' || dialect === 'mysql') return dialect; + throw new OrganizationOwnershipPlanRefusal( + 'driver-unsupported', + `The connected driver (${describeDriver(driver)}) names no SQL dialect the ceremony can write statements for, so ` + + 'none was run. The ceremony supports SQLite, PostgreSQL and MySQL, read from the driver\'s dialectName, or from ' + + 'its config.client when that is a recognised client string; a client class is read only through dialectName.', + ); +} + +/** The driver, as the refusal names it: its name, its `dialectName`, its client. A client class is named, never printed. */ +function describeDriver(driver: unknown): string { + const d = driver as { name?: unknown; dialectName?: unknown; config?: { client?: unknown } } | null | undefined; + const parts = [typeof d?.name === 'string' && d.name ? d.name : 'unnamed driver']; + if (typeof d?.dialectName === 'string') parts.push(`dialectName ${JSON.stringify(d.dialectName)}`); + const client = d?.config?.client; + if (typeof client === 'string') parts.push(`client ${JSON.stringify(client)}`); + else if (typeof client === 'function') parts.push(`client class ${client.name || '(anonymous)'}`); + else if (client === undefined || client === null) parts.push('no client'); + else parts.push(`client of type ${typeof client}`); + return parts.join(', '); } /** sha256 over the inventory, so a plan names the table it was computed from. */ @@ -568,12 +605,12 @@ interface Catalog { } async function readCatalog(sql: Sql, reader: PlanReader, inventoried: ReadonlySet): Promise { - const listSql = physicalTableListSql(reader.client); + const listSql = physicalTableListSql(sql.dialect); if (!listSql) { throw new OrganizationOwnershipPlanRefusal( 'driver-unsupported', - `The connected driver (${reader.client ?? 'unknown client'}) has no catalog statement this plan can run, so ` + - 'no table can be enumerated. The ceremony supports SQLite, PostgreSQL and MySQL.', + `The ${sql.dialect} dialect has no catalog statement this plan can run, so no table can be enumerated. ` + + 'The ceremony supports SQLite, PostgreSQL and MySQL.', ); } let rows: Record[]; @@ -1075,15 +1112,9 @@ export async function buildOrganizationOwnershipPlan(opts: { onAttribution?: AttributionSink; }): Promise { const inventory = opts.inventory ?? ORGANIZATION_OWNERSHIP_INVENTORY; - const dialect = dialectOf(opts.reader.client); - if (!dialect) { - throw new OrganizationOwnershipPlanRefusal( - 'driver-unsupported', - `The connected driver (${opts.reader.client ?? 'unknown client'}) has no catalog statement this plan can run, so ` + - 'no table can be enumerated. The ceremony supports SQLite, PostgreSQL and MySQL.', - ); - } - const sql = new Sql(opts.reader, dialect); + // The reader's dialect was resolved from the driver before this call + // (`ceremonyDialectOf`, which refuses an unresolved one). + const sql = new Sql(opts.reader, opts.reader.dialect); const byObject = new Map(inventory.map((e) => [e.object, e])); const catalog = await readCatalog(sql, opts.reader, new Set(byObject.keys())); diff --git a/packages/metadata-core/src/legacy-organization-row.test.ts b/packages/metadata-core/src/legacy-organization-row.test.ts index 99ab2b31764..56655f25196 100644 --- a/packages/metadata-core/src/legacy-organization-row.test.ts +++ b/packages/metadata-core/src/legacy-organization-row.test.ts @@ -9,7 +9,7 @@ */ import { describe, expect, it } from 'vitest'; -import { LegacyOrganizationRowsUnreadable, readLegacyOrganizationRows } from './legacy-organization-row.js'; +import { dialectOfDriver, LegacyOrganizationRowsUnreadable, readLegacyOrganizationRows } from './legacy-organization-row.js'; /** A SQL driver double: the column list and the legacy select, by statement. */ function sqlDriver(opts: { @@ -151,3 +151,19 @@ describe('readLegacyOrganizationRows — the presence check is the driver\'s cat await expect(readLegacyOrganizationRows(catalogDriver([]))).rejects.toThrow(/column list came back empty/); }); }); + +describe('dialectOfDriver — the one driver-dialect resolver, shared with the organization-ownership ceremony', () => { + it.each([ + ['dialectName first: SqliteWasmDriver\'s client is a dialect class', { dialectName: 'sqlite', config: { client: class Client_WasmSqlite {} } }, 'sqlite'], + ['dialectName postgres', { dialectName: 'postgres', config: { client: 'pg' } }, 'postgres'], + ['dialectName mysql', { dialectName: 'mysql', config: { client: 'mysql2' } }, 'mysql'], + ['no dialectName: a recognised client string', { config: { client: 'postgresql' } }, 'postgres'], + ['dialectName is the whole answer: unknown beside a recognised client', { dialectName: 'unknown', config: { client: 'pg' } }, 'unresolved'], + ['a client class with no dialectName', { config: { client: class Client_PG {} } }, 'unresolved'], + ['an unrecognised client string', { config: { client: 'oracledb' } }, 'unresolved'], + ['no dialectName and no client (memory, MongoDB)', {}, 'none'], + ['no driver', null, 'none'], + ])('%s', (_label, driver, expected) => { + expect(dialectOfDriver(driver)).toBe(expected); + }); +}); diff --git a/packages/metadata-core/src/legacy-organization-row.ts b/packages/metadata-core/src/legacy-organization-row.ts index 3034a6f17b7..17c876ee186 100644 --- a/packages/metadata-core/src/legacy-organization-row.ts +++ b/packages/metadata-core/src/legacy-organization-row.ts @@ -102,8 +102,18 @@ function dialectOfName(name: string): Dialect | null { * The driver's dialect, `'none'` when it is not a SQL driver at all, or * `'unresolved'` when it is one whose dialect this read cannot name. * `dialectName` first: `SqliteWasmDriver`'s `config.client` is a class. + * + * Exported as the ONE resolver for every raw-SQL reader that must pick a + * dialect from a driver it was handed: this read, and the + * organization-ownership ceremony (`os migrate organization-ownership`), which + * refuses on anything but the three dialects instead of guessing one. A + * `dialectName` string is the whole answer, recognised or not; a `config.client` + * is read only when there is no `dialectName`, and only as a string — a client + * CLASS names no spelling, so it is `'unresolved'`, never a default. */ -function dialectOfDriver(driver: RawDriver | null | undefined): Dialect | 'none' | 'unresolved' { +export function dialectOfDriver( + driver: { dialectName?: unknown; config?: { client?: unknown } } | null | undefined, +): Dialect | 'none' | 'unresolved' { if (!driver) return 'none'; if (typeof driver.dialectName === 'string') return dialectOfName(driver.dialectName) ?? 'unresolved'; const client = driver.config?.client;