fix: prevent stale drawing saves
This commit is contained in:
+108
-16
@@ -9,6 +9,17 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react";
|
|||||||
import { DEFAULT_TITLE, type DrawingMeta, type ScenePayload } from "./shared";
|
import { DEFAULT_TITLE, type DrawingMeta, type ScenePayload } from "./shared";
|
||||||
|
|
||||||
type DrawingResponse = DrawingMeta & ScenePayload;
|
type DrawingResponse = DrawingMeta & ScenePayload;
|
||||||
|
type ApiErrorBody = { ok: false; error?: string; drawing?: DrawingMeta };
|
||||||
|
type UpdateResponse = { ok: true; drawing: DrawingMeta };
|
||||||
|
|
||||||
|
class RequestError extends Error {
|
||||||
|
constructor(
|
||||||
|
readonly status: number,
|
||||||
|
readonly body: ApiErrorBody,
|
||||||
|
) {
|
||||||
|
super(body.error ?? `Request failed: ${status}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const EXCALIDRAW_THEME_TOKEN_MAP = [
|
const EXCALIDRAW_THEME_TOKEN_MAP = [
|
||||||
["--island-bg-color", "--app-island-bg"],
|
["--island-bg-color", "--app-island-bg"],
|
||||||
@@ -70,14 +81,18 @@ async function requestJson<T>(url: string, init?: RequestInit): Promise<T> {
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const body = (await response.json()) as T & { error?: string };
|
const body = (await response.json()) as T & ApiErrorBody;
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
throw new Error(body.error ?? `Request failed: ${response.status}`);
|
throw new RequestError(response.status, body);
|
||||||
}
|
}
|
||||||
|
|
||||||
return body;
|
return body;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function isConflictError(error: unknown): error is RequestError & { body: ApiErrorBody & { drawing: DrawingMeta } } {
|
||||||
|
return error instanceof RequestError && error.status === 409 && error.body.drawing !== undefined;
|
||||||
|
}
|
||||||
|
|
||||||
function DrawerIcon() {
|
function DrawerIcon() {
|
||||||
return (
|
return (
|
||||||
<svg viewBox="0 0 24 24" aria-hidden="true">
|
<svg viewBox="0 0 24 24" aria-hidden="true">
|
||||||
@@ -106,8 +121,11 @@ export function App() {
|
|||||||
const timeoutRef = useRef<number | null>(null);
|
const timeoutRef = useRef<number | null>(null);
|
||||||
const inFlightSaveRef = useRef<Promise<void> | null>(null);
|
const inFlightSaveRef = useRef<Promise<void> | null>(null);
|
||||||
const loadVersionRef = useRef(0);
|
const loadVersionRef = useRef(0);
|
||||||
|
const loadedRevisionRef = useRef<number | null>(null);
|
||||||
|
const loadDrawingRef = useRef<(drawingId: string) => Promise<void>>(async () => {});
|
||||||
const themeSyncFrameRef = useRef<number | null>(null);
|
const themeSyncFrameRef = useRef<number | null>(null);
|
||||||
const toastTimeoutRef = useRef<number | null>(null);
|
const toastTimeoutRef = useRef<number | null>(null);
|
||||||
|
const [editorReloadNonce, setEditorReloadNonce] = useState(0);
|
||||||
|
|
||||||
const activeDrawing = useMemo(
|
const activeDrawing = useMemo(
|
||||||
() => drawings.find((drawing) => drawing.id === activeId) ?? null,
|
() => drawings.find((drawing) => drawing.id === activeId) ?? null,
|
||||||
@@ -120,9 +138,17 @@ export function App() {
|
|||||||
return list;
|
return list;
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
const clearPendingSaveTimeout = useCallback(() => {
|
||||||
|
if (timeoutRef.current !== null) {
|
||||||
|
clearTimeout(timeoutRef.current);
|
||||||
|
timeoutRef.current = null;
|
||||||
|
}
|
||||||
|
}, []);
|
||||||
|
|
||||||
const saveScene = useCallback(async (isManual = false) => {
|
const saveScene = useCallback(async (isManual = false) => {
|
||||||
const drawingId = currentIdRef.current;
|
const drawingId = currentIdRef.current;
|
||||||
const nextScene = latestSceneRef.current;
|
const nextScene = latestSceneRef.current;
|
||||||
|
const expectedRevision = loadedRevisionRef.current;
|
||||||
if (!drawingId || !nextScene) {
|
if (!drawingId || !nextScene) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -136,12 +162,15 @@ export function App() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const promise = requestJson<{ ok: true; drawing: DrawingMeta }>(`/api/drawings/${drawingId}`, {
|
const promise = requestJson<UpdateResponse>(`/api/drawings/${drawingId}`, {
|
||||||
method: "PUT",
|
method: "PUT",
|
||||||
body: JSON.stringify(nextScene),
|
body: JSON.stringify({ ...nextScene, expectedRevision }),
|
||||||
})
|
})
|
||||||
.then((body) => {
|
.then((body) => {
|
||||||
setDrawings((current) => replaceMeta(current, body.drawing));
|
setDrawings((current) => replaceMeta(current, body.drawing));
|
||||||
|
if (currentIdRef.current === drawingId) {
|
||||||
|
loadedRevisionRef.current = body.drawing.revision;
|
||||||
|
}
|
||||||
if (isManual) {
|
if (isManual) {
|
||||||
setToastMessage("Saved");
|
setToastMessage("Saved");
|
||||||
if (toastTimeoutRef.current !== null) {
|
if (toastTimeoutRef.current !== null) {
|
||||||
@@ -151,6 +180,20 @@ export function App() {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
.catch((saveError: unknown) => {
|
.catch((saveError: unknown) => {
|
||||||
|
if (isConflictError(saveError)) {
|
||||||
|
clearPendingSaveTimeout();
|
||||||
|
setError(null);
|
||||||
|
setDrawings((current) => replaceMeta(current, saveError.body.drawing));
|
||||||
|
if (isManual) {
|
||||||
|
setToastMessage("Reloading latest...");
|
||||||
|
if (toastTimeoutRef.current !== null) {
|
||||||
|
clearTimeout(toastTimeoutRef.current);
|
||||||
|
toastTimeoutRef.current = null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return loadDrawingRef.current(drawingId);
|
||||||
|
}
|
||||||
|
|
||||||
setError(saveError instanceof Error ? saveError.message : "Save failed");
|
setError(saveError instanceof Error ? saveError.message : "Save failed");
|
||||||
if (isManual) {
|
if (isManual) {
|
||||||
setToastMessage("Save failed");
|
setToastMessage("Save failed");
|
||||||
@@ -167,12 +210,11 @@ export function App() {
|
|||||||
|
|
||||||
inFlightSaveRef.current = promise;
|
inFlightSaveRef.current = promise;
|
||||||
await promise;
|
await promise;
|
||||||
}, []);
|
}, [clearPendingSaveTimeout]);
|
||||||
|
|
||||||
const flushPendingSave = useCallback(async () => {
|
const flushPendingSave = useCallback(async () => {
|
||||||
if (timeoutRef.current !== null) {
|
if (timeoutRef.current !== null) {
|
||||||
clearTimeout(timeoutRef.current);
|
clearPendingSaveTimeout();
|
||||||
timeoutRef.current = null;
|
|
||||||
await saveScene();
|
await saveScene();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -180,18 +222,17 @@ export function App() {
|
|||||||
if (inFlightSaveRef.current) {
|
if (inFlightSaveRef.current) {
|
||||||
await inFlightSaveRef.current;
|
await inFlightSaveRef.current;
|
||||||
}
|
}
|
||||||
}, [saveScene]);
|
}, [clearPendingSaveTimeout, saveScene]);
|
||||||
|
|
||||||
const triggerManualSave = useCallback(async () => {
|
const triggerManualSave = useCallback(async () => {
|
||||||
if (timeoutRef.current !== null) {
|
if (timeoutRef.current !== null) {
|
||||||
clearTimeout(timeoutRef.current);
|
clearPendingSaveTimeout();
|
||||||
timeoutRef.current = null;
|
|
||||||
}
|
}
|
||||||
if (inFlightSaveRef.current) {
|
if (inFlightSaveRef.current) {
|
||||||
await inFlightSaveRef.current;
|
await inFlightSaveRef.current;
|
||||||
}
|
}
|
||||||
await saveScene(true);
|
await saveScene(true);
|
||||||
}, [saveScene]);
|
}, [clearPendingSaveTimeout, saveScene]);
|
||||||
|
|
||||||
const scheduleSave = useCallback(() => {
|
const scheduleSave = useCallback(() => {
|
||||||
if (timeoutRef.current !== null) {
|
if (timeoutRef.current !== null) {
|
||||||
@@ -230,11 +271,13 @@ export function App() {
|
|||||||
currentIdRef.current = drawing.id;
|
currentIdRef.current = drawing.id;
|
||||||
currentTitleRef.current = drawing.title;
|
currentTitleRef.current = drawing.title;
|
||||||
latestSceneRef.current = nextScene;
|
latestSceneRef.current = nextScene;
|
||||||
|
loadedRevisionRef.current = drawing.revision;
|
||||||
|
|
||||||
setDrawings(sortDrawings(list));
|
setDrawings(replaceMeta(list, drawing));
|
||||||
setActiveId(drawing.id);
|
setActiveId(drawing.id);
|
||||||
setActiveTitle(drawing.title);
|
setActiveTitle(drawing.title);
|
||||||
setScene(nextScene);
|
setScene(nextScene);
|
||||||
|
setEditorReloadNonce((current) => current + 1);
|
||||||
} catch (loadError) {
|
} catch (loadError) {
|
||||||
const list = await refreshList();
|
const list = await refreshList();
|
||||||
if (list.length === 0) {
|
if (list.length === 0) {
|
||||||
@@ -261,6 +304,8 @@ export function App() {
|
|||||||
[refreshList],
|
[refreshList],
|
||||||
);
|
);
|
||||||
|
|
||||||
|
loadDrawingRef.current = loadDrawing;
|
||||||
|
|
||||||
const navigateToDrawing = useCallback(
|
const navigateToDrawing = useCallback(
|
||||||
async (drawingId: string, options?: { replace?: boolean; flush?: boolean }) => {
|
async (drawingId: string, options?: { replace?: boolean; flush?: boolean }) => {
|
||||||
const replace = options?.replace ?? false;
|
const replace = options?.replace ?? false;
|
||||||
@@ -324,18 +369,29 @@ export function App() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const body = await requestJson<{ ok: true; drawing: DrawingMeta }>(`/api/drawings/${drawingId}`, {
|
const body = await requestJson<UpdateResponse>(`/api/drawings/${drawingId}`, {
|
||||||
method: "PUT",
|
method: "PUT",
|
||||||
body: JSON.stringify({ title: currentTitleRef.current }),
|
body: JSON.stringify({
|
||||||
|
title: currentTitleRef.current,
|
||||||
|
expectedRevision: loadedRevisionRef.current,
|
||||||
|
}),
|
||||||
});
|
});
|
||||||
|
|
||||||
currentTitleRef.current = body.drawing.title;
|
currentTitleRef.current = body.drawing.title;
|
||||||
|
loadedRevisionRef.current = body.drawing.revision;
|
||||||
setActiveTitle(body.drawing.title);
|
setActiveTitle(body.drawing.title);
|
||||||
setDrawings((current) => replaceMeta(current, body.drawing));
|
setDrawings((current) => replaceMeta(current, body.drawing));
|
||||||
} catch (renameError) {
|
} catch (renameError) {
|
||||||
|
if (isConflictError(renameError)) {
|
||||||
|
clearPendingSaveTimeout();
|
||||||
|
setDrawings((current) => replaceMeta(current, renameError.body.drawing));
|
||||||
|
await loadDrawingRef.current(drawingId);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
setError(renameError instanceof Error ? renameError.message : "Rename failed");
|
setError(renameError instanceof Error ? renameError.message : "Rename failed");
|
||||||
}
|
}
|
||||||
}, []);
|
}, [clearPendingSaveTimeout]);
|
||||||
|
|
||||||
const syncThemeTokens = useCallback(() => {
|
const syncThemeTokens = useCallback(() => {
|
||||||
const appShell = appShellRef.current;
|
const appShell = appShellRef.current;
|
||||||
@@ -399,6 +455,42 @@ export function App() {
|
|||||||
};
|
};
|
||||||
}, [loadDrawing, navigateToDrawing]);
|
}, [loadDrawing, navigateToDrawing]);
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
const checkForExternalChanges = async () => {
|
||||||
|
try {
|
||||||
|
const list = await refreshList();
|
||||||
|
const drawingId = currentIdRef.current;
|
||||||
|
const loadedRevision = loadedRevisionRef.current;
|
||||||
|
const serverDrawing = drawingId ? list.find((drawing) => drawing.id === drawingId) : null;
|
||||||
|
|
||||||
|
if (serverDrawing && loadedRevision !== null && serverDrawing.revision > loadedRevision) {
|
||||||
|
clearPendingSaveTimeout();
|
||||||
|
await loadDrawingRef.current(serverDrawing.id);
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Focus checks only discover external edits; direct save/load requests report failures.
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
const handleFocus = () => {
|
||||||
|
void checkForExternalChanges();
|
||||||
|
};
|
||||||
|
|
||||||
|
const handleVisibilityChange = () => {
|
||||||
|
if (document.visibilityState === "visible") {
|
||||||
|
void checkForExternalChanges();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
window.addEventListener("focus", handleFocus);
|
||||||
|
document.addEventListener("visibilitychange", handleVisibilityChange);
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
window.removeEventListener("focus", handleFocus);
|
||||||
|
document.removeEventListener("visibilitychange", handleVisibilityChange);
|
||||||
|
};
|
||||||
|
}, [clearPendingSaveTimeout, refreshList]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
return () => {
|
return () => {
|
||||||
if (timeoutRef.current !== null) {
|
if (timeoutRef.current !== null) {
|
||||||
@@ -482,7 +574,7 @@ export function App() {
|
|||||||
) : (
|
) : (
|
||||||
<div className="editor-frame">
|
<div className="editor-frame">
|
||||||
<Excalidraw
|
<Excalidraw
|
||||||
key={activeDrawing?.id ?? activeId}
|
key={`${activeDrawing?.id ?? activeId}:${editorReloadNonce}`}
|
||||||
initialData={sceneToInitialData(scene)}
|
initialData={sceneToInitialData(scene)}
|
||||||
onChange={(elements, appState, files) => {
|
onChange={(elements, appState, files) => {
|
||||||
const nextScene = sceneFromEditor(elements, appState, files);
|
const nextScene = sceneFromEditor(elements, appState, files);
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ describe("api", () => {
|
|||||||
const created = await api.createDrawing().json();
|
const created = await api.createDrawing().json();
|
||||||
|
|
||||||
expect(created.appState.theme).toBe("dark");
|
expect(created.appState.theme).toBe("dark");
|
||||||
|
expect(created.revision).toBe(0);
|
||||||
cleanup();
|
cleanup();
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -70,6 +71,7 @@ describe("api", () => {
|
|||||||
elements: [{ id: "shape" }],
|
elements: [{ id: "shape" }],
|
||||||
appState: {},
|
appState: {},
|
||||||
files: {},
|
files: {},
|
||||||
|
expectedRevision: created.revision,
|
||||||
}),
|
}),
|
||||||
}),
|
}),
|
||||||
{ params: { id: created.id } },
|
{ params: { id: created.id } },
|
||||||
@@ -77,6 +79,90 @@ describe("api", () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
expect(response.status).toBe(200);
|
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();
|
||||||
|
|
||||||
|
const response = await api.updateDrawing(
|
||||||
|
Object.assign(
|
||||||
|
new Request(`http://local/api/drawings/${created.id}`, {
|
||||||
|
method: "PUT",
|
||||||
|
body: JSON.stringify({
|
||||||
|
title: "Synced",
|
||||||
|
elements: [{ id: "shape" }],
|
||||||
|
appState: {},
|
||||||
|
files: {},
|
||||||
|
expectedRevision: 0,
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
{ params: { id: created.id } },
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(response.status).toBe(200);
|
||||||
|
const body = await response.json();
|
||||||
|
expect(body.drawing.title).toBe("Synced");
|
||||||
|
expect(body.drawing.revision).toBe(1);
|
||||||
|
cleanup();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("returns 409 for stale expected revisions without overwriting", 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: "server" }],
|
||||||
|
appState: {},
|
||||||
|
files: {},
|
||||||
|
expectedRevision: 0,
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
{ params: { id: created.id } },
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
const conflict = await api.updateDrawing(
|
||||||
|
Object.assign(
|
||||||
|
new Request(`http://local/api/drawings/${created.id}`, {
|
||||||
|
method: "PUT",
|
||||||
|
body: JSON.stringify({
|
||||||
|
elements: [{ id: "stale" }],
|
||||||
|
appState: {},
|
||||||
|
files: {},
|
||||||
|
expectedRevision: 0,
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
{ params: { id: created.id } },
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(conflict.status).toBe(409);
|
||||||
|
const body = await conflict.json();
|
||||||
|
expect(body).toEqual({
|
||||||
|
ok: false,
|
||||||
|
error: "Drawing changed externally",
|
||||||
|
drawing: {
|
||||||
|
id: created.id,
|
||||||
|
title: created.title,
|
||||||
|
createdAt: body.drawing.createdAt,
|
||||||
|
updatedAt: body.drawing.updatedAt,
|
||||||
|
revision: 1,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const stored = await api.getDrawing(
|
||||||
|
Object.assign(new Request(`http://local/api/drawings/${created.id}`), { params: { id: created.id } }),
|
||||||
|
).json();
|
||||||
|
expect(stored.elements).toEqual([{ id: "server" }]);
|
||||||
cleanup();
|
cleanup();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
+43
-16
@@ -1,4 +1,5 @@
|
|||||||
import { type DrawingStore } from "./db";
|
import { type DrawingStore } from "./db";
|
||||||
|
import { type DrawingMeta } from "./shared";
|
||||||
import { HttpError, normalizeTitle, parseJsonBody, parseSceneText } from "./scene";
|
import { HttpError, normalizeTitle, parseJsonBody, parseSceneText } from "./scene";
|
||||||
|
|
||||||
type RouteRequest = Request & {
|
type RouteRequest = Request & {
|
||||||
@@ -22,6 +23,29 @@ function errorResponse(error: unknown): Response {
|
|||||||
return json({ ok: false, error: "Internal server error" }, { status: 500 });
|
return json({ ok: false, error: "Internal server error" }, { status: 500 });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function toMeta(drawing: DrawingMeta): DrawingMeta {
|
||||||
|
return {
|
||||||
|
id: drawing.id,
|
||||||
|
title: drawing.title,
|
||||||
|
createdAt: drawing.createdAt,
|
||||||
|
updatedAt: drawing.updatedAt,
|
||||||
|
revision: drawing.revision,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseExpectedRevision(raw: Record<string, unknown>): number | undefined {
|
||||||
|
if (!Object.hasOwn(raw, "expectedRevision")) {
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
const value = raw.expectedRevision;
|
||||||
|
if (typeof value !== "number" || !Number.isSafeInteger(value) || value < 0) {
|
||||||
|
throw new HttpError(400, "expectedRevision must be a non-negative integer");
|
||||||
|
}
|
||||||
|
|
||||||
|
return value;
|
||||||
|
}
|
||||||
|
|
||||||
export function createApi(store: DrawingStore) {
|
export function createApi(store: DrawingStore) {
|
||||||
return {
|
return {
|
||||||
health() {
|
health() {
|
||||||
@@ -36,10 +60,7 @@ export function createApi(store: DrawingStore) {
|
|||||||
const drawing = store.createDrawing();
|
const drawing = store.createDrawing();
|
||||||
return json(
|
return json(
|
||||||
{
|
{
|
||||||
id: drawing.id,
|
...toMeta(drawing),
|
||||||
title: drawing.title,
|
|
||||||
createdAt: drawing.createdAt,
|
|
||||||
updatedAt: drawing.updatedAt,
|
|
||||||
...drawing.scene,
|
...drawing.scene,
|
||||||
},
|
},
|
||||||
{ status: 201 },
|
{ status: 201 },
|
||||||
@@ -55,10 +76,7 @@ export function createApi(store: DrawingStore) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
return json({
|
return json({
|
||||||
id: drawing.id,
|
...toMeta(drawing),
|
||||||
title: drawing.title,
|
|
||||||
createdAt: drawing.createdAt,
|
|
||||||
updatedAt: drawing.updatedAt,
|
|
||||||
...drawing.scene,
|
...drawing.scene,
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
@@ -77,29 +95,38 @@ export function createApi(store: DrawingStore) {
|
|||||||
Object.hasOwn(raw, "elements") ||
|
Object.hasOwn(raw, "elements") ||
|
||||||
Object.hasOwn(raw, "appState") ||
|
Object.hasOwn(raw, "appState") ||
|
||||||
Object.hasOwn(raw, "files");
|
Object.hasOwn(raw, "files");
|
||||||
|
const expectedRevision = parseExpectedRevision(raw);
|
||||||
|
|
||||||
if (!hasTitle && !hasScene) {
|
if (!hasTitle && !hasScene) {
|
||||||
throw new HttpError(400, "Nothing to update");
|
throw new HttpError(400, "Nothing to update");
|
||||||
}
|
}
|
||||||
|
|
||||||
const scene = hasScene ? parseSceneText(text) : undefined;
|
const scene = hasScene ? parseSceneText(text) : undefined;
|
||||||
const updated = store.updateDrawing(id, {
|
const result = store.updateDrawing(id, {
|
||||||
scene,
|
scene,
|
||||||
title: hasTitle ? normalizeTitle(raw.title) : undefined,
|
title: hasTitle ? normalizeTitle(raw.title) : undefined,
|
||||||
|
expectedRevision,
|
||||||
});
|
});
|
||||||
|
|
||||||
if (!updated) {
|
if (!result.ok && result.reason === "not_found") {
|
||||||
return json({ ok: false, error: "Drawing not found" }, { status: 404 });
|
return json({ ok: false, error: "Drawing not found" }, { status: 404 });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!result.ok && result.reason === "conflict") {
|
||||||
|
return json(
|
||||||
|
{
|
||||||
|
ok: false,
|
||||||
|
error: "Drawing changed externally",
|
||||||
|
drawing: toMeta(result.drawing),
|
||||||
|
},
|
||||||
|
{ status: 409 },
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
const updated = result.drawing;
|
||||||
return json({
|
return json({
|
||||||
ok: true,
|
ok: true,
|
||||||
drawing: {
|
drawing: toMeta(updated),
|
||||||
id: updated.id,
|
|
||||||
title: updated.title,
|
|
||||||
createdAt: updated.createdAt,
|
|
||||||
updatedAt: updated.updatedAt,
|
|
||||||
},
|
|
||||||
});
|
});
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
return errorResponse(error);
|
return errorResponse(error);
|
||||||
|
|||||||
+63
-2
@@ -1,4 +1,5 @@
|
|||||||
import { afterEach, describe, expect, test } from "bun:test";
|
import { afterEach, describe, expect, test } from "bun:test";
|
||||||
|
import { Database } from "bun:sqlite";
|
||||||
import { mkdtempSync, rmSync } from "node:fs";
|
import { mkdtempSync, rmSync } from "node:fs";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { join } from "node:path";
|
||||||
@@ -27,8 +28,10 @@ describe("drawing store", () => {
|
|||||||
const created = store.createDrawing();
|
const created = store.createDrawing();
|
||||||
|
|
||||||
expect(created.title).toBe("Untitled");
|
expect(created.title).toBe("Untitled");
|
||||||
|
expect(created.revision).toBe(0);
|
||||||
expect(created.scene.appState.theme).toBe("dark");
|
expect(created.scene.appState.theme).toBe("dark");
|
||||||
expect(store.listDrawings()).toHaveLength(1);
|
expect(store.listDrawings()).toHaveLength(1);
|
||||||
|
expect(store.listDrawings()[0]?.revision).toBe(0);
|
||||||
|
|
||||||
const updated = store.updateDrawing(created.id, {
|
const updated = store.updateDrawing(created.id, {
|
||||||
title: "Flow",
|
title: "Flow",
|
||||||
@@ -39,12 +42,70 @@ describe("drawing store", () => {
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(updated?.title).toBe("Flow");
|
expect(updated.ok).toBe(true);
|
||||||
expect(updated?.scene.elements).toHaveLength(1);
|
expect(updated.ok ? updated.drawing.title : null).toBe("Flow");
|
||||||
|
expect(updated.ok ? updated.drawing.revision : null).toBe(1);
|
||||||
|
expect(updated.ok ? updated.drawing.scene.elements : []).toHaveLength(1);
|
||||||
|
|
||||||
const removed = store.deleteDrawing(created.id);
|
const removed = store.deleteDrawing(created.id);
|
||||||
expect(removed.deleted).toBe(true);
|
expect(removed.deleted).toBe(true);
|
||||||
expect(store.listDrawings()).toHaveLength(1);
|
expect(store.listDrawings()).toHaveLength(1);
|
||||||
expect(removed.next?.id).not.toBe(created.id);
|
expect(removed.next?.id).not.toBe(created.id);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("rejects stale expected revisions without overwriting the scene", () => {
|
||||||
|
const store = createTempStore();
|
||||||
|
const created = store.createDrawing();
|
||||||
|
|
||||||
|
const first = store.updateDrawing(created.id, {
|
||||||
|
expectedRevision: 0,
|
||||||
|
scene: {
|
||||||
|
elements: [{ id: "first" }],
|
||||||
|
appState: {},
|
||||||
|
files: {},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(first.ok).toBe(true);
|
||||||
|
|
||||||
|
const stale = store.updateDrawing(created.id, {
|
||||||
|
expectedRevision: 0,
|
||||||
|
scene: {
|
||||||
|
elements: [{ id: "stale" }],
|
||||||
|
appState: {},
|
||||||
|
files: {},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(stale.ok).toBe(false);
|
||||||
|
expect(stale.ok ? null : stale.reason).toBe("conflict");
|
||||||
|
expect(store.getDrawing(created.id)?.revision).toBe(1);
|
||||||
|
expect(store.getDrawing(created.id)?.scene.elements).toEqual([{ id: "first" }]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("migrates existing databases to include revision", () => {
|
||||||
|
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
|
||||||
|
);
|
||||||
|
|
||||||
|
INSERT INTO drawings (id, title, data)
|
||||||
|
VALUES ('legacy', 'Legacy', '{"elements":[],"appState":{},"files":{}}');
|
||||||
|
`);
|
||||||
|
db.close(false);
|
||||||
|
|
||||||
|
const store = createDrawingStore(databasePath);
|
||||||
|
expect(store.getDrawing("legacy")?.revision).toBe(0);
|
||||||
|
|
||||||
|
const updated = store.updateDrawing("legacy", { title: "Migrated" });
|
||||||
|
expect(updated.ok ? updated.drawing.revision : null).toBe(1);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -11,10 +11,16 @@ type DrawingRow = {
|
|||||||
data: string;
|
data: string;
|
||||||
createdAt: string;
|
createdAt: string;
|
||||||
updatedAt: string;
|
updatedAt: string;
|
||||||
|
revision: number;
|
||||||
};
|
};
|
||||||
|
|
||||||
type DrawingMetaRow = Omit<DrawingRow, "data">;
|
type DrawingMetaRow = Omit<DrawingRow, "data">;
|
||||||
|
|
||||||
|
export type DrawingUpdateResult =
|
||||||
|
| { ok: true; drawing: DrawingRecord }
|
||||||
|
| { ok: false; reason: "not_found" }
|
||||||
|
| { ok: false; reason: "conflict"; drawing: DrawingRecord };
|
||||||
|
|
||||||
export type DrawingStore = ReturnType<typeof createDrawingStore>;
|
export type DrawingStore = ReturnType<typeof createDrawingStore>;
|
||||||
|
|
||||||
function createId(): string {
|
function createId(): string {
|
||||||
@@ -44,6 +50,7 @@ function readMeta(row: DrawingMetaRow | null | undefined): DrawingMeta | null {
|
|||||||
title: row.title,
|
title: row.title,
|
||||||
createdAt: row.createdAt,
|
createdAt: row.createdAt,
|
||||||
updatedAt: row.updatedAt,
|
updatedAt: row.updatedAt,
|
||||||
|
revision: row.revision,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -70,19 +77,26 @@ export function createDrawingStore(databasePath = resolveDatabasePath()) {
|
|||||||
title TEXT NOT NULL,
|
title TEXT NOT NULL,
|
||||||
data TEXT NOT NULL,
|
data TEXT NOT NULL,
|
||||||
created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||||
updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP
|
updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||||
|
revision INTEGER NOT NULL DEFAULT 0
|
||||||
);
|
);
|
||||||
|
|
||||||
CREATE INDEX IF NOT EXISTS drawings_updated_at_idx
|
CREATE INDEX IF NOT EXISTS drawings_updated_at_idx
|
||||||
ON drawings(updated_at DESC, created_at DESC);
|
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(`
|
const listQuery = db.query(`
|
||||||
SELECT
|
SELECT
|
||||||
id,
|
id,
|
||||||
title,
|
title,
|
||||||
created_at AS createdAt,
|
created_at AS createdAt,
|
||||||
updated_at AS updatedAt
|
updated_at AS updatedAt,
|
||||||
|
revision
|
||||||
FROM drawings
|
FROM drawings
|
||||||
ORDER BY updated_at DESC, created_at DESC
|
ORDER BY updated_at DESC, created_at DESC
|
||||||
`);
|
`);
|
||||||
@@ -93,7 +107,8 @@ export function createDrawingStore(databasePath = resolveDatabasePath()) {
|
|||||||
title,
|
title,
|
||||||
data,
|
data,
|
||||||
created_at AS createdAt,
|
created_at AS createdAt,
|
||||||
updated_at AS updatedAt
|
updated_at AS updatedAt,
|
||||||
|
revision
|
||||||
FROM drawings
|
FROM drawings
|
||||||
WHERE id = ?1
|
WHERE id = ?1
|
||||||
`);
|
`);
|
||||||
@@ -105,22 +120,40 @@ export function createDrawingStore(databasePath = resolveDatabasePath()) {
|
|||||||
|
|
||||||
const updateSceneQuery = db.query(`
|
const updateSceneQuery = db.query(`
|
||||||
UPDATE drawings
|
UPDATE drawings
|
||||||
SET data = ?2, updated_at = CURRENT_TIMESTAMP
|
SET data = ?2, updated_at = CURRENT_TIMESTAMP, revision = revision + 1
|
||||||
WHERE id = ?1
|
WHERE id = ?1
|
||||||
`);
|
`);
|
||||||
|
|
||||||
const updateTitleQuery = db.query(`
|
const updateTitleQuery = db.query(`
|
||||||
UPDATE drawings
|
UPDATE drawings
|
||||||
SET title = ?2, updated_at = CURRENT_TIMESTAMP
|
SET title = ?2, updated_at = CURRENT_TIMESTAMP, revision = revision + 1
|
||||||
WHERE id = ?1
|
WHERE id = ?1
|
||||||
`);
|
`);
|
||||||
|
|
||||||
const updateBothQuery = db.query(`
|
const updateBothQuery = db.query(`
|
||||||
UPDATE drawings
|
UPDATE drawings
|
||||||
SET title = ?2, data = ?3, updated_at = CURRENT_TIMESTAMP
|
SET title = ?2, data = ?3, updated_at = CURRENT_TIMESTAMP, revision = revision + 1
|
||||||
WHERE id = ?1
|
WHERE id = ?1
|
||||||
`);
|
`);
|
||||||
|
|
||||||
|
const updateSceneExpectedQuery = db.query(`
|
||||||
|
UPDATE drawings
|
||||||
|
SET data = ?2, updated_at = CURRENT_TIMESTAMP, revision = revision + 1
|
||||||
|
WHERE id = ?1 AND revision = ?3
|
||||||
|
`);
|
||||||
|
|
||||||
|
const updateTitleExpectedQuery = db.query(`
|
||||||
|
UPDATE drawings
|
||||||
|
SET title = ?2, updated_at = CURRENT_TIMESTAMP, revision = revision + 1
|
||||||
|
WHERE id = ?1 AND revision = ?3
|
||||||
|
`);
|
||||||
|
|
||||||
|
const updateBothExpectedQuery = db.query(`
|
||||||
|
UPDATE drawings
|
||||||
|
SET title = ?2, data = ?3, updated_at = CURRENT_TIMESTAMP, revision = revision + 1
|
||||||
|
WHERE id = ?1 AND revision = ?4
|
||||||
|
`);
|
||||||
|
|
||||||
const deleteQuery = db.query(`
|
const deleteQuery = db.query(`
|
||||||
DELETE FROM drawings
|
DELETE FROM drawings
|
||||||
WHERE id = ?1
|
WHERE id = ?1
|
||||||
@@ -148,28 +181,50 @@ export function createDrawingStore(databasePath = resolveDatabasePath()) {
|
|||||||
return getLatestDrawing() ?? createDrawing();
|
return getLatestDrawing() ?? createDrawing();
|
||||||
}
|
}
|
||||||
|
|
||||||
function updateDrawing(id: string, update: { scene?: ScenePayload; title?: string }): DrawingRecord | null {
|
function updateDrawing(
|
||||||
const existing = getDrawing(id);
|
id: string,
|
||||||
if (!existing) {
|
update: { scene?: ScenePayload; title?: string; expectedRevision?: number },
|
||||||
return null;
|
): DrawingUpdateResult {
|
||||||
}
|
|
||||||
|
|
||||||
const hasScene = update.scene !== undefined;
|
const hasScene = update.scene !== undefined;
|
||||||
const hasTitle = update.title !== undefined;
|
const hasTitle = update.title !== undefined;
|
||||||
|
|
||||||
if (!hasScene && !hasTitle) {
|
if (!hasScene && !hasTitle) {
|
||||||
return existing;
|
const existing = getDrawing(id);
|
||||||
|
return existing ? { ok: true, drawing: existing } : { ok: false, reason: "not_found" };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let changes: number;
|
||||||
if (hasScene && hasTitle) {
|
if (hasScene && hasTitle) {
|
||||||
updateBothQuery.run(id, normalizeTitle(update.title), JSON.stringify(update.scene));
|
const title = normalizeTitle(update.title);
|
||||||
|
const scene = JSON.stringify(update.scene);
|
||||||
|
changes =
|
||||||
|
update.expectedRevision === undefined
|
||||||
|
? Number(updateBothQuery.run(id, title, scene).changes)
|
||||||
|
: Number(updateBothExpectedQuery.run(id, title, scene, update.expectedRevision).changes);
|
||||||
} else if (hasScene) {
|
} else if (hasScene) {
|
||||||
updateSceneQuery.run(id, JSON.stringify(update.scene));
|
const scene = JSON.stringify(update.scene);
|
||||||
|
changes =
|
||||||
|
update.expectedRevision === undefined
|
||||||
|
? Number(updateSceneQuery.run(id, scene).changes)
|
||||||
|
: Number(updateSceneExpectedQuery.run(id, scene, update.expectedRevision).changes);
|
||||||
} else {
|
} else {
|
||||||
updateTitleQuery.run(id, normalizeTitle(update.title));
|
const title = normalizeTitle(update.title);
|
||||||
|
changes =
|
||||||
|
update.expectedRevision === undefined
|
||||||
|
? Number(updateTitleQuery.run(id, title).changes)
|
||||||
|
: Number(updateTitleExpectedQuery.run(id, title, update.expectedRevision).changes);
|
||||||
}
|
}
|
||||||
|
|
||||||
return getDrawing(id);
|
const drawing = getDrawing(id);
|
||||||
|
if (changes > 0 && drawing) {
|
||||||
|
return { ok: true, drawing };
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!drawing) {
|
||||||
|
return { ok: false, reason: "not_found" };
|
||||||
|
}
|
||||||
|
|
||||||
|
return { ok: false, reason: "conflict", drawing };
|
||||||
}
|
}
|
||||||
|
|
||||||
function deleteDrawing(id: string): { deleted: boolean; next: DrawingMeta | null } {
|
function deleteDrawing(id: string): { deleted: boolean; next: DrawingMeta | null } {
|
||||||
|
|||||||
@@ -54,8 +54,10 @@ describe("mcp tool handlers", () => {
|
|||||||
expect(result).toEqual({
|
expect(result).toEqual({
|
||||||
id: result.id,
|
id: result.id,
|
||||||
title: "explicit",
|
title: "explicit",
|
||||||
|
revision: 1,
|
||||||
url: `http://example.test/d/${result.id}`,
|
url: `http://example.test/d/${result.id}`,
|
||||||
});
|
});
|
||||||
|
expect(drawing?.revision).toBe(1);
|
||||||
expect(drawing?.scene).toEqual(scene);
|
expect(drawing?.scene).toEqual(scene);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -69,6 +71,7 @@ describe("mcp tool handlers", () => {
|
|||||||
expect(drawing).toMatchObject({
|
expect(drawing).toMatchObject({
|
||||||
id: result.id,
|
id: result.id,
|
||||||
title: "read",
|
title: "read",
|
||||||
|
revision: 1,
|
||||||
url: `http://example.test/d/${result.id}`,
|
url: `http://example.test/d/${result.id}`,
|
||||||
scene,
|
scene,
|
||||||
});
|
});
|
||||||
@@ -83,10 +86,12 @@ describe("mcp tool handlers", () => {
|
|||||||
files: {},
|
files: {},
|
||||||
};
|
};
|
||||||
|
|
||||||
tools.replace_drawing({ id: result.id, title: "replaced", scene: nextScene });
|
const replaced = tools.replace_drawing({ id: result.id, title: "replaced", scene: nextScene });
|
||||||
|
|
||||||
expect(store.listDrawings()).toHaveLength(1);
|
expect(store.listDrawings()).toHaveLength(1);
|
||||||
|
expect(replaced.revision).toBe(2);
|
||||||
expect(store.getDrawing(result.id)?.title).toBe("replaced");
|
expect(store.getDrawing(result.id)?.title).toBe("replaced");
|
||||||
|
expect(store.getDrawing(result.id)?.revision).toBe(2);
|
||||||
expect(store.getDrawing(result.id)?.scene).toEqual(nextScene);
|
expect(store.getDrawing(result.id)?.scene).toEqual(nextScene);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -94,7 +99,7 @@ describe("mcp tool handlers", () => {
|
|||||||
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() });
|
||||||
|
|
||||||
tools.patch_drawing({
|
const patched = tools.patch_drawing({
|
||||||
id: result.id,
|
id: result.id,
|
||||||
patch: [
|
patch: [
|
||||||
{ op: "replace", path: "/elements/0/x", value: 42 },
|
{ op: "replace", path: "/elements/0/x", value: 42 },
|
||||||
@@ -102,6 +107,8 @@ describe("mcp tool handlers", () => {
|
|||||||
],
|
],
|
||||||
});
|
});
|
||||||
|
|
||||||
|
expect(patched.revision).toBe(2);
|
||||||
|
expect(store.getDrawing(result.id)?.revision).toBe(2);
|
||||||
expect(store.getDrawing(result.id)?.scene.elements).toEqual([{ id: "one", type: "rectangle", x: 42, y: 20 }]);
|
expect(store.getDrawing(result.id)?.scene.elements).toEqual([{ id: "one", type: "rectangle", x: 42, y: 20 }]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
+9
-3
@@ -75,10 +75,11 @@ function notFound(id: string): never {
|
|||||||
throw new Error(`Drawing not found: ${id}`);
|
throw new Error(`Drawing not found: ${id}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
function mutationResult(publicBaseUrl: string, drawing: { id: string; title: string }) {
|
function mutationResult(publicBaseUrl: string, drawing: { id: string; title: string; revision: number }) {
|
||||||
return {
|
return {
|
||||||
id: drawing.id,
|
id: drawing.id,
|
||||||
title: drawing.title,
|
title: drawing.title,
|
||||||
|
revision: drawing.revision,
|
||||||
url: drawingUrl(publicBaseUrl, drawing.id),
|
url: drawingUrl(publicBaseUrl, drawing.id),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -88,8 +89,12 @@ function updateExistingDrawing(
|
|||||||
id: string,
|
id: string,
|
||||||
update: { scene?: ScenePayload; title?: string },
|
update: { scene?: ScenePayload; title?: string },
|
||||||
) {
|
) {
|
||||||
const updated = store.updateDrawing(id, update);
|
const result = store.updateDrawing(id, update);
|
||||||
return updated ?? notFound(id);
|
if (!result.ok) {
|
||||||
|
return notFound(id);
|
||||||
|
}
|
||||||
|
|
||||||
|
return result.drawing;
|
||||||
}
|
}
|
||||||
|
|
||||||
function applyScenePatch(scene: ScenePayload, patch: Operation[]): ScenePayload {
|
function applyScenePatch(scene: ScenePayload, patch: Operation[]): ScenePayload {
|
||||||
@@ -115,6 +120,7 @@ export function createMcpToolHandlers(store: DrawingStore, publicBaseUrl: string
|
|||||||
title: drawing.title,
|
title: drawing.title,
|
||||||
createdAt: drawing.createdAt,
|
createdAt: drawing.createdAt,
|
||||||
updatedAt: drawing.updatedAt,
|
updatedAt: drawing.updatedAt,
|
||||||
|
revision: drawing.revision,
|
||||||
url: drawingUrl(publicBaseUrl, drawing.id),
|
url: drawingUrl(publicBaseUrl, drawing.id),
|
||||||
scene: drawing.scene,
|
scene: drawing.scene,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ export type DrawingMeta = {
|
|||||||
title: string;
|
title: string;
|
||||||
createdAt: string;
|
createdAt: string;
|
||||||
updatedAt: string;
|
updatedAt: string;
|
||||||
|
revision: number;
|
||||||
};
|
};
|
||||||
|
|
||||||
export type DrawingRecord = DrawingMeta & {
|
export type DrawingRecord = DrawingMeta & {
|
||||||
|
|||||||
Reference in New Issue
Block a user