From dc9e7bd12d546c14a5ccffb1b85cd2d6697b3fcb Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 18 Jun 2025 11:24:30 +0200 Subject: [PATCH] [project-history] add support for resync of history-ot ranges (#26475) * [project-history] add support for resync of history-ot ranges * [project-history] avoid compressing sharejs and history-ot upgrades * [document-updater] improve error message of some assertions ... by migrating the assertions like this: ```diff -stub.calledWith().should.equal(true) +stub.should.have.been.calledWith() ``` ```diff -stub.called.should.equal(false) +stub.should.not.have.been.called ``` * [document-updater] move content field in resyncDocContent * [document-updater] add support for resync of history-ot ranges GitOrigin-RevId: e6104686a26934a5f25a8f095cbe00c163fbbaa7 --- .../app/js/DocumentManager.js | 5 - services/document-updater/app/js/Limits.js | 15 + .../app/js/ProjectHistoryRedisManager.js | 43 +- .../test/unit/js/Limits/LimitsTests.js | 84 ++ .../ProjectHistoryRedisManagerTests.js | 166 ++- .../project-history/app/js/SyncManager.js | 139 +- .../app/js/UpdateCompressor.js | 4 +- services/project-history/app/js/types.ts | 6 + .../test/acceptance/js/SyncTests.js | 1191 ++++++++++++++++- 9 files changed, 1579 insertions(+), 74 deletions(-) diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index 6080c1c97d..3fb3d10a6e 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -535,11 +535,6 @@ const DocumentManager = { if (opts.historyRangesMigration) { historyRangesSupport = opts.historyRangesMigration === 'forwards' } - if (!Array.isArray(lines)) { - const file = StringFileData.fromRaw(lines) - // TODO(24596): tc support for history-ot - lines = file.getLines() - } await ProjectHistoryRedisManager.promises.queueResyncDocContent( projectId, diff --git a/services/document-updater/app/js/Limits.js b/services/document-updater/app/js/Limits.js index 268ccd3f9b..cbd9293042 100644 --- a/services/document-updater/app/js/Limits.js +++ b/services/document-updater/app/js/Limits.js @@ -28,4 +28,19 @@ module.exports = { // since we didn't hit the limit in the loop, the document is within the allowed length return false }, + + /** + * @param {StringFileRawData} raw + * @param {number} maxDocLength + */ + stringFileDataContentIsTooLarge(raw, maxDocLength) { + let n = raw.content.length + if (n <= maxDocLength) return false // definitely under the limit, no need to calculate the total size + for (const tc of raw.trackedChanges ?? []) { + if (tc.tracking.type !== 'delete') continue + n -= tc.range.length + if (n <= maxDocLength) return false // under the limit now, no need to calculate the exact size + } + return true + }, } diff --git a/services/document-updater/app/js/ProjectHistoryRedisManager.js b/services/document-updater/app/js/ProjectHistoryRedisManager.js index 9a9985d99a..78e9c2ea4c 100644 --- a/services/document-updater/app/js/ProjectHistoryRedisManager.js +++ b/services/document-updater/app/js/ProjectHistoryRedisManager.js @@ -8,13 +8,14 @@ const rclient = require('@overleaf/redis-wrapper').createClient( ) const logger = require('@overleaf/logger') const metrics = require('./Metrics') -const { docIsTooLarge } = require('./Limits') +const { docIsTooLarge, stringFileDataContentIsTooLarge } = require('./Limits') const { addTrackedDeletesToContent, extractOriginOrSource } = require('./Utils') const HistoryConversions = require('./HistoryConversions') const OError = require('@overleaf/o-error') /** * @import { Ranges } from './types' + * @import { StringFileRawData } from 'overleaf-editor-core/lib/types' */ const ProjectHistoryRedisManager = { @@ -180,7 +181,7 @@ const ProjectHistoryRedisManager = { * @param {string} projectId * @param {string} projectHistoryId * @param {string} docId - * @param {string[]} lines + * @param {string[] | StringFileRawData} lines * @param {Ranges} ranges * @param {string[]} resolvedCommentIds * @param {number} version @@ -204,13 +205,8 @@ const ProjectHistoryRedisManager = { 'queue doc content resync' ) - let content = lines.join('\n') - if (historyRangesSupport) { - content = addTrackedDeletesToContent(content, ranges.changes ?? []) - } - const projectUpdate = { - resyncDocContent: { content, version }, + resyncDocContent: { version }, projectHistoryId, path: pathname, doc: docId, @@ -219,17 +215,38 @@ const ProjectHistoryRedisManager = { }, } - if (historyRangesSupport) { - projectUpdate.resyncDocContent.ranges = - HistoryConversions.toHistoryRanges(ranges) - projectUpdate.resyncDocContent.resolvedCommentIds = resolvedCommentIds + let content = '' + if (Array.isArray(lines)) { + content = lines.join('\n') + if (historyRangesSupport) { + content = addTrackedDeletesToContent(content, ranges.changes ?? []) + projectUpdate.resyncDocContent.ranges = + HistoryConversions.toHistoryRanges(ranges) + projectUpdate.resyncDocContent.resolvedCommentIds = resolvedCommentIds + } + } else { + content = lines.content + projectUpdate.resyncDocContent.historyOTRanges = { + comments: lines.comments, + trackedChanges: lines.trackedChanges, + } } + projectUpdate.resyncDocContent.content = content const jsonUpdate = JSON.stringify(projectUpdate) // Do an optimised size check on the docLines using the serialised // project update length as an upper bound const sizeBound = jsonUpdate.length - if (docIsTooLarge(sizeBound, lines, Settings.max_doc_length)) { + if (Array.isArray(lines)) { + if (docIsTooLarge(sizeBound, lines, Settings.max_doc_length)) { + throw new OError( + 'blocking resync doc content insert into project history queue: doc is too large', + { projectId, docId, docSize: sizeBound } + ) + } + } else if ( + stringFileDataContentIsTooLarge(lines, Settings.max_doc_length) + ) { throw new OError( 'blocking resync doc content insert into project history queue: doc is too large', { projectId, docId, docSize: sizeBound } diff --git a/services/document-updater/test/unit/js/Limits/LimitsTests.js b/services/document-updater/test/unit/js/Limits/LimitsTests.js index 34a5c13c26..11ca38746a 100644 --- a/services/document-updater/test/unit/js/Limits/LimitsTests.js +++ b/services/document-updater/test/unit/js/Limits/LimitsTests.js @@ -81,4 +81,88 @@ describe('Limits', function () { }) }) }) + + describe('stringFileDataContentIsTooLarge', function () { + it('should handle small docs', function () { + expect( + this.Limits.stringFileDataContentIsTooLarge({ content: '' }, 123) + ).to.equal(false) + }) + it('should handle docs at the limit', function () { + expect( + this.Limits.stringFileDataContentIsTooLarge( + { content: 'x'.repeat(123) }, + 123 + ) + ).to.equal(false) + }) + it('should handle docs above the limit', function () { + expect( + this.Limits.stringFileDataContentIsTooLarge( + { content: 'x'.repeat(123 + 1) }, + 123 + ) + ).to.equal(true) + }) + it('should handle docs above the limit and below with tracked-deletes removed', function () { + expect( + this.Limits.stringFileDataContentIsTooLarge( + { + content: 'x'.repeat(123 + 1), + trackedChanges: [ + { + range: { pos: 1, length: 1 }, + tracking: { + type: 'delete', + ts: '2025-06-16T14:31:44.910Z', + userId: 'user-id', + }, + }, + ], + }, + 123 + ) + ).to.equal(false) + }) + it('should handle docs above the limit and above with tracked-deletes removed', function () { + expect( + this.Limits.stringFileDataContentIsTooLarge( + { + content: 'x'.repeat(123 + 2), + trackedChanges: [ + { + range: { pos: 1, length: 1 }, + tracking: { + type: 'delete', + ts: '2025-06-16T14:31:44.910Z', + userId: 'user-id', + }, + }, + ], + }, + 123 + ) + ).to.equal(true) + }) + it('should handle docs above the limit and with tracked-inserts', function () { + expect( + this.Limits.stringFileDataContentIsTooLarge( + { + content: 'x'.repeat(123 + 1), + trackedChanges: [ + { + range: { pos: 1, length: 1 }, + tracking: { + type: 'insert', + ts: '2025-06-16T14:31:44.910Z', + userId: 'user-id', + }, + }, + ], + }, + 123 + ) + ).to.equal(true) + }) + }) }) diff --git a/services/document-updater/test/unit/js/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.js b/services/document-updater/test/unit/js/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.js index 760385b176..ad6c121dfb 100644 --- a/services/document-updater/test/unit/js/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.js +++ b/services/document-updater/test/unit/js/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.js @@ -15,6 +15,7 @@ describe('ProjectHistoryRedisManager', function () { this.Limits = { docIsTooLarge: sinon.stub().returns(false), + stringFileDataContentIsTooLarge: sinon.stub().returns(false), } this.ProjectHistoryRedisManager = SandboxedModule.require(modulePath, { @@ -61,22 +62,18 @@ describe('ProjectHistoryRedisManager', function () { }) it('should queue an update', function () { - this.multi.rpush - .calledWithExactly( - `ProjectHistory:Ops:${this.project_id}`, - this.ops[0], - this.ops[1] - ) - .should.equal(true) + this.multi.rpush.should.have.been.calledWithExactly( + `ProjectHistory:Ops:${this.project_id}`, + this.ops[0], + this.ops[1] + ) }) it('should set the queue timestamp if not present', function () { - this.multi.setnx - .calledWithExactly( - `ProjectHistory:FirstOpTimestamp:${this.project_id}`, - Date.now() - ) - .should.equal(true) + this.multi.setnx.should.have.been.calledWithExactly( + `ProjectHistory:FirstOpTimestamp:${this.project_id}`, + Date.now() + ) }) }) @@ -118,9 +115,10 @@ describe('ProjectHistoryRedisManager', function () { file: this.file_id, } - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(update) + ) }) }) @@ -166,9 +164,10 @@ describe('ProjectHistoryRedisManager', function () { doc: this.doc_id, } - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(update) + ) }) it('should queue an update with file metadata', async function () { @@ -350,9 +349,10 @@ describe('ProjectHistoryRedisManager', function () { doc: this.doc_id, } - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(update) + ) }) it('should not forward ranges if history ranges support is undefined', async function () { @@ -402,9 +402,10 @@ describe('ProjectHistoryRedisManager', function () { doc: this.doc_id, } - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(update) + ) }) it('should pass "false" as the createdBlob field if not provided', async function () { @@ -432,9 +433,10 @@ describe('ProjectHistoryRedisManager', function () { doc: this.doc_id, } - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(update) + ) }) it('should pass through the value of the createdBlob field', async function () { @@ -463,9 +465,10 @@ describe('ProjectHistoryRedisManager', function () { doc: this.doc_id, } - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(update) + ) }) }) @@ -493,8 +496,8 @@ describe('ProjectHistoryRedisManager', function () { beforeEach(async function () { this.update = { resyncDocContent: { - content: 'one\ntwo', version: this.version, + content: 'one\ntwo', }, projectHistoryId: this.projectHistoryId, path: this.pathname, @@ -516,19 +519,18 @@ describe('ProjectHistoryRedisManager', function () { }) it('should check if the doc is too large', function () { - this.Limits.docIsTooLarge - .calledWith( - JSON.stringify(this.update).length, - this.lines, - this.settings.max_doc_length - ) - .should.equal(true) + this.Limits.docIsTooLarge.should.have.been.calledWith( + JSON.stringify(this.update).length, + this.lines, + this.settings.max_doc_length + ) }) it('should queue an update', function () { - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(this.update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(this.update) + ) }) }) @@ -551,9 +553,8 @@ describe('ProjectHistoryRedisManager', function () { }) it('should not queue an update if the doc is too large', function () { - this.ProjectHistoryRedisManager.promises.queueOps.called.should.equal( - false - ) + this.ProjectHistoryRedisManager.promises.queueOps.should.not.have.been + .called }) }) @@ -561,10 +562,10 @@ describe('ProjectHistoryRedisManager', function () { beforeEach(async function () { this.update = { resyncDocContent: { - content: 'onedeleted\ntwo', version: this.version, ranges: this.ranges, resolvedCommentIds: this.resolvedCommentIds, + content: 'onedeleted\ntwo', }, projectHistoryId: this.projectHistoryId, path: this.pathname, @@ -601,9 +602,76 @@ describe('ProjectHistoryRedisManager', function () { }) it('should queue an update', function () { - this.ProjectHistoryRedisManager.promises.queueOps - .calledWithExactly(this.project_id, JSON.stringify(this.update)) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(this.update) + ) + }) + }) + + describe('history-ot', function () { + beforeEach(async function () { + this.lines = { + content: 'onedeleted\ntwo', + comments: [{ id: 'id1', ranges: [{ pos: 0, length: 3 }] }], + trackedChanges: [ + { + range: { pos: 3, length: 7 }, + tracking: { + type: 'delete', + userId: 'user-id', + ts: '2025-06-16T14:31:44.910Z', + }, + }, + ], + } + this.update = { + resyncDocContent: { + version: this.version, + historyOTRanges: { + comments: this.lines.comments, + trackedChanges: this.lines.trackedChanges, + }, + content: this.lines.content, + }, + projectHistoryId: this.projectHistoryId, + path: this.pathname, + doc: this.doc_id, + meta: { ts: new Date() }, + } + + await this.ProjectHistoryRedisManager.promises.queueResyncDocContent( + this.project_id, + this.projectHistoryId, + this.doc_id, + this.lines, + this.ranges, + this.resolvedCommentIds, + this.version, + this.pathname, + true + ) + }) + + it('should include tracked deletes in the update', function () { + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(this.update) + ) + }) + + it('should check the doc length without tracked deletes', function () { + this.Limits.stringFileDataContentIsTooLarge.should.have.been.calledWith( + this.lines, + this.settings.max_doc_length + ) + }) + + it('should queue an update', function () { + this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly( + this.project_id, + JSON.stringify(this.update) + ) }) }) }) diff --git a/services/project-history/app/js/SyncManager.js b/services/project-history/app/js/SyncManager.js index ef8caf69eb..43cb61be9f 100644 --- a/services/project-history/app/js/SyncManager.js +++ b/services/project-history/app/js/SyncManager.js @@ -23,6 +23,7 @@ import { isInsert, isDelete } from './Utils.js' /** * @import { Comment as HistoryComment, TrackedChange as HistoryTrackedChange } from 'overleaf-editor-core' + * @import { CommentRawData, TrackedChangeRawData } from 'overleaf-editor-core/lib/types' * @import { Comment, Entity, ResyncDocContentUpdate, RetainOp, TrackedChange } from './types' * @import { TrackedChangeTransition, TrackingDirective, TrackingType, Update } from './types' * @import { ProjectStructureUpdate } from './types' @@ -764,11 +765,19 @@ class SyncUpdateExpander { } const persistedComments = file.getComments().toArray() - await this.queueUpdatesForOutOfSyncComments( - update, - pathname, - persistedComments - ) + if (update.resyncDocContent.historyOTRanges) { + this.queueUpdatesForOutOfSyncCommentsHistoryOT( + update, + pathname, + file.getComments().toRaw() + ) + } else { + await this.queueUpdatesForOutOfSyncComments( + update, + pathname, + persistedComments + ) + } const persistedChanges = file.getTrackedChanges().asSorted() await this.queueUpdatesForOutOfSyncTrackedChanges( @@ -825,6 +834,91 @@ class SyncUpdateExpander { return expandedUpdate } + /** + * Queue updates for out of sync comments + * + * @param {ResyncDocContentUpdate} update + * @param {string} pathname + * @param {CommentRawData[]} persistedComments + */ + queueUpdatesForOutOfSyncCommentsHistoryOT( + update, + pathname, + persistedComments + ) { + const expectedComments = + update.resyncDocContent.historyOTRanges?.comments ?? [] + const expectedCommentsById = new Map( + expectedComments.map(comment => [comment.id, comment]) + ) + const persistedCommentsById = new Map( + persistedComments.map(comment => [comment.id, comment]) + ) + + // Delete any persisted comment that is not in the expected comment list. + for (const persistedComment of persistedComments) { + if (!expectedCommentsById.has(persistedComment.id)) { + this.expandedUpdates.push({ + doc: update.doc, + op: [{ deleteComment: persistedComment.id }], + meta: { + pathname, + resync: true, + origin: this.origin, + ts: update.meta.ts, + }, + }) + } + } + + for (const expectedComment of expectedComments) { + const persistedComment = persistedCommentsById.get(expectedComment.id) + if ( + persistedComment && + commentRangesAreInSyncHistoryOT(persistedComment, expectedComment) + ) { + if (expectedComment.resolved === persistedComment.resolved) { + // Both comments are identical; do nothing + } else { + // Only the resolved state differs + this.expandedUpdates.push({ + doc: update.doc, + op: [ + { + commentId: expectedComment.id, + resolved: expectedComment.resolved, + }, + ], + meta: { + pathname, + resync: true, + origin: this.origin, + ts: update.meta.ts, + }, + }) + } + } else { + // New comment or ranges differ + this.expandedUpdates.push({ + doc: update.doc, + op: [ + { + commentId: expectedComment.id, + ranges: expectedComment.ranges, + resolved: expectedComment.resolved, + }, + ], + meta: { + pathname, + resync: true, + origin: this.origin, + ts: update.meta.ts, + }, + }) + } + } + } + /** * Queue updates for out of sync comments * @@ -951,6 +1045,7 @@ class SyncUpdateExpander { for (const transition of getTrackedChangesTransitions( persistedChanges, expectedChanges, + update.resyncDocContent.historyOTRanges?.trackedChanges || [], expectedContent.length )) { if (transition.pos > cursor) { @@ -1018,6 +1113,25 @@ class SyncUpdateExpander { } } +/** + * Compares the ranges in the persisted and expected comments + * + * @param {CommentRawData} persistedComment + * @param {CommentRawData} expectedComment + */ +function commentRangesAreInSyncHistoryOT(persistedComment, expectedComment) { + if (persistedComment.ranges.length !== expectedComment.ranges.length) { + return false + } + for (let i = 0; i < persistedComment.ranges.length; i++) { + const persistedRange = persistedComment.ranges[i] + const expectedRange = expectedComment.ranges[i] + if (persistedRange.pos !== expectedRange.pos) return false + if (persistedRange.length !== expectedRange.length) return false + } + return true +} + /** * Compares the ranges in the persisted and expected comments * @@ -1049,11 +1163,13 @@ function commentRangesAreInSync(persistedComment, expectedComment) { * * @param {readonly HistoryTrackedChange[]} persistedChanges * @param {TrackedChange[]} expectedChanges + * @param {TrackedChangeRawData[]} persistedChangesHistoryOT * @param {number} docLength */ function getTrackedChangesTransitions( persistedChanges, expectedChanges, + persistedChangesHistoryOT, docLength ) { /** @type {TrackedChangeTransition[]} */ @@ -1076,6 +1192,19 @@ function getTrackedChangesTransitions( }) } + for (const change of persistedChangesHistoryOT) { + transitions.push({ + stage: 'expected', + pos: change.range.pos, + tracking: change.tracking, + }) + transitions.push({ + stage: 'expected', + pos: change.range.pos + change.range.length, + tracking: { type: 'none' }, + }) + } + for (const change of expectedChanges) { const op = change.op const pos = op.hpos ?? op.p diff --git a/services/project-history/app/js/UpdateCompressor.js b/services/project-history/app/js/UpdateCompressor.js index a6b3789b56..5ae7591a7f 100644 --- a/services/project-history/app/js/UpdateCompressor.js +++ b/services/project-history/app/js/UpdateCompressor.js @@ -169,7 +169,9 @@ export function concatUpdatesWithSameVersion(updates) { lastUpdate.op != null && lastUpdate.v === update.v && lastUpdate.doc === update.doc && - lastUpdate.pathname === update.pathname + lastUpdate.pathname === update.pathname && + EditOperationBuilder.isValid(update.op[0]) === + EditOperationBuilder.isValid(lastUpdate.op[0]) ) { lastUpdate.op = lastUpdate.op.concat(update.op) if (update.meta.doc_hash == null) { diff --git a/services/project-history/app/js/types.ts b/services/project-history/app/js/types.ts index 96701e587f..c11b7741e3 100644 --- a/services/project-history/app/js/types.ts +++ b/services/project-history/app/js/types.ts @@ -3,6 +3,8 @@ import { LinkedFileData, RawEditOperation, RawOrigin, + CommentRawData, + TrackedChangeRawData, } from 'overleaf-editor-core/lib/types' export type Update = @@ -118,6 +120,10 @@ export type ResyncDocContentUpdate = { content: string version: number ranges?: Ranges + historyOTRanges?: { + comments: CommentRawData[] + trackedChanges: TrackedChangeRawData[] + } resolvedCommentIds?: string[] } projectHistoryId: string diff --git a/services/project-history/test/acceptance/js/SyncTests.js b/services/project-history/test/acceptance/js/SyncTests.js index 89e002d4dd..f7420e6cdb 100644 --- a/services/project-history/test/acceptance/js/SyncTests.js +++ b/services/project-history/test/acceptance/js/SyncTests.js @@ -1225,7 +1225,7 @@ describe('Syncing with web and doc-updater', function () { ) }) - it('should fix comments in the history store', function (done) { + it('should add comments in the history store', function (done) { const commentId = 'comment-id' const addComment = MockHistoryStore() .post(`/api/projects/${historyId}/legacy_changes`, body => { @@ -1315,6 +1315,1195 @@ describe('Syncing with web and doc-updater', function () { } ) }) + + it('should add comments in the history store (history-ot)', function (done) { + const commentId = 'comment-id' + const addComment = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + commentId, + ranges: [{ pos: 1, length: 10 }], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + historyOTRanges: { + comments: [ + { + id: commentId, + ranges: [ + { + pos: 1, + length: 10, + }, + ], + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + addComment.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should add tracked changes in the history store', function (done) { + const fixTrackedChange = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + textOperation: [ + { + r: 1, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + r: 1, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + 1, + ], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + ranges: { + changes: [ + { + id: 'id1', + op: { + d: 'a', + p: 0, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + { + id: 'id2', + op: { + i: '\n', + p: 0, + hpos: 1, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + fixTrackedChange.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should add tracked changes in the history store (history-ot)', function (done) { + const fixTrackedChange = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + textOperation: [ + { + r: 1, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + r: 1, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + 1, + ], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + historyOTRanges: { + trackedChanges: [ + { + range: { pos: 0, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + range: { pos: 1, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + fixTrackedChange.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + }) + + describe("when a doc's ranges are out of sync", function () { + const commentId = 'comment-id' + beforeEach(function () { + MockHistoryStore() + .get(`/api/projects/${historyId}/latest/history`) + .reply(200, { + chunk: { + history: { + snapshot: { + files: { + 'main.tex': { + hash: '0a207c060e61f3b88eaee0a8cd0696f46fb155eb', + rangesHash: '0a207c060e61f3b88eaee0a8cd0696f46fb155ec', + stringLength: 3, + }, + }, + }, + changes: [], + }, + startVersion: 0, + }, + }) + + MockHistoryStore() + .get( + `/api/projects/${historyId}/blobs/0a207c060e61f3b88eaee0a8cd0696f46fb155eb` + ) + .reply(200, 'a\nb') + + MockHistoryStore() + .get( + `/api/projects/${historyId}/blobs/0a207c060e61f3b88eaee0a8cd0696f46fb155ec` + ) + .reply( + 200, + JSON.stringify({ + comments: [{ id: commentId, ranges: [{ pos: 0, length: 3 }] }], + trackedChanges: [ + { + range: { pos: 0, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + range: { pos: 2, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + ], + }) + ) + }) + + it('should fix comments in the history store', function (done) { + const addComment = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + commentId, + ranges: [{ pos: 1, length: 2 }], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + ranges: { + comments: [ + { + id: commentId, + op: { + c: 'a', + p: 0, + hpos: 1, + hlen: 2, + t: commentId, + }, + meta: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + ], + changes: [ + { + id: 'id1', + op: { + d: 'a', + p: 0, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + { + id: 'id2', + op: { + i: '\n', + p: 1, + hpos: 2, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + addComment.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should fix resolved state for comments in the history store', function (done) { + const addComment = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + commentId, + resolved: true, + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + resolvedCommentIds: [commentId], + ranges: { + comments: [ + { + id: commentId, + op: { + c: 'a', + p: 0, + hpos: 0, + hlen: 3, + t: commentId, + }, + meta: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + ], + changes: [ + { + id: 'id1', + op: { + d: 'a', + p: 0, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + { + id: 'id2', + op: { + i: '\n', + p: 1, + hpos: 2, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + addComment.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should fix comments in the history store (history-ot)', function (done) { + const addComment = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + commentId, + ranges: [{ pos: 1, length: 2 }], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + historyOTRanges: { + comments: [ + { + id: commentId, + ranges: [ + { + pos: 1, + length: 2, + }, + ], + }, + ], + trackedChanges: [ + { + range: { pos: 0, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + range: { pos: 2, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + addComment.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should fix resolved state for comments in the history store (history-ot)', function (done) { + const addComment = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + commentId, + resolved: true, + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + historyOTRanges: { + comments: [ + { + id: commentId, + ranges: [ + { + pos: 0, + length: 3, + }, + ], + resolved: true, + }, + ], + trackedChanges: [ + { + range: { pos: 0, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + range: { pos: 2, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + addComment.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should fix tracked changes in the history store', function (done) { + const fixTrackedChange = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + textOperation: [ + 1, + { + r: 1, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + { + r: 1, + tracking: { + type: 'none', + }, + }, + ], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + ranges: { + comments: [ + { + id: commentId, + op: { + c: 'a', + p: 0, + hpos: 0, + hlen: 3, + t: commentId, + }, + meta: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + ], + changes: [ + { + id: 'id1', + op: { + d: 'a', + p: 0, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + { + id: 'id2', + op: { + i: '\n', + p: 0, + hpos: 1, + }, + metadata: { + user_id: 'user-id', + ts: this.timestamp, + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + fixTrackedChange.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should fix tracked changes in the history store (history-ot)', function (done) { + const fixTrackedChange = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + textOperation: [ + 1, + { + r: 1, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + { + r: 1, + tracking: { + type: 'none', + }, + }, + ], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + historyOTRanges: { + comments: [ + { + id: commentId, + ranges: [ + { + pos: 0, + length: 3, + }, + ], + }, + ], + trackedChanges: [ + { + range: { pos: 0, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + range: { pos: 1, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + fixTrackedChange.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) + + it('should fix both comments and tracked changes in the history store (history-ot)', function (done) { + const fixTrackedChange = MockHistoryStore() + .post(`/api/projects/${historyId}/legacy_changes`, body => { + expect(body).to.deep.equal([ + // not merged due to comment operation using history-ot and tracked-changes operation using sharejs ot + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + commentId, + ranges: [{ pos: 1, length: 2 }], + }, + ], + origin: { kind: 'test-origin' }, + }, + { + v2Authors: [], + authors: [], + timestamp: this.timestamp.toJSON(), + operations: [ + { + pathname: 'main.tex', + textOperation: [ + 1, + { + r: 1, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + { + r: 1, + tracking: { + type: 'none', + }, + }, + ], + }, + ], + origin: { kind: 'test-origin' }, + }, + ]) + return true + }) + .query({ end_version: 0 }) + .reply(204) + + async.series( + [ + cb => { + ProjectHistoryClient.resyncHistory(this.project_id, cb) + }, + cb => { + const update = { + projectHistoryId: historyId, + resyncProjectStructure: { + docs: [{ path: '/main.tex' }], + files: [], + }, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + const update = { + path: '/main.tex', + projectHistoryId: historyId, + resyncDocContent: { + content: 'a\nb', + historyOTRanges: { + comments: [ + { + id: commentId, + ranges: [ + { + pos: 1, + length: 2, + }, + ], + }, + ], + trackedChanges: [ + { + range: { pos: 0, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'delete', + userId: 'user-id', + }, + }, + { + range: { pos: 1, length: 1 }, + tracking: { + ts: this.timestamp.toJSON(), + type: 'insert', + userId: 'user-id', + }, + }, + ], + }, + }, + doc: this.doc_id, + meta: { + ts: this.timestamp, + }, + } + ProjectHistoryClient.pushRawUpdate(this.project_id, update, cb) + }, + cb => { + ProjectHistoryClient.flushProject(this.project_id, cb) + }, + ], + error => { + if (error) { + return done(error) + } + assert( + fixTrackedChange.isDone(), + `/api/projects/${historyId}/changes should have been called` + ) + done() + } + ) + }) }) describe('resyncProjectStructureOnly', function () {