fix: skip no-op scene revision bumps
This commit is contained in:
@@ -84,6 +84,46 @@ describe("api", () => {
|
|||||||
cleanup();
|
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 () => {
|
test("updates scene and title with a matching expected revision", async () => {
|
||||||
const { api, cleanup } = withApi();
|
const { api, cleanup } = withApi();
|
||||||
const created = await api.createDrawing().json();
|
const created = await api.createDrawing().json();
|
||||||
|
|||||||
@@ -82,4 +82,57 @@ describe("drawing store", () => {
|
|||||||
expect(store.getDrawing(created.id)?.scene.elements).toEqual([{ id: "first" }]);
|
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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -190,32 +190,46 @@ export function createDrawingStore(databasePath = resolveDatabasePath()) {
|
|||||||
): DrawingUpdateResult {
|
): DrawingUpdateResult {
|
||||||
const hasScene = update.scene !== undefined;
|
const hasScene = update.scene !== undefined;
|
||||||
const hasTitle = update.title !== 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) {
|
if (!hasScene && !hasTitle) {
|
||||||
const existing = getDrawing(id);
|
return { ok: true, drawing: existing };
|
||||||
return existing ? { ok: true, drawing: existing } : { ok: false, reason: "not_found" };
|
}
|
||||||
|
|
||||||
|
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;
|
let changes: number;
|
||||||
if (hasScene && hasTitle) {
|
if (sceneChanged && titleChanged) {
|
||||||
const title = normalizeTitle(update.title);
|
|
||||||
const scene = JSON.stringify(update.scene);
|
|
||||||
changes =
|
changes =
|
||||||
update.expectedRevision === undefined
|
update.expectedRevision === undefined
|
||||||
? Number(updateBothQuery.run(id, title, scene).changes)
|
? Number(updateBothQuery.run(id, nextTitle, nextScene).changes)
|
||||||
: Number(updateBothExpectedQuery.run(id, title, scene, update.expectedRevision).changes);
|
: Number(updateBothExpectedQuery.run(id, nextTitle, nextScene, update.expectedRevision).changes);
|
||||||
} else if (hasScene) {
|
} else if (sceneChanged) {
|
||||||
const scene = JSON.stringify(update.scene);
|
|
||||||
changes =
|
changes =
|
||||||
update.expectedRevision === undefined
|
update.expectedRevision === undefined
|
||||||
? Number(updateSceneQuery.run(id, scene).changes)
|
? Number(updateSceneQuery.run(id, nextScene).changes)
|
||||||
: Number(updateSceneExpectedQuery.run(id, scene, update.expectedRevision).changes);
|
: Number(updateSceneExpectedQuery.run(id, nextScene, update.expectedRevision).changes);
|
||||||
} else {
|
} else {
|
||||||
const title = normalizeTitle(update.title);
|
|
||||||
changes =
|
changes =
|
||||||
update.expectedRevision === undefined
|
update.expectedRevision === undefined
|
||||||
? Number(updateTitleQuery.run(id, title).changes)
|
? Number(updateTitleQuery.run(id, nextTitle).changes)
|
||||||
: Number(updateTitleExpectedQuery.run(id, title, update.expectedRevision).changes);
|
: Number(updateTitleExpectedQuery.run(id, nextTitle, update.expectedRevision).changes);
|
||||||
}
|
}
|
||||||
|
|
||||||
const drawing = getDrawing(id);
|
const drawing = getDrawing(id);
|
||||||
|
|||||||
@@ -95,6 +95,18 @@ describe("mcp tool handlers", () => {
|
|||||||
expect(store.getDrawing(result.id)?.scene).toEqual(nextScene);
|
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", () => {
|
test("patch_drawing replaces an element property and removes an element", () => {
|
||||||
const { store, tools } = createTempTools();
|
const { store, tools } = createTempTools();
|
||||||
const result = tools.create_drawing({ title: "patch", scene: testScene() });
|
const result = tools.create_drawing({ title: "patch", scene: testScene() });
|
||||||
|
|||||||
Reference in New Issue
Block a user