diff --git a/src/merge-commit.test.ts b/src/merge-commit.test.ts index 7e5fd7d..38c1bd3 100644 --- a/src/merge-commit.test.ts +++ b/src/merge-commit.test.ts @@ -4,6 +4,7 @@ import { MemoryPatchStore } from "./adapters/memory-patch-store"; import { mergeStrings3 } from "./merge3"; import { StringCodec, StringDocument } from "./string-document"; import { legacyPatchId } from "./patch-id"; +import { hashString } from "./value-hash"; import type { DocCodec, Inconsistency, Patch, PatchEnvelope } from "./types"; const doc = (s: string) => new StringDocument(s); @@ -184,6 +185,25 @@ describe("a history written before merge commits recorded their merged value", ( return all; } + it("is what a current session without merge3 writes: no inexact marker", async () => { + // The reviewer's case: a current apply-all writer, read by exact readers + // without changing its envelopes. + const [a, b] = await writers(applyAllCodec, 2); + const root = a.commit(doc("hello\n")); + b.applyRemote(root); + const left = a.commit(doc("helloA\n")); + const right = b.commit(doc("helloB\n")); + a.applyRemote(right); + const authored = `${a.getDocument()}tail\n`; + const last = a.commit(doc(authored)); + a.close(); + b.close(); + for (const p of [root, left, right, last]) expect(p.inexact).toBeUndefined(); + for (const c of [codec, otherCodec]) { + expect(graph(c, [root, left, right, last]).version(last.time).toString()).toBe(authored); + } + }); + it("keeps the values its authors saw, whatever the merge algorithm", async () => { let merges = 0; for (let seed = 1; seed <= 20; seed++) { @@ -380,3 +400,51 @@ describe("replay checkpoints of a large document", () => { expect(applied).toBeLessThan(4 * all.length); }); }); + +describe("a merge commit made without the exact value of its parents", () => { + const id = legacyPatchId; + const patch = (t: number, parents: number[], from: string, to: string): Patch => ({ + time: id(t), + parents: parents.map(id), + patch: doc(from).makePatch(doc(to)), + hash: hashString(to), + }); + // 2 deletes a line that 3 edits concurrently; 4 merges them (the edit + // wins) and is snapshotted; 5 edits a line that 6 deletes concurrently. + const ps = [ + patch(1, [], "", "a\nb\nc\n"), + patch(2, [1], "a\nb\nc\n", "a\nc\n"), + patch(3, [1], "a\nb\nc\n", "a\nB\nc\n"), + patch(4, [2, 3], "a\nB\nc\n", "a\nB\nc\nd\n"), + patch(5, [4], "a\nB\nc\nd\n", "a\nB\nC!\nd\n"), + patch(6, [3], "a\nB\nc\n", "a\nB\n"), + ]; + const snapshot: Patch = { ...ps[3], isSnapshot: true, snapshot: "a\nB\nc\nd\n" }; + + it("is marked inexact, and read with merge3", async () => { + // A client with the snapshot, 5 and 6, but not 3 (below the snapshot): + // it cannot compute the exact merge of 5 and 6. + const s = new Session({ + codec, + patchStore: new MemoryPatchStore([snapshot, ps[4], ps[5]]), + clientId: "late", + }); + await s.init(); + expect(s.getHeads()).toEqual([id(5), id(6)].sort()); + const merge = s.commit(doc(`${s.getDocument()}g\n`)); + s.close(); + expect(merge.parents).toHaveLength(2); + expect(merge.hash).toBeUndefined(); + expect(merge.mergeParent).toBeUndefined(); + expect(merge.inexact).toBe(true); + // Read with merge3, like any merge commit of this version, not by + // replaying the history in time order (which depends on the history a + // client has loaded, and here differs). + const full = graph(codec, [...ps, snapshot, merge]); + const value = full.version(merge.time).toString(); + const merged = full.exactValueOf(merge.parents!)!; + expect(value).toBe(codec.applyPatch(merged, merge.patch).toString()); + const legacy = graph(codec, [...ps, snapshot, { ...merge, inexact: undefined }]); + expect(legacy.version(merge.time).toString()).not.toBe(value); + }); +}); diff --git a/src/patch-graph.ts b/src/patch-graph.ts index 316afbf..8aabe32 100644 --- a/src/patch-graph.ts +++ b/src/patch-graph.ts @@ -60,17 +60,21 @@ function isRoot(patch: Patch): boolean { } // A merge commit (several parents) that does not record its merged value -// (see Patch.mergeParent) and has no hash, so was written by a version that -// applied every patch in time order (patchflow 0.8 and earlier; 0.9 was never -// used in production), or with the value of that same fallback (see -// Session.commit). Its value is computed the way its author computed the value -// it is a diff from (see legacyValueOfSet), not with merge3, so a history -// written before exact merges keeps its values. +// (see Patch.mergeParent), has no hash and is not marked inexact, so was +// written by a version that applied every patch in time order (patchflow 0.8 +// and earlier; 0.9 was never used in production). Its value is computed the +// way its author computed the value it is a diff from (see legacyValueOfSet), +// not with merge3, so a history written before exact merges keeps its values. +// A merge commit written by this version without the exact value of its +// parents is marked inexact (Patch.inexact) and read with merge3, which, unlike +// replaying the history in time order, does not depend on how much of the +// history a client has loaded. function isLegacyMerge(patch: Patch): boolean { return ( (patch.parents?.length ?? 0) > 1 && patch.mergeParent == null && patch.hash == null && + !patch.inexact && patch.patch != null ); } @@ -776,6 +780,13 @@ export class PatchGraph { return hashString(this.codec.toString(doc)); } + // Whether this graph computes exact values (the codec provides merge3 and + // the strategy is not apply-all). Otherwise every value applies all patches + // in time order, as patchflow 0.8 did. + computesExactValues(): boolean { + return this.codec.merge3 != null && this.mergeStrategy !== "apply-all"; + } + // The exact merged value of a set of patches (e.g. the parents of a new // patch), or undefined if it cannot be computed from the loaded history or // the graph does not compute exact values. diff --git a/src/session.ts b/src/session.ts index a932993..009e3f4 100644 --- a/src/session.ts +++ b/src/session.ts @@ -367,6 +367,7 @@ export class Session extends EventEmitter { source: opts.source, meta: opts.meta, hash: parentsValue == null ? undefined : this.graph.hashOf(nextDoc), + ...this.inexactMarker(parentsValue), ...this.recordMerge(parents, parentsValue), }; this.graph.add([envelope]); @@ -405,6 +406,15 @@ export class Session extends EventEmitter { return best == null ? {} : { mergeParent: best.parent, mergePatch: best.patch }; } + // A patch committed without the exact value of its parents by a session + // that computes exact values (e.g. before the history needed for it is + // loaded) is marked inexact (see Patch.inexact). A session that never + // computes exact values (no merge3, or apply-all) writes history the way + // patchflow 0.8 did, and its patches are read that way: no marker. + private inexactMarker(parentsValue: Document | undefined): Pick { + return parentsValue == null && this.graph.computesExactValues() ? { inexact: true } : {}; + } + // Merge a remote patch and refresh the current document. applyRemote(env: PatchEnvelope): void { this.applyRemoteBatch([env]); @@ -743,6 +753,7 @@ export class Session extends EventEmitter { version: nextVersion, file: true, hash: parentsValue == null ? undefined : this.graph.hashOf(newDoc), + ...this.inexactMarker(parentsValue), ...this.recordMerge(parents, parentsValue), }; this.graph.add([envelope]); diff --git a/src/types.ts b/src/types.ts index 5061d0a..9cc6a3a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -54,6 +54,12 @@ export interface Patch { // patch-graph.ts. mergeParent?: PatchId; mergePatch?: unknown; + // Set by this version on a patch committed without the exact value of its + // parents (e.g. before the history needed for it was loaded): `patch` is a + // diff from an approximation of that value, and the patch has no hash. It + // tells such a patch apart from one written by a version before exact + // merges (see isLegacyMerge in patch-graph.ts). + inexact?: boolean; } // A value that does not match the hash its author recorded (Patch.hash).