Account for gaps in the LocalReference array when packing segments (m…

…icrosoft#9565) When Zamboni combines TextSegment's, it needs to update LocalReference's that refer to the segments. It wasn't doing this correctly in the case where, say, a reference is found at position 0, then some number of segments without references were appended, then a segment containing a reference was appended. The text corresponding to the segments without references wasn't accounted in the final segment. A gap needs to be introduced in the LocalReference array to make the references correct.
tylerbutler · Mar 24, 2022 · d081771 · d081771
1 parent be349b4
commit d081771
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 0 deletions.
diff --git a/packages/dds/merge-tree/src/localReference.ts b/packages/dds/merge-tree/src/localReference.ts
@@ -3,6 +3,7 @@
  * Licensed under the MIT License.
  */
 
+import { assert } from "@fluidframework/common-utils";
 import { Client } from "./client";
 import {
     ISegment,
@@ -133,8 +134,14 @@ export class LocalReferenceCollection {
             if (!seg1.localRefs) {
                 seg1.localRefs = new LocalReferenceCollection(seg1);
             }
+            assert(seg1.localRefs.refsByOffset.length === seg1.cachedLength, "LocalReferences array contains a gap");
             seg1.localRefs.append(seg2.localRefs);
         }
+        else if (seg1.localRefs) {
+            // Since creating the LocalReferenceCollection, we may have appended
+            // segments that had no local references. Account for them now by padding the array.
+            seg1.localRefs.refsByOffset.length += seg2.cachedLength;
+        }
     }
 
     public hierRefCount: number = 0;

diff --git a/packages/dds/sequence/src/test/sharedString.spec.ts b/packages/dds/sequence/src/test/sharedString.spec.ts
@@ -373,6 +373,76 @@ describe("SharedString", () => {
             }
         });
 
+        it("can maintain consistency of LocalReference's when segments are packed", async () => {
+            // sharedString.insertMarker(0, ReferenceType.Tile, { nodeType: "Paragraph" });
+
+            const collection1 = sharedString.getIntervalCollection("test2");
+            containerRuntimeFactory.processAllMessages();
+            const collection2 = sharedString2.getIntervalCollection("test2");
+
+            sharedString.insertText(0, "a");
+            sharedString.insertText(1, "b");
+            sharedString.insertText(2, "c");
+            sharedString.insertText(3, "d");
+            sharedString.insertText(4, "e");
+            sharedString.insertText(5, "f");
+
+            containerRuntimeFactory.processAllMessages();
+
+            assert.strictEqual(sharedString.getText(), "abcdef", "incorrect text 1");
+            assert.strictEqual(sharedString2.getText(), "abcdef", "incorrect text 2");
+
+            const interval1 = collection1.add(2, 2, IntervalType.SlideOnRemove);
+            const id1 = interval1.getIntervalId();
+
+            containerRuntimeFactory.processAllMessages();
+
+            assert.notStrictEqual(collection2.getIntervalById(id1), undefined, "interval not found 1");
+            assert.strictEqual(collection2.getIntervalById(id1).start.toPosition(), 2, "1");
+            assert.strictEqual(collection2.getIntervalById(id1).end.toPosition(), 2, "2");
+
+            sharedString.insertText(0, "a");
+            sharedString.insertText(1, "b");
+            sharedString.insertText(2, "c");
+            sharedString.insertText(3, "d");
+            sharedString.insertText(4, "e");
+            sharedString.insertText(5, "f");
+
+            containerRuntimeFactory.processAllMessages();
+
+            assert.strictEqual(sharedString.getText(), "abcdefabcdef", "incorrect text 2");
+            assert.strictEqual(sharedString2.getText(), "abcdefabcdef", "incorrect text 3");
+
+            const interval2 = collection1.add(5, 5, IntervalType.SlideOnRemove);
+            const id2 = interval2.getIntervalId();
+
+            const interval3 = collection1.add(2, 2, IntervalType.SlideOnRemove);
+            const id3 = interval3.getIntervalId();
+
+            containerRuntimeFactory.processAllMessages();
+
+            assert.strictEqual(collection2.getIntervalById(id1).start.toPosition(), 8, "5");
+            assert.strictEqual(collection2.getIntervalById(id1).end.toPosition(), 8, "6");
+
+            assert.strictEqual(collection2.getIntervalById(id2).start.toPosition(), 5, "3");
+            assert.strictEqual(collection2.getIntervalById(id2).end.toPosition(), 5, "4");
+
+            assert.strictEqual(collection2.getIntervalById(id3).start.toPosition(), 2, "5");
+            assert.strictEqual(collection2.getIntervalById(id3).end.toPosition(), 2, "6");
+
+            // Summarize to cause Zamboni to pack segments. Confirm consistency after packing.
+            await sharedString2.summarize();
+
+            assert.strictEqual(collection2.getIntervalById(id1).start.toPosition(), 8, "5");
+            assert.strictEqual(collection2.getIntervalById(id1).end.toPosition(), 8, "6");
+
+            assert.strictEqual(collection2.getIntervalById(id2).start.toPosition(), 5, "3");
+            assert.strictEqual(collection2.getIntervalById(id2).end.toPosition(), 5, "4");
+
+            assert.strictEqual(collection2.getIntervalById(id3).start.toPosition(), 2, "5");
+            assert.strictEqual(collection2.getIntervalById(id3).end.toPosition(), 2, "6");
+        });
+
         it("can insert text", async () => {
             // Insert text in first shared string.
             sharedString.insertText(0, "hello");