fix(desktop): prevent false autosave conflicts
This commit is contained in:
1 parent
19b24113a4
commit
e07b5c3217
5 files changed
+105
-9
No files matched your search
@@ -2088,9 +2088,33 @@ const RichEditorPane = ({
|
||||
|
||||
useEffect(() => {
|
||||
const handleSyncCompleted = (event: Event) => {
|
||||
const result = (event as CustomEvent<{ failed?: number; conflicted?: number }>).detail;
|
||||
const result = (event as CustomEvent<{
|
||||
failed?: number;
|
||||
conflicted?: number;
|
||||
syncedMemos?: ReadonlyMap<string, MemoDetail>;
|
||||
}>).detail;
|
||||
const memoId = memoRef.current?.id;
|
||||
|
||||
const syncedMemo = memoId ? result?.syncedMemos?.get(memoId) : null;
|
||||
if (syncedMemo && memoRef.current?.id === syncedMemo.id) {
|
||||
// Keep the live editor document, title, and tags intact while moving
|
||||
// its concurrency base to the revision just acknowledged by this
|
||||
// device. This closes the query-refresh window after an autosave.
|
||||
memoRef.current = {
|
||||
...memoRef.current,
|
||||
revision: syncedMemo.revision,
|
||||
contentHash: syncedMemo.contentHash,
|
||||
updatedAt: syncedMemo.updatedAt,
|
||||
};
|
||||
if (editSessionRef.current?.memoId === syncedMemo.id) {
|
||||
editSessionRef.current = {
|
||||
...editSessionRef.current,
|
||||
baseRevision: syncedMemo.revision,
|
||||
baseContentHash: syncedMemo.contentHash,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
if (memoId && (result?.conflicted ?? 0) > 0) {
|
||||
void localDb.syncQueue.get(getMemoUpdateQueueId(memoId)).then((item) => {
|
||||
if (!item || item.status !== "conflict" || memoRef.current?.id !== memoId) {
|
||||
|
||||
@@ -1,6 +1,13 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
|
||||
const { isStagedResourceReferenced, mergeMemoIdMappings, orderBootstrapNotebooks, rewriteStagedResource } = await import("./desktop-sync.ts");
|
||||
const {
|
||||
isStagedResourceReferenced,
|
||||
mergeMemoIdMappings,
|
||||
mergeSyncedMemos,
|
||||
orderBootstrapNotebooks,
|
||||
resolveDesktopMemoSyncBase,
|
||||
rewriteStagedResource,
|
||||
} = await import("./desktop-sync.ts");
|
||||
|
||||
describe("desktop staged resource sync", () => {
|
||||
test("rewrites placeholders in memo JSON and markdown", () => {
|
||||
@@ -39,6 +46,17 @@ describe("desktop staged resource sync", () => {
|
||||
|
||||
expect(retained.get("memo_local_1")).toBe("memo_remote_1");
|
||||
});
|
||||
|
||||
test("keeps the latest acknowledged memo base across sync phases", () => {
|
||||
const created = { id: "memo_remote_1", revision: 0 };
|
||||
const updated = { id: "memo_remote_1", revision: 1 };
|
||||
const retained = mergeSyncedMemos(
|
||||
new Map([[created.id, created]]),
|
||||
new Map([[updated.id, updated]]),
|
||||
);
|
||||
|
||||
expect(retained.get("memo_remote_1")).toEqual(updated);
|
||||
});
|
||||
});
|
||||
|
||||
describe("desktop bootstrap sync", () => {
|
||||
@@ -54,3 +72,19 @@ describe("desktop bootstrap sync", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("desktop memo sync base", () => {
|
||||
test("repairs a legacy local autosave revision that is ahead of the cloud", () => {
|
||||
expect(resolveDesktopMemoSyncBase(
|
||||
{ revision: 3, contentHash: "cloud-3" },
|
||||
{ expectedRevision: 9, expectedContentHash: "local-autosave-9" },
|
||||
)).toEqual({ expectedRevision: 3, expectedContentHash: "cloud-3" });
|
||||
});
|
||||
|
||||
test("keeps a genuinely stale base so the server update remains protected", () => {
|
||||
expect(resolveDesktopMemoSyncBase(
|
||||
{ revision: 9, contentHash: "cloud-9" },
|
||||
{ expectedRevision: 3, expectedContentHash: "cloud-3" },
|
||||
)).toEqual({ expectedRevision: 3, expectedContentHash: "cloud-3" });
|
||||
});
|
||||
});
|
||||
@@ -164,6 +164,24 @@ const applyRemoteNotebook = async (notebook: DesktopRpcResponses["notebook.list"
|
||||
});
|
||||
};
|
||||
|
||||
export const resolveDesktopMemoSyncBase = (
|
||||
current: { revision: number; contentHash: string },
|
||||
expected: { expectedRevision: number; expectedContentHash: string },
|
||||
) => {
|
||||
// Older desktop clients advanced the cached cloud revision for every local
|
||||
// autosave. A base ahead of the actual server is therefore a local counter,
|
||||
// not evidence of a concurrent remote edit. Rebase that impossible state so
|
||||
// existing drafts recover automatically. Bases behind the server remain
|
||||
// conflicts and keep the normal adopt-cloud/copy-draft protection.
|
||||
if (expected.expectedRevision > current.revision) {
|
||||
return {
|
||||
expectedRevision: current.revision,
|
||||
expectedContentHash: current.contentHash,
|
||||
};
|
||||
}
|
||||
return expected;
|
||||
};
|
||||
|
||||
const acknowledge = async (
|
||||
item: DesktopOutboxItem,
|
||||
remoteMemo?: DesktopRpcResponses["memo.get"]["memo"],
|
||||
@@ -196,11 +214,12 @@ const syncOutboxItem = async (item: DesktopOutboxItem, stagedRewrites: StagedRes
|
||||
const memoId = String(payload.memoId ?? item.entityId);
|
||||
const editSessionResponse = await api.createMemoEditSession(memoId);
|
||||
const editSession = editSessionResponse.editSession;
|
||||
const expectedBase = {
|
||||
const queuedBase = {
|
||||
expectedRevision: Number(payload.expectedRevision),
|
||||
expectedContentHash: String(payload.expectedContentHash ?? ""),
|
||||
};
|
||||
const currentBase = { revision: editSession.baseRevision, contentHash: editSession.baseContentHash };
|
||||
const expectedBase = resolveDesktopMemoSyncBase(currentBase, queuedBase);
|
||||
if (!isMemoSyncBaseCurrent(currentBase, expectedBase)) {
|
||||
throw new ApiRequestError(
|
||||
"Note changed before the offline draft could sync.",
|
||||
@@ -210,8 +229,8 @@ const syncOutboxItem = async (item: DesktopOutboxItem, stagedRewrites: StagedRes
|
||||
);
|
||||
}
|
||||
const data = await api.updateMemo(memoId, {
|
||||
expectedRevision: Number(payload.expectedRevision),
|
||||
expectedContentHash: String(payload.expectedContentHash ?? ""),
|
||||
expectedRevision: expectedBase.expectedRevision,
|
||||
expectedContentHash: expectedBase.expectedContentHash,
|
||||
editSessionId: editSession.id,
|
||||
title: String(payload.title ?? ""),
|
||||
contentJson: payload.contentJson as TiptapDoc,
|
||||
@@ -354,6 +373,7 @@ const syncOutbox = async (stagedRewrites: StagedResourceRewrite[], onlyKinds?: S
|
||||
let conflicted = 0;
|
||||
const response = await request("sync.outbox.list", { limit: 100 });
|
||||
const memoIdMappings = new Map<string, string>();
|
||||
const syncedMemos = new Map<string, DesktopRpcResponses["memo.get"]["memo"]>();
|
||||
for (const item of response.items) {
|
||||
if (onlyKinds && !onlyKinds.has(item.kind)) continue;
|
||||
try {
|
||||
@@ -361,6 +381,9 @@ const syncOutbox = async (stagedRewrites: StagedResourceRewrite[], onlyKinds?: S
|
||||
if (item.kind === "memo.create" && result && typeof result === "object" && "id" in result && typeof result.id === "string") {
|
||||
memoIdMappings.set(item.entityId, result.id);
|
||||
}
|
||||
if (result && typeof result === "object" && "id" in result && typeof result.id === "string" && "contentJson" in result) {
|
||||
syncedMemos.set(result.id, result as DesktopRpcResponses["memo.get"]["memo"]);
|
||||
}
|
||||
synced += 1;
|
||||
} catch (error) {
|
||||
const conflict = error instanceof ApiRequestError
|
||||
@@ -370,7 +393,7 @@ const syncOutbox = async (stagedRewrites: StagedResourceRewrite[], onlyKinds?: S
|
||||
else failed += 1;
|
||||
}
|
||||
}
|
||||
return { attempted: onlyKinds ? synced + failed + conflicted : response.items.length, synced, failed, conflicted, memoIdMappings };
|
||||
return { attempted: onlyKinds ? synced + failed + conflicted : response.items.length, synced, failed, conflicted, memoIdMappings, syncedMemos };
|
||||
};
|
||||
|
||||
const applyBootstrap = async (page: SyncBootstrapResponse) => {
|
||||
@@ -437,6 +460,7 @@ export type DesktopSyncResult = {
|
||||
failed: number;
|
||||
conflicted: number;
|
||||
memoIdMappings: Map<string, string>;
|
||||
syncedMemos: Map<string, DesktopRpcResponses["memo.get"]["memo"]>;
|
||||
};
|
||||
|
||||
export const mergeMemoIdMappings = (
|
||||
@@ -447,22 +471,30 @@ export const mergeMemoIdMappings = (
|
||||
return target;
|
||||
};
|
||||
|
||||
export const mergeSyncedMemos = <T>(target: Map<string, T>, source: ReadonlyMap<string, T>) => {
|
||||
for (const [memoId, memo] of source) target.set(memoId, memo);
|
||||
return target;
|
||||
};
|
||||
|
||||
let activeSync: Promise<DesktopSyncResult> | null = null;
|
||||
|
||||
export const syncDesktopData = () => {
|
||||
if (activeSync) return activeSync;
|
||||
activeSync = (async () => {
|
||||
const memoIdMappings = new Map<string, string>();
|
||||
const syncedMemos = new Map<string, DesktopRpcResponses["memo.get"]["memo"]>();
|
||||
if (typeof navigator !== "undefined" && !navigator.onLine) {
|
||||
return { attempted: 0, synced: 0, failed: 0, conflicted: 0, memoIdMappings };
|
||||
return { attempted: 0, synced: 0, failed: 0, conflicted: 0, memoIdMappings, syncedMemos };
|
||||
}
|
||||
try {
|
||||
const creates = await syncOutbox([], new Set(["memo.create"]));
|
||||
mergeMemoIdMappings(memoIdMappings, creates.memoIdMappings);
|
||||
mergeSyncedMemos(syncedMemos, creates.syncedMemos);
|
||||
await remapStagedResourceMemoIds(creates.memoIdMappings);
|
||||
const stagedResources = await syncStagedResources(creates.memoIdMappings);
|
||||
const outbox = await syncOutbox(stagedResources.rewrites);
|
||||
mergeMemoIdMappings(memoIdMappings, outbox.memoIdMappings);
|
||||
mergeSyncedMemos(syncedMemos, outbox.syncedMemos);
|
||||
if (stagedResources.failed === 0 && outbox.failed === 0 && outbox.conflicted === 0 && creates.conflicted === 0) {
|
||||
await removeSyncedStagedResources(stagedResources.stagedIds);
|
||||
}
|
||||
@@ -476,6 +508,7 @@ export const syncDesktopData = () => {
|
||||
failed: creates.failed + outbox.failed + stagedResources.failed,
|
||||
conflicted: creates.conflicted + outbox.conflicted,
|
||||
memoIdMappings,
|
||||
syncedMemos,
|
||||
};
|
||||
} catch (error) {
|
||||
lastSyncFailed = true;
|
||||
@@ -483,7 +516,7 @@ export const syncDesktopData = () => {
|
||||
// A create may already have been acknowledged before a later upload or
|
||||
// pull failed. Preserve its id mapping so the UI cannot keep editing an
|
||||
// obsolete temporary id merely because the overall sync was partial.
|
||||
return { attempted: 0, synced: 0, failed: 1, conflicted: 0, memoIdMappings };
|
||||
return { attempted: 0, synced: 0, failed: 1, conflicted: 0, memoIdMappings, syncedMemos };
|
||||
}
|
||||
})().finally(() => { activeSync = null; });
|
||||
return activeSync;
|
||||
|
||||
@@ -1017,7 +1017,11 @@ fn update_memo(database: &Connection, params: &Value) -> Result<Value, String> {
|
||||
if changed == 0 {
|
||||
return Err(format!("Memo not found or deleted: {memo_id}"));
|
||||
}
|
||||
tx.execute("UPDATE memo_contents SET content_json = ?2, content_markdown = ?3, content_text = ?4, content_hash = ?5, revision = revision + 1, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE memo_id = ?1", rusqlite::params![memo_id, content_json_text, markdown, text, hash]).map_err(|e| e.to_string())?;
|
||||
// `revision` is the last acknowledged cloud revision, not a counter for
|
||||
// local autosaves. Advancing it here makes several saves on one device
|
||||
// look newer than the cloud and produces a false revision conflict once
|
||||
// the coalesced outbox item is synced.
|
||||
tx.execute("UPDATE memo_contents SET content_json = ?2, content_markdown = ?3, content_text = ?4, content_hash = ?5, updated_at = strftime('%Y-%m-%dT%H:%M:%fZ','now') WHERE memo_id = ?1", rusqlite::params![memo_id, content_json_text, markdown, text, hash]).map_err(|e| e.to_string())?;
|
||||
tx.commit().map_err(|e| e.to_string())?;
|
||||
enqueue_change(
|
||||
database,
|
||||
|
||||
@@ -107,6 +107,7 @@ await request("memo.update", updatePayload("latest autosave"));
|
||||
const coalescedUpdates = (await request("sync.outbox.list", { limit: 200 })).items.filter((item) => item.kind === "memo.update" && item.entityId === coalesced.memo.id);
|
||||
assert.equal(coalescedUpdates.length, 1, "offline autosaves should coalesce into one sidecar outbox item");
|
||||
assert.equal(coalescedUpdates[0].payload.contentMarkdown, "latest autosave");
|
||||
assert.equal((await request("memo.get", { memoId: coalesced.memo.id })).memo.revision, 0, "local autosaves must not advance the acknowledged cloud revision");
|
||||
|
||||
const batchNotebook = (await request("notebook.create", { name: "Batch destination" })).notebook;
|
||||
const batchFirst = (await request("memo.create", { notebookId: inbox.id, title: "Batch one", contentMarkdown: "batch one", tags: ["batch-tag"] })).memo;
|
||||
|
||||
Reference in new issue
Block a user