fix(sync): reconcile merged memo placeholders
This commit is contained in:
1 parent
0aff87e2ef
commit
9f458c0aa3
4 files changed
+111
-5
No files matched your search
@@ -5,6 +5,7 @@ import {
|
||||
acquireMaintenanceLease,
|
||||
createMemoEditSession,
|
||||
fetchEdgeEverApp,
|
||||
mergeMemosRecord,
|
||||
updateMemoRecord,
|
||||
} from "./index.ts";
|
||||
|
||||
@@ -83,6 +84,31 @@ const getSeedMemo = (sqlite) => sqlite.query(
|
||||
).get();
|
||||
|
||||
describe("database write optimizations", () => {
|
||||
test("returns the completed merge when the same source set is retried", async () => {
|
||||
const { database, sqlite } = createDatabase();
|
||||
const first = getSeedMemo(sqlite);
|
||||
const secondId = "memo_merge_retry_source";
|
||||
sqlite.query(
|
||||
`INSERT INTO memos (id, workspace_id, notebook_id, title, excerpt, tags_json, created_by, updated_by)
|
||||
VALUES (?, ?, ?, 'Retry source', 'retry body', '[]', 'user', 'user')`,
|
||||
).run(secondId, first.workspace_id, first.notebook_id);
|
||||
sqlite.query(
|
||||
`INSERT INTO memo_contents (memo_id, content_json, content_markdown, content_text, content_hash)
|
||||
VALUES (?, '{"type":"doc","content":[]}', 'retry body', 'retry body', 'retry-hash')`,
|
||||
).run(secondId);
|
||||
|
||||
const input = { memoIds: [first.id, secondId], title: "Idempotent merge" };
|
||||
const actor = { actorType: "user", actorId: "user_owner" };
|
||||
const completed = await mergeMemosRecord(database, first.workspace_id, input, actor, "owner");
|
||||
const retried = await mergeMemosRecord(database, first.workspace_id, input, actor, "owner");
|
||||
|
||||
expect(retried.id).toBe(completed.id);
|
||||
expect(retried.sourceMemoIds).toEqual(input.memoIds);
|
||||
expect(sqlite.query("SELECT COUNT(*) AS count FROM memos WHERE is_deleted = 0 AND title = 'Idempotent merge'").get().count).toBe(1);
|
||||
expect(sqlite.query("SELECT COUNT(*) AS count FROM audit_events WHERE action = 'memo.merge' AND entity_id = ?").get(completed.id).count).toBe(1);
|
||||
sqlite.close();
|
||||
});
|
||||
|
||||
test("reuses a matching live edit session without another insert", async () => {
|
||||
const { database, sqlite } = createDatabase();
|
||||
const memo = getSeedMemo(sqlite);
|
||||
|
||||
+29
-2
@@ -2922,7 +2922,7 @@ const moveMemosRecord = async (
|
||||
return moveMemosToNotebook(db, workspaceId, memoIds, notebookId, actor, actorLabel);
|
||||
};
|
||||
|
||||
const mergeMemosRecord = async (
|
||||
export const mergeMemosRecord = async (
|
||||
db: D1Database,
|
||||
workspaceId: string,
|
||||
input: { memoIds: string[]; notebookId?: string; title?: string },
|
||||
@@ -2944,7 +2944,7 @@ const mergeMemosRecord = async (
|
||||
m.source_memo_ids, m.merge_source_count, m.merged_into_memo_id
|
||||
FROM memos m
|
||||
INNER JOIN memo_contents c ON c.memo_id = m.id
|
||||
WHERE m.workspace_id = ? AND m.is_deleted = 0 AND m.id IN (${placeholders})`
|
||||
WHERE m.workspace_id = ? AND m.id IN (${placeholders})`
|
||||
)
|
||||
.bind(workspaceId, ...uniqueMemoIds)
|
||||
.all<MemoDetailRow>();
|
||||
@@ -2953,6 +2953,33 @@ const mergeMemosRecord = async (
|
||||
throw new AppError("missing_memos", "One or more memos cannot be merged.", 400);
|
||||
}
|
||||
|
||||
const activeRows = rows.results.filter((row) => !row.is_deleted);
|
||||
if (activeRows.length !== uniqueMemoIds.length) {
|
||||
// A desktop outbox retry can arrive after the first merge committed but
|
||||
// before the client acknowledged its local placeholder. When every source
|
||||
// now points at the same live merge result, return that result instead of
|
||||
// treating the retry as a new merge. This makes the operation recoverable
|
||||
// after a lost response without ever creating a second server-side memo.
|
||||
const mergedTargetIds = new Set(
|
||||
rows.results
|
||||
.filter((row) => row.is_deleted && row.merged_into_memo_id)
|
||||
.map((row) => row.merged_into_memo_id as string),
|
||||
);
|
||||
if (activeRows.length === 0 && mergedTargetIds.size === 1) {
|
||||
const [mergedTargetId] = mergedTargetIds;
|
||||
const completedMerge = await getMemoDetail(db, workspaceId, mergedTargetId);
|
||||
const completedSourceIds = new Set(completedMerge?.sourceMemoIds ?? []);
|
||||
if (
|
||||
completedMerge
|
||||
&& completedMerge.sourceMemoIds.length === uniqueMemoIds.length
|
||||
&& uniqueMemoIds.every((memoId) => completedSourceIds.has(memoId))
|
||||
) {
|
||||
return completedMerge;
|
||||
}
|
||||
}
|
||||
throw new AppError("missing_memos", "One or more memos cannot be merged.", 400);
|
||||
}
|
||||
|
||||
if (input.notebookId && !(await getNotebook(db, workspaceId, input.notebookId))) {
|
||||
throw new AppError("not_found", "Target notebook not found", 404);
|
||||
}
|
||||
|
||||
@@ -1287,9 +1287,35 @@ fn sync_outbox_ack(database: &Connection, params: &Value) -> Result<Value, Strin
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
if remote_exists {
|
||||
database
|
||||
.execute("DELETE FROM memos WHERE id = ?1", [&entity_id])
|
||||
// `acknowledge` caches the remote memo before acknowledging the
|
||||
// outbox item, so the remote id normally already exists here.
|
||||
// A merged local memo can still own resources moved from its
|
||||
// sources. Deleting it directly is rejected by the resources
|
||||
// foreign key and leaves both the local placeholder and remote
|
||||
// memo visible forever. Repoint every surviving relationship
|
||||
// before removing the placeholder, and keep the reconciliation
|
||||
// atomic so a crash cannot strand a partially remapped merge.
|
||||
let tx = database
|
||||
.unchecked_transaction()
|
||||
.map_err(|e| e.to_string())?;
|
||||
tx.execute(
|
||||
"UPDATE resources SET memo_id = ?1 WHERE memo_id = ?2",
|
||||
rusqlite::params![remote_id, entity_id],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
tx.execute(
|
||||
"UPDATE resources SET original_memo_id = ?1 WHERE original_memo_id = ?2",
|
||||
rusqlite::params![remote_id, entity_id],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
tx.execute(
|
||||
"UPDATE memos SET merged_into_memo_id = ?1 WHERE merged_into_memo_id = ?2",
|
||||
rusqlite::params![remote_id, entity_id],
|
||||
)
|
||||
.map_err(|e| e.to_string())?;
|
||||
tx.execute("DELETE FROM memos WHERE id = ?1", [&entity_id])
|
||||
.map_err(|e| e.to_string())?;
|
||||
tx.commit().map_err(|e| e.to_string())?;
|
||||
} else {
|
||||
database
|
||||
.execute(
|
||||
|
||||
@@ -125,10 +125,37 @@ assert.equal((await request("memo.list", { notebookId: batchNotebook.id, limit:
|
||||
assert.equal((await request("memo.emptyTrash")).deleted, 1);
|
||||
assert.equal((await request("memo.list", { trash: true, notebookId: batchNotebook.id, limit: 20 })).totalCount, 0);
|
||||
|
||||
await request("resource.cache", {
|
||||
resource: {
|
||||
id: "resource-from-merge-source",
|
||||
memoId: first.memo.id,
|
||||
originalMemoId: null,
|
||||
kind: "attachment",
|
||||
mimeType: "application/zip",
|
||||
filename: "merged-source.zip",
|
||||
byteSize: 42,
|
||||
sha256: "resource-hash",
|
||||
width: null,
|
||||
height: null,
|
||||
},
|
||||
});
|
||||
const merged = await request("memo.merge", { memoIds: [first.memo.id, second.memo.id] });
|
||||
assert.equal(merged.memo.sourceMemoIds.length, 2);
|
||||
const trash = await request("memo.list", { trash: true, limit: 20 });
|
||||
assert.ok(trash.memos.some((memo) => memo.id === first.memo.id));
|
||||
const mergeOutbox = (await request("sync.outbox.list", { limit: 200 })).items.find((item) => item.kind === "memo.merge" && item.entityId === merged.memo.id);
|
||||
assert.ok(mergeOutbox, "memo merge should be queued");
|
||||
const remoteMergedMemo = { ...merged.memo, id: "memo_remote_merged" };
|
||||
await request("sync.apply", {
|
||||
changes: [{ entityType: "memo", operation: "upsert", entityId: remoteMergedMemo.id, memo: remoteMergedMemo, notebook: null }],
|
||||
});
|
||||
await request("sync.outbox.ack", { id: mergeOutbox.id, remoteMemo: remoteMergedMemo });
|
||||
await assert.rejects(() => request("memo.get", { memoId: merged.memo.id, includeDeleted: true }), /Query returned no rows/);
|
||||
assert.equal((await request("memo.get", { memoId: first.memo.id, includeDeleted: true })).memo.mergedIntoMemoId, remoteMergedMemo.id);
|
||||
const mergedResource = (await request("resource.list", { limit: 200 })).resources.find((resource) => resource.id === "resource-from-merge-source");
|
||||
assert.equal(mergedResource.memoId, remoteMergedMemo.id, "merge acknowledgement should remap resources before deleting the local memo");
|
||||
assert.equal(mergedResource.originalMemoId, first.memo.id, "merge acknowledgement should preserve the resource's source memo");
|
||||
assert.ok(!(await request("sync.outbox.list", { limit: 200 })).items.some((item) => item.id === mergeOutbox.id), "merge acknowledgement should clear the outbox item");
|
||||
|
||||
await request("template.cache", {
|
||||
template: {
|
||||
@@ -194,7 +221,7 @@ await request("storage.restore", { path: backupWithResource.path });
|
||||
await assert.rejects(() => request("memo.get", { memoId: afterBackup.memo.id }), /Query returned no rows/);
|
||||
assert.equal(readFileSync(join(stagedResourceDirectory, "stage-test.bin"), "utf8"), "offline attachment snapshot", "restore should recover staged resources");
|
||||
const outbox = await request("sync.outbox.list", { limit: 100 });
|
||||
assert.ok(outbox.items.some((item) => item.kind === "memo.merge"));
|
||||
assert.ok(!outbox.items.some((item) => item.id === mergeOutbox.id), "a restored backup must not resurrect an acknowledged merge");
|
||||
const conflictMemo = await request("memo.create", { notebookId: inbox.id, title: "Conflict test", contentMarkdown: "local conflict", tags: [] });
|
||||
const conflictCandidate = (await request("sync.outbox.list", { limit: 200 })).items.find((item) => item.entityId === conflictMemo.memo.id);
|
||||
assert.ok(conflictCandidate, "conflict test should create an outbox item");
|
||||
|
||||
Reference in new issue
Block a user