From 38593f6f67a508b4cd4bbb676ea2377fbf4e90f6 Mon Sep 17 00:00:00 2001 From: que01 Date: Thu, 27 Aug 2026 18:11:52 +0800 Subject: [PATCH] feat: save, undo and redo direct edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 5 of the map editor. EditSession becomes the single owner of the constraint set, replacing the pair of refs step 4 kept alongside it: it now holds the saved operations too, and `fragment()` assembles the constraints plus every operation they reference — the shape both preview and save send. That removes the class of bug that produced the earlier 400, because callers can no longer ship a constraint whose provenance points at nothing. Save goes through `expectedDocumentVersion`. A 409 adopts the server's version so the next attempt is checked against reality, and says which version won instead of failing silently. Undo of a saved gesture appends an inverse operation rather than rewriting persisted history; undo of an unsaved one just moves the cursor. Discard drops unsaved commands, cancels anything in flight, and returns the map to the baseline. One gesture mints one operation id, released on pointerup. Reusing an id across gestures put two operations with the same id in the document, which validateEditDocument() rejects. The API test now covers the payload the client actually sends — `constraints` + `operations` merged into the active document, rather than a whole document — and asserts that a save survives a reload as `exact`. Sending constraints without their operations is asserted to be rejected rather than written. Co-Authored-By: Claude Opus 5 (1M context) --- test/workbench-edit-api.js | 47 ++++++ workbench/client/src/App.tsx | 165 +++++++++++++++++----- workbench/client/src/edit/session.test.ts | 69 ++++++++- workbench/client/src/edit/session.ts | 41 +++++- 4 files changed, 280 insertions(+), 42 deletions(-) diff --git a/test/workbench-edit-api.js b/test/workbench-edit-api.js index c7a710c..ee01d9b 100644 --- a/test/workbench-edit-api.js +++ b/test/workbench-edit-api.js @@ -72,5 +72,52 @@ const rebased = rebaseEdits(area, checkpoint.manifest.id); assert.equal(rebased.counts.exact, 0); assert.deepEqual(rebased.constraints, []); +// The map saves `constraints` + `operations`, not a whole document. That merge path +// is what the client actually exercises, and leaving the operations out is exactly +// how the first wiring produced a 400. +const edgeHandle = state.handles.handles.find((handle) => handle.kind === 'road-edge-offset'); +assert.ok(edgeHandle, 'the fixture must publish a road edge handle'); +const clientConstraint = { + id: `constraint:${edgeHandle.handleId}`, + kind: 'road-edge-offset', + anchor: edgeHandle.anchor, + anchorSnapshot: { + coordinate: edgeHandle.position, + tangentAzimuth: 0, + roadLengthMeters: 120, + osmNodeIds: ['1'], + }, + value: { offsetMeters: 0.5, transition: 'smoothstep' }, + enabled: true, + status: 'exact', + provenance: { operationId: 'operation:client:1', createdAt: '2026-08-27T00:00:00.000Z' }, +}; +const clientOperation = { + id: clientConstraint.provenance.operationId, + createdAt: clientConstraint.provenance.createdAt, + constraintIds: [clientConstraint.id], +}; +assert.throws( + () => saveEdits(area, { expectedDocumentVersion: 1, constraints: [clientConstraint], operations: [] }), + /provenance\.operationId/, + 'a constraint saved without its operation must be rejected, not written', +); +const clientSaved = saveEdits(area, { + expectedDocumentVersion: 1, + constraints: [clientConstraint], + operations: [clientOperation], +}); +assert.equal(clientSaved.documentVersion, 2); + +// A save that does not survive a reload as `exact` was cosmetic. +const reloaded = editState(area); +assert.equal(reloaded.documentVersion, 2); +assert.deepEqual( + reloaded.document.constraints.map((item) => item.id), + [clientConstraint.id], +); +assert.equal(reloaded.constraintStates[0].status, 'exact'); +assert.equal(reloaded.constraintStates[0].applied, true); + fs.rmSync(workspace, { recursive: true, force: true }); console.log('workbench edit API tests passed'); diff --git a/workbench/client/src/App.tsx b/workbench/client/src/App.tsx index 4f606e6..48e1a0c 100644 --- a/workbench/client/src/App.tsx +++ b/workbench/client/src/App.tsx @@ -1,6 +1,6 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import { Download, RefreshCw, Save, Upload } from 'lucide-react'; -import { api } from './lib/api'; +import { api, isVersionConflict } from './lib/api'; import { MapCanvas } from './components/MapCanvas'; import { directEditEnabled, intervalEditingSupported } from './edit/flag'; import { PreviewRequester, type PreviewDraft } from './edit/preview-request'; @@ -15,6 +15,7 @@ import type { RoadConstraint, RoadEditOperation, RoadIntervalAnchor, + VersionConflict, } from './edit/types'; import { Button } from './ui/button'; import type { GeoFeature, GeoJson, Override, Road, WorkbenchState } from './types/state'; @@ -112,10 +113,10 @@ function App() { .then((value) => { if (!('handles' in value)) return; setManifest(value.handles); - // A drag drafts on top of whatever is already saved, so keep the active - // document's constraints rather than sending a lone constraint. - savedConstraints.current = value.document.constraints; - savedOperations.current = value.document.operations; + // The session owns the constraint set from here on, so a drag drafts on top + // of the active document instead of sending a lone constraint. + sessionRef.current?.load(value.document.constraints, value.document.operations, value.documentVersion); + touch(); }) // A manifest failure must not take the workbench down with it; the map just // shows no handles. @@ -149,19 +150,32 @@ function App() { }, [editHandles, manifest, selected, state]); const [preview, setPreview] = useState> | null>(null); const [editDiagnostics, setEditDiagnostics] = useState([]); - // The working set: the saved document plus whatever this session has drafted. - // Advanced on pointerup, not on every move, so an abandoned drag leaves nothing - // behind — and so a later range drag can re-anchor the edit a value drag made. - const savedConstraints = useRef([]); - /** Their operations. A constraint without its operation is rejected as a 400. */ - const savedOperations = useRef([]); const sessionRef = useRef(null); if (directEditEnabled && !sessionRef.current) sessionRef.current = new EditSession(); + // The session is a mutable object in a ref, so React cannot see it change. + // Bumping this after every mutation is what keeps the buttons in step with it. + const [editRevision, setEditRevision] = useState(0); + const touch = () => setEditRevision((value) => value + 1); + const session = sessionRef.current; + /** + * One gesture, one operation id. Minted on the first draft of a drag and cleared + * on release: reusing an id across gestures would put two operations with the + * same id in the document, which `validateEditDocument()` rejects outright. + */ + const gesture = useRef<{ operationId: string; createdAt: string } | null>(null); + const gestureIdentity = () => { + if (!gesture.current) + gesture.current = { + operationId: `operation:${editRevision}:${Date.now()}`, + createdAt: new Date().toISOString(), + }; + return gesture.current; + }; const requester = useMemo(() => { - const session = sessionRef.current; - if (!session) return null; + const owner = sessionRef.current; + if (!owner) return null; return new PreviewRequester({ - session, + session: owner, send: (request, signal) => api.editPreview(request, signal), onSettled: (outcome, response) => { setEditDiagnostics(response.diagnostics); @@ -175,31 +189,28 @@ function App() { onError: (error) => setStatus(`预览失败：${(error as Error).message}`), }); }, []); - /** A drag becomes one drafted constraint layered over the saved document. */ + /** A drag becomes one drafted constraint layered over the session's document. */ const draftFor = (handle: EditHandle, value: number): PreviewDraft | null => { const roadId = 'roadId' in handle.anchor ? handle.anchor.roadId : null; const road = roadId ? state?.compiled.model.roads.find((item) => item.id === roadId) : undefined; // Without a centerline there is no snapshot to record, so drop the drag rather // than send a constraint the server would have to reject. - if (!road || road.centerline.length < 2) return null; + if (!session || !road || road.centerline.length < 2) return null; const constraint = draftConstraint( handle, handle.anchor, constraintValueFor(handle, value), anchorSnapshotFor(handle, road.centerline, road.sourceNodeIds), - { - constraintId: `constraint:${handle.handleId}`, - operationId: `operation:${handle.handleId}`, - createdAt: new Date().toISOString(), - }, + { constraintId: `constraint:${handle.handleId}`, ...gestureIdentity() }, ); const operation = operationFor(constraint); + const base = session.fragment(); // Both halves travel: validateEditDocument() rejects a constraint whose // provenance.operationId is not a recorded operation, and that check covers the // already-saved constraints too, not just the one being dragged. return { - constraints: [...savedConstraints.current.filter((item) => item.id !== constraint.id), constraint], - operations: [...savedOperations.current.filter((item) => item.id !== operation.id), operation], + constraints: [...base.constraints.filter((item) => item.id !== constraint.id), constraint], + operations: [...base.operations.filter((item) => item.id !== operation.id), operation], }; }; /** @@ -208,23 +219,86 @@ function App() { * constraint on it. With no constraint yet there is nothing to re-anchor. */ const rangeDraftFor = (range: IntervalRangeHandle, anchor: RoadIntervalAnchor): PreviewDraft | null => { + const base = session?.fragment(); const onRoad = (item: RoadConstraint) => item.anchor.type === 'road-interval' && item.anchor.roadId === range.roadId; - if (!savedConstraints.current.some(onRoad)) return null; + if (!base || !base.constraints.some(onRoad)) return null; return { - constraints: savedConstraints.current.map((item) => + constraints: base.constraints.map((item) => onRoad(item) ? { ...item, anchor: { ...item.anchor, startStation: anchor.startStation, endStation: anchor.endStation } } : item, ), - operations: savedOperations.current, + operations: base.operations, }; }; - /** Both drag kinds commit their result here so the next drag builds on it. */ - const advance = (draft: PreviewDraft | null) => { - if (!draft) return; - savedConstraints.current = draft.constraints ?? savedConstraints.current; - savedOperations.current = draft.operations ?? savedOperations.current; + /** + * Pointerup closes the gesture: one undoable command, then the id is released so + * the next drag mints its own. An abandoned drag commits nothing. + */ + const commitGesture = (draft: PreviewDraft | null, constraintIds: string[]) => { + if (!draft || !session) return; + requester?.flush(draft); + session.commit(draft.constraints ?? [], constraintIds, gestureIdentity()); + gesture.current = null; + touch(); + }; + /** Re-previews whatever the session now holds, after undo/redo moved the cursor. */ + const previewSession = () => { + const fragment = session?.fragment(); + if (fragment) requester?.flush(fragment); + }; + const undoEdit = () => { + // A saved gesture cannot be un-applied — persisted history is append-only — so + // it is reversed by an inverse operation, which needs its own identity. + const reversed = session?.undo({ + operationId: `operation:undo:${Date.now()}`, + createdAt: new Date().toISOString(), + }); + if (!reversed) return; + touch(); + previewSession(); + }; + const redoEdit = () => { + if (!session?.redo()) return; + touch(); + previewSession(); + }; + const discardEdits = () => { + if (!session) return; + requester?.cancel(); + session.discard(); + gesture.current = null; + touch(); + setPreview(null); + setEditDiagnostics([]); + setStatus('已放弃未保存的直接编辑'); + }; + const saveDirectEdits = async () => { + if (!session) return; + const fragment = session.fragment(); + try { + const result = await api.saveEdits({ + expectedDocumentVersion: session.documentVersion, + constraints: fragment.constraints, + operations: fragment.operations, + }); + session.markSaved(result.documentVersion); + touch(); + setStatus(`直接编辑已保存，文档版本 ${result.documentVersion}`); + } catch (error) { + if (!isVersionConflict(error)) { + setStatus(`保存失败：${(error as Error).message}`); + return; + } + // Single-writer protection fired: another tab moved the document. Adopt the + // server's version so the next attempt is checked against reality, and say so + // rather than failing silently. + const current = (error.body as VersionConflict | null)?.current?.documentVersion; + if (Number.isInteger(current)) session.setDocumentVersion(current as number); + touch(); + setStatus(`保存冲突：文档已被改到版本 ${current ?? '未知'}，请刷新后重做本次编辑。`); + } }; const stage = (change: Override) => setStaged((current) => [...current.filter((item) => item.id !== change.id), change]); @@ -312,6 +386,22 @@ function App() { + {directEditEnabled ? ( + <> + + + + + + ) : null}