diff --git a/src/api.test.ts b/src/api.test.ts index e9b9ab1..ca72bb2 100644 --- a/src/api.test.ts +++ b/src/api.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { Database } from "bun:sqlite"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -59,6 +60,36 @@ describe("api", () => { cleanup(); }); + test("returns 500 when stored scene payload is invalid", () => { + const dir = mkdtempSync(join(tmpdir(), "excali-api-")); + const databasePath = join(dir, "test.sqlite"); + const db = new Database(databasePath, { create: true, strict: true }); + db.exec(` + CREATE TABLE drawings ( + id TEXT PRIMARY KEY, + title TEXT NOT NULL, + data TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + revision INTEGER NOT NULL DEFAULT 0 + ); + + INSERT INTO drawings (id, title, data, revision) + VALUES ('broken', 'Broken', '{"appState":{},"files":{}}', 0); + `); + db.close(false); + + const store = createDrawingStore(databasePath); + const api = createApi(store); + const response = api.getDrawing( + Object.assign(new Request("http://local/api/drawings/broken"), { params: { id: "broken" } }), + ); + + expect(response.status).toBe(500); + store.close(); + rmSync(dir, { recursive: true, force: true }); + }); + test("updates a drawing scene", async () => { const { api, cleanup } = withApi(); const created = await api.createDrawing().json(); diff --git a/src/api.ts b/src/api.ts index 1efe1ca..f1508b6 100644 --- a/src/api.ts +++ b/src/api.ts @@ -68,17 +68,21 @@ export function createApi(store: DrawingStore) { }, getDrawing(request: RouteRequest) { - const id = request.params?.id; - const drawing = id ? store.getDrawing(id) : null; + try { + const id = request.params?.id; + const drawing = id ? store.getDrawing(id) : null; - if (!drawing) { - return json({ ok: false, error: "Drawing not found" }, { status: 404 }); + if (!drawing) { + return json({ ok: false, error: "Drawing not found" }, { status: 404 }); + } + + return json({ + ...toMeta(drawing), + ...drawing.scene, + }); + } catch (error) { + return errorResponse(error); } - - return json({ - ...toMeta(drawing), - ...drawing.scene, - }); }, async updateDrawing(request: RouteRequest) { diff --git a/src/db.test.ts b/src/db.test.ts index 52f3951..52523dc 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -83,7 +83,7 @@ describe("drawing store", () => { expect(store.getDrawing(created.id)?.scene.elements).toEqual([{ id: "first" }]); }); - test("migrates existing databases to include revision", () => { + test("fails to open databases missing the revision column", () => { const dir = mkdtempSync(join(tmpdir(), "excali-")); cleanup.push(dir); const databasePath = join(dir, "test.sqlite"); @@ -102,10 +102,31 @@ describe("drawing store", () => { `); db.close(false); - const store = createDrawingStore(databasePath); - expect(store.getDrawing("legacy")?.revision).toBe(0); + expect(() => createDrawingStore(databasePath)).toThrow(/revision/i); + }); - const updated = store.updateDrawing("legacy", { title: "Migrated" }); - expect(updated.ok ? updated.drawing.revision : null).toBe(1); + test("throws when reading drawings with invalid stored scene payloads", () => { + const dir = mkdtempSync(join(tmpdir(), "excali-")); + cleanup.push(dir); + const databasePath = join(dir, "test.sqlite"); + const db = new Database(databasePath, { create: true, strict: true }); + db.exec(` + CREATE TABLE drawings ( + id TEXT PRIMARY KEY, + title TEXT NOT NULL, + data TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + revision INTEGER NOT NULL DEFAULT 0 + ); + + INSERT INTO drawings (id, title, data, revision) + VALUES ('broken', 'Broken', '{"appState":{},"files":{}}', 0); + `); + db.close(false); + + const store = createDrawingStore(databasePath); + expect(() => store.getDrawing("broken")).toThrow(/Invalid stored scene/); + store.close(); }); }); diff --git a/src/db.ts b/src/db.ts index af9a9e2..dcbf882 100644 --- a/src/db.ts +++ b/src/db.ts @@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto"; import { mkdirSync } from "node:fs"; import { dirname } from "node:path"; import { DEFAULT_TITLE, type DrawingMeta, type DrawingRecord, type ScenePayload } from "./shared"; -import { coerceStoredScene, emptyScene, normalizeTitle } from "./scene"; +import { emptyScene, normalizeScene, normalizeTitle } from "./scene"; type DrawingRow = { id: string; @@ -59,9 +59,17 @@ function readRow(row: DrawingRow | null | undefined): DrawingRecord | null { return null; } + let scene: ScenePayload; + try { + scene = normalizeScene(JSON.parse(row.data)); + } catch (error) { + const message = error instanceof Error ? error.message : "Unknown scene parsing error"; + throw new Error(`Invalid stored scene for drawing ${row.id}: ${message}`); + } + return { ...readMeta(row)!, - scene: coerceStoredScene(JSON.parse(row.data)), + scene, }; } @@ -85,11 +93,6 @@ export function createDrawingStore(databasePath = resolveDatabasePath()) { ON drawings(updated_at DESC, created_at DESC); `); - const columns = db.query("PRAGMA table_info(drawings)").all() as Array<{ name: string }>; - if (!columns.some((column) => column.name === "revision")) { - db.exec("ALTER TABLE drawings ADD COLUMN revision INTEGER NOT NULL DEFAULT 0"); - } - const listQuery = db.query(` SELECT id, diff --git a/src/scene.ts b/src/scene.ts index 32435e9..fb49566 100644 --- a/src/scene.ts +++ b/src/scene.ts @@ -87,11 +87,3 @@ export function parseJsonBody(text: string): T { export function parseSceneText(text: string): ScenePayload { return normalizeScene(parseJsonBody(text)); } - -export function coerceStoredScene(input: unknown): ScenePayload { - try { - return normalizeScene(input); - } catch { - return emptyScene(); - } -}