diff --git a/.changeset/loro-js-tree-local-positions.md b/.changeset/loro-js-tree-local-positions.md new file mode 100644 index 000000000..594857ff2 --- /dev/null +++ b/.changeset/loro-js-tree-local-positions.md @@ -0,0 +1,10 @@ +--- +"loro.js": patch +--- + +Fix `loro.js` local tree moves and creates to match `loro-crdt`: + +- Creating or moving a node between two siblings that share a fractional index (for example two roots created concurrently at index 0) threw `fractional index bounds are not ordered`. It now gives the following siblings with that index new positions in the same transaction, as Rust does. +- Moving a node to the position it already has, including `move(node, parent)` without an index when it is already last, `node.move(parent)`, and `moveAfter`/`moveBefore` its current neighbor, no longer records an op. +- `moveAfter` and `moveBefore` within the same parent counted the moved node itself, so moving a node after a later sibling threw `tree index … is out of range` or landed one slot off. +- Moving a node under one of its own deleted descendants threw `cannot move a tree node below itself or its descendant`. A deleted node's parent is the deleted root, as in Rust, so the move is allowed (and the node becomes deleted). diff --git a/loro-js/src/runtime/containers.ts b/loro-js/src/runtime/containers.ts index d7e9c46df..2ffbdc5b2 100644 --- a/loro-js/src/runtime/containers.ts +++ b/loro-js/src/runtime/containers.ts @@ -1,4 +1,4 @@ -import { bytesToHex } from "../codec/bytes"; +import { bytesEqual, bytesToHex } from "../codec/bytes"; import { containerTypeFromHistoricalByte, containerTypeToHistoricalByte, @@ -7,7 +7,7 @@ import { PostcardReader, PostcardWriter } from "../codec/postcard"; import type { ContainerId as CodecContainerId, Id as CodecId } from "../codec/types"; import { formatContainerId, formatTreeId, parseContainerId } from "./ids"; import type { LoroDoc } from "./document"; -import { fractionalIndexBetween } from "./fractional-index"; +import { fractionalIndexBetween, fractionalIndexesBetween } from "./fractional-index"; import { OrderedIndex } from "./ordered-index"; import { SequenceIndex, SequenceSpan } from "./sequence-index"; import type { SequenceIdRun } from "./sequence-index"; @@ -3028,11 +3028,18 @@ export class LoroTree< ); } + /** + * Port of Rust's `generate_fi_at`: the position for a node placed at + * `index` among `parent`'s children, not counting `exclude`. When both + * neighbors share a position there is no index between them, so the right + * neighbor and every following sibling with that position get new + * positions in `rearranged`, in order. + */ _positionFor( parent: CodecId | undefined, index?: number, exclude?: CodecId, - ): Uint8Array { + ): { position: Uint8Array; rearranged: { id: CodecId; position: Uint8Array }[] } { const children = this._children.get(treeParentKey(parent)); const excluded = exclude === undefined ? undefined : this._nodes.get(formatTreeId(exclude)); @@ -3053,10 +3060,29 @@ export class LoroTree< ? siblingIndex + 1 : siblingIndex, ); - return fractionalIndexBetween( - position === 0 ? undefined : siblingAt(position - 1)!.position, - position === length ? undefined : siblingAt(position)!.position, - ); + const left = position === 0 ? undefined : siblingAt(position - 1)!.position; + const right = position === length ? undefined : siblingAt(position)!; + if (left === undefined || right === undefined || !bytesEqual(left, right.position)) { + return { position: fractionalIndexBetween(left, right?.position), rearranged: [] }; + } + const reset = [right]; + let nextRight: Uint8Array | undefined; + for (let sibling = position + 1; sibling < length; sibling += 1) { + const record = siblingAt(sibling)!; + if (!bytesEqual(record.position, left)) { + nextRight = record.position; + break; + } + reset.push(record); + } + const positions = fractionalIndexesBetween(left, nextRight, reset.length + 1); + return { + position: positions[0]!, + rearranged: reset.map((record, offset) => ({ + id: record.id, + position: positions[offset + 1]!, + })), + }; } _setRecord(record: TreeNodeRecord): void { @@ -3198,12 +3224,28 @@ export class LoroTreeNode = Record): void { + // Rust's mov_after: the index is counted without this node. const parent = target.parent(); - this.move(parent, target.index() + 1); + let index = target.index() + 1; + if (this.#hasParent(parent) && this.index() < index) index -= 1; + this.move(parent, index); } moveBefore(target: LoroTreeNode): void { - this.move(target.parent(), target.index()); + const parent = target.parent(); + let index = target.index(); + if (this.#hasParent(parent) && index >= 1 && this.index() < index) index -= 1; + this.move(parent, index); + } + + #hasParent(parent: LoroTreeNode | undefined): boolean { + const record = this.#record(); + return ( + !record.deleted && + (parent === undefined + ? record.parent === undefined + : record.parent !== undefined && formatTreeId(record.parent) === parent.id) + ); } parent(): LoroTreeNode | undefined { diff --git a/loro-js/src/runtime/document.ts b/loro-js/src/runtime/document.ts index c1fc58a78..ced47c814 100644 --- a/loro-js/src/runtime/document.ts +++ b/loro-js/src/runtime/document.ts @@ -4229,15 +4229,27 @@ export class LoroDoc = Record = Record `${id.counter}@${id.peer}`)); diff --git a/loro-js/src/runtime/fractional-index.ts b/loro-js/src/runtime/fractional-index.ts index 652ac8ecd..3066f8c11 100644 --- a/loro-js/src/runtime/fractional-index.ts +++ b/loro-js/src/runtime/fractional-index.ts @@ -25,6 +25,36 @@ export function fractionalIndexBetween( return terminated(between); } +/** + * `n` increasing indexes between the bounds, spread by bisection. Port of + * Rust's `FractionalIndex::generate_n_evenly`. + */ +export function fractionalIndexesBetween( + lower: Uint8Array | undefined, + upper: Uint8Array | undefined, + n: number, +): Uint8Array[] { + const output: Uint8Array[] = []; + const generate = ( + low: Uint8Array | undefined, + high: Uint8Array | undefined, + count: number, + ): void => { + if (count === 0) return; + const middle = fractionalIndexBetween(low, high); + if (count === 1) { + output.push(middle); + return; + } + const half = Math.floor(count / 2); + generate(low, middle, half); + output.push(middle); + generate(middle, high, count - half - 1); + }; + generate(lower, upper, n); + return output; +} + function terminated(bytes: number[]): Uint8Array { return Uint8Array.from([...bytes, TERMINATOR]); } diff --git a/loro-js/tests/tree-local-positions.test.ts b/loro-js/tests/tree-local-positions.test.ts new file mode 100644 index 000000000..e3164cf93 --- /dev/null +++ b/loro-js/tests/tree-local-positions.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, test } from "vitest"; + +import { LoroDoc } from "../src/index"; +import type { LoroTree } from "../src/index"; + +// Expected values are what loro-crdt returns for the same calls. + +function shape(tree: LoroTree): string[] { + return tree.toJSON().map((node) => `${node.id}:${node.fractional_index}`); +} + +/** Two roots created concurrently at index 0 share the position 80. */ +function equalPositions(): LoroDoc { + const p1 = new LoroDoc(); + p1.setPeerId(1); + const p2 = new LoroDoc(); + p2.setPeerId(2); + p1.getTree("tree").createNode(); + p1.commit(); + p2.getTree("tree").createNode(); + p2.commit(); + p1.import(p2.export({ mode: "update" })); + expect(shape(p1.getTree("tree"))).toEqual(["0@1:80", "0@2:80"]); + return p1; +} + +describe("local tree positions", () => { + // Rust's generate_fi_at: there is no index between two equal positions, so + // the right neighbor is moved to a new position in the same transaction. + test("rearranges equal neighbors when creating a node between them", () => { + const doc = equalPositions(); + const tree = doc.getTree("tree"); + const before = doc.opCount(); + const node = tree.createNode(undefined, 1); + doc.commit(); + expect(node.id).toBe("1@1"); + expect(doc.opCount() - before).toBe(2); + expect(shape(tree)).toEqual(["0@1:80", "1@1:817F80", "0@2:8180"]); + + const replay = new LoroDoc(); + replay.import(doc.export({ mode: "update" })); + expect(shape(replay.getTree("tree"))).toEqual(shape(tree)); + }); + + test("rearranges equal neighbors when moving a node between them", () => { + const doc = equalPositions(); + const tree = doc.getTree("tree"); + const extra = tree.createNode(); + doc.commit(); + const before = doc.opCount(); + tree.move(extra.id, undefined, 1); + doc.commit(); + expect(doc.opCount() - before).toBe(2); + expect(shape(tree)).toEqual(["0@1:80", "1@1:817F80", "0@2:8180"]); + }); + + test("records nothing for a move to the current position", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const a = tree.createNode(); + const b = tree.createNode(); + const c = tree.createNode(); + doc.commit(); + const count = (edit: () => void): number => { + const before = doc.opCount(); + edit(); + doc.commit(); + return doc.opCount() - before; + }; + + expect(count(() => tree.move(a.id, undefined, 0))).toBe(0); + // Without an index the node goes last; it already is. + expect(count(() => tree.move(c.id, undefined))).toBe(0); + expect(count(() => c.move(undefined))).toBe(0); + expect(count(() => b.moveAfter(a))).toBe(0); + expect(count(() => a.moveBefore(b))).toBe(0); + expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, b.id, c.id]); + }); + + // Rust's mov_after/mov_before count the index without the moved node. + test("moves after and before a sibling in the same parent", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const a = tree.createNode(); + const b = tree.createNode(); + const c = tree.createNode(); + doc.commit(); + + a.moveAfter(c); + doc.commit(); + expect(tree.toJSON().map((node) => node.id)).toEqual([b.id, c.id, a.id]); + a.moveBefore(b); + doc.commit(); + expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, b.id, c.id]); + b.moveAfter(c); + doc.commit(); + expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, c.id, b.id]); + }); + + test("moves within a parent in time independent of the sibling count", () => { + /** Best of three runs of 1,000 same-parent moves, half of them no-ops. */ + function movesMs(siblings: number): number { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const nodes = Array.from({ length: siblings }, () => tree.createNode()); + doc.commit(); + let best = Infinity; + for (let run = 0; run < 3; run += 1) { + const started = performance.now(); + for (let step = 0; step < 500; step += 1) { + const node = nodes[(step * 7_919) % siblings]!; + tree.move(node.id, undefined, node.index()!); + tree.move(node.id, undefined, 0); + } + doc.commit(); + best = Math.min(best, performance.now() - started); + } + return best; + } + // Counting the siblings by listing them made each move linear in their + // number (16x the siblings took about 16x the time). + const ratio = movesMs(16_000) / movesMs(1_000); + expect(ratio).toBeLessThan(4); + }, 60_000); + + // Rust's is_ancestor_of treats a deleted node's parent as the deleted root. + test("moves a node under its own deleted child", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const parent = tree.createNode(); + const child = tree.createNode(parent.id); + doc.commit(); + tree.delete(child.id); + doc.commit(); + const before = doc.opCount(); + tree.move(parent.id, child.id, 0); + doc.commit(); + expect(doc.opCount() - before).toBe(1); + expect(tree.toJSON()).toEqual([]); + expect(() => tree.move(child.id, child.id, 0)).toThrow(/below itself/); + }); +});