From e4cc2a081610ce3a3b53ef6db7d8f33e8802bb5d Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Fri, 28 Jun 2024 08:10:50 -0400 Subject: [PATCH] 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 --- .../project-history/app/js/ChunkTranslator.js | 10 +- .../ChunkTranslator/ChunkTranslatorTests.js | 169 ++++++++++++++++++ 2 files changed, 173 insertions(+), 6 deletions(-) diff --git a/services/project-history/app/js/ChunkTranslator.js b/services/project-history/app/js/ChunkTranslator.js index 2648cc0b44..6d9eb37b5d 100644 --- a/services/project-history/app/js/ChunkTranslator.js +++ b/services/project-history/app/js/ChunkTranslator.js @@ -463,11 +463,10 @@ class TextUpdateBuilder { tc.range.overlaps(resultRetentionRange) ) + const sourceOffset = this.sourceCursor - this.result.length for (const trackedDelete of trackedDeletes) { 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 (retain.tracking.type === 'delete') { @@ -566,12 +565,11 @@ class TextUpdateBuilder { .sort((a, b) => a.range.start - b.range.start) let scanCursor = this.result.length + const sourceOffset = this.sourceCursor - this.result.length for (const trackedDelete of trackedDeletes) { const resultTrackDeleteRange = trackedDelete.range - const sourceTrackDeleteRange = trackedDelete.range.moveBy( - this.sourceCursor - this.result.length - ) + const sourceTrackDeleteRange = trackedDelete.range.moveBy(sourceOffset) if (scanCursor < resultTrackDeleteRange.start) { this.changes.push({ diff --git a/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js b/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js index 92d012465c..fa451ce807 100644 --- a/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js +++ b/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() + } + ) + }) + }) }) }) })