Ver Fonte

Merge pull request #32628 from overleaf/em-bg-history-bug

Prevent incorrect merging of ops with different hpos offsets

GitOrigin-RevId: d171a93dcb29e952c7105afd81c3b29ca7e9788e
Eric Mc Sween há 4 meses atrás
pai
commit
77952e5d21

+ 10 - 0
services/project-history/app/js/UpdateCompressor.js

@@ -394,6 +394,16 @@ function _concatTwoUpdates(firstUpdate, secondUpdate) {
     firstOp.p <= secondOp.p && secondOp.p <= firstOp.p + firstSize
     firstOp.p <= secondOp.p && secondOp.p <= firstOp.p + firstSize
   const combinedLengthUnderLimit = firstSize + secondSize < MAX_UPDATE_SIZE
   const combinedLengthUnderLimit = firstSize + secondSize < MAX_UPDATE_SIZE
 
 
+  // When ops come from a multi-component update, the history position offset
+  // (hpos - p) may differ between ops because each component's hpos is computed
+  // against a different tracked-change state. Merging ops with different
+  // offsets would produce incorrect history positions, so we bail out.
+  const firstHposOffset = (firstOp.hpos ?? firstOp.p) - firstOp.p
+  const secondHposOffset = (secondOp.hpos ?? secondOp.p) - secondOp.p
+  if (firstHposOffset !== secondHposOffset) {
+    return [firstUpdate, secondUpdate]
+  }
+
   // Two inserts
   // Two inserts
   if (
   if (
     firstOp.i != null &&
     firstOp.i != null &&

+ 90 - 0
services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js

@@ -877,6 +877,36 @@ describe('UpdateCompressor', function () {
         ])
         ])
       })
       })
 
 
+      it('should not merge inserts with different hpos offsets (multi-component update)', function () {
+        // hpos offsets: first op has offset 10 (13-3),
+        // second op has offset 12 (18-6)
+        expect(
+          this.UpdateCompressor.compressUpdates([
+            {
+              op: { p: 3, i: 'foo', hpos: 13 },
+              meta: { ts: this.ts1, user_id: this.user_id },
+              v: 42,
+            },
+            {
+              op: { p: 6, i: 'bar', hpos: 18 },
+              meta: { ts: this.ts2, user_id: this.user_id },
+              v: 43,
+            },
+          ])
+        ).to.deep.equal([
+          {
+            op: { p: 3, i: 'foo', hpos: 13 },
+            meta: { ts: this.ts1, user_id: this.user_id },
+            v: 42,
+          },
+          {
+            op: { p: 6, i: 'bar', hpos: 18 },
+            meta: { ts: this.ts2, user_id: this.user_id },
+            v: 43,
+          },
+        ])
+      })
+
       it('should not merge updates from different users', function () {
       it('should not merge updates from different users', function () {
         expect(
         expect(
           this.UpdateCompressor.compressUpdates([
           this.UpdateCompressor.compressUpdates([
@@ -1091,6 +1121,36 @@ describe('UpdateCompressor', function () {
         ])
         ])
       })
       })
 
 
+      it('should not merge deletes with different hpos offsets (multi-component update)', function () {
+        // hpos offsets: first op has offset 10 (13-3),
+        // second op has offset 12 (15-3)
+        expect(
+          this.UpdateCompressor.compressUpdates([
+            {
+              op: { p: 3, d: 'foo', hpos: 13 },
+              meta: { ts: this.ts1, user_id: this.user_id },
+              v: 42,
+            },
+            {
+              op: { p: 3, d: 'bar', hpos: 15 },
+              meta: { ts: this.ts2, user_id: this.user_id },
+              v: 43,
+            },
+          ])
+        ).to.deep.equal([
+          {
+            op: { p: 3, d: 'foo', hpos: 13 },
+            meta: { ts: this.ts1, user_id: this.user_id },
+            v: 42,
+          },
+          {
+            op: { p: 3, d: 'bar', hpos: 15 },
+            meta: { ts: this.ts2, user_id: this.user_id },
+            v: 43,
+          },
+        ])
+      })
+
       it('should not merge when the deletes are tracked', function () {
       it('should not merge when the deletes are tracked', function () {
         // TODO: We should be able to lift that constraint, but it would
         // TODO: We should be able to lift that constraint, but it would
         // require recalculating the hpos on the second op.
         // require recalculating the hpos on the second op.
@@ -1264,6 +1324,36 @@ describe('UpdateCompressor', function () {
           },
           },
         ])
         ])
       })
       })
+
+      it('should not merge insert and delete with different hpos offsets (multi-component update)', function () {
+        // hpos offsets: first op has offset 10 (13-3),
+        // second op has offset 12 (17-5)
+        expect(
+          this.UpdateCompressor.compressUpdates([
+            {
+              op: { p: 3, i: 'foo', hpos: 13 },
+              meta: { ts: this.ts1, user_id: this.user_id },
+              v: 42,
+            },
+            {
+              op: { p: 5, d: 'o', hpos: 17 },
+              meta: { ts: this.ts2, user_id: this.user_id },
+              v: 43,
+            },
+          ])
+        ).to.deep.equal([
+          {
+            op: { p: 3, i: 'foo', hpos: 13 },
+            meta: { ts: this.ts1, user_id: this.user_id },
+            v: 42,
+          },
+          {
+            op: { p: 5, d: 'o', hpos: 17 },
+            meta: { ts: this.ts2, user_id: this.user_id },
+            v: 43,
+          },
+        ])
+      })
     })
     })
 
 
     describe('delete - insert', function () {
     describe('delete - insert', function () {