From d081771336157859dbfc4c2fa3651e53933608c3 Mon Sep 17 00:00:00 2001
From: Paul Leathers <pleath@users.noreply.github.com>
Date: Wed, 23 Mar 2022 17:49:11 -0700
Subject: [PATCH] Account for gaps in the LocalReference array when packing
 segments (#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.
---
 packages/dds/merge-tree/src/localReference.ts |  7 ++
 .../sequence/src/test/sharedString.spec.ts    | 70 +++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/packages/dds/merge-tree/src/localReference.ts b/packages/dds/merge-tree/src/localReference.ts
index 79b5ab670e1a..977d8e83489d 100644
--- 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
index 304677e5c1d2..5a66ed623a58 100644
--- 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");