From b0986b6d47445de657ecbb58e0a48fd3bc4d82d3 Mon Sep 17 00:00:00 2001 From: AngryJay91 Date: Tue, 31 Mar 2026 11:34:30 +0900 Subject: [PATCH 1/2] fix: add optimistic locking to docs to prevent edit conflicts (#15) - Migration 009: ADD COLUMN version INTEGER DEFAULT 1 to docs table - PUT /api/v2/docs/:id: checks client version against DB, returns 409 if mismatch - GET /api/v2/docs/:id: returns version field in response - DocsEditor.vue: sends version on save, shows conflict warning on 409 Closes #15 --- scaffold/pm-api/sql/009-doc-versioning.sql | 6 +++ scaffold/pm-api/src/routes/v2-docs.ts | 56 ++++++++++++++++++--- scaffold/spec-site/src/pages/DocsEditor.vue | 35 +++++++++++-- 3 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 scaffold/pm-api/sql/009-doc-versioning.sql diff --git a/scaffold/pm-api/sql/009-doc-versioning.sql b/scaffold/pm-api/sql/009-doc-versioning.sql new file mode 100644 index 00000000..3206d3b9 --- /dev/null +++ b/scaffold/pm-api/sql/009-doc-versioning.sql @@ -0,0 +1,6 @@ +-- Migration 009: Add version column to docs for optimistic locking +-- Prevents silent data loss when multiple users edit the same document simultaneously. +-- Strategy: version-based (integer increment), not CRDT. +-- On save: WHERE version = ? + increment. If 0 rows updated → 409 Conflict. + +ALTER TABLE docs ADD COLUMN version INTEGER DEFAULT 1; diff --git a/scaffold/pm-api/src/routes/v2-docs.ts b/scaffold/pm-api/src/routes/v2-docs.ts index d7b6bf25..b3df55ae 100644 --- a/scaffold/pm-api/src/routes/v2-docs.ts +++ b/scaffold/pm-api/src/routes/v2-docs.ts @@ -10,7 +10,7 @@ app.get('/', async (c) => { return c.json({ docs: rows }) }) -// GET /:id - document detail +// GET /:id - document detail (includes version for optimistic locking) app.get('/:id', async (c) => { const id = c.req.param('id') const { rows } = await queryOrThrow('SELECT * FROM docs WHERE id = ?', [id]) @@ -18,17 +18,57 @@ app.get('/:id', async (c) => { return c.json({ doc: rows[0] }) }) -// PUT /:id - create/update document +// PUT /:id - create/update document with optimistic locking +// Body: { title, content, version? } +// - New doc (INSERT): version is ignored; server sets version = 1 +// - Existing doc (UPDATE): must supply version matching current row +// → 409 Conflict if version mismatch (someone else saved in between) app.put('/:id', async (c) => { const id = c.req.param('id') - const body = await c.req.json<{ title: string; content: string }>() + const body = await c.req.json<{ title: string; content: string; version?: number }>() const createdBy = c.get('userName') || 'unknown' - await executeOrThrow( - `INSERT INTO docs (id, title, content, created_by) VALUES (?, ?, ?, ?) - ON CONFLICT(id) DO UPDATE SET title = excluded.title, content = excluded.content, updated_at = CURRENT_TIMESTAMP`, - [id, body.title, body.content, createdBy], + + // Check if doc already exists + const { rows } = await queryOrThrow('SELECT version FROM docs WHERE id = ?', [id]) + + if (rows.length === 0) { + // New document — INSERT with version = 1 + await executeOrThrow( + `INSERT INTO docs (id, title, content, created_by, version) VALUES (?, ?, ?, ?, 1)`, + [id, body.title, body.content, createdBy], + ) + return c.json({ ok: true, version: 1 }) + } + + // Existing document — optimistic locking + const currentVersion = (rows[0] as { version: number }).version ?? 1 + const clientVersion = body.version + + if (clientVersion === undefined || clientVersion === null) { + // Legacy client without version support — allow save but warn + // (graceful degradation: skips conflict check) + await executeOrThrow( + `UPDATE docs SET title = ?, content = ?, updated_at = CURRENT_TIMESTAMP, version = version + 1 WHERE id = ?`, + [body.title, body.content, id], + ) + return c.json({ ok: true, version: currentVersion + 1 }) + } + + // Versioned save: only update if version matches + const result = await executeOrThrow( + `UPDATE docs SET title = ?, content = ?, updated_at = CURRENT_TIMESTAMP, version = version + 1 + WHERE id = ? AND version = ?`, + [body.title, body.content, id, clientVersion], ) - return c.json({ ok: true }) + + if (result.rowsAffected === 0) { + return c.json( + { error: 'Conflict: document was modified by someone else. Please refresh and try again.', code: 'VERSION_CONFLICT' }, + 409, + ) + } + + return c.json({ ok: true, version: currentVersion + 1 }) }) export default app diff --git a/scaffold/spec-site/src/pages/DocsEditor.vue b/scaffold/spec-site/src/pages/DocsEditor.vue index 3860caa4..4d1f3ca5 100644 --- a/scaffold/spec-site/src/pages/DocsEditor.vue +++ b/scaffold/spec-site/src/pages/DocsEditor.vue @@ -12,13 +12,16 @@ const isNew = computed(() => !docId.value) const title = ref('') const content = ref('') const saving = ref(false) +const docVersion = ref(undefined) +const conflictError = ref('') onMounted(async () => { if (docId.value) { - const { data } = await apiGet<{ doc: { title: string; content: string } }>(`/api/v2/docs/${docId.value}`) + const { data } = await apiGet<{ doc: { title: string; content: string; version?: number } }>(`/api/v2/docs/${docId.value}`) if (data?.doc) { title.value = data.doc.title content.value = data.doc.content + docVersion.value = data.doc.version } } }) @@ -30,14 +33,28 @@ function generateId(title: string): string { async function save() { if (!title.value.trim()) { alert('Please enter a title'); return } saving.value = true + conflictError.value = '' const id = docId.value || generateId(title.value) - const { error } = await apiPut(`/api/v2/docs/${id}`, { title: title.value, content: content.value }) + const { data, error } = await apiPut<{ ok: boolean; version?: number }>(`/api/v2/docs/${id}`, { + title: title.value, + content: content.value, + version: docVersion.value, + }) saving.value = false + + if (error?.includes('HTTP 409') || error?.includes('VERSION_CONFLICT')) { + conflictError.value = 'Someone else edited this document while you were working. Please refresh the page to get the latest version, then re-apply your changes.' + return + } if (error) { alert(error); return } + + // Update local version from server response + if (data?.version !== undefined) { + docVersion.value = data.version + } + router.push(`/docs/${id}`) } - -// renderMarkdown imported from @/utils/markdown