diff --git a/docs/database-backends.md b/docs/database-backends.md index fbf5ff5a..0fdd4753 100644 --- a/docs/database-backends.md +++ b/docs/database-backends.md @@ -189,6 +189,19 @@ Tested against PostgreSQL 16 and MySQL 8. **MariaDB is not a substitute for MySQL when testing** — it accepts DDL that MySQL 8 rejects, which has hidden a real defect here more than once. +### What neither of them covers + +Both harnesses build a `DatabaseContext` of their own, so neither runs +`createCurrentRepositoryContext()` — the one the application actually uses. +That gap hid a hardcoded `dialect: "sqlite"` in it: every engine reported +itself as SQLite at runtime while all three test passes stayed green, which on +MySQL meant `upsert` reached for `onConflictDoUpdate` and died with a +TypeError on the first write. + +Anything the factory decides from the dialect needs its own test against the +factory. Asserting it through a hand-built context proves nothing about what +runs in production. + ## Known limits - The desktop app always uses SQLite. It embeds its own backend and cannot ship diff --git a/src/backend/database/repositories/factory.ts b/src/backend/database/repositories/factory.ts index 12eaee51..9600f439 100644 --- a/src/backend/database/repositories/factory.ts +++ b/src/backend/database/repositories/factory.ts @@ -47,9 +47,21 @@ import { UserRepository } from "./user-repository.js"; import { VaultProfileRepository } from "./vault-profile-repository.js"; import { VaultTokenRepository } from "./vault-token-repository.js"; +/** + * The context every repository runs against. + * + * The dialect has to be resolved, not assumed: it is what `returning.ts` reads + * to decide whether it can ask for RETURNING, and whether an upsert spells + * itself `onConflictDoUpdate` or `onDuplicateKeyUpdate`. Reporting "sqlite" + * while connected to MySQL makes the second of those a TypeError on the first + * write. + * + * Both cross-dialect harnesses build a DatabaseContext themselves, so neither + * exercises this function — see tests/database/repositories/factory-context. + */ export function createCurrentRepositoryContext(): DatabaseContext { return { - dialect: "sqlite", + dialect: resolveDatabaseDialect(), drizzle: getDb(), }; } diff --git a/src/backend/tests/database/repositories/factory-context.test.ts b/src/backend/tests/database/repositories/factory-context.test.ts new file mode 100644 index 00000000..5f9fe8d8 --- /dev/null +++ b/src/backend/tests/database/repositories/factory-context.test.ts @@ -0,0 +1,66 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { DATABASE_DIALECT_ENV } from "../../../database/db/dialect.js"; + +// getDb() throws unless a database was initialized; the context's handle is +// not what this file is about. +vi.mock("../../../database/db/index.js", () => ({ + getDb: () => ({}), + getSqlite: () => ({}), + DatabaseSaveTrigger: { forceSave: vi.fn() }, +})); + +const { createCurrentRepositoryContext, createCurrentRepositoryWriteHook } = + await import("../../../database/repositories/factory.js"); + +// Neither cross-dialect harness reaches this function: both +// tests/database/repositories/test-support.ts and scripts/verify-dialects.mjs +// construct a DatabaseContext of their own. That is why the production path +// could report "sqlite" while connected to MySQL with CI green on all three +// engines, and why this asserts on the real factory rather than a fixture. +describe("createCurrentRepositoryContext", () => { + const saved = process.env[DATABASE_DIALECT_ENV]; + + beforeEach(() => { + delete process.env[DATABASE_DIALECT_ENV]; + }); + + afterEach(() => { + if (saved === undefined) delete process.env[DATABASE_DIALECT_ENV]; + else process.env[DATABASE_DIALECT_ENV] = saved; + }); + + it("defaults to sqlite when nothing is configured", () => { + expect(createCurrentRepositoryContext().dialect).toBe("sqlite"); + }); + + it("reports the configured dialect", () => { + for (const dialect of ["sqlite", "postgres", "mysql"]) { + process.env[DATABASE_DIALECT_ENV] = dialect; + expect(createCurrentRepositoryContext().dialect).toBe(dialect); + } + }); + + it("rejects an unsupported dialect rather than falling back to sqlite", () => { + process.env[DATABASE_DIALECT_ENV] = "oracle"; + expect(() => createCurrentRepositoryContext()).toThrow(/oracle/); + }); +}); + +describe("createCurrentRepositoryWriteHook", () => { + const saved = process.env[DATABASE_DIALECT_ENV]; + + afterEach(() => { + if (saved === undefined) delete process.env[DATABASE_DIALECT_ENV]; + else process.env[DATABASE_DIALECT_ENV] = saved; + }); + + it("installs a persist hook only for sqlite", () => { + process.env[DATABASE_DIALECT_ENV] = "sqlite"; + expect(createCurrentRepositoryWriteHook("test")).toBeTypeOf("function"); + + for (const dialect of ["postgres", "mysql"]) { + process.env[DATABASE_DIALECT_ENV] = dialect; + expect(createCurrentRepositoryWriteHook("test")).toBeUndefined(); + } + }); +});