diff --git a/services/project-history/app/js/ChunkTranslator.js b/services/project-history/app/js/ChunkTranslator.js index b5707fa3a0..c1b2a1d7c7 100644 --- a/services/project-history/app/js/ChunkTranslator.js +++ b/services/project-history/app/js/ChunkTranslator.js @@ -447,32 +447,52 @@ class TextUpdateBuilder { * @param {RetainOp} retain */ applyRetain(retain) { - const rangeOfRetention = new Range(this.sourceCursor, retain.length) - let cursor = this.sourceCursor + const resultRetentionRange = new Range(this.result.length, retain.length) + const sourceRetentionRange = new Range(this.sourceCursor, retain.length) + + let scanCursor = this.result.length if (retain.tracking) { // We are modifying existing tracked deletes. We need to treat removal // (type insert/none) of a tracked delete as an insertion. Similarly, any // range we introduce as a tracked deletion must be reported as a deletion. const trackedDeletes = this.trackedChanges.trackedChanges.filter( tc => - tc.tracking.type === 'delete' && tc.range.overlaps(rangeOfRetention) + tc.tracking.type === 'delete' && + tc.range.overlaps(resultRetentionRange) ) + for (const trackedDelete of trackedDeletes) { - if (cursor < trackedDelete.range.start) { + const resultTrackedDelete = trackedDelete.range + const sourceTrackedDelete = trackedDelete.range.moveBy( + this.sourceCursor - this.result.length + ) + + if (scanCursor < resultTrackedDelete.start) { if (retain.tracking.type === 'delete') { this.changes.push({ - d: this.source.slice(cursor, trackedDelete.range.start), + d: this.source.slice( + this.sourceCursor, + sourceTrackedDelete.start + ), p: this.result.length, }) } - this.result += this.source.slice(cursor, trackedDelete.range.start) - cursor = trackedDelete.range.start + this.result += this.source.slice( + this.sourceCursor, + sourceTrackedDelete.start + ) + scanCursor = resultTrackedDelete.start + this.sourceCursor = sourceTrackedDelete.start } - const endOfInsertion = Math.min( - trackedDelete.range.end, - rangeOfRetention.end + const endOfInsertionResult = Math.min( + resultTrackedDelete.end, + resultRetentionRange.end ) - const text = this.source.slice(cursor, endOfInsertion) + const endOfInsertionSource = Math.min( + sourceTrackedDelete.end, + sourceRetentionRange.end + ) + const text = this.source.slice(this.sourceCursor, endOfInsertionSource) if ( retain.tracking.type === 'none' || retain.tracking.type === 'insert' @@ -483,16 +503,22 @@ class TextUpdateBuilder { }) } this.result += text - cursor = endOfInsertion - if (cursor >= rangeOfRetention.end) { + // skip the tracked delete itself + scanCursor = endOfInsertionResult + this.sourceCursor = endOfInsertionSource + + if (scanCursor >= resultRetentionRange.end) { break } } } - if (cursor < rangeOfRetention.end) { + if (scanCursor < resultRetentionRange.end) { // The last region is not a tracked delete. But we should still handle // a new tracked delete as a deletion. - const text = this.source.slice(cursor, rangeOfRetention.end) + const text = this.source.slice( + this.sourceCursor, + sourceRetentionRange.end + ) if (retain.tracking?.type === 'delete') { this.changes.push({ d: text, @@ -501,7 +527,7 @@ class TextUpdateBuilder { } this.result += text } - this.sourceCursor += retain.length + this.sourceCursor = sourceRetentionRange.end } /** @@ -525,34 +551,49 @@ class TextUpdateBuilder { * @param {RemoveOp} deletion */ applyDelete(deletion) { - const rangeOfDeletion = new Range(this.sourceCursor, deletion.length) + const sourceDeletionRange = new Range(this.sourceCursor, deletion.length) + const resultDeletionRange = new Range(this.result.length, deletion.length) + const trackedDeletes = this.trackedChanges.trackedChanges .filter( tc => - tc.tracking.type === 'delete' && tc.range.overlaps(rangeOfDeletion) + tc.tracking.type === 'delete' && + tc.range.overlaps(resultDeletionRange) ) .sort((a, b) => a.range.start - b.range.start) - let cursor = this.sourceCursor + + let scanCursor = this.result.length + for (const trackedDelete of trackedDeletes) { - if (cursor < trackedDelete.range.start) { + const resultTrackDeleteRange = trackedDelete.range + const sourceTrackDeleteRange = trackedDelete.range.moveBy( + this.sourceCursor - this.result.length + ) + + if (scanCursor < resultTrackDeleteRange.start) { this.changes.push({ - d: this.source.slice(cursor, trackedDelete.range.start), + d: this.source.slice(this.sourceCursor, sourceTrackDeleteRange.start), p: this.result.length, }) } // skip the tracked delete itself - cursor = Math.min(trackedDelete.range.end, rangeOfDeletion.end) - if (cursor >= rangeOfDeletion.end) { + scanCursor = Math.min(resultTrackDeleteRange.end, resultDeletionRange.end) + this.sourceCursor = Math.min( + sourceTrackDeleteRange.end, + sourceDeletionRange.end + ) + + if (scanCursor >= resultDeletionRange.end) { break } } - if (cursor < rangeOfDeletion.end) { + if (scanCursor < resultDeletionRange.end) { this.changes.push({ - d: this.source.slice(cursor, rangeOfDeletion.end), + d: this.source.slice(this.sourceCursor, sourceDeletionRange.end), p: this.result.length, }) } - this.sourceCursor = rangeOfDeletion.end + this.sourceCursor = sourceDeletionRange.end } finish() { diff --git a/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js b/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js index d1303666e3..28b232df55 100644 --- a/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js +++ b/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js @@ -2277,6 +2277,479 @@ describe('ChunkTranslator', function () { } ) }) + + it('should properly create changes when deleting after moved track deletes', function (done) { + this.chunk = { + chunk: { + startVersion: 0, + history: { + snapshot: { + files: { + 'main.tex': { + hash: this.fileHash, + stringLength: 34, + }, + }, + }, + changes: [ + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // // He[ll]o planet world, this is a test + 2, + { + r: 2, + tracking: { + type: 'delete', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + 30, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // {...} is a tracked insert + // He[ll]o {TEST }planet world, this is a test + 6, + { + i: 'TEST ', + tracking: { + type: 'insert', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + 28, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // {...} is a tracked insert + // He[ll]o {TEST }planet world, [this] is a test + 25, + { + r: 4, + tracking: { + type: 'delete', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + 10, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // {...} is a tracked insert + 2, // He|[ll]o {TEST }planet world, [this] is a test + -2, // He|o {TEST }planet world, [this] is a test + 2, // Heo |{TEST }planet world, [this] is a test + { + r: 5, + tracking: { + type: 'none', + userId: '65f1baeb3cee7c3563fbd1d1', + ts: '2024-03-27T13:20:23.253Z', + }, + }, // Heo TEST| planet world, [this] is a test + 14, // Heo TEST planet world, |[this] is a test + -4, // Heo TEST planet world, | is a test + 10, // Heo TEST planet world, is a test| + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + ], + }, + }, + authors: [this.author1.id], + } + + this.ChunkTranslator.convertToDiffUpdates( + this.projectId, + this.chunk, + 'main.tex', + 0, + 4, + (error, diff) => { + expect(error).to.be.null + expect(diff.updates).to.deep.equal([ + { + op: [ + { + d: 'll', + p: 2, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 0, + }, + { + op: [ + { + i: 'TEST ', + p: 4, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 1, + }, + { + op: [ + { + d: 'this', + p: 23, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 2, + }, + { + op: [], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 3, + }, + ]) + done() + } + ) + }) + + it('should properly create changes when retaining after moved track deletes', function (done) { + this.chunk = { + chunk: { + startVersion: 0, + history: { + snapshot: { + files: { + 'main.tex': { + hash: this.fileHash, + stringLength: 34, + }, + }, + }, + changes: [ + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // // He[ll]o planet world, this is a test + 2, + { + r: 2, + tracking: { + type: 'delete', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + 30, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // He[ll]o planet world, [this] is a test + 20, + { + r: 4, + tracking: { + type: 'delete', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + 10, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // {...} is a tracked insert + // He[ll]o planet world, [this] {TEST }is a test + 25, + { + i: 'TEST ', + tracking: { + type: 'insert', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + 9, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // {...} is a tracked insert + 2, // He|[ll]o planet world, [this] {TEST }is a test + -2, // He|o planet world, [this] {TEST }is a test + { + r: 39, + tracking: { + type: 'none', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + ], // He|o planet world, this TEST is a test + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + ], + }, + }, + authors: [this.author1.id], + } + + this.ChunkTranslator.convertToDiffUpdates( + this.projectId, + this.chunk, + 'main.tex', + 0, + 4, + (error, diff) => { + expect(error).to.be.null + expect(diff.updates).to.deep.equal([ + { + op: [ + { + d: 'll', + p: 2, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 0, + }, + { + op: [ + { + d: 'this', + p: 18, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 1, + }, + { + op: [ + { + i: 'TEST ', + p: 19, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 2, + }, + { + op: [ + { + i: 'this', + p: 18, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 3, + }, + ]) + done() + } + ) + }) + + it('should handle deletion that starts before tracked delete', function (done) { + this.chunk = { + chunk: { + startVersion: 0, + history: { + snapshot: { + files: { + 'main.tex': { + hash: this.fileHash, + stringLength: 34, + }, + }, + }, + changes: [ + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + // Hello planet world, [this] is a test + 20, + { + r: 4, + tracking: { + type: 'delete', + userId: this.author1.id, + ts: this.date.toISOString(), + }, + }, + 10, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + { + operations: [ + { + pathname: 'main.tex', + textOperation: [ + // [...] is a tracked delete + 5, // Hello| planet world, [this] is a test + -25, // Hellotest + 4, + ], + }, + ], + timestamp: this.date.toISOString(), + authors: [this.author1.id], + }, + ], + }, + }, + authors: [this.author1.id], + } + + this.ChunkTranslator.convertToDiffUpdates( + this.projectId, + this.chunk, + 'main.tex', + 0, + 2, + (error, diff) => { + expect(error).to.be.null + expect(diff.updates).to.deep.equal([ + { + op: [ + { + d: 'this', + p: 20, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 0, + }, + { + op: [ + { + d: ' planet world, ', + p: 5, + }, + { + d: ' is a ', + p: 5, + }, + ], + meta: { + users: [this.author1.id], + start_ts: this.date.getTime(), + end_ts: this.date.getTime(), + }, + v: 1, + }, + ]) + done() + } + ) + }) }) }) })