Просмотр исходного кода

Fix history diffs when deleting over many tracked deletes (#19193)

* Fix history diffs when deleting over many tracked deletes

As we are looping through tracked deletes, the offset between the result
positions and the source positions must be kept constant. Otherwise, the
tracked deletes are translated as we delete text and move the source
cursor.

GitOrigin-RevId: b2417a75219aaa16bf5c61e0ebcb0586cae6aef2
Eric Mc Sween 2 лет назад
Родитель
Сommit
e4cc2a0816

+ 4 - 6
services/project-history/app/js/ChunkTranslator.js

@@ -463,11 +463,10 @@ class TextUpdateBuilder {
             tc.range.overlaps(resultRetentionRange)
             tc.range.overlaps(resultRetentionRange)
         )
         )
 
 
+      const sourceOffset = this.sourceCursor - this.result.length
       for (const trackedDelete of trackedDeletes) {
       for (const trackedDelete of trackedDeletes) {
         const resultTrackedDelete = trackedDelete.range
         const resultTrackedDelete = trackedDelete.range
-        const sourceTrackedDelete = trackedDelete.range.moveBy(
-          this.sourceCursor - this.result.length
-        )
+        const sourceTrackedDelete = trackedDelete.range.moveBy(sourceOffset)
 
 
         if (scanCursor < resultTrackedDelete.start) {
         if (scanCursor < resultTrackedDelete.start) {
           if (retain.tracking.type === 'delete') {
           if (retain.tracking.type === 'delete') {
@@ -566,12 +565,11 @@ class TextUpdateBuilder {
       .sort((a, b) => a.range.start - b.range.start)
       .sort((a, b) => a.range.start - b.range.start)
 
 
     let scanCursor = this.result.length
     let scanCursor = this.result.length
+    const sourceOffset = this.sourceCursor - this.result.length
 
 
     for (const trackedDelete of trackedDeletes) {
     for (const trackedDelete of trackedDeletes) {
       const resultTrackDeleteRange = trackedDelete.range
       const resultTrackDeleteRange = trackedDelete.range
-      const sourceTrackDeleteRange = trackedDelete.range.moveBy(
-        this.sourceCursor - this.result.length
-      )
+      const sourceTrackDeleteRange = trackedDelete.range.moveBy(sourceOffset)
 
 
       if (scanCursor < resultTrackDeleteRange.start) {
       if (scanCursor < resultTrackDeleteRange.start) {
         this.changes.push({
         this.changes.push({

+ 169 - 0
services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js

@@ -2738,6 +2738,175 @@ describe('ChunkTranslator', function () {
           }
           }
         )
         )
       })
       })
+
+      describe('whith multiple tracked deletes', function () {
+        beforeEach(function () {
+          this.fileContents = 'Hello planet world universe, this is a test'
+          this.ranges = JSON.stringify({
+            trackedChanges: [
+              {
+                range: { pos: 6, length: 7 },
+                tracking: {
+                  type: 'delete',
+                  userId: this.author1.id,
+                  ts: '2024-01-01T00:00:00.000Z',
+                },
+              },
+              {
+                range: { pos: 18, length: 9 },
+                tracking: {
+                  type: 'delete',
+                  userId: this.author1.id,
+                  ts: '2024-01-01T00:00:00.000Z',
+                },
+              },
+            ],
+          })
+          this.HistoryStoreManager.getProjectBlob
+            .withArgs(this.historyId, this.rangesHash)
+            .yields(null, this.ranges)
+          this.HistoryStoreManager.getProjectBlob
+            .withArgs(this.historyId, this.fileHash)
+            .yields(null, this.fileContents)
+        })
+
+        it('should handle a deletion that spans multiple tracked deletes', function (done) {
+          this.chunk = {
+            chunk: {
+              startVersion: 0,
+              history: {
+                snapshot: {
+                  files: {
+                    'main.tex': {
+                      hash: this.fileHash,
+                      rangesHash: this.rangesHash,
+                      stringLength: this.fileContents.length,
+                    },
+                  },
+                },
+                changes: [
+                  {
+                    operations: [
+                      {
+                        pathname: 'main.tex',
+                        textOperation: [
+                          // [...] is a tracked delete
+                          6, // Hello |[planet ]world[ universe], this is a test
+                          -21, // Hello|, this is a test
+                          16,
+                        ],
+                      },
+                    ],
+                    timestamp: this.date.toISOString(),
+                    authors: [this.author1.id],
+                  },
+                ],
+              },
+            },
+            authors: [this.author1.id],
+          }
+
+          this.ChunkTranslator.convertToDiffUpdates(
+            this.projectId,
+            this.chunk,
+            'main.tex',
+            0,
+            1,
+            (error, diff) => {
+              expect(error).to.be.null
+              expect(diff.updates).to.deep.equal([
+                {
+                  op: [
+                    {
+                      d: 'world',
+                      p: 6,
+                    },
+                  ],
+                  meta: {
+                    users: [this.author1.id],
+                    start_ts: this.date.getTime(),
+                    end_ts: this.date.getTime(),
+                  },
+                  v: 0,
+                },
+              ])
+              done()
+            }
+          )
+        })
+
+        it('should handle a tracked deletion that spans multiple tracked deletes', function (done) {
+          this.chunk = {
+            chunk: {
+              startVersion: 0,
+              history: {
+                snapshot: {
+                  files: {
+                    'main.tex': {
+                      hash: this.fileHash,
+                      rangesHash: this.rangesHash,
+                      stringLength: this.fileContents.length,
+                    },
+                  },
+                },
+                changes: [
+                  {
+                    operations: [
+                      {
+                        pathname: 'main.tex',
+                        textOperation: [
+                          // [...] is a tracked delete
+                          6, // Hello |[planet ]world[ universe], this is a test
+                          {
+                            r: 21,
+                            tracking: {
+                              type: 'delete',
+                              userId: this.author1.id,
+                              ts: '2024-01-01T00:00:00.000Z',
+                            },
+                          }, // Hello [planet world universe]|, this is a test
+                          16,
+                        ],
+                      },
+                    ],
+                    timestamp: this.date.toISOString(),
+                    authors: [this.author1.id],
+                  },
+                ],
+              },
+            },
+            authors: [this.author1.id],
+          }
+
+          this.ChunkTranslator.convertToDiffUpdates(
+            this.projectId,
+            this.chunk,
+            'main.tex',
+            0,
+            1,
+            (error, diff) => {
+              expect(error).to.be.null
+              expect(diff.updates).to.deep.equal([
+                {
+                  op: [
+                    {
+                      d: 'world',
+                      p: 6,
+                    },
+                  ],
+                  meta: {
+                    users: [this.author1.id],
+                    start_ts: this.date.getTime(),
+                    end_ts: this.date.getTime(),
+                  },
+                  v: 0,
+                },
+              ])
+              done()
+            }
+          )
+        })
+      })
     })
     })
   })
   })
 })
 })