diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index fe485a52ea..a9a720ed13 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -237,10 +237,14 @@ const DocumentManager = { changeIds = [] } - const { lines, version, ranges } = await DocumentManager.getDoc( - projectId, - docId - ) + const { + lines, + version, + ranges, + pathname, + projectHistoryId, + historyRangesSupport, + } = await DocumentManager.getDoc(projectId, docId) if (lines == null || version == null) { throw new Errors.NotFoundError(`document not found: ${docId}`) } @@ -256,6 +260,26 @@ const DocumentManager = { newRanges, {} ) + + if (historyRangesSupport) { + const historyUpdates = RangesManager.getHistoryUpdatesForAcceptedChanges({ + docId, + acceptedChangeIds: changeIds, + changes: ranges.changes || [], + lines, + pathname, + projectHistoryId, + }) + + if (historyUpdates.length === 0) { + return + } + + await ProjectHistoryRedisManager.promises.queueOps( + projectId, + ...historyUpdates.map(op => JSON.stringify(op)) + ) + } }, async updateCommentState(projectId, docId, commentId, userId, resolved) { diff --git a/services/document-updater/app/js/RangesManager.js b/services/document-updater/app/js/RangesManager.js index 3ca3d3a780..f92bb3aeee 100644 --- a/services/document-updater/app/js/RangesManager.js +++ b/services/document-updater/app/js/RangesManager.js @@ -5,7 +5,7 @@ const logger = require('@overleaf/logger') const OError = require('@overleaf/o-error') const Metrics = require('./Metrics') const _ = require('lodash') -const { isInsert, isDelete, isComment } = require('./Utils') +const { isInsert, isDelete, isComment, getDocLength } = require('./Utils') /** * @typedef {import('./types').Comment} Comment @@ -15,6 +15,7 @@ const { isInsert, isDelete, isComment } = require('./Utils') * @typedef {import('./types').HistoryDeleteOp} HistoryDeleteOp * @typedef {import('./types').HistoryDeleteTrackedChange} HistoryDeleteTrackedChange * @typedef {import('./types').HistoryInsertOp} HistoryInsertOp + * @typedef {import('./types').HistoryRetainOp} HistoryRetainOp * @typedef {import('./types').HistoryOp} HistoryOp * @typedef {import('./types').HistoryUpdate} HistoryUpdate * @typedef {import('./types').InsertOp} InsertOp @@ -138,6 +139,118 @@ const RangesManager = { return newRanges }, + /** + * + * @param {object} args + * @param {string} args.docId + * @param {string[]} args.acceptedChangeIds + * @param {TrackedChange[]} args.changes + * @param {string} args.pathname + * @param {string} args.projectHistoryId + * @param {string[]} args.lines + */ + getHistoryUpdatesForAcceptedChanges({ + docId, + acceptedChangeIds, + changes, + pathname, + projectHistoryId, + lines, + }) { + /** @type {(change: TrackedChange) => boolean} */ + const isAccepted = change => acceptedChangeIds.includes(change.id) + + const historyOps = [] + + // Keep ops in order of offset, with deletes before inserts + const sortedChanges = changes.slice().sort(function (c1, c2) { + const result = c1.op.p - c2.op.p + if (result !== 0) { + return result + } else if (isInsert(c1.op) && isDelete(c2.op)) { + return 1 + } else if (isDelete(c1.op) && isInsert(c2.op)) { + return -1 + } else { + return 0 + } + }) + + const docLength = getDocLength(lines) + let historyDocLength = docLength + for (const change of sortedChanges) { + if (isDelete(change.op)) { + historyDocLength += change.op.d.length + } + } + + let unacceptedDeletes = 0 + for (const change of sortedChanges) { + /** @type {HistoryOp | undefined} */ + let op + + if (isDelete(change.op)) { + if (isAccepted(change)) { + op = { + p: change.op.p, + d: change.op.d, + } + if (unacceptedDeletes > 0) { + op.hpos = op.p + unacceptedDeletes + } + } else { + unacceptedDeletes += change.op.d.length + } + } else if (isInsert(change.op)) { + if (isAccepted(change)) { + op = { + p: change.op.p, + r: change.op.i, + tracking: { + type: 'none', + userId: change.metadata.user_id, + ts: change.metadata.ts, + }, + } + if (unacceptedDeletes > 0) { + op.hpos = op.p + unacceptedDeletes + } + } + } + + if (!op) { + continue + } + + /** @type {HistoryUpdate} */ + const historyOp = { + doc: docId, + op: [op], + meta: { + ...change.metadata, + doc_length: docLength, + pathname, + }, + } + + if (projectHistoryId) { + historyOp.projectHistoryId = projectHistoryId + } + + if (historyOp.meta && historyDocLength !== docLength) { + historyOp.meta.history_doc_length = historyDocLength + } + + historyOps.push(historyOp) + + if (isDelete(change.op) && isAccepted(change)) { + historyDocLength -= change.op.d.length + } + } + + return historyOps + }, + _getRanges(rangesTracker) { // Return the minimal data structure needed, since most documents won't have any // changes or comments diff --git a/services/document-updater/app/js/UpdateManager.js b/services/document-updater/app/js/UpdateManager.js index 5e4b8abc7c..e7cc69c1f4 100644 --- a/services/document-updater/app/js/UpdateManager.js +++ b/services/document-updater/app/js/UpdateManager.js @@ -7,7 +7,6 @@ const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager') const RealTimeRedisManager = require('./RealTimeRedisManager') const ShareJsUpdateManager = require('./ShareJsUpdateManager') const HistoryManager = require('./HistoryManager') -const _ = require('lodash') const logger = require('@overleaf/logger') const Metrics = require('./Metrics') const Errors = require('./Errors') @@ -15,7 +14,7 @@ const DocumentManager = require('./DocumentManager') const RangesManager = require('./RangesManager') const SnapshotManager = require('./SnapshotManager') const Profiler = require('./Profiler') -const { isInsert, isDelete } = require('./Utils') +const { isInsert, isDelete, getDocLength } = require('./Utils') /** * @typedef {import("./types").DeleteOp} DeleteOp @@ -308,11 +307,7 @@ const UpdateManager = { ranges, historyRangesSupport ) { - let docLength = _.reduce(lines, (chars, line) => chars + line.length, 0) - // Add newline characters. Lines are joined by newlines, but the last line - // doesn't include a newline. We must make a special case for an empty list - // so that it doesn't report a doc length of -1. - docLength += Math.max(lines.length - 1, 0) + let docLength = getDocLength(lines) let historyDocLength = docLength for (const change of ranges.changes ?? []) { if ('d' in change.op) { diff --git a/services/document-updater/app/js/Utils.js b/services/document-updater/app/js/Utils.js index 115407181d..1655d9c779 100644 --- a/services/document-updater/app/js/Utils.js +++ b/services/document-updater/app/js/Utils.js @@ -1,4 +1,5 @@ // @ts-check +const _ = require('lodash') /** * @typedef {import('./types').CommentOp} CommentOp @@ -38,6 +39,22 @@ function isComment(op) { return 'c' in op && op.c != null } +/** + * Get the length of a document from its lines + * + * @param {string[]} lines + * @returns {number} + */ +function getDocLength(lines) { + let docLength = _.reduce(lines, (chars, line) => chars + line.length, 0) + // Add newline characters. Lines are joined by newlines, but the last line + // doesn't include a newline. We must make a special case for an empty list + // so that it doesn't report a doc length of -1. + docLength += Math.max(lines.length - 1, 0) + + return docLength +} + /** * Adds given tracked deletes to the given content. * @@ -66,4 +83,10 @@ function addTrackedDeletesToContent(content, trackedChanges) { return result } -module.exports = { isInsert, isDelete, isComment, addTrackedDeletesToContent } +module.exports = { + isInsert, + isDelete, + isComment, + addTrackedDeletesToContent, + getDocLength, +} diff --git a/services/document-updater/app/js/types.ts b/services/document-updater/app/js/types.ts index 6d451f4205..8ec33df405 100644 --- a/services/document-updater/app/js/types.ts +++ b/services/document-updater/app/js/types.ts @@ -1,4 +1,7 @@ +import { TrackingPropsRawData } from 'overleaf-editor-core/types/lib/types' + export type Update = { + doc: string op: Op[] v: number meta?: { @@ -8,7 +11,7 @@ export type Update = { projectHistoryId?: string } -export type Op = InsertOp | DeleteOp | CommentOp +export type Op = InsertOp | DeleteOp | CommentOp | RetainOp export type InsertOp = { i: string @@ -16,6 +19,11 @@ export type InsertOp = { u?: boolean } +export type RetainOp = { + r: string + p: number +} + export type DeleteOp = { d: string p: number @@ -52,7 +60,7 @@ export type TrackedChange = { } } -export type HistoryOp = HistoryInsertOp | HistoryDeleteOp | HistoryCommentOp +export type HistoryOp = HistoryInsertOp | HistoryDeleteOp | HistoryCommentOp | HistoryRetainOp export type HistoryInsertOp = InsertOp & { commentIds?: string[] @@ -60,6 +68,11 @@ export type HistoryInsertOp = InsertOp & { trackedDeleteRejection?: boolean } +export type HistoryRetainOp = RetainOp & { + hpos?: number + tracking?: TrackingPropsRawData +} + export type HistoryDeleteOp = DeleteOp & { hpos?: number trackedChanges?: HistoryDeleteTrackedChange[] @@ -78,7 +91,8 @@ export type HistoryCommentOp = CommentOp & { export type HistoryUpdate = { op: HistoryOp[] - v: number + doc: string + v?: number meta?: { pathname?: string doc_length?: number diff --git a/services/document-updater/test/acceptance/js/RangesTests.js b/services/document-updater/test/acceptance/js/RangesTests.js index 09aaa2bcf8..0cb1bfbd30 100644 --- a/services/document-updater/test/acceptance/js/RangesTests.js +++ b/services/document-updater/test/acceptance/js/RangesTests.js @@ -6,6 +6,9 @@ const { db, ObjectId } = require('../../../app/js/mongodb') const MockWebApi = require('./helpers/MockWebApi') const DocUpdaterClient = require('./helpers/DocUpdaterClient') const DocUpdaterApp = require('./helpers/DocUpdaterApp') +const RangesManager = require('../../../app/js/RangesManager') + +const sandbox = sinon.createSandbox() describe('Ranges', function () { before(function (done) { @@ -309,7 +312,7 @@ describe('Ranges', function () { describe('accepting a change', function () { beforeEach(function (done) { - sinon.spy(MockWebApi, 'setDocument') + sandbox.spy(MockWebApi, 'setDocument') this.project_id = DocUpdaterClient.randomId() this.user_id = DocUpdaterClient.randomId() this.id_seed = '587357bd35e64f6157' @@ -361,7 +364,7 @@ describe('Ranges', function () { }) }) afterEach(function () { - MockWebApi.setDocument.restore() + sandbox.restore() }) it('should remove the change after accepting', function (done) { @@ -424,6 +427,219 @@ describe('Ranges', function () { }) }) + describe('accepting multiple changes', function () { + beforeEach(function (done) { + this.getHistoryUpdatesSpy = sandbox.spy( + RangesManager, + 'getHistoryUpdatesForAcceptedChanges' + ) + + this.project_id = DocUpdaterClient.randomId() + this.user_id = DocUpdaterClient.randomId() + this.doc = { + id: DocUpdaterClient.randomId(), + lines: ['aaa', 'bbb', 'ccc', 'ddd', 'eee'], + } + + DocUpdaterClient.enableHistoryRangesSupport(this.doc.id, (error, res) => { + if (error) { + throw error + } + MockWebApi.insertDoc(this.project_id, this.doc.id, { + lines: this.doc.lines, + version: 0, + }) + + DocUpdaterClient.preloadDoc(this.project_id, this.doc.id, error => { + if (error != null) { + throw error + } + + this.id_seed_1 = 'tc_1' + this.id_seed_2 = 'tc_2' + this.id_seed_3 = 'tc_3' + + this.updates = [ + { + doc: this.doc.id, + op: [{ d: 'bbb', p: 4 }], + v: 0, + meta: { + user_id: this.user_id, + tc: this.id_seed_1, + }, + }, + { + doc: this.doc.id, + op: [{ d: 'ccc', p: 5 }], + v: 1, + meta: { + user_id: this.user_id, + tc: this.id_seed_2, + }, + }, + { + doc: this.doc.id, + op: [{ d: 'ddd', p: 6 }], + v: 2, + meta: { + user_id: this.user_id, + tc: this.id_seed_3, + }, + }, + ] + + DocUpdaterClient.sendUpdates( + this.project_id, + this.doc.id, + this.updates, + error => { + if (error != null) { + throw error + } + setTimeout(() => { + DocUpdaterClient.getDoc( + this.project_id, + this.doc.id, + (error, res, data) => { + if (error != null) { + throw error + } + const { ranges } = data + const changeOps = ranges.changes + .map(change => change.op) + .flat() + changeOps.should.deep.equal([ + { d: 'bbb', p: 4 }, + { d: 'ccc', p: 5 }, + { d: 'ddd', p: 6 }, + ]) + done() + } + ) + }, 200) + } + ) + }) + }) + }) + afterEach(function () { + sandbox.restore() + }) + + it('accepting changes in order', function (done) { + DocUpdaterClient.acceptChanges( + this.project_id, + this.doc.id, + [ + this.id_seed_1 + '000001', + this.id_seed_2 + '000001', + this.id_seed_3 + '000001', + ], + error => { + if (error != null) { + throw error + } + + const historyUpdates = this.getHistoryUpdatesSpy.returnValues[0] + expect(historyUpdates[0]).to.deep.equal({ + doc: this.doc.id, + meta: { + pathname: '/a/b/c.tex', + doc_length: 10, + history_doc_length: 19, + ts: historyUpdates[0].meta.ts, + user_id: this.user_id, + }, + op: [{ p: 4, d: 'bbb' }], + }) + + expect(historyUpdates[1]).to.deep.equal({ + doc: this.doc.id, + meta: { + pathname: '/a/b/c.tex', + doc_length: 10, + history_doc_length: 16, + ts: historyUpdates[1].meta.ts, + user_id: this.user_id, + }, + op: [{ p: 5, d: 'ccc' }], + }) + + expect(historyUpdates[2]).to.deep.equal({ + doc: this.doc.id, + meta: { + pathname: '/a/b/c.tex', + doc_length: 10, + history_doc_length: 13, + ts: historyUpdates[2].meta.ts, + user_id: this.user_id, + }, + op: [{ p: 6, d: 'ddd' }], + }) + + done() + } + ) + }) + + it('accepting changes in reverse order', function (done) { + DocUpdaterClient.acceptChanges( + this.project_id, + this.doc.id, + [ + this.id_seed_3 + '000001', + this.id_seed_2 + '000001', + this.id_seed_1 + '000001', + ], + error => { + if (error != null) { + throw error + } + + const historyUpdates = this.getHistoryUpdatesSpy.returnValues[0] + expect(historyUpdates[0]).to.deep.equal({ + doc: this.doc.id, + meta: { + pathname: '/a/b/c.tex', + doc_length: 10, + history_doc_length: 19, + ts: historyUpdates[0].meta.ts, + user_id: this.user_id, + }, + op: [{ p: 4, d: 'bbb' }], + }) + + expect(historyUpdates[1]).to.deep.equal({ + doc: this.doc.id, + meta: { + pathname: '/a/b/c.tex', + doc_length: 10, + history_doc_length: 16, + ts: historyUpdates[1].meta.ts, + user_id: this.user_id, + }, + op: [{ p: 5, d: 'ccc' }], + }) + + expect(historyUpdates[2]).to.deep.equal({ + doc: this.doc.id, + meta: { + pathname: '/a/b/c.tex', + doc_length: 10, + history_doc_length: 13, + ts: historyUpdates[2].meta.ts, + user_id: this.user_id, + }, + op: [{ p: 6, d: 'ddd' }], + }) + + done() + } + ) + }) + }) + describe('deleting a comment range', function () { before(function (done) { this.project_id = DocUpdaterClient.randomId() diff --git a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js b/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js index 9a3234628c..04cc1feebe 100644 --- a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js +++ b/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js @@ -119,6 +119,10 @@ module.exports = DocUpdaterClient = { ) }, + enableHistoryRangesSupport(docId, cb) { + rclient.sadd(keys.historyRangesSupport(), docId, cb) + }, + preloadDoc(projectId, docId, callback) { DocUpdaterClient.getDoc(projectId, docId, callback) }, @@ -193,6 +197,16 @@ module.exports = DocUpdaterClient = { ) }, + acceptChanges(projectId, docId, changeIds, callback) { + request.post( + { + url: `http://localhost:3003/project/${projectId}/doc/${docId}/change/accept`, + json: { change_ids: changeIds }, + }, + callback + ) + }, + removeComment(projectId, docId, comment, callback) { request.del( `http://localhost:3003/project/${projectId}/doc/${docId}/comment/${comment}`, diff --git a/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js b/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js index d4e3ab9250..b932a43835 100644 --- a/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js +++ b/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js @@ -637,6 +637,265 @@ describe('RangesManager', function () { }) }) }) + + describe('getHistoryUpdatesForAcceptedChanges', function () { + beforeEach(function () { + this.RangesManager = SandboxedModule.require(MODULE_PATH, { + requires: { + '@overleaf/ranges-tracker': (this.RangesTracker = + SandboxedModule.require('@overleaf/ranges-tracker')), + '@overleaf/metrics': {}, + }, + }) + }) + + it('should create history updates for accepted track inserts', function () { + // 'one two three four five' <-- text before changes + const ranges = { + comments: [], + changes: makeRanges([ + { i: 'lorem', p: 0 }, + { i: 'ipsum', p: 15 }, + ]), + } + const lines = ['loremone two thipsumree four five'] + + const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({ + docId: this.doc_id, + acceptedChangeIds: ranges.changes.map(change => change.id), + changes: ranges.changes, + pathname: '', + projectHistoryId: '', + lines, + }) + + expect(result).to.deep.equal([ + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 33, + pathname: '', + ts: ranges.changes[0].metadata.ts, + }, + op: [ + { + r: 'lorem', + p: 0, + tracking: { + type: 'none', + userId: this.user_id, + ts: ranges.changes[0].metadata.ts, + }, + }, + ], + }, + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 33, + pathname: '', + ts: ranges.changes[1].metadata.ts, + }, + op: [ + { + r: 'ipsum', + p: 15, + tracking: { + type: 'none', + userId: this.user_id, + ts: ranges.changes[1].metadata.ts, + }, + }, + ], + }, + ]) + }) + + it('should create history updates for accepted track deletes', function () { + // 'one two three four five' <-- text before changes + const ranges = { + comments: [], + changes: makeRanges([ + { d: 'two', p: 4 }, + { d: 'three', p: 5 }, + ]), + } + const lines = ['one four five'] + + const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({ + docId: this.doc_id, + acceptedChangeIds: ranges.changes.map(change => change.id), + changes: ranges.changes, + pathname: '', + projectHistoryId: '', + lines, + }) + + expect(result).to.deep.equal([ + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 15, + history_doc_length: 23, + pathname: '', + ts: ranges.changes[0].metadata.ts, + }, + op: [ + { + d: 'two', + p: 4, + }, + ], + }, + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 15, + history_doc_length: 20, + pathname: '', + ts: ranges.changes[1].metadata.ts, + }, + op: [ + { + d: 'three', + p: 5, + }, + ], + }, + ]) + }) + + it('should create history updates with unaccepted deletes', function () { + // 'one two three four five' <-- text before changes + const ranges = { + comments: [], + changes: makeRanges([ + { d: 'two', p: 4 }, + { d: 'three', p: 5 }, + ]), + } + const lines = ['one four five'] + + const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({ + docId: this.doc_id, + acceptedChangeIds: [ranges.changes[1].id], + changes: ranges.changes, + pathname: '', + projectHistoryId: '', + lines, + }) + + expect(result).to.deep.equal([ + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 15, + history_doc_length: 23, + pathname: '', + ts: ranges.changes[1].metadata.ts, + }, + op: [ + { + d: 'three', + p: 5, + hpos: 8, + }, + ], + }, + ]) + }) + + it('should create history updates with mixed track changes', function () { + // 'one two three four five' <-- text before changes + const ranges = { + comments: [], + changes: makeRanges([ + { d: 'two', p: 4 }, + { d: 'three', p: 5 }, + { i: 'xxx ', p: 6 }, + { d: 'five', p: 15 }, + ]), + } + const lines = ['one xxx four '] + + const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({ + docId: this.doc_id, + acceptedChangeIds: [ + ranges.changes[0].id, + // ranges.changes[1].id - second delete is not accepted + ranges.changes[2].id, + ranges.changes[3].id, + ], + changes: ranges.changes, + pathname: '', + projectHistoryId: '', + lines, + }) + + expect(result).to.deep.equal([ + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 15, + history_doc_length: 27, + pathname: '', + ts: ranges.changes[0].metadata.ts, + }, + op: [ + { + d: 'two', + p: 4, + }, + ], + }, + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 15, + history_doc_length: 24, + pathname: '', + ts: ranges.changes[1].metadata.ts, + }, + op: [ + { + r: 'xxx ', + p: 6, + hpos: 11, + tracking: { + type: 'none', + userId: this.user_id, + ts: ranges.changes[1].metadata.ts, + }, + }, + ], + }, + { + doc: this.doc_id, + meta: { + user_id: TEST_USER_ID, + doc_length: 15, + history_doc_length: 24, + pathname: '', + ts: ranges.changes[2].metadata.ts, + }, + op: [ + { + d: 'five', + p: 15, + hpos: 20, + }, + ], + }, + ]) + }) + }) }) function makeRanges(ops) { @@ -646,7 +905,7 @@ function makeRanges(ops) { changes.push({ id: id.toString(), op, - metadata: { user_id: TEST_USER_ID }, + metadata: { user_id: TEST_USER_ID, ts: new Date() }, }) id += 1 } diff --git a/services/project-history/app/js/UpdateCompressor.js b/services/project-history/app/js/UpdateCompressor.js index 021edae0a5..8b7bca80b1 100644 --- a/services/project-history/app/js/UpdateCompressor.js +++ b/services/project-history/app/js/UpdateCompressor.js @@ -71,6 +71,8 @@ function adjustLengthByOp(length, op, opts = {}) { } else { return length - op.d.length } + } else if ('r' in op && op.r != null) { + return length } else if ('c' in op && op.c != null) { return length } else { diff --git a/services/project-history/app/js/UpdateTranslator.js b/services/project-history/app/js/UpdateTranslator.js index ded0c4f4d7..8150ba8d96 100644 --- a/services/project-history/app/js/UpdateTranslator.js +++ b/services/project-history/app/js/UpdateTranslator.js @@ -12,6 +12,7 @@ import * as OperationsCompressor from './OperationsCompressor.js' * @typedef {import('./types').DeleteOp} DeleteCommentUpdate * @typedef {import('./types').DeleteOp} DeleteOp * @typedef {import('./types').InsertOp} InsertOp + * @typedef {import('./types').RetainOp} RetainOp * @typedef {import('./types').Op} Op * @typedef {import('./types').RawScanOp} RawScanOp * @typedef {import('./types').RenameUpdate} RenameUpdate @@ -292,7 +293,7 @@ class OperationsBuilder { return } - if (!isInsert(op) && !isDelete(op)) { + if (!isInsert(op) && !isDelete(op) && !isRetain(op)) { throw new Errors.UnexpectedOpTypeError('unexpected op type', { op }) } @@ -330,6 +331,14 @@ class OperationsBuilder { } } + if (isRetain(op)) { + if (op.tracking) { + this.retain(op.r.length, { tracking: op.tracking }) + } else { + this.retain(op.r.length) + } + } + if (isDelete(op)) { const changes = op.trackedChanges ?? [] @@ -465,6 +474,14 @@ function isInsert(op) { return 'i' in op && op.i != null } +/** + * @param {Op} op + * @returns {op is RetainOp} + */ +function isRetain(op) { + return 'r' in op && op.r != null +} + /** * @param {Op} op * @returns {op is DeleteOp} diff --git a/services/project-history/app/js/types.ts b/services/project-history/app/js/types.ts index cf46092d48..b52b24e20d 100644 --- a/services/project-history/app/js/types.ts +++ b/services/project-history/app/js/types.ts @@ -62,7 +62,14 @@ export type RenameUpdate = ProjectUpdateBase & { new_pathname: string } -export type Op = InsertOp | DeleteOp | CommentOp +export type Op = RetainOp | InsertOp | DeleteOp | CommentOp + +export type RetainOp = { + r: string + p: number + hpos?: number + tracking?: TrackingProps +} export type InsertOp = { i: string diff --git a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js index ad64125705..8a6da56a97 100644 --- a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js +++ b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js @@ -97,6 +97,42 @@ describe('UpdateCompressor', function () { ]) }) + it('should not ignore retain ops with tracking data', function () { + expect( + this.UpdateCompressor.convertToSingleOpUpdates([ + { + op: [ + (this.op1 = { p: 0, i: 'Foo' }), + (this.op2 = { + p: 9, + r: 'baz', + tracking: { type: 'none', ts: this.ts1, userId: this.user_id }, + }), + (this.op3 = { p: 6, i: 'bar' }), + ], + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 10 }, + v: 42, + }, + ]) + ).to.deep.equal([ + { + op: this.op1, + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 10 }, + v: 42, + }, + { + op: this.op2, + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 13 }, + v: 42, + }, + { + op: this.op3, + meta: { ts: this.ts1, user_id: this.user_id, doc_length: 13 }, + v: 42, + }, + ]) + }) + it('should update doc_length when splitting after an insert', function () { expect( this.UpdateCompressor.convertToSingleOpUpdates([ diff --git a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js index 2b994539d5..447281ada1 100644 --- a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js +++ b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js @@ -568,6 +568,118 @@ describe('UpdateTranslator', function () { ]) }) + it('should translate retains without tracking data', function () { + const updates = [ + { + update: { + doc: this.doc_id, + op: [ + { + p: 3, + r: 'lo', + }, + ], + v: this.version, + meta: { + user_id: this.user_id, + ts: new Date(this.timestamp).getTime(), + pathname: '/main.tex', + doc_length: 20, + }, + }, + }, + ] + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [20], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + [this.doc_id]: { + pathname: 'main.tex', + v: 0, + }, + }, + }, + ]) + }) + + it('can translate retains with tracking data', function () { + const updates = [ + { + update: { + doc: this.doc_id, + op: [ + { + p: 3, + r: 'lo', + tracking: { + type: 'none', + ts: this.timestamp, + userId: this.user_id, + }, + }, + ], + v: this.version, + meta: { + user_id: this.user_id, + ts: new Date(this.timestamp).getTime(), + pathname: '/main.tex', + doc_length: 20, + }, + }, + }, + ] + + const changes = this.UpdateTranslator.convertToChanges( + this.project_id, + updates + ).map(change => change.toRaw()) + + expect(changes).to.deep.equal([ + { + authors: [], + operations: [ + { + pathname: 'main.tex', + textOperation: [ + 3, + { + r: 2, + tracking: { + type: 'none', + ts: this.timestamp, + userId: this.user_id, + }, + }, + 15, + ], + }, + ], + v2Authors: [this.user_id], + timestamp: this.timestamp, + v2DocVersions: { + '59bfd450e3028c4d40a1e9ab': { + pathname: 'main.tex', + v: 0, + }, + }, + }, + ]) + }) + it('can translate insertions at the start and end (with zero retained)', function () { const updates = [ {