From 80ad24580a34e17c443512368f0e163df0c30e5f Mon Sep 17 00:00:00 2001 From: Z8MB1E Date: Sat, 3 Oct 2026 04:08:29 -0400 Subject: [PATCH] fix(tech-tree): save canvas positions in one request so auto-arrange cannot overload the server Every canvas position write (Auto-arrange, Save positions, drags, freeze pins) used to send one PATCH per technology, all at once. On a large tree that was dozens of concurrent full document updates, which exhausted the database pool and could crash the server. - Add POST /api/tech-tree/positions: validates `{ positions: [{ id, x, y }] }` (null x/y clears a pin, capped at 2000) and applies every position in a single UPDATE on the tree position columns. Access mirrors the technologies update rule (technologies:update or Intelligence membership). - Route all canvas position writes through it; failures now surface in the toolbar instead of being swallowed. - Auto-arrange is optimistic: the canvas re-lays out on click and the pins are restored if the save fails. A spinner status chip shows while a layout save runs, and layout controls and dragging are locked until it finishes. - Integration tests for validation and the bulk write; the e2e checks the busy indicator and that no per-node PATCHes are sent. --- src/app/api/tech-tree/positions/route.ts | 52 +++++++ .../admin/tech-tree/TechTreeCanvas.tsx | 130 +++++++++++------ src/lib/tech-tree/positions.ts | 81 +++++++++++ tests/e2e/tech-tree.e2e.spec.ts | 16 +- tests/int/tech-tree-positions.int.spec.ts | 137 ++++++++++++++++++ 5 files changed, 371 insertions(+), 45 deletions(-) create mode 100644 src/app/api/tech-tree/positions/route.ts create mode 100644 src/lib/tech-tree/positions.ts create mode 100644 tests/int/tech-tree-positions.int.spec.ts diff --git a/src/app/api/tech-tree/positions/route.ts b/src/app/api/tech-tree/positions/route.ts new file mode 100644 index 0000000..ac07586 --- /dev/null +++ b/src/app/api/tech-tree/positions/route.ts @@ -0,0 +1,52 @@ +import { NextRequest, NextResponse } from "next/server"; +import config from "@payload-config"; +import { getPayload } from "payload"; + +import { parsePositionUpdates, writeTreePositions } from "@/lib/tech-tree/positions"; +import { hasIntelligenceQualification } from "@/utils/access-control/hasIntelligenceQualification"; +import { hasPermission } from "@/utils/access-control/hasPermission"; + +export const dynamic = "force-dynamic"; +export const runtime = "nodejs"; + +/** + * Bulk Tech Tree position write: `{ positions: [{ id, x, y }] }`, where a + * null x/y returns a technology to the automatic layout. Used by every canvas + * position write (drags, Save positions, Auto-arrange, freeze pins) so they + * cost one request and one SQL statement instead of a PATCH per node. + * + * Access mirrors the technologies collection's update rule: the + * `technologies:update` permission OR Intelligence division membership. + */ +export async function POST(req: NextRequest) { + const payload = await getPayload({ config: await config }); + const { user } = await payload.auth({ headers: req.headers, canSetHeaders: false }); + if (!user) { + return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + } + const allowed = + (await hasPermission(payload, user, "technologies:update")) || + (await hasIntelligenceQualification(payload, user)); + if (!allowed) { + return NextResponse.json({ error: "Forbidden" }, { status: 403 }); + } + + let body: unknown; + try { + body = await req.json(); + } catch { + return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); + } + const parsed = parsePositionUpdates(body); + if (!parsed.ok) { + return NextResponse.json({ error: parsed.error }, { status: 400 }); + } + + try { + const updated = await writeTreePositions(payload, parsed.updates); + return NextResponse.json({ ok: true, updated }); + } catch (err) { + payload.logger.error(`[TechTree] Bulk position write failed: ${err}`); + return NextResponse.json({ error: "Could not save positions." }, { status: 500 }); + } +} diff --git a/src/components/admin/tech-tree/TechTreeCanvas.tsx b/src/components/admin/tech-tree/TechTreeCanvas.tsx index 0a34af0..da38826 100644 --- a/src/components/admin/tech-tree/TechTreeCanvas.tsx +++ b/src/components/admin/tech-tree/TechTreeCanvas.tsx @@ -17,7 +17,14 @@ import { ReactFlowProvider, } from "@xyflow/react"; import "@xyflow/react/dist/style.css"; -import { LayoutGridIcon, MagnetIcon, PlusIcon, SaveIcon, TriangleAlertIcon } from "lucide-react"; +import { + LayoutGridIcon, + Loader2Icon, + MagnetIcon, + PlusIcon, + SaveIcon, + TriangleAlertIcon, +} from "lucide-react"; import { useRouter } from "next/navigation"; import React from "react"; @@ -239,6 +246,27 @@ function buildFlowEdges( }); } +/** + * Persist canvas positions in ONE request (one SQL statement server-side). + * Never PATCH per node: dozens of concurrent document updates exhausted the + * database pool and could take the server down. Null x/y clears a pin. + */ +async function saveTreePositions( + updates: { id: number; x: number | null; y: number | null }[], +): Promise { + if (updates.length === 0) return; + const res = await fetch("/api/tech-tree/positions", { + method: "POST", + credentials: "same-origin", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ positions: updates }), + }); + if (!res.ok) { + const body = await res.json().catch(() => null); + throw new Error(body?.error || `Could not save positions (HTTP ${res.status}).`); + } +} + async function fetchCollection(slug: string): Promise { const res = await fetch(`/api/${slug}?limit=0&depth=0&sort=name`, { credentials: "same-origin", @@ -255,7 +283,9 @@ function CanvasInner({ data }: { data: TechTreeData }) { const [resources, setResources] = React.useState(data.resources); const [selectedId, setSelectedId] = React.useState(null); const [createOpen, setCreateOpen] = React.useState(false); - const [arranging, setArranging] = React.useState(false); + // Label of the layout write in flight ("Arranging...", "Saving positions..."); + // null when idle. Drives the toolbar status chip and locks layout controls. + const [layoutBusy, setLayoutBusy] = React.useState(null); // Manual edge routing, loaded from this browser's storage on mount. Empty = // the edge draws its automatic path. @@ -314,16 +344,8 @@ function CanvasInner({ data }: { data: TechTreeData }) { /** Persist freeze pins (skipping ids whose own PATCH already carries one). */ const persistPins = React.useCallback( async (pins: Map, skip: Set = new Set()) => { - await Promise.all( - [...pins] - .filter(([id]) => !skip.has(id)) - .map(([id, pin]) => - fetch(`/api/technologies/${id}`, { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ treePosition: pin }), - }), - ), + await saveTreePositions( + [...pins].filter(([id]) => !skip.has(id)).map(([id, pin]) => ({ id, x: pin.x, y: pin.y })), ); }, [], @@ -476,7 +498,7 @@ function CanvasInner({ data }: { data: TechTreeData }) { /** * Persist one or more card positions to `treePosition` (optimistic local - * update first, then PATCH each). Used by card drags (a multi-selection + * update first, then one bulk write). Used by card drags (a multi-selection * moves several cards at once) and by category-band drags (every member). */ const persistPositions = React.useCallback( @@ -489,17 +511,12 @@ function CanvasInner({ data }: { data: TechTreeData }) { return p ? { ...t, treePosition: { x: p.x, y: p.y } } : t; }), ); - await Promise.all( - positions.map((p) => - fetch(`/api/technologies/${p.id}`, { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ treePosition: { x: p.x, y: p.y } }), - }).catch(() => { - // Optimistic position stays locally; a reload restores the stored value. - }), - ), - ); + try { + await saveTreePositions(positions); + } catch (e) { + // The optimistic position stays on screen; a reload shows what is stored. + setConnectError(e instanceof Error ? e.message : "Could not save positions."); + } }, [], ); @@ -599,39 +616,48 @@ function CanvasInner({ data }: { data: TechTreeData }) { setSaveStatus("Nothing to save."); return; } + if (layoutBusy) return; setTechs((prev) => prev.map((t) => { const pin = pins.get(t.id); return pin ? { ...t, treePosition: pin } : t; }), ); + setLayoutBusy("Saving positions..."); + setConnectError(null); try { await persistPins(pins); setSaveStatus(`Positions saved. ${pins.size} node(s).`); } catch (e) { - setSaveStatus("Could not save positions."); + setConnectError(e instanceof Error ? e.message : "Could not save positions."); + } finally { + setLayoutBusy(null); } - }, [techs, currentPositions, persistPins]); + }, [techs, currentPositions, persistPins, layoutBusy]); const handleAutoArrange = React.useCallback(async () => { - const withPositions = techs.filter((t) => t.treePosition !== null); - if (withPositions.length === 0) return; - setArranging(true); + const pinned = techs.filter((t) => t.treePosition !== null); + if (pinned.length === 0 || layoutBusy) return; + const previous = new Map(pinned.map((t) => [t.id, t.treePosition])); + + // Optimistic: re-lay the canvas out at once so the click visibly does + // something, then persist every cleared pin in a single request. + setLayoutBusy("Arranging..."); + setConnectError(null); + setTechs((prev) => prev.map((t) => (t.treePosition ? { ...t, treePosition: null } : t))); try { - await Promise.all( - withPositions.map((t) => - fetch(`/api/technologies/${t.id}`, { - method: "PATCH", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ treePosition: { x: null, y: null } }), - }), - ), + await saveTreePositions(pinned.map((t) => ({ id: t.id, x: null, y: null }))); + setSaveStatus(`Layout reset. ${pinned.length} node(s) re-arranged.`); + } catch (e) { + // Put the pins back so the canvas matches what is actually stored. + setTechs((prev) => + prev.map((t) => (previous.has(t.id) ? { ...t, treePosition: previous.get(t.id) ?? null } : t)), ); - setTechs((prev) => prev.map((t) => ({ ...t, treePosition: null }))); + setConnectError(e instanceof Error ? e.message : "Could not reset the layout."); } finally { - setArranging(false); + setLayoutBusy(null); } - }, [techs]); + }, [techs, layoutBusy]); const techById = React.useMemo( () => new Map(techs.map((t) => [String(t.id), t])), @@ -656,7 +682,9 @@ function CanvasInner({ data }: { data: TechTreeData }) { if (options.freeze) { const pins = pinsToFreeze(currentPositions(), nextTechs, nextCategories); nextTechs = applyPins(nextTechs, pins); - void persistPins(pins); + persistPins(pins).catch((e: unknown) => + setConnectError(e instanceof Error ? e.message : "Could not save positions."), + ); } setTechs(nextTechs); setCategories(nextCategories); @@ -701,6 +729,9 @@ function CanvasInner({ data }: { data: TechTreeData }) { fitView fitViewOptions={{ padding: 0.15, maxZoom: 0.85 }} minZoom={0.08} + // Freeze dragging while a layout write is in flight so a drag cannot + // race (and be overwritten by) the bulk save. + nodesDraggable={layoutBusy === null} snapToGrid={snapToGrid} snapGrid={[SNAP_GRID, SNAP_GRID]} nodesConnectable={data.canUpdate} @@ -771,7 +802,17 @@ function CanvasInner({ data }: { data: TechTreeData }) { {connectError} ) : null} - {saveStatus ? ( + {layoutBusy ? ( + + + {layoutBusy} + + ) : saveStatus ? ( - {arranging ? "Arranging..." : "Auto-arrange"} + Auto-arrange ) : null} {data.canCreate ? ( diff --git a/src/lib/tech-tree/positions.ts b/src/lib/tech-tree/positions.ts new file mode 100644 index 0000000..fe227c1 --- /dev/null +++ b/src/lib/tech-tree/positions.ts @@ -0,0 +1,81 @@ +import { sql } from "@payloadcms/db-postgres"; +import type { Payload } from "payload"; + +/** + * Bulk Tech Tree position writes. + * + * The canvas used to persist positions with one REST PATCH per technology, all + * fired at once. Auto-arrange or Save positions on a 60+ node tree therefore + * hit the server with 60+ concurrent full document updates (auth, permission + * lookups, populated responses each), which exhausted the database pool and + * could take the server down. Every canvas position write now goes through + * `writeTreePositions`: one request, one SQL statement. + * + * Only the `tree_position_x` / `tree_position_y` columns are touched. The + * technologies collection has no hooks, so bypassing the Local API loses + * nothing; `updated_at` is bumped to keep the documents honest. + */ + +/** Hard cap per request so a malformed client cannot build a giant statement. */ +export const MAX_POSITION_UPDATES = 2000; + +export type TreePositionUpdate = { + id: number; + /** Both numbers to pin a node, both null to return it to the automatic layout. */ + x: number | null; + y: number | null; +}; + +export type ParseResult = + | { ok: true; updates: TreePositionUpdate[] } + | { ok: false; error: string }; + +/** Validate a `{ positions: [...] }` request body. Pure. */ +export function parsePositionUpdates(body: unknown): ParseResult { + const positions = (body as { positions?: unknown } | null)?.positions; + if (!Array.isArray(positions)) return { ok: false, error: "positions must be an array." }; + if (positions.length > MAX_POSITION_UPDATES) { + return { ok: false, error: `At most ${MAX_POSITION_UPDATES} positions per request.` }; + } + + const byId = new Map(); + for (const entry of positions) { + const { id, x, y } = (entry ?? {}) as { id?: unknown; x?: unknown; y?: unknown }; + if (typeof id !== "number" || !Number.isInteger(id) || id <= 0) { + return { ok: false, error: "Each position needs a positive integer id." }; + } + const cleared = x === null && y === null; + const pinned = + typeof x === "number" && Number.isFinite(x) && typeof y === "number" && Number.isFinite(y); + if (!cleared && !pinned) { + return { ok: false, error: `Position for ${id} must have numeric x and y, or both null.` }; + } + // Last write wins for a repeated id. + byId.set( + id, + cleared + ? { id, x: null, y: null } + : { id, x: Math.round(x as number), y: Math.round(y as number) }, + ); + } + return { ok: true, updates: [...byId.values()] }; +} + +/** Write every position in a single UPDATE. Returns the number of rows touched. */ +export async function writeTreePositions( + payload: Payload, + updates: TreePositionUpdate[], +): Promise { + if (updates.length === 0) return 0; + const values = sql.join( + updates.map((u) => sql`(${u.id}::integer, ${u.x}::numeric, ${u.y}::numeric)`), + sql`, `, + ); + const result = await payload.db.drizzle.execute(sql` + UPDATE technologies AS t + SET tree_position_x = v.x, tree_position_y = v.y, updated_at = now() + FROM (VALUES ${values}) AS v(id, x, y) + WHERE t.id = v.id + `); + return (result as { rowCount?: number | null }).rowCount ?? 0; +} diff --git a/tests/e2e/tech-tree.e2e.spec.ts b/tests/e2e/tech-tree.e2e.spec.ts index 822c016..9ad7626 100644 --- a/tests/e2e/tech-tree.e2e.spec.ts +++ b/tests/e2e/tech-tree.e2e.spec.ts @@ -238,8 +238,22 @@ test.describe("Tech Tree admin view", () => { new RegExp(`translate\\(${stored.x}px,\\s*${stored.y}px\\)`), ); - // Auto-arrange clears the manual position. + // Auto-arrange clears the manual position with ONE bulk request (never a + // PATCH per node) and shows a busy indicator while it saves. The save is + // held briefly so the indicator is observable. + let perNodePatches = 0; + page.on("request", (req) => { + if (req.method() === "PATCH" && req.url().includes("/api/technologies/")) perNodePatches += 1; + }); + await page.route("**/api/tech-tree/positions", async (route) => { + await new Promise((resolve) => setTimeout(resolve, 800)); + await route.continue(); + }); await page.getByTestId("tech-tree-auto-arrange").click(); + await expect(page.getByTestId("tech-tree-layout-busy")).toBeVisible(); + await expect(page.getByTestId("tech-tree-layout-busy")).toHaveCount(0, { timeout: 15_000 }); + await page.unroute("**/api/tech-tree/positions"); + expect(perNodePatches).toBe(0); await expect .poll( async () => { diff --git a/tests/int/tech-tree-positions.int.spec.ts b/tests/int/tech-tree-positions.int.spec.ts new file mode 100644 index 0000000..cac338e --- /dev/null +++ b/tests/int/tech-tree-positions.int.spec.ts @@ -0,0 +1,137 @@ +import { getPayload, Payload } from "payload"; +import config from "@/payload.config"; + +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { + MAX_POSITION_UPDATES, + parsePositionUpdates, + writeTreePositions, +} from "@/lib/tech-tree/positions"; + +const RUN = `ttpos-${Date.now().toString(36)}`; +const TIMEOUT = 30_000; + +describe("tech tree position bulk-write validation", () => { + it("accepts pins and clears", () => { + const result = parsePositionUpdates({ + positions: [ + { id: 1, x: 10.4, y: 20.6 }, + { id: 2, x: null, y: null }, + ], + }); + expect(result).toEqual({ + ok: true, + updates: [ + { id: 1, x: 10, y: 21 }, + { id: 2, x: null, y: null }, + ], + }); + }); + + it("keeps the last write for a repeated id", () => { + const result = parsePositionUpdates({ + positions: [ + { id: 3, x: 1, y: 1 }, + { id: 3, x: null, y: null }, + ], + }); + expect(result).toEqual({ ok: true, updates: [{ id: 3, x: null, y: null }] }); + }); + + it("rejects malformed bodies", () => { + expect(parsePositionUpdates(null).ok).toBe(false); + expect(parsePositionUpdates({ positions: "nope" }).ok).toBe(false); + expect(parsePositionUpdates({ positions: [{ id: 0, x: 1, y: 1 }] }).ok).toBe(false); + expect(parsePositionUpdates({ positions: [{ id: 1.5, x: 1, y: 1 }] }).ok).toBe(false); + // Half-cleared and non-finite coordinates are both invalid. + expect(parsePositionUpdates({ positions: [{ id: 1, x: 5, y: null }] }).ok).toBe(false); + expect(parsePositionUpdates({ positions: [{ id: 1, x: Number.NaN, y: 1 }] }).ok).toBe(false); + }); + + it("caps the batch size", () => { + const positions = Array.from({ length: MAX_POSITION_UPDATES + 1 }, (_, i) => ({ + id: i + 1, + x: 0, + y: 0, + })); + expect(parsePositionUpdates({ positions }).ok).toBe(false); + }); +}); + +describe("tech tree position bulk write (database)", () => { + let payload: Payload; + const ids: number[] = []; + + beforeAll(async () => { + payload = await getPayload({ config }); + for (const label of ["a", "b", "c"]) { + const doc = await payload.create({ + collection: "technologies", + data: { + name: `${RUN}-${label}`, + summary: "Bulk position write fixture.", + type: "upgrade", + approvalStatus: "in_progress", + researchCosts: { minimumResearchDuration: 1 }, + }, + overrideAccess: true, + depth: 0, + }); + ids.push(doc.id); + } + }, TIMEOUT); + + afterAll(async () => { + for (const id of ids) { + await payload.delete({ collection: "technologies", id, overrideAccess: true }); + } + }, TIMEOUT); + + const positionOf = async (id: number) => { + const doc = await payload.findByID({ + collection: "technologies", + id, + overrideAccess: true, + depth: 0, + }); + return doc.treePosition ?? null; + }; + + it( + "pins many technologies in one write and clears them again", + async () => { + const pinned = await writeTreePositions(payload, [ + { id: ids[0], x: 46, y: 92 }, + { id: ids[1], x: 368, y: 184 }, + { id: ids[2], x: 0, y: 0 }, + ]); + expect(pinned).toBe(3); + expect(await positionOf(ids[0])).toMatchObject({ x: 46, y: 92 }); + expect(await positionOf(ids[1])).toMatchObject({ x: 368, y: 184 }); + expect(await positionOf(ids[2])).toMatchObject({ x: 0, y: 0 }); + + // Auto-arrange path: clear every pin in a single statement. + const cleared = await writeTreePositions( + payload, + ids.map((id) => ({ id, x: null, y: null })), + ); + expect(cleared).toBe(3); + for (const id of ids) { + const position = await positionOf(id); + expect(position?.x ?? null).toBeNull(); + expect(position?.y ?? null).toBeNull(); + } + }, + TIMEOUT, + ); + + it( + "ignores ids that do not exist", + async () => { + const touched = await writeTreePositions(payload, [{ id: 999_999_999, x: 1, y: 1 }]); + expect(touched).toBe(0); + }, + TIMEOUT, + ); +});