concurrency

This commit is contained in:
Zimeng Xiong
2026-02-07 12:45:25 -08:00
parent dd0f381ed1
commit de254d46f2
12 changed files with 429 additions and 31 deletions
@@ -3,6 +3,7 @@ import request from "supertest";
import fs from "fs";
import path from "path";
import os from "os";
import JSZip from "jszip";
import { getTestPrisma, setupTestDb, cleanupTestDb } from "./testUtils";
type LegacyDbOptions = {
@@ -156,6 +157,111 @@ const createLegacySqliteDb = (opts: LegacyDbOptions): string => {
return filePath;
};
const createExcalidashArchiveWithDuplicateDrawingIds = async (): Promise<string> => {
const dir = createTempDir();
const filePath = path.join(dir, "duplicate-drawing-ids.excalidash");
const zip = new JSZip();
const manifest = {
format: "excalidash",
formatVersion: 1,
exportedAt: new Date().toISOString(),
unorganizedFolder: "Unorganized",
collections: [] as any[],
drawings: [
{
id: "duplicate-drawing-id",
name: "Drawing One",
filePath: "Unorganized/drawing-1.excalidraw",
collectionId: null,
},
{
id: "duplicate-drawing-id",
name: "Drawing Two",
filePath: "Unorganized/drawing-2.excalidraw",
collectionId: null,
},
],
};
zip.file("excalidash.manifest.json", JSON.stringify(manifest));
zip.file(
"Unorganized/drawing-1.excalidraw",
JSON.stringify({ type: "excalidraw", version: 2, source: "test", elements: [], appState: {}, files: {} })
);
zip.file(
"Unorganized/drawing-2.excalidraw",
JSON.stringify({ type: "excalidraw", version: 2, source: "test", elements: [], appState: {}, files: {} })
);
const buffer = await zip.generateAsync({ type: "nodebuffer" });
fs.writeFileSync(filePath, buffer);
return filePath;
};
const createLegacySqliteDbWithDuplicateDrawingIds = (): string => {
const dir = createTempDir();
const filePath = path.join(dir, "legacy-duplicate-ids.db");
const db = openWritableDb(filePath);
try {
db.exec(`
CREATE TABLE "Drawing" (
id TEXT,
name TEXT NOT NULL,
elements TEXT NOT NULL,
appState TEXT NOT NULL,
files TEXT,
preview TEXT,
version INTEGER,
collectionId TEXT,
collectionName TEXT,
createdAt TEXT,
updatedAt TEXT
);
`);
const now = new Date("2024-01-03T00:00:00.000Z").toISOString();
const insertDrawing = db.prepare(
`INSERT INTO "Drawing"
(id, name, elements, appState, files, preview, version, collectionId, collectionName, createdAt, updatedAt)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`
);
insertDrawing.run(
"legacy-duplicate-id",
"Legacy Drawing A",
JSON.stringify([]),
JSON.stringify({}),
JSON.stringify({}),
null,
1,
null,
null,
now,
now,
);
insertDrawing.run(
"legacy-duplicate-id",
"Legacy Drawing B",
JSON.stringify([]),
JSON.stringify({}),
JSON.stringify({}),
null,
1,
null,
null,
now,
now,
);
} finally {
db.close();
}
return filePath;
};
describe("Import compatibility (legacy exports)", () => {
const uploadsDir = path.resolve(__dirname, "../../uploads");
const userAgent = "vitest-import-compat";
@@ -287,4 +393,52 @@ describe("Import compatibility (legacy exports)", () => {
expect(res.status).toBe(400);
expect(res.body.error).toBe("Invalid legacy DB");
});
it("rejects .excalidash verify when manifest has duplicate drawing IDs", async () => {
const archive = await createExcalidashArchiveWithDuplicateDrawingIds();
const res = await request(app)
.post("/import/excalidash/verify")
.set("User-Agent", userAgent)
.set(csrfHeaderName, csrfToken)
.attach("archive", archive);
expect(res.status).toBe(400);
expect(String(res.body.message || "")).toContain("Duplicate drawing id");
});
it("rejects .excalidash import when manifest has duplicate drawing IDs", async () => {
const archive = await createExcalidashArchiveWithDuplicateDrawingIds();
const res = await request(app)
.post("/import/excalidash")
.set("User-Agent", userAgent)
.set(csrfHeaderName, csrfToken)
.attach("archive", archive);
expect(res.status).toBe(400);
expect(String(res.body.message || "")).toContain("Duplicate drawing id");
});
it("rejects legacy verify when DB has duplicate drawing IDs", async () => {
const legacyDb = createLegacySqliteDbWithDuplicateDrawingIds();
const res = await request(app)
.post("/import/sqlite/legacy/verify")
.set("User-Agent", userAgent)
.set(csrfHeaderName, csrfToken)
.attach("db", legacyDb);
expect(res.status).toBe(400);
expect(String(res.body.message || "")).toContain("Duplicate drawing id");
});
it("rejects legacy import when DB has duplicate drawing IDs", async () => {
const legacyDb = createLegacySqliteDbWithDuplicateDrawingIds();
const res = await request(app)
.post("/import/sqlite/legacy")
.set("User-Agent", userAgent)
.set(csrfHeaderName, csrfToken)
.attach("db", legacyDb);
expect(res.status).toBe(400);
expect(String(res.body.message || "")).toContain("Duplicate drawing id");
});
});
+1
View File
@@ -560,6 +560,7 @@ const drawingUpdateSchema = drawingBaseSchema
elements: elementSchema.array().optional(),
appState: appStateSchema.optional(),
files: filesFieldSchema,
version: z.number().int().positive().optional(),
})
.refine(
(data) => {
+23 -1
View File
@@ -310,7 +310,12 @@ export const registerDashboardRoutes = (
appState?: Record<string, unknown>;
preview?: string | null;
files?: Record<string, unknown>;
version?: number;
};
const isSceneUpdate =
payload.elements !== undefined ||
payload.appState !== undefined ||
payload.files !== undefined;
const data: Prisma.DrawingUpdateInput = { version: { increment: 1 } };
if (payload.name !== undefined) data.name = payload.name;
@@ -334,11 +339,28 @@ export const registerDashboardRoutes = (
}
}
const updateWhere: Prisma.DrawingWhereInput = { id, userId: req.user.id };
if (isSceneUpdate && payload.version !== undefined) {
updateWhere.version = payload.version;
}
const updateResult = await prisma.drawing.updateMany({
where: { id, userId: req.user.id },
where: updateWhere,
data,
});
if (updateResult.count === 0) {
if (isSceneUpdate && payload.version !== undefined) {
const latestDrawing = await prisma.drawing.findFirst({
where: { id, userId: req.user.id },
select: { version: true },
});
return res.status(409).json({
error: "Conflict",
code: "VERSION_CONFLICT",
message: "Drawing has changed since this editor state was loaded.",
currentVersion: latestDrawing?.version ?? null,
});
}
return res.status(404).json({ error: "Drawing not found" });
}
+2 -2
View File
@@ -340,8 +340,8 @@ export const createDrawing = async (
};
export const updateDrawing = async (id: string, data: Partial<Drawing>) => {
const response = await api.put<{ success: true }>(`/drawings/${id}`, data);
return response.data;
const response = await api.put<Drawing>(`/drawings/${id}`, data);
return deserializeDrawing(response.data);
};
export const deleteDrawing = async (id: string) => {
+60
View File
@@ -0,0 +1,60 @@
import { fireEvent, render, screen } from "@testing-library/react";
import { MemoryRouter } from "react-router-dom";
import { describe, expect, it, vi } from "vitest";
vi.mock("./Sidebar", () => ({
Sidebar: () => <div data-testid="sidebar">sidebar</div>,
}));
vi.mock("./Logo", () => ({
Logo: () => <div data-testid="logo">logo</div>,
}));
vi.mock("./UploadStatus", () => ({
UploadStatus: () => <div data-testid="upload-status">upload-status</div>,
}));
import { Layout } from "./Layout";
describe("Layout", () => {
it("removes active resize listeners on unmount", () => {
const addSpy = vi.spyOn(document, "addEventListener");
const removeSpy = vi.spyOn(document, "removeEventListener");
const { unmount } = render(
<MemoryRouter>
<Layout
collections={[]}
selectedCollectionId={undefined}
onSelectCollection={() => {}}
onCreateCollection={() => {}}
onEditCollection={() => {}}
onDeleteCollection={() => {}}
>
<div>content</div>
</Layout>
</MemoryRouter>
);
fireEvent.mouseDown(screen.getByTitle("Drag to resize sidebar"));
const mouseMoveAdd = addSpy.mock.calls.find(([event]) => event === "mousemove");
const mouseUpAdd = addSpy.mock.calls.find(([event]) => event === "mouseup");
expect(mouseMoveAdd?.[1]).toBeTypeOf("function");
expect(mouseUpAdd?.[1]).toBeTypeOf("function");
unmount();
expect(
removeSpy.mock.calls.some(
([event, handler]) => event === "mousemove" && handler === mouseMoveAdd?.[1]
)
).toBe(true);
expect(
removeSpy.mock.calls.some(
([event, handler]) => event === "mouseup" && handler === mouseUpAdd?.[1]
)
).toBe(true);
});
});
+48
View File
@@ -0,0 +1,48 @@
import { render, screen, waitFor } from "@testing-library/react";
import axios from "axios";
import { MemoryRouter } from "react-router-dom";
import { describe, expect, it, vi } from "vitest";
import { AuthProvider, useAuth } from "./AuthContext";
const Probe = () => {
const { loading, authEnabled } = useAuth();
return (
<div>
<span data-testid="loading">{String(loading)}</span>
<span data-testid="auth-enabled">{String(authEnabled)}</span>
</div>
);
};
describe("AuthProvider", () => {
it("defaults to auth-enabled mode if /auth/status fails", async () => {
const storage = new Map<string, string>();
Object.defineProperty(window, "localStorage", {
configurable: true,
value: {
getItem: (key: string) => storage.get(key) ?? null,
setItem: (key: string, value: string) => {
storage.set(key, value);
},
removeItem: (key: string) => {
storage.delete(key);
},
},
});
vi.spyOn(axios, "get").mockRejectedValueOnce(new Error("network down"));
render(
<MemoryRouter>
<AuthProvider>
<Probe />
</AuthProvider>
</MemoryRouter>
);
await waitFor(() => {
expect(screen.getByTestId("loading").textContent).toBe("false");
});
expect(screen.getByTestId("auth-enabled").textContent).toBe("true");
});
});
+5 -8
View File
@@ -11,6 +11,7 @@ import clsx from 'clsx';
import { ConfirmModal } from '../components/ConfirmModal';
import { useUpload } from '../context/UploadContext';
import { DragOverlayPortal, getSelectionBounds, type Point, type SelectionBounds } from './dashboard/shared';
import { isLatestRequest, mergeUniqueDrawings } from './dashboard/pagination';
const PAGE_SIZE = 24;
@@ -92,7 +93,7 @@ export const Dashboard: React.FC = () => {
}),
api.getCollections()
]);
if (requestVersion !== listRequestVersionRef.current) return;
if (!isLatestRequest(requestVersion, listRequestVersionRef.current)) return;
setDrawings(drawingsRes.drawings);
setTotalCount(drawingsRes.totalCount);
setCollections(collectionsData);
@@ -100,7 +101,7 @@ export const Dashboard: React.FC = () => {
} catch (err) {
console.error('Failed to fetch data:', err);
} finally {
if (requestVersion === listRequestVersionRef.current) {
if (isLatestRequest(requestVersion, listRequestVersionRef.current)) {
setIsLoading(false);
}
}
@@ -117,12 +118,8 @@ export const Dashboard: React.FC = () => {
sortField: sortConfig.field,
sortDirection: sortConfig.direction,
});
if (requestVersion !== listRequestVersionRef.current) return;
setDrawings(prev => {
const seen = new Set(prev.map((d) => d.id));
const nextPage = drawingsRes.drawings.filter((d) => !seen.has(d.id));
return [...prev, ...nextPage];
});
if (!isLatestRequest(requestVersion, listRequestVersionRef.current)) return;
setDrawings(prev => mergeUniqueDrawings(prev, drawingsRes.drawings));
setTotalCount(drawingsRes.totalCount);
} catch (err) {
console.error('Failed to fetch more data:', err);
+45 -20
View File
@@ -19,7 +19,9 @@ import {
getColorFromString,
getFilesDelta,
getInitialsFromName,
hasRenderableElements,
haveSameElements,
isSuspiciousEmptySnapshot,
} from './editor/shared';
import type { ElementVersionInfo } from './editor/shared';
@@ -124,7 +126,9 @@ export const Editor: React.FC = () => {
const latestFilesRef = useRef<any>(null);
const lastSyncedFilesRef = useRef<Record<string, any>>({});
const latestAppStateRef = useRef<any>(null);
const debouncedSaveRef = useRef<((drawingId: string, elements: readonly any[], appState: any) => void) | null>(null);
const debouncedSaveRef = useRef<((drawingId: string, elements: readonly any[], appState: any, files?: Record<string, any>) => void) | null>(null);
const currentDrawingVersionRef = useRef<number | null>(null);
const lastPersistedElementsRef = useRef<readonly any[]>([]);
const emitFilesDeltaIfNeeded = useCallback(
(nextFiles: Record<string, any>) => {
@@ -362,7 +366,7 @@ export const Editor: React.FC = () => {
// Persist after file data becomes available so new tabs (tab3) load correctly.
if (didEmit && id && latestAppStateRef.current && debouncedSaveRef.current) {
debouncedSaveRef.current(id, latestElementsRef.current, latestAppStateRef.current);
debouncedSaveRef.current(id, latestElementsRef.current, latestAppStateRef.current, latestFilesRef.current || {});
}
};
}
@@ -428,11 +432,11 @@ export const Editor: React.FC = () => {
scrollToContent: true,
}), []);
const saveDataRef = useRef<((drawingId: string, elements: readonly any[], appState: any) => Promise<void>) | null>(null);
const saveDataRef = useRef<((drawingId: string, elements: readonly any[], appState: any, files?: Record<string, any>) => Promise<void>) | null>(null);
const savePreviewRef = useRef<((drawingId: string, elements: readonly any[], appState: any, files: any) => Promise<void>) | null>(null);
const saveLibraryRef = useRef<((items: any[]) => Promise<void>) | null>(null);
saveDataRef.current = async (drawingId: string, elements: readonly any[], appState: any) => {
saveDataRef.current = async (drawingId: string, elements: readonly any[], appState: any, files?: Record<string, any>) => {
if (!drawingId) return;
try {
@@ -442,24 +446,38 @@ export const Editor: React.FC = () => {
gridSize: appState?.gridSize || null,
};
const snapshot = latestElementsRef.current ?? elements ?? [];
const persistableElements = Array.isArray(snapshot) ? snapshot : [];
const persistableElements = Array.isArray(elements) ? elements : [];
if (isSuspiciousEmptySnapshot(lastPersistedElementsRef.current, persistableElements)) {
console.warn("[Editor] Skipping suspicious empty snapshot save", { drawingId });
return;
}
const persistableFiles = files ?? latestFilesRef.current ?? {};
console.log("[Editor] Saving drawing", {
drawingId,
elementCount: persistableElements.length,
hasRenderableElements: persistableElements.some((el: any) => !el?.isDeleted),
hasRenderableElements: hasRenderableElements(persistableElements),
appState: persistableAppState,
});
await api.updateDrawing(drawingId, {
const updated = await api.updateDrawing(drawingId, {
elements: persistableElements,
appState: persistableAppState,
files: latestFilesRef.current || {},
files: persistableFiles,
version: currentDrawingVersionRef.current ?? undefined,
});
if (typeof updated.version === "number") {
currentDrawingVersionRef.current = updated.version;
}
lastPersistedElementsRef.current = persistableElements;
console.log("[Editor] Save complete", { drawingId });
} catch (err) {
if (api.isAxiosError(err) && err.response?.status === 409) {
console.warn("[Editor] Version conflict while saving drawing", { drawingId });
toast.error("Drawing changed in another tab. Refresh to load latest.");
return;
}
console.error('Failed to save drawing', err);
toast.error("Failed to save changes");
}
@@ -509,9 +527,9 @@ export const Editor: React.FC = () => {
const debouncedSave = useCallback(
debounce((drawingId, elements, appState) => {
debounce((drawingId, elements, appState, files) => {
if (saveDataRef.current) {
saveDataRef.current(drawingId, elements, appState);
saveDataRef.current(drawingId, elements, appState, files);
}
}, 1000),
[] // Empty dependency array = Stable across renders
@@ -587,6 +605,8 @@ export const Editor: React.FC = () => {
latestElementsRef.current = [];
latestFilesRef.current = {};
lastSyncedFilesRef.current = {};
currentDrawingVersionRef.current = null;
lastPersistedElementsRef.current = [];
excalidrawAPI.current = null;
setIsReady(false);
setIsSceneLoading(true);
@@ -614,6 +634,8 @@ export const Editor: React.FC = () => {
latestElementsRef.current = elements;
latestFilesRef.current = files;
lastSyncedFilesRef.current = files;
currentDrawingVersionRef.current = typeof data.version === "number" ? data.version : null;
lastPersistedElementsRef.current = elements;
elements.forEach((el: any) => {
recordElementVersion(el);
@@ -657,6 +679,8 @@ export const Editor: React.FC = () => {
latestElementsRef.current = [];
latestFilesRef.current = {};
lastSyncedFilesRef.current = {};
currentDrawingVersionRef.current = null;
lastPersistedElementsRef.current = [];
setLoadError(message);
setInitialData(null);
} finally {
@@ -678,7 +702,7 @@ export const Editor: React.FC = () => {
latestElementsRef.current = elements;
latestFilesRef.current = files;
if (!id) return;
await saveDataRef.current(id, elements, appState);
await saveDataRef.current(id, elements, appState, files);
savePreviewRef.current(id, elements, appState, files);
toast.success("Saved changes to server");
}
@@ -729,8 +753,8 @@ export const Editor: React.FC = () => {
latestElementsRef.current = allElements;
const hasRenderableElements = allElements.some((el: any) => !el?.isDeleted);
if (isBootstrappingScene.current && !hasRenderableElements) {
const hasRenderable = hasRenderableElements(allElements);
if (isBootstrappingScene.current && !hasRenderable) {
console.log("[Editor] Bootstrapping guard active", {
drawingId: id,
elementCount: allElements.length,
@@ -741,19 +765,20 @@ export const Editor: React.FC = () => {
// Trigger Sync (Throttled)
broadcastChanges(allElements, currentFiles);
const filesSnapshot = currentFiles;
latestFilesRef.current = filesSnapshot;
// Trigger Fast Save
console.log("[Editor] Queueing save", {
drawingId: id,
elementCount: allElements.length,
hasRenderableElements,
hasRenderableElements: hasRenderable,
});
if (id) {
debouncedSave(id, allElements, appState);
debouncedSave(id, allElements, appState, filesSnapshot);
}
// Trigger Slow Preview Gen
const filesSnapshot = currentFiles;
latestFilesRef.current = filesSnapshot;
console.log("[Editor] Queueing preview save", {
drawingId: id,
fileCount: Object.keys(filesSnapshot).length,
@@ -779,7 +804,7 @@ export const Editor: React.FC = () => {
// Persist after file data becomes available (covers the "tab 3" case).
if (didEmit && latestAppStateRef.current && debouncedSaveRef.current) {
debouncedSaveRef.current(id, latestElementsRef.current, latestAppStateRef.current);
debouncedSaveRef.current(id, latestElementsRef.current, latestAppStateRef.current, nextFiles);
}
}, 1000);
@@ -823,7 +848,7 @@ export const Editor: React.FC = () => {
latestFilesRef.current = files;
await Promise.all([
saveDataRef.current(id, elements, appState),
saveDataRef.current(id, elements, appState, files),
savePreviewRef.current(id, elements, appState, files)
]);
console.log("[Editor] Saved on back navigation", { drawingId: id });
@@ -0,0 +1,31 @@
import { describe, expect, it } from "vitest";
import { isLatestRequest, mergeUniqueDrawings } from "./pagination";
import type { DrawingSummary } from "../../types";
const drawing = (id: string, name = id): DrawingSummary => ({
id,
name,
collectionId: null,
preview: null,
version: 1,
createdAt: Date.now(),
updatedAt: Date.now(),
});
describe("dashboard pagination helpers", () => {
it("accepts only latest request version", () => {
expect(isLatestRequest(3, 3)).toBe(true);
expect(isLatestRequest(2, 3)).toBe(false);
expect(isLatestRequest(4, 3)).toBe(false);
});
it("merges pages without duplicating IDs", () => {
const existing = [drawing("a"), drawing("b")];
const incoming = [drawing("b", "b-new"), drawing("c")];
const merged = mergeUniqueDrawings(existing, incoming);
expect(merged.map((d) => d.id)).toEqual(["a", "b", "c"]);
expect(merged[1].name).toBe("b");
});
});
@@ -0,0 +1,13 @@
import type { DrawingSummary } from "../../types";
export const isLatestRequest = (requestVersion: number, currentVersion: number): boolean =>
requestVersion === currentVersion;
export const mergeUniqueDrawings = (
existing: DrawingSummary[],
incoming: DrawingSummary[]
): DrawingSummary[] => {
const seen = new Set(existing.map((d) => d.id));
const nextPage = incoming.filter((d) => !seen.has(d.id));
return [...existing, ...nextPage];
};
+32
View File
@@ -0,0 +1,32 @@
import { describe, expect, it } from "vitest";
import {
hasRenderableElements,
isSuspiciousEmptySnapshot,
} from "./shared";
describe("editor/shared scene guards", () => {
it("detects renderable elements", () => {
expect(hasRenderableElements([{ id: "a", isDeleted: false }])).toBe(true);
expect(
hasRenderableElements([
{ id: "a", isDeleted: true },
{ id: "b", isDeleted: true },
])
).toBe(false);
});
it("flags empty snapshot after a previously non-empty persisted scene", () => {
const previous = [{ id: "a", isDeleted: false }];
expect(isSuspiciousEmptySnapshot(previous, [])).toBe(true);
});
it("does not flag empty snapshot for already-empty drawings", () => {
expect(isSuspiciousEmptySnapshot([], [])).toBe(false);
});
it("does not flag non-empty snapshots", () => {
const previous = [{ id: "a", isDeleted: false }];
const next = [{ id: "a", isDeleted: true }];
expect(isSuspiciousEmptySnapshot(previous, next)).toBe(false);
});
});
+15
View File
@@ -17,6 +17,21 @@ export const haveSameElements = (a: readonly any[] = [], b: readonly any[] = [])
return true;
};
export const hasRenderableElements = (elements: readonly any[] = []): boolean =>
elements.some((element: any) => !element?.isDeleted);
/**
* Guard against transient empty snapshots (e.g. hydration/reload races) from
* overwriting a previously persisted non-empty drawing.
*/
export const isSuspiciousEmptySnapshot = (
previousPersisted: readonly any[] = [],
nextSnapshot: readonly any[] = []
): boolean => {
if (!Array.isArray(nextSnapshot) || nextSnapshot.length > 0) return false;
return hasRenderableElements(previousPersisted);
};
const buildFileSignature = (file: any): string => {
const mimeType = typeof file?.mimeType === "string" ? file.mimeType : "";
const id = typeof file?.id === "string" ? file.id : "";