diff --git a/src/api.test.ts b/src/api.test.ts index e9b9ab1..2f12b6e 100644 --- a/src/api.test.ts +++ b/src/api.test.ts @@ -84,6 +84,46 @@ describe("api", () => { cleanup(); }); + test("does not increment revisions for equivalent normalized scenes", async () => { + const { api, cleanup } = withApi(); + const created = await api.createDrawing().json(); + + await api.updateDrawing( + Object.assign( + new Request(`http://local/api/drawings/${created.id}`, { + method: "PUT", + body: JSON.stringify({ + elements: [{ id: "shape" }], + appState: { gridSize: 20 }, + files: {}, + expectedRevision: 0, + }), + }), + { params: { id: created.id } }, + ), + ); + + const response = await api.updateDrawing( + Object.assign( + new Request(`http://local/api/drawings/${created.id}`, { + method: "PUT", + body: JSON.stringify({ + elements: [{ id: "shape" }], + appState: { gridSize: 20, selectedElementIds: { shape: true } }, + files: {}, + expectedRevision: 0, + }), + }), + { params: { id: created.id } }, + ), + ); + + expect(response.status).toBe(200); + const body = await response.json(); + expect(body.drawing.revision).toBe(1); + cleanup(); + }); + test("updates scene and title with a matching expected revision", async () => { const { api, cleanup } = withApi(); const created = await api.createDrawing().json(); diff --git a/src/db.test.ts b/src/db.test.ts index 409c938..c67a1a5 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -82,4 +82,57 @@ describe("drawing store", () => { expect(store.getDrawing(created.id)?.scene.elements).toEqual([{ id: "first" }]); }); + test("keeps the revision unchanged when the scene is already stored", () => { + const store = createTempStore(); + const created = store.createDrawing(); + const scene = { + elements: [{ id: "same" }], + appState: {}, + files: {}, + }; + + const first = store.updateDrawing(created.id, { scene }); + expect(first.ok ? first.drawing.revision : null).toBe(1); + + const repeated = store.updateDrawing(created.id, { scene }); + expect(repeated.ok).toBe(true); + expect(repeated.ok ? repeated.drawing.revision : null).toBe(1); + expect(store.getDrawing(created.id)?.revision).toBe(1); + }); + + test("accepts stale expected revisions when the scene is already stored", () => { + const store = createTempStore(); + const created = store.createDrawing(); + const scene = { + elements: [{ id: "same" }], + appState: {}, + files: {}, + }; + + const first = store.updateDrawing(created.id, { expectedRevision: 0, scene }); + expect(first.ok ? first.drawing.revision : null).toBe(1); + + const repeated = store.updateDrawing(created.id, { expectedRevision: 0, scene }); + expect(repeated.ok).toBe(true); + expect(repeated.ok ? repeated.drawing.revision : null).toBe(1); + expect(store.getDrawing(created.id)?.scene.elements).toEqual([{ id: "same" }]); + }); + + test("increments the revision when the title changes but the scene does not", () => { + const store = createTempStore(); + const created = store.createDrawing(); + const scene = { + elements: [{ id: "same" }], + appState: {}, + files: {}, + }; + + const first = store.updateDrawing(created.id, { expectedRevision: 0, scene }); + expect(first.ok ? first.drawing.revision : null).toBe(1); + + const renamed = store.updateDrawing(created.id, { expectedRevision: 1, title: "Renamed", scene }); + expect(renamed.ok).toBe(true); + expect(renamed.ok ? renamed.drawing.title : null).toBe("Renamed"); + expect(renamed.ok ? renamed.drawing.revision : null).toBe(2); + }); }); diff --git a/src/db.ts b/src/db.ts index dcbf882..adf2601 100644 --- a/src/db.ts +++ b/src/db.ts @@ -190,32 +190,46 @@ export function createDrawingStore(databasePath = resolveDatabasePath()) { ): DrawingUpdateResult { const hasScene = update.scene !== undefined; const hasTitle = update.title !== undefined; + const existingRow = getQuery.get(id) as DrawingRow | null | undefined; + if (!existingRow) { + return { ok: false, reason: "not_found" }; + } + + const existing = readRow(existingRow)!; if (!hasScene && !hasTitle) { - const existing = getDrawing(id); - return existing ? { ok: true, drawing: existing } : { ok: false, reason: "not_found" }; + return { ok: true, drawing: existing }; + } + + const nextTitle = hasTitle ? normalizeTitle(update.title) : existingRow.title; + const nextScene = hasScene ? JSON.stringify(update.scene) : existingRow.data; + const titleChanged = nextTitle !== existingRow.title; + const sceneChanged = nextScene !== existingRow.data; + + if (!titleChanged && !sceneChanged) { + return { ok: true, drawing: existing }; + } + + if (update.expectedRevision !== undefined && update.expectedRevision !== existingRow.revision) { + return { ok: false, reason: "conflict", drawing: existing }; } let changes: number; - if (hasScene && hasTitle) { - const title = normalizeTitle(update.title); - const scene = JSON.stringify(update.scene); + if (sceneChanged && titleChanged) { changes = update.expectedRevision === undefined - ? Number(updateBothQuery.run(id, title, scene).changes) - : Number(updateBothExpectedQuery.run(id, title, scene, update.expectedRevision).changes); - } else if (hasScene) { - const scene = JSON.stringify(update.scene); + ? Number(updateBothQuery.run(id, nextTitle, nextScene).changes) + : Number(updateBothExpectedQuery.run(id, nextTitle, nextScene, update.expectedRevision).changes); + } else if (sceneChanged) { changes = update.expectedRevision === undefined - ? Number(updateSceneQuery.run(id, scene).changes) - : Number(updateSceneExpectedQuery.run(id, scene, update.expectedRevision).changes); + ? Number(updateSceneQuery.run(id, nextScene).changes) + : Number(updateSceneExpectedQuery.run(id, nextScene, update.expectedRevision).changes); } else { - const title = normalizeTitle(update.title); changes = update.expectedRevision === undefined - ? Number(updateTitleQuery.run(id, title).changes) - : Number(updateTitleExpectedQuery.run(id, title, update.expectedRevision).changes); + ? Number(updateTitleQuery.run(id, nextTitle).changes) + : Number(updateTitleExpectedQuery.run(id, nextTitle, update.expectedRevision).changes); } const drawing = getDrawing(id); diff --git a/src/mcp-server.test.ts b/src/mcp-server.test.ts index 356fda9..5b087b4 100644 --- a/src/mcp-server.test.ts +++ b/src/mcp-server.test.ts @@ -95,6 +95,18 @@ describe("mcp tool handlers", () => { expect(store.getDrawing(result.id)?.scene).toEqual(nextScene); }); + test("replace_drawing keeps the revision when the scene is already stored", () => { + const { store, tools } = createTempTools(); + const scene = testScene(); + const result = tools.create_drawing({ title: "replace noop", scene }); + + const replaced = tools.replace_drawing({ id: result.id, scene }); + + expect(replaced.revision).toBe(1); + expect(store.getDrawing(result.id)?.revision).toBe(1); + expect(store.getDrawing(result.id)?.scene).toEqual(scene); + }); + test("patch_drawing replaces an element property and removes an element", () => { const { store, tools } = createTempTools(); const result = tools.create_drawing({ title: "patch", scene: testScene() });