From 86508f78b23fc1b314bcc27a5c45f378259fd580 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() + } + ) + }) + }) }) }) })