diff --git a/src/store/postgres.ts b/src/store/postgres.ts index ea47ca0..4ba0ca7 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -335,11 +335,10 @@ export class PostgresStore implements HeimdallStore { AND a.atttypid = to_regtype(required.type_name) ) UNION ALL - SELECT 'missing' FROM unnest($5::text[], $6::text[], $7::text[], $8::text[]) AS required(table_name, columns, index_name, predicate) + SELECT 'missing' FROM unnest($5::text[], $6::text[], $7::text[]) AS required(table_name, columns, predicate) WHERE NOT EXISTS ( SELECT 1 FROM pg_index i WHERE i.indrelid = to_regclass(required.table_name) - AND (required.index_name = '' OR i.indexrelid = to_regclass(required.index_name)) AND i.indisunique AND i.indimmediate AND coalesce(regexp_replace(lower(pg_get_expr(i.indpred, i.indrelid)), '[()[:space:]]', '', 'g'), '') = required.predicate @@ -352,7 +351,7 @@ export class PostgresStore implements HeimdallStore { ) = required.columns ) UNION ALL - SELECT 'privilege' FROM unnest($9::text[], $10::text[]) AS required(table_name, privilege) + SELECT 'privilege' FROM unnest($8::text[], $9::text[]) AS required(table_name, privilege) WHERE to_regclass(required.table_name) IS NOT NULL AND NOT has_table_privilege(to_regclass(required.table_name), required.privilege) `, @@ -363,7 +362,6 @@ export class PostgresStore implements HeimdallStore { REQUIRED_COLUMNS.map((required) => required.type), REQUIRED_KEYS.map((required) => required.table), REQUIRED_KEYS.map((required) => required.columns), - REQUIRED_KEYS.map((required) => required.index), REQUIRED_KEYS.map((required) => required.predicate), privileges.map((required) => required.table), privileges.map((required) => required.privilege), diff --git a/src/store/schema.ts b/src/store/schema.ts index f385031..91db7fc 100644 --- a/src/store/schema.ts +++ b/src/store/schema.ts @@ -223,26 +223,24 @@ export const REQUIRED_COLUMNS = [ /** * Every unique key the schema makes: the primary keys and unique constraints - * of the created tables, and each unique index it creates by name with that - * index's predicate. The store relies on each as an immediate unique index - * whose key columns are exactly these, with exactly this predicate: ON + * of the created tables, and each unique index it creates, with that index's + * predicate. The store relies on each as an immediate unique index on its + * table whose key columns are exactly these, with exactly this predicate: ON * CONFLICT refuses a deferrable arbiter and cannot infer a partial or wider * one, and the partial attempt index is what keeps one unconsumed completion - * per attempt (R21.1). `index` is empty for a constraint, which may be - * satisfied by any such index on its table. + * per attempt (R21.1). Any index with that shape serves, whatever its name. */ export const REQUIRED_KEYS = [ ...createdTables.flatMap(({ table, lines }) => lines.flatMap((line) => { const columns = /^(\w+) .*\bPRIMARY KEY\b/.exec(line)?.[1] ?? keyColumns(line, "PRIMARY KEY") ?? keyColumns(line, "UNIQUE"); - return columns ? [{ table, columns, index: "", predicate: "" }] : []; + return columns ? [{ table, columns, predicate: "" }] : []; }) ), - ...[...schemaText.matchAll(/^CREATE UNIQUE INDEX IF NOT EXISTS (\w+)\s+ON (\w+)\(([^)]*)\)(?:\s+WHERE ([^;]+))?;/gm)].map((match) => ({ - table: match[2]!, - columns: columnList(match[3]!), - index: match[1]!, - predicate: comparablePredicate(match[4] ?? ""), + ...[...schemaText.matchAll(/^CREATE UNIQUE INDEX IF NOT EXISTS \w+\s+ON (\w+)\(([^)]*)\)(?:\s+WHERE ([^;]+))?;/gm)].map((match) => ({ + table: match[1]!, + columns: columnList(match[2]!), + predicate: comparablePredicate(match[3] ?? ""), })), ]; diff --git a/tests/store.test.ts b/tests/store.test.ts index ffec529..25500c7 100644 --- a/tests/store.test.ts +++ b/tests/store.test.ts @@ -234,13 +234,12 @@ describe("PostgresStore.checkSchema", () => { expect(REQUIRED_KEYS).toHaveLength(12); expect(REQUIRED_KEYS).toEqual( expect.arrayContaining([ - { table: "linked_identities", columns: "provider,provider_user_id", index: "", predicate: "" }, - { table: "private_command_receipts", columns: "app_slug,idempotency_key", index: "", predicate: "" }, - { table: "entitlement_snapshots", columns: "account_id,provider,scope", index: "", predicate: "" }, + { table: "linked_identities", columns: "provider,provider_user_id", predicate: "" }, + { table: "private_command_receipts", columns: "app_slug,idempotency_key", predicate: "" }, + { table: "entitlement_snapshots", columns: "account_id,provider,scope", predicate: "" }, { table: "auth_completions", columns: "app_slug,attempt_id", - index: "auth_completions_attempt_unconsumed_unique_idx", predicate: comparablePredicate("attempt_id IS NOT NULL AND consumed_at IS NULL"), }, ]) @@ -360,6 +359,17 @@ describe.skipIf(!adminUrl)("startup against a live Postgres (HEIMDALL_TEST_PG_AD const admin = adminUrl ? new Pool({ connectionString: adminUrl, max: 2 }) : undefined; const cleanup: Array<() => Promise> = []; + /** + * A pool the test itself uses. The forced drops end its connections, + * possibly while pg is still closing them after end() resolved; that is + * cleanup, not a finding, so it is not left to become an unhandled error. + */ + function fixturePool(connectionString: string): Pool { + const pool = new Pool({ connectionString, max: 2 }); + pool.on("error", () => undefined); + return pool; + } + function urlFor(database: string, user?: string, password?: string): string { const url = new URL(adminUrl!); url.pathname = `/${database}`; @@ -370,7 +380,7 @@ describe.skipIf(!adminUrl)("startup against a live Postgres (HEIMDALL_TEST_PG_AD beforeAll(async () => { await admin!.query(`CREATE DATABASE ${template}`); - const pool = new Pool({ connectionString: urlFor(template) }); + const pool = fixturePool(urlFor(template)); await new PostgresStore(pool).ensureSchema(); await pool.end(); await admin!.query(`CREATE ROLE ${role} LOGIN PASSWORD '${rolePassword}'`); @@ -390,10 +400,7 @@ describe.skipIf(!adminUrl)("startup against a live Postgres (HEIMDALL_TEST_PG_AD async function appliedDatabase(): Promise<{ name: string; owner: Pool }> { const name = `heimdall_check_${randomUUID().replace(/-/g, "")}`; await admin!.query(`CREATE DATABASE ${name} TEMPLATE ${template}`); - const owner = new Pool({ connectionString: urlFor(name), max: 2 }); - // The forced drop ends this fixture pool's connections, possibly while - // they are still closing; that is cleanup, not a finding. - owner.on("error", () => undefined); + const owner = fixturePool(urlFor(name)); cleanup.push(async () => { await owner.end(); await admin!.query(`DROP DATABASE IF EXISTS ${name} WITH (FORCE)`); @@ -450,7 +457,7 @@ describe.skipIf(!adminUrl)("startup against a live Postgres (HEIMDALL_TEST_PG_AD await grant(owner, STORE_TABLE_PRIVILEGES); expect(await start(urlFor(name, role, rolePassword), false)).toBe("started"); - const pool = new Pool({ connectionString: urlFor(name, role, rolePassword) }); + const pool = fixturePool(urlFor(name, role, rolePassword)); cleanup.push(async () => { await pool.end(); }); @@ -474,7 +481,7 @@ describe.skipIf(!adminUrl)("startup against a live Postgres (HEIMDALL_TEST_PG_AD expect(message, `${table} without ${privilege}`).toBe(refusal("SCHEMA_PRIVILEGES")); expect(message).not.toContain("CANARY"); - const pool = new Pool({ connectionString: urlFor(name, role, rolePassword) }); + const pool = fixturePool(urlFor(name, role, rolePassword)); const failure = await exerciseStore(new PostgresStore(pool)).then( () => undefined, (error: unknown) => (error as { code?: string }).code