diff --git a/libraries/overleaf-editor-core/lib/file_data/comment_list.js b/libraries/overleaf-editor-core/lib/file_data/comment_list.js index 9c7086e184..5452d2b288 100644 --- a/libraries/overleaf-editor-core/lib/file_data/comment_list.js +++ b/libraries/overleaf-editor-core/lib/file_data/comment_list.js @@ -69,9 +69,12 @@ class CommentList { /** * @param {Range} range - * @param {{ commentIds: string[] }} opts + * @param {{ commentIds?: string[] }} opts */ applyInsert(range, opts = { commentIds: [] }) { + if (!opts.commentIds) { + opts.commentIds = [] + } for (const [commentId, comment] of this.comments) { comment.applyInsert( range.pos, @@ -85,13 +88,21 @@ class CommentList { * @param {Range} range */ applyDelete(range) { - for (const [commentId, comment] of this.comments) { + for (const [, comment] of this.comments) { comment.applyDelete(range) - if (comment.isEmpty()) { - this.delete(commentId) - } } } + + /** + * + * @param {Range} range + * @returns {string[]} + */ + idsCoveringRange(range) { + return Array.from(this.comments.entries()) + .filter(([, comment]) => comment.ranges.some(r => r.contains(range))) + .map(([id]) => id) + } } module.exports = CommentList diff --git a/libraries/overleaf-editor-core/lib/file_data/string_file_data.js b/libraries/overleaf-editor-core/lib/file_data/string_file_data.js index e96d2b9687..73b7e27254 100644 --- a/libraries/overleaf-editor-core/lib/file_data/string_file_data.js +++ b/libraries/overleaf-editor-core/lib/file_data/string_file_data.js @@ -34,7 +34,11 @@ class StringFileData extends FileData { * @returns {StringFileData} */ static fromRaw(raw) { - return new StringFileData(raw.content, raw.comments || []) + return new StringFileData( + raw.content, + raw.comments || [], + raw.trackedChanges || [] + ) } /** diff --git a/libraries/overleaf-editor-core/lib/file_data/tracked_change.js b/libraries/overleaf-editor-core/lib/file_data/tracked_change.js index ad4b8ee85d..7c10873ea1 100644 --- a/libraries/overleaf-editor-core/lib/file_data/tracked_change.js +++ b/libraries/overleaf-editor-core/lib/file_data/tracked_change.js @@ -8,12 +8,19 @@ const TrackingProps = require('./tracking_props') class TrackedChange { /** - * * @param {Range} range * @param {TrackingProps} tracking */ constructor(range, tracking) { + /** + * @readonly + * @type {Range} + */ this.range = range + /** + * @readonly + * @type {TrackingProps} + */ this.tracking = tracking } @@ -60,17 +67,22 @@ class TrackedChange { * Merges another tracked change into this, updating the range and tracking * timestamp * @param {TrackedChange} other - * @returns {void} + * @returns {TrackedChange} */ merge(other) { if (!this.canMerge(other)) { throw new Error('Cannot merge tracked changes') } - this.range = this.range.merge(other.range) - this.tracking.ts = - this.tracking.ts.getTime() > other.tracking.ts.getTime() - ? this.tracking.ts - : other.tracking.ts + return new TrackedChange( + this.range.merge(other.range), + new TrackingProps( + this.tracking.type, + this.tracking.userId, + this.tracking.ts.getTime() > other.tracking.ts.getTime() + ? this.tracking.ts + : other.tracking.ts + ) + ) } } diff --git a/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js b/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js index d6399b76e7..b9264062b8 100644 --- a/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js +++ b/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js @@ -46,6 +46,16 @@ class TrackedChangeList { return this.trackedChanges.filter(change => range.contains(change.range)) } + /** + * Returns the tracking props for a given range. + * @param {Range} range + * @returns {TrackingProps | undefined} + */ + propsAtRange(range) { + return this.trackedChanges.find(change => change.range.contains(range)) + ?.tracking + } + /** * Removes the tracked changes that are fully included in the range * @param {Range} range @@ -80,7 +90,7 @@ class TrackedChangeList { const last = newTrackedChanges[newTrackedChanges.length - 1] const current = this.trackedChanges[i] if (last.canMerge(current)) { - last.merge(current) + newTrackedChanges[newTrackedChanges.length - 1] = last.merge(current) } else { newTrackedChanges.push(current) } @@ -103,8 +113,12 @@ class TrackedChangeList { trackedChange.range.startIsAfter(cursor) || cursor === trackedChange.range.start ) { - trackedChange.range = trackedChange.range.moveBy(insertedText.length) - newTrackedChanges.push(trackedChange) + newTrackedChanges.push( + new TrackedChange( + trackedChange.range.moveBy(insertedText.length), + trackedChange.tracking + ) + ) } else if (cursor === trackedChange.range.end) { // The insertion is at the end of the tracked change. So we don't need // to move it. @@ -116,17 +130,15 @@ class TrackedChangeList { cursor, insertedText.length ) - const firstPart = new TrackedChange( - firstRange, - trackedChange.tracking.clone() - ) - newTrackedChanges.push(firstPart) + const firstPart = new TrackedChange(firstRange, trackedChange.tracking) + if (!firstPart.range.isEmpty()) { + newTrackedChanges.push(firstPart) + } // second part will be added at the end if it is a tracked insertion - const thirdPart = new TrackedChange( - thirdRange, - trackedChange.tracking.clone() - ) - newTrackedChanges.push(thirdPart) + const thirdPart = new TrackedChange(thirdRange, trackedChange.tracking) + if (!thirdPart.range.isEmpty()) { + newTrackedChanges.push(thirdPart) + } } else { newTrackedChanges.push(trackedChange) } @@ -157,11 +169,19 @@ class TrackedChangeList { if (deletedRange.contains(trackedChange.range)) { continue } else if (deletedRange.overlaps(trackedChange.range)) { - trackedChange.range = trackedChange.range.subtract(deletedRange) - newTrackedChanges.push(trackedChange) + const newRange = trackedChange.range.subtract(deletedRange) + if (!newRange.isEmpty()) { + newTrackedChanges.push( + new TrackedChange(newRange, trackedChange.tracking) + ) + } } else if (trackedChange.range.startIsAfter(cursor)) { - trackedChange.range = trackedChange.range.moveBy(-length) - newTrackedChanges.push(trackedChange) + newTrackedChanges.push( + new TrackedChange( + trackedChange.range.moveBy(-length), + trackedChange.tracking + ) + ) } else { newTrackedChanges.push(trackedChange) } @@ -188,28 +208,41 @@ class TrackedChangeList { } else if (retainedRange.overlaps(trackedChange.range)) { if (trackedChange.range.contains(retainedRange)) { const [leftRange, rightRange] = trackedChange.range.splitAt(cursor) - newTrackedChanges.push( - new TrackedChange(leftRange, trackedChange.tracking.clone()) - ) - newTrackedChanges.push( - new TrackedChange( - rightRange.moveBy(length).shrinkBy(length), - trackedChange.tracking.clone() + if (!leftRange.isEmpty()) { + newTrackedChanges.push( + new TrackedChange(leftRange, trackedChange.tracking) ) - ) + } + if (!rightRange.isEmpty() && rightRange.length > length) { + newTrackedChanges.push( + new TrackedChange( + rightRange.moveBy(length).shrinkBy(length), + trackedChange.tracking + ) + ) + } } else if (retainedRange.start <= trackedChange.range.start) { // overlaps to the left const [, reducedRange] = trackedChange.range.splitAt( retainedRange.end ) - trackedChange.range = reducedRange - newTrackedChanges.push(trackedChange) + if (!reducedRange.isEmpty()) { + newTrackedChanges.push( + new TrackedChange(reducedRange, trackedChange.tracking) + ) + } } else { // overlaps to the right const [reducedRange] = trackedChange.range.splitAt(cursor) - trackedChange.range = reducedRange - newTrackedChanges.push(trackedChange) + if (!reducedRange.isEmpty()) { + newTrackedChanges.push( + new TrackedChange(reducedRange, trackedChange.tracking) + ) + } } + } else { + // keep the range + newTrackedChanges.push(trackedChange) } } if (opts.tracking?.type === 'delete' || opts.tracking?.type === 'insert') { diff --git a/libraries/overleaf-editor-core/lib/file_data/tracking_props.js b/libraries/overleaf-editor-core/lib/file_data/tracking_props.js index f79c018c3c..a03246b521 100644 --- a/libraries/overleaf-editor-core/lib/file_data/tracking_props.js +++ b/libraries/overleaf-editor-core/lib/file_data/tracking_props.js @@ -1,6 +1,6 @@ // @ts-check /** - * @typedef {import("../types").TrackedChangeRawData} TrackedChangeRawData + * @typedef {import("../types").TrackingPropsRawData} TrackingPropsRawData */ class TrackingProps { @@ -22,6 +22,7 @@ class TrackingProps { */ this.userId = userId /** + * @readonly * @type {Date} */ this.ts = ts @@ -29,13 +30,16 @@ class TrackingProps { /** * - * @param {TrackedChangeRawData['tracking']} raw - * @returns + * @param {TrackingPropsRawData} raw + * @returns {TrackingProps} */ static fromRaw(raw) { return new TrackingProps(raw.type, raw.userId, new Date(raw.ts)) } + /** + * @returns {TrackingPropsRawData} + */ toRaw() { return { type: this.type, @@ -44,8 +48,15 @@ class TrackingProps { } } - clone() { - return new TrackingProps(this.type, this.userId, this.ts) + equals(other) { + if (!(other instanceof TrackingProps)) { + return false + } + return ( + this.type === other.type && + this.userId === other.userId && + this.ts.getTime() === other.ts.getTime() + ) } } diff --git a/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js b/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js index dfd4c08faa..a4c80eff76 100644 --- a/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js +++ b/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js @@ -1,11 +1,14 @@ // @ts-check -/** @typedef {import('./edit_operation')} EditOperation */ +/** + * @typedef {import('./edit_operation')} EditOperation + * @typedef {import('../types').RawEditOperation} RawEditOperation + */ const TextOperation = require('./text_operation') class EditOperationBuilder { /** * - * @param {object} raw + * @param {RawEditOperation} raw * @returns {EditOperation} */ static fromJSON(raw) { diff --git a/libraries/overleaf-editor-core/lib/operation/scan_op.js b/libraries/overleaf-editor-core/lib/operation/scan_op.js index 9c9c0df132..dbdb3cce84 100644 --- a/libraries/overleaf-editor-core/lib/operation/scan_op.js +++ b/libraries/overleaf-editor-core/lib/operation/scan_op.js @@ -5,9 +5,15 @@ const { InvalidInsertionError, UnprocessableError, } = require('../errors') +const TrackingProps = require('../file_data/tracking_props') -/** @typedef {{ result: string, inputCursor: number}} ApplyContext */ -/** @typedef {{ length: number, inputCursor: number, readonly inputLength: number}} LengthApplyContext */ +/** + * @typedef {{ length: number, inputCursor: number, readonly inputLength: number}} LengthApplyContext + * @typedef {import('../types').RawScanOp} RawScanOp + * @typedef {import('../types').RawInsertOp} RawInsertOp + * @typedef {import('../types').RawRetainOp} RawRetainOp + * @typedef {import('../types').RawRemoveOp} RawRemoveOp + */ class ScanOp { constructor() { @@ -16,16 +22,6 @@ class ScanOp { } } - /** - * Applies an operation to a string - * @param {string} input - * @param {ApplyContext} current - * @returns {ApplyContext} - */ - apply(input, current) { - throw new Error('abstract method') - } - /** * Applies an operation to a length * @param {LengthApplyContext} current @@ -35,12 +31,15 @@ class ScanOp { throw new Error('abstract method') } + /** + * @returns {RawScanOp} + */ toJSON() { throw new Error('abstract method') } /** - * @param {object} raw + * @param {RawScanOp} raw * @returns {ScanOp} */ static fromJSON(raw) { @@ -87,7 +86,13 @@ class ScanOp { } class InsertOp extends ScanOp { - constructor(insertion) { + /** + * + * @param {string} insertion + * @param {TrackingProps | undefined} tracking + * @param {string[] | undefined} commentIds + */ + constructor(insertion, tracking = undefined, commentIds = undefined) { super() if (typeof insertion !== 'string') { throw new InvalidInsertionError('insertion must be a string') @@ -95,12 +100,17 @@ class InsertOp extends ScanOp { if (containsNonBmpChars(insertion)) { throw new InvalidInsertionError('insertion contains non-BMP characters') } + /** @type {string} */ this.insertion = insertion + /** @type {TrackingProps | undefined} */ + this.tracking = tracking + /** @type {string[] | undefined} */ + this.commentIds = commentIds } /** * - * @param {{i: string} | string} op + * @param {RawInsertOp} op * @returns {InsertOp} */ static fromJSON(op) { @@ -113,21 +123,11 @@ class InsertOp extends ScanOp { 'insert operation must have a string property' ) } - return new InsertOp(op.i) - } - - /** - * @inheritdoc - * @param {string} input - * @param {ApplyContext} current - * @returns {ApplyContext} - * */ - apply(input, current) { - if (containsNonBmpChars(this.insertion)) { - throw new InvalidInsertionError(input, this.toJSON()) - } - current.result += this.insertion - return current + return new InsertOp( + op.i, + op.tracking && TrackingProps.fromRaw(op.tracking), + op.commentIds + ) } /** @@ -145,24 +145,69 @@ class InsertOp extends ScanOp { if (!(other instanceof InsertOp)) { return false } - return this.insertion === other.insertion + if (this.insertion !== other.insertion) { + return false + } + if (this.tracking) { + if (!this.tracking.equals(other.tracking)) { + return false + } + } else if (other.tracking) { + return false + } + + if (this.commentIds) { + return ( + this.commentIds.length === other.commentIds?.length && + this.commentIds.every(id => other.commentIds?.includes(id)) + ) + } + return !other.commentIds } canMergeWith(other) { - return other instanceof InsertOp + if (!(other instanceof InsertOp)) { + return false + } + if (this.tracking) { + if (!this.tracking.equals(other.tracking)) { + return false + } + } else if (other.tracking) { + return false + } + if (this.commentIds) { + return ( + this.commentIds.length === other.commentIds?.length && + this.commentIds.every(id => other.commentIds?.includes(id)) + ) + } + return !other.commentIds } mergeWith(other) { - if (!(other instanceof InsertOp)) { + if (!this.canMergeWith(other)) { throw new Error('Cannot merge with incompatible operation') } this.insertion += other.insertion + // We already have the same tracking info and commentIds } + /** + * @returns {RawInsertOp} + */ toJSON() { - // TODO: Once we add metadata to the operation, generate an object rather - // than the compact representation. - return this.insertion + if (!this.tracking && !this.commentIds) { + return this.insertion + } + const obj = { i: this.insertion } + if (this.tracking) { + obj.tracking = this.tracking.toRaw() + } + if (this.commentIds) { + obj.commentIds = this.commentIds + } + return obj } toString() { @@ -171,34 +216,19 @@ class InsertOp extends ScanOp { } class RetainOp extends ScanOp { - constructor(length) { + /** + * @param {number} length + * @param {TrackingProps | undefined} tracking + */ + constructor(length, tracking = undefined) { super() if (length < 0) { throw new Error('length must be non-negative') } + /** @type {number} */ this.length = length - } - - /** - * @inheritdoc - * @param {string} input - * @param {ApplyContext} current - * @returns {ApplyContext} - * */ - apply(input, current) { - if (current.inputCursor + this.length > input.length) { - throw new ApplyError( - "Operation can't retain more chars than are left in the string.", - this.toJSON(), - input - ) - } - current.result += input.slice( - current.inputCursor, - current.inputCursor + this.length - ) - current.inputCursor += this.length - return current + /** @type {TrackingProps | undefined} */ + this.tracking = tracking } /** @@ -221,8 +251,8 @@ class RetainOp extends ScanOp { /** * - * @param {number | {r: number}} op - * @returns + * @param {RawRetainOp} op + * @returns {RetainOp} */ static fromJSON(op) { if (typeof op === 'number') { @@ -232,6 +262,9 @@ class RetainOp extends ScanOp { if (typeof op.r !== 'number') { throw new Error('retain operation must have a number property') } + if (op.tracking) { + return new RetainOp(op.r, TrackingProps.fromRaw(op.tracking)) + } return new RetainOp(op.r) } @@ -240,24 +273,40 @@ class RetainOp extends ScanOp { if (!(other instanceof RetainOp)) { return false } - return this.length === other.length + if (this.length !== other.length) { + return false + } + if (this.tracking) { + return this.tracking.equals(other.tracking) + } + return !other.tracking } canMergeWith(other) { - return other instanceof RetainOp + if (!(other instanceof RetainOp)) { + return false + } + if (this.tracking) { + return this.tracking.equals(other.tracking) + } + return !other.tracking } mergeWith(other) { - if (!(other instanceof RetainOp)) { + if (!this.canMergeWith(other)) { throw new Error('Cannot merge with incompatible operation') } this.length += other.length } + /** + * @returns {RawRetainOp} + */ toJSON() { - // TODO: Once we add metadata to the operation, generate an object rather - // than the compact representation. - return this.length + if (!this.tracking) { + return this.length + } + return { r: this.length, tracking: this.tracking.toRaw() } } toString() { @@ -271,20 +320,10 @@ class RemoveOp extends ScanOp { if (length < 0) { throw new Error('length must be non-negative') } + /** @type {number} */ this.length = length } - /** - * @inheritdoc - * @param {string} _input - * @param {ApplyContext} current - * @returns {ApplyContext} - */ - apply(_input, current) { - current.inputCursor += this.length - return current - } - /** * @inheritdoc * @param {LengthApplyContext} current @@ -297,7 +336,7 @@ class RemoveOp extends ScanOp { /** * - * @param {number} op + * @param {RawRemoveOp} op * @returns {RemoveOp} */ static fromJSON(op) { @@ -320,12 +359,15 @@ class RemoveOp extends ScanOp { } mergeWith(other) { - if (!(other instanceof RemoveOp)) { + if (!this.canMergeWith(other)) { throw new Error('Cannot merge with incompatible operation') } this.length += other.length } + /** + * @returns {RawRemoveOp} + */ toJSON() { return -this.length } @@ -335,20 +377,35 @@ class RemoveOp extends ScanOp { } } +/** + * @param {RawScanOp} op + * @returns {op is RawRetainOp} + */ function isRetain(op) { return ( (typeof op === 'number' && op > 0) || - (typeof op === 'object' && typeof op.r === 'number' && op.r > 0) + (typeof op === 'object' && + 'r' in op && + typeof op.r === 'number' && + op.r > 0) ) } +/** + * @param {RawScanOp} op + * @returns {op is RawInsertOp} + */ function isInsert(op) { return ( typeof op === 'string' || - (typeof op === 'object' && typeof op.i === 'string') + (typeof op === 'object' && 'i' in op && typeof op.i === 'string') ) } +/** + * @param {RawScanOp} op + * @returns {op is RawRemoveOp} + */ function isRemove(op) { return typeof op === 'number' && op < 0 } diff --git a/libraries/overleaf-editor-core/lib/operation/text_operation.js b/libraries/overleaf-editor-core/lib/operation/text_operation.js index c0d2748cf5..93d581b846 100644 --- a/libraries/overleaf-editor-core/lib/operation/text_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/text_operation.js @@ -12,7 +12,6 @@ const containsNonBmpChars = require('../util').containsNonBmpChars const EditOperation = require('./edit_operation') const { - ScanOp, RetainOp, InsertOp, RemoveOp, @@ -26,7 +25,13 @@ const { InvalidInsertionError, TooLongError, } = require('../errors') -/** @typedef {import('../file_data/string_file_data')} StringFileData */ +const Range = require('../file_data/range') +const TrackingProps = require('../file_data/tracking_props') +/** + * @typedef {import('../file_data/string_file_data')} StringFileData + * @typedef {import('../types').RawTextOperation} RawTextOperation + * @typedef {import('../operation/scan_op').ScanOp} ScanOp + */ /** * Create an empty text operation. @@ -85,8 +90,10 @@ class TextOperation extends EditOperation { /** * Skip over a given number of characters. * @param {number | {r: number}} n + * @param {{tracking?: TrackingProps}} opts + * @returns {TextOperation} */ - retain(n) { + retain(n, opts = {}) { if (n === 0) { return this } @@ -95,6 +102,7 @@ class TextOperation extends EditOperation { throw new Error('retain expects an integer or a retain object') } const newOp = RetainOp.fromJSON(n) + newOp.tracking = opts.tracking if (newOp.length === 0) { return this @@ -117,12 +125,16 @@ class TextOperation extends EditOperation { /** * Insert a string at the current position. * @param {string | {i: string}} insertValue + * @param {{tracking?: TrackingProps, commentIds?: string[]}} opts + * @returns {TextOperation} */ - insert(insertValue) { + insert(insertValue, opts = {}) { if (!isInsert(insertValue)) { throw new Error('insert expects a string or an insert object') } const newOp = InsertOp.fromJSON(insertValue) + newOp.tracking = opts.tracking + newOp.commentIds = opts.commentIds if (newOp.insertion === '') { return this } @@ -154,6 +166,7 @@ class TextOperation extends EditOperation { /** * Remove a string at the current position. * @param {number | string} n + * @returns {TextOperation} */ remove(n) { if (typeof n === 'string') { @@ -198,6 +211,7 @@ class TextOperation extends EditOperation { /** * @inheritdoc + * @returns {RawTextOperation} */ toJSON() { return { textOperation: this.ops.map(op => op.toJSON()) } @@ -205,16 +219,24 @@ class TextOperation extends EditOperation { /** * Converts a plain JS object into an operation and validates it. + * @param {RawTextOperation} obj + * @returns {TextOperation} */ static fromJSON = function ({ textOperation: ops }) { const o = new TextOperation() for (const op of ops) { if (isRetain(op)) { - o.retain(op) + const retain = RetainOp.fromJSON(op) + o.retain(retain.length, { tracking: retain.tracking }) } else if (isInsert(op)) { - o.insert(op) + const insert = InsertOp.fromJSON(op) + o.insert(insert.insertion, { + commentIds: insert.commentIds, + tracking: insert.tracking, + }) } else if (isRemove(op)) { - o.remove(op) + const remove = RemoveOp.fromJSON(op) + o.remove(-remove.length) } else { throw new UnprocessableError('unknown operation: ' + JSON.stringify(op)) } @@ -248,10 +270,42 @@ class TextOperation extends EditOperation { } const ops = this.ops - const { inputCursor, result } = ops.reduce( - (intermediate, op) => op.apply(str, intermediate), - { result: '', inputCursor: 0 } - ) + let inputCursor = 0 + let result = '' + for (const op of ops) { + if (op instanceof RetainOp) { + if (inputCursor + op.length > str.length) { + throw new ApplyError( + "Operation can't retain more chars than are left in the string.", + op.toJSON(), + str + ) + } + file.trackedChanges.applyRetain(result.length, op.length, { + tracking: op.tracking, + }) + result += str.slice(inputCursor, inputCursor + op.length) + inputCursor += op.length + } else if (op instanceof InsertOp) { + if (containsNonBmpChars(op.insertion)) { + throw new InvalidInsertionError(str, op.toJSON()) + } + file.trackedChanges.applyInsert(result.length, op.insertion, { + tracking: op.tracking, + }) + file.comments.applyInsert( + new Range(result.length, op.insertion.length), + { commentIds: op.commentIds } + ) + result += op.insertion + } else if (op instanceof RemoveOp) { + file.trackedChanges.applyDelete(result.length, op.length) + file.comments.applyDelete(new Range(result.length, op.length)) + inputCursor += op.length + } else { + throw new UnprocessableError('Unknown ScanOp type during apply') + } + } if (inputCursor !== str.length) { throw new TextOperation.ApplyError( @@ -311,14 +365,65 @@ class TextOperation extends EditOperation { for (let i = 0, l = ops.length; i < l; i++) { const op = ops[i] if (op instanceof RetainOp) { - inverse.retain(op.length) - strIndex += op.length + // Where we need to end up after the retains + const target = strIndex + op.length + // A previous retain could have overriden some tracking info. Now we + // need to restore it. + const previousRanges = previousState.trackedChanges.inRange( + new Range(strIndex, op.length) + ) + + let removeTrackingInfoIfNeeded + if (op.tracking) { + removeTrackingInfoIfNeeded = new TrackingProps( + 'none', + op.tracking.userId, + op.tracking.ts + ) + } + + for (const trackedChange of previousRanges) { + if (strIndex < trackedChange.range.start) { + inverse.retain(trackedChange.range.start - strIndex, { + tracking: removeTrackingInfoIfNeeded, + }) + strIndex = trackedChange.range.start + } + if (trackedChange.range.end < strIndex + op.length) { + inverse.retain(trackedChange.range.length, { + tracking: trackedChange.tracking, + }) + strIndex = trackedChange.range.end + } + if (trackedChange.range.end !== strIndex) { + // No need to split the range at the end + const [left] = trackedChange.range.splitAt(strIndex) + inverse.retain(left.length, { tracking: trackedChange.tracking }) + strIndex = left.end + } + } + if (strIndex < target) { + inverse.retain(target - strIndex, { + tracking: removeTrackingInfoIfNeeded, + }) + strIndex = target + } } else if (op instanceof InsertOp) { inverse.remove(op.insertion.length) } else if (op instanceof RemoveOp) { - // remove op - inverse.insert(str.slice(strIndex, strIndex + op.length)) - strIndex += op.length + const segments = calculateTrackingCommentSegments( + strIndex, + op.length, + previousState.comments, + previousState.trackedChanges + ) + for (const segment of segments) { + inverse.insert(str.slice(strIndex, strIndex + segment.length), { + tracking: segment.tracking, + commentIds: segment.commentIds, + }) + strIndex += segment.length + } } else { throw new UnprocessableError('unknown scanop during inversion') } @@ -410,7 +515,10 @@ class TextOperation extends EditOperation { } if (op2 instanceof InsertOp) { - operation.insert(op2.insertion) + operation.insert(op2.insertion, { + tracking: op2.tracking, + commentIds: op2.commentIds, + }) op2 = ops2[i2++] continue } @@ -427,48 +535,70 @@ class TextOperation extends EditOperation { } if (op1 instanceof RetainOp && op2 instanceof RetainOp) { + // If both have tracking info, use the latter one. Otherwise use the + // tracking info from the former. + const tracking = op2.tracking ?? op1.tracking if (op1.length > op2.length) { - operation.retain(op2.length) - op1 = ScanOp.fromJSON(op1.length - op2.length) + operation.retain(op2.length, { + tracking, + }) + op1 = new RetainOp(op1.length - op2.length, op1.tracking) op2 = ops2[i2++] } else if (op1.length === op2.length) { - operation.retain(op1.length) + operation.retain(op1.length, { + tracking, + }) op1 = ops1[i1++] op2 = ops2[i2++] } else { - operation.retain(op1.length) - op2 = ScanOp.fromJSON(op2.length - op1.length) + operation.retain(op1.length, { + tracking, + }) + op2 = new RetainOp(op2.length - op1.length, op2.tracking) op1 = ops1[i1++] } } else if (op1 instanceof InsertOp && op2 instanceof RemoveOp) { if (op1.insertion.length > op2.length) { - op1 = ScanOp.fromJSON(op1.insertion.slice(op2.length)) + op1 = new InsertOp( + op1.insertion.slice(op2.length), + op1.tracking, + op1.commentIds + ) op2 = ops2[i2++] } else if (op1.insertion.length === op2.length) { op1 = ops1[i1++] op2 = ops2[i2++] } else { - op2 = ScanOp.fromJSON(-op2.length + op1.insertion.length) + op2 = RemoveOp.fromJSON(op1.insertion.length - op2.length) op1 = ops1[i1++] } } else if (op1 instanceof InsertOp && op2 instanceof RetainOp) { + const opts = { + // Prefer the latter tracking info + tracking: op2.tracking ?? op1.tracking, + commentIds: op1.commentIds, + } if (op1.insertion.length > op2.length) { - operation.insert(op1.insertion.slice(0, op2.length)) - op1 = ScanOp.fromJSON(op1.insertion.slice(op2.length)) + operation.insert(op1.insertion.slice(0, op2.length), opts) + op1 = new InsertOp( + op1.insertion.slice(op2.length), + op1.tracking, + op1.commentIds + ) op2 = ops2[i2++] } else if (op1.insertion.length === op2.length) { - operation.insert(op1.insertion) + operation.insert(op1.insertion, opts) op1 = ops1[i1++] op2 = ops2[i2++] } else { - operation.insert(op1.insertion) - op2 = ScanOp.fromJSON(op2.length - op1.insertion.length) + operation.insert(op1.insertion, opts) + op2 = new RetainOp(op2.length - op1.insertion.length, op2.tracking) op1 = ops1[i1++] } } else if (op1 instanceof RetainOp && op2 instanceof RemoveOp) { if (op1.length > op2.length) { operation.remove(-op2.length) - op1 = ScanOp.fromJSON(op1.length - op2.length) + op1 = new RetainOp(op1.length - op2.length, op1.tracking) op2 = ops2[i2++] } else if (op1.length === op2.length) { operation.remove(-op2.length) @@ -476,7 +606,7 @@ class TextOperation extends EditOperation { op2 = ops2[i2++] } else { operation.remove(op1.length) - op2 = ScanOp.fromJSON(-op2.length + op1.length) + op2 = RemoveOp.fromJSON(op1.length - op2.length) op1 = ops1[i1++] } } else { @@ -498,6 +628,7 @@ class TextOperation extends EditOperation { * heart of OT. * @param {TextOperation} operation1 * @param {TextOperation} operation2 + * @returns {[TextOperation, TextOperation]} */ static transform(operation1, operation2) { if (operation1.baseLength !== operation2.baseLength) { @@ -526,14 +657,20 @@ class TextOperation extends EditOperation { // => insert the string in the corresponding prime operation, skip it in // the other one. If both op1 and op2 are insert ops, prefer op1. if (op1 instanceof InsertOp) { - operation1prime.insert(op1.insertion) + operation1prime.insert(op1.insertion, { + tracking: op1.tracking, + commentIds: op1.commentIds, + }) operation2prime.retain(op1.insertion.length) op1 = ops1[i1++] continue } if (op2 instanceof InsertOp) { operation1prime.retain(op2.insertion.length) - operation2prime.insert(op2.insertion) + operation2prime.insert(op2.insertion, { + tracking: op2.tracking, + commentIds: op2.commentIds, + }) op2 = ops2[i2++] continue } @@ -552,9 +689,21 @@ class TextOperation extends EditOperation { let minl if (op1 instanceof RetainOp && op2 instanceof RetainOp) { // Simple case: retain/retain + + // If both have tracking info, we use the one from op1 + /** @type {TrackingProps | undefined} */ + let operation1primeTracking + /** @type {TrackingProps | undefined} */ + let operation2primeTracking + if (op1.tracking) { + operation1primeTracking = op1.tracking + } else { + operation2primeTracking = op2.tracking + } + if (op1.length > op2.length) { minl = op2.length - op1 = ScanOp.fromJSON(op1.length - op2.length) + op1 = new RetainOp(op1.length - op2.length, op1.tracking) op2 = ops2[i2++] } else if (op1.length === op2.length) { minl = op2.length @@ -562,30 +711,30 @@ class TextOperation extends EditOperation { op2 = ops2[i2++] } else { minl = op1.length - op2 = ScanOp.fromJSON(op2.length - op1.length) + op2 = new RetainOp(op2.length - op1.length, op2.tracking) op1 = ops1[i1++] } - operation1prime.retain(minl) - operation2prime.retain(minl) + operation1prime.retain(minl, { tracking: operation1primeTracking }) + operation2prime.retain(minl, { tracking: operation2primeTracking }) } else if (op1 instanceof RemoveOp && op2 instanceof RemoveOp) { // Both operations remove the same string at the same position. We don't // need to produce any operations, we just skip over the remove ops and // handle the case that one operation removes more than the other. if (op1.length > op2.length) { - op1 = ScanOp.fromJSON(-op1.length - -op2.length) + op1 = RemoveOp.fromJSON(op2.length - op1.length) op2 = ops2[i2++] } else if (op1.length === op2.length) { op1 = ops1[i1++] op2 = ops2[i2++] } else { - op2 = ScanOp.fromJSON(-op2.length - -op1.length) + op2 = RemoveOp.fromJSON(op1.length - op2.length) op1 = ops1[i1++] } // next two cases: remove/retain and retain/remove } else if (op1 instanceof RemoveOp && op2 instanceof RetainOp) { if (op1.length > op2.length) { minl = op2.length - op1 = ScanOp.fromJSON(-op1.length + op2.length) + op1 = RemoveOp.fromJSON(op2.length - op1.length) op2 = ops2[i2++] } else if (op1.length === op2.length) { minl = op2.length @@ -593,14 +742,14 @@ class TextOperation extends EditOperation { op2 = ops2[i2++] } else { minl = op1.length - op2 = ScanOp.fromJSON(op2.length + -op1.length) + op2 = new RetainOp(op2.length - op1.length, op2.tracking) op1 = ops1[i1++] } operation1prime.remove(minl) } else if (op1 instanceof RetainOp && op2 instanceof RemoveOp) { if (op1.length > op2.length) { minl = op2.length - op1 = ScanOp.fromJSON(op1.length + -op2.length) + op1 = new RetainOp(op1.length - op2.length, op1.tracking) op2 = ops2[i2++] } else if (op1.length === op2.length) { minl = op1.length @@ -608,7 +757,7 @@ class TextOperation extends EditOperation { op2 = ops2[i2++] } else { minl = op1.length - op2 = ScanOp.fromJSON(-op2.length + op1.length) + op2 = RemoveOp.fromJSON(op1.length - op2.length) op1 = ops1[i1++] } operation2prime.remove(minl) @@ -660,4 +809,77 @@ function getStartIndex(operation) { return 0 } +/** + * Constructs the segments defined as each overlapping range of tracked + * changes and comments. Each segment can have it's own tracking props and + * attached comment ids. + * + * The quick brown fox jumps over the lazy dog + * Tracked inserts ---------- ----- + * Tracked deletes ------ + * Comment 1 ------- + * Comment 2 ---- + * Comment 3 ----------------- + * + * Approx. boundaries: | | | || | | | | + * + * @param {number} cursor + * @param {number} length + * @param {import('../file_data/comment_list')} commentsList + * @param {import('../file_data/tracked_change_list')} trackedChangeList + * @returns {{length: number, commentIds?: string[], tracking?: TrackingProps}[]} + */ +function calculateTrackingCommentSegments( + cursor, + length, + commentsList, + trackedChangeList +) { + const breaks = new Set() + const opStart = cursor + const opEnd = cursor + length + // Utility function to limit breaks to the boundary set by the operation range + function addBreak(rangeBoundary) { + if (rangeBoundary < opStart || rangeBoundary > opEnd) { + return + } + breaks.add(rangeBoundary) + } + // Add comment boundaries + for (const comment of commentsList.comments.values()) { + for (const range of comment.ranges) { + addBreak(range.end) + addBreak(range.start) + } + } + // Add tracked change boundaries + for (const trackedChange of trackedChangeList.trackedChanges) { + addBreak(trackedChange.range.start) + addBreak(trackedChange.range.end) + } + // Add operation boundaries + addBreak(opStart) + addBreak(opEnd) + + // Sort the boundaries so that we can construct ranges between them + const sortedBreaks = Array.from(breaks).sort((a, b) => a - b) + + const separateRanges = [] + for (let i = 1; i < sortedBreaks.length; i++) { + const start = sortedBreaks[i - 1] + const end = sortedBreaks[i] + const currentRange = new Range(start, end - start) + // The comment ids that cover the current range is part of this sub-range + const commentIds = commentsList.idsCoveringRange(currentRange) + // The tracking info that covers the current range is part of this sub-range + const tracking = trackedChangeList.propsAtRange(currentRange) + separateRanges.push({ + length: currentRange.length, + commentIds: commentIds.length > 0 ? commentIds : undefined, + tracking, + }) + } + return separateRanges +} + module.exports = TextOperation diff --git a/libraries/overleaf-editor-core/lib/types.ts b/libraries/overleaf-editor-core/lib/types.ts index ba6d52a491..b2655485b5 100644 --- a/libraries/overleaf-editor-core/lib/types.ts +++ b/libraries/overleaf-editor-core/lib/types.ts @@ -30,6 +30,32 @@ export type TrackingPropsRawData = { export type StringFileRawData = { content: string comments?: CommentRawData[] + trackedChanges?: TrackedChangeRawData[] } export type RawV2DocVersions = Record + +export type RawInsertOp = + | { + i: string + commentIds?: string[] + tracking?: TrackingPropsRawData + } + | string + +export type RawRemoveOp = number +export type RawRetainOp = + | { + r: number + commentIds?: string[] + tracking?: TrackingPropsRawData + } + | number + +export type RawScanOp = RawInsertOp | RawRemoveOp | RawRetainOp + +export type RawTextOperation = { + textOperation: RawScanOp[] +} + +export type RawEditOperation = RawTextOperation diff --git a/libraries/overleaf-editor-core/test/comments_list.test.js b/libraries/overleaf-editor-core/test/comments_list.test.js index ee052c4e41..e28f7bbad3 100644 --- a/libraries/overleaf-editor-core/test/comments_list.test.js +++ b/libraries/overleaf-editor-core/test/comments_list.test.js @@ -413,7 +413,7 @@ describe('commentList', function () { }) }) - it('should delete entire comment', function () { + it('should leave comment without ranges', function () { const commentList = CommentList.fromRaw([ { id: 'comm1', @@ -436,6 +436,7 @@ describe('commentList', function () { ranges: [{ pos: 5, length: 10 }], resolved: false, }, + { id: 'comm2', ranges: [], resolved: false }, { id: 'comm3', ranges: [{ pos: 20, length: 15 }], diff --git a/libraries/overleaf-editor-core/test/scan_op.test.js b/libraries/overleaf-editor-core/test/scan_op.test.js index dc1e9d466d..80ab69114e 100644 --- a/libraries/overleaf-editor-core/test/scan_op.test.js +++ b/libraries/overleaf-editor-core/test/scan_op.test.js @@ -7,6 +7,7 @@ const { RemoveOp, } = require('../lib/operation/scan_op') const { UnprocessableError, ApplyError } = require('../lib/errors') +const TrackingProps = require('../lib/file_data/tracking_props') describe('ScanOp', function () { describe('fromJSON', function () { @@ -41,7 +42,9 @@ describe('ScanOp', function () { }) it('throws an error for invalid input', function () { - expect(() => ScanOp.fromJSON({})).to.throw(UnprocessableError) + expect(() => ScanOp.fromJSON(/** @type {any} */ ({}))).to.throw( + UnprocessableError + ) }) it('throws an error for zero', function () { @@ -63,6 +66,27 @@ describe('RetainOp', function () { expect(op1.equals(op2)).to.be.false }) + it('is not equal to another RetainOp with no tracking info', function () { + const op1 = new RetainOp( + 4, + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new RetainOp(4) + expect(op1.equals(op2)).to.be.false + }) + + it('is not equal to another RetainOp with different tracking info', function () { + const op1 = new RetainOp( + 4, + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new RetainOp( + 4, + new TrackingProps('insert', 'user2', new Date('2024-01-01T00:00:00.000Z')) + ) + expect(op1.equals(op2)).to.be.false + }) + it('is not equal to an InsertOp', function () { const op1 = new RetainOp(1) const op2 = new InsertOp('a') @@ -83,6 +107,43 @@ describe('RetainOp', function () { expect(op1.equals(new RetainOp(3))).to.be.true }) + it('cannot merge with another RetainOp if tracking info is different', function () { + const op1 = new RetainOp( + 4, + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new RetainOp( + 4, + new TrackingProps('insert', 'user2', new Date('2024-01-01T00:00:00.000Z')) + ) + expect(op1.canMergeWith(op2)).to.be.false + expect(() => op1.mergeWith(op2)).to.throw(Error) + }) + + it('can merge with another RetainOp if tracking info is the same', function () { + const op1 = new RetainOp( + 4, + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new RetainOp( + 4, + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + op1.mergeWith(op2) + expect( + op1.equals( + new RetainOp( + 8, + new TrackingProps( + 'insert', + 'user1', + new Date('2024-01-01T00:00:00.000Z') + ) + ) + ) + ).to.be.true + }) + it('cannot merge with an InsertOp', function () { const op1 = new RetainOp(1) const op2 = new InsertOp('a') @@ -112,16 +173,6 @@ describe('RetainOp', function () { expect(length).to.equal(13) expect(inputCursor).to.equal(13) }) - - it('adds from the input to the result when applied', function () { - const op = new RetainOp(3) - const { result, inputCursor } = op.apply('abcdefghi', { - result: 'xyz', - inputCursor: 3, - }) - expect(result).to.equal('xyzdef') - expect(inputCursor).to.equal(6) - }) }) describe('InsertOp', function () { @@ -137,6 +188,60 @@ describe('InsertOp', function () { expect(op1.equals(op2)).to.be.false }) + it('is not equal to another InsertOp with no tracking info', function () { + const op1 = new InsertOp( + 'a', + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new InsertOp('a') + expect(op1.equals(op2)).to.be.false + }) + + it('is not equal to another InsertOp with different tracking info', function () { + const op1 = new InsertOp( + 'a', + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new InsertOp( + 'a', + new TrackingProps('insert', 'user2', new Date('2024-01-01T00:00:00.000Z')) + ) + expect(op1.equals(op2)).to.be.false + }) + + it('is not equal to another InsertOp with no comment ids', function () { + const op1 = new InsertOp('a', undefined, ['1']) + const op2 = new InsertOp('a') + expect(op1.equals(op2)).to.be.false + }) + + it('is not equal to another InsertOp with tracking info', function () { + const op1 = new InsertOp('a', undefined) + const op2 = new InsertOp( + 'a', + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + expect(op1.equals(op2)).to.be.false + }) + + it('is not equal to another InsertOp with comment ids', function () { + const op1 = new InsertOp('a') + const op2 = new InsertOp('a', undefined, ['1']) + expect(op1.equals(op2)).to.be.false + }) + + it('is not equal to another InsertOp with different comment ids', function () { + const op1 = new InsertOp('a', undefined, ['1']) + const op2 = new InsertOp('a', undefined, ['2']) + expect(op1.equals(op2)).to.be.false + }) + + it('is not equal to another InsertOp with overlapping comment ids', function () { + const op1 = new InsertOp('a', undefined, ['1']) + const op2 = new InsertOp('a', undefined, ['2', '1']) + expect(op1.equals(op2)).to.be.false + }) + it('is not equal to a RetainOp', function () { const op1 = new InsertOp('a') const op2 = new RetainOp(1) @@ -157,6 +262,103 @@ describe('InsertOp', function () { expect(op1.equals(new InsertOp('ab'))).to.be.true }) + it('cannot merge with another InsertOp if comment id info is different', function () { + const op1 = new InsertOp('a', undefined, ['1']) + const op2 = new InsertOp('b', undefined, ['1', '2']) + expect(op1.canMergeWith(op2)).to.be.false + expect(() => op1.mergeWith(op2)).to.throw(Error) + }) + + it('cannot merge with another InsertOp if comment id info is different while tracking info matches', function () { + const op1 = new InsertOp( + 'a', + new TrackingProps( + 'insert', + 'user1', + new Date('2024-01-01T00:00:00.000Z') + ), + ['1', '2'] + ) + const op2 = new InsertOp( + 'b', + new TrackingProps( + 'insert', + 'user1', + new Date('2024-01-01T00:00:00.000Z') + ), + ['3'] + ) + expect(op1.canMergeWith(op2)).to.be.false + expect(() => op1.mergeWith(op2)).to.throw(Error) + }) + + it('cannot merge with another InsertOp if comment id is present in other and tracking info matches', function () { + const op1 = new InsertOp( + 'a', + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new InsertOp( + 'b', + new TrackingProps( + 'insert', + 'user1', + new Date('2024-01-01T00:00:00.000Z') + ), + ['1'] + ) + expect(op1.canMergeWith(op2)).to.be.false + expect(() => op1.mergeWith(op2)).to.throw(Error) + }) + + it('cannot merge with another InsertOp if tracking info is different', function () { + const op1 = new InsertOp( + 'a', + new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z')) + ) + const op2 = new InsertOp( + 'b', + new TrackingProps('insert', 'user2', new Date('2024-01-01T00:00:00.000Z')) + ) + expect(op1.canMergeWith(op2)).to.be.false + expect(() => op1.mergeWith(op2)).to.throw(Error) + }) + + it('can merge with another InsertOp if tracking and comment info is the same', function () { + const op1 = new InsertOp( + 'a', + new TrackingProps( + 'insert', + 'user1', + new Date('2024-01-01T00:00:00.000Z') + ), + ['1', '2'] + ) + const op2 = new InsertOp( + 'b', + new TrackingProps( + 'insert', + 'user1', + new Date('2024-01-01T00:00:00.000Z') + ), + ['1', '2'] + ) + expect(op1.canMergeWith(op2)).to.be.true + op1.mergeWith(op2) + expect( + op1.equals( + new InsertOp( + 'ab', + new TrackingProps( + 'insert', + 'user1', + new Date('2024-01-01T00:00:00.000Z') + ), + ['1', '2'] + ) + ) + ).to.be.true + }) + it('cannot merge with a RetainOp', function () { const op1 = new InsertOp('a') const op2 = new RetainOp(1) @@ -187,16 +389,6 @@ describe('InsertOp', function () { expect(inputCursor).to.equal(20) }) - it('adds from the insertion to the result when applied', function () { - const op = new InsertOp('ghi') - const { result, inputCursor } = op.apply('abcdef', { - result: 'xyz', - inputCursor: 3, - }) - expect(result).to.equal('xyzghi') - expect(inputCursor).to.equal(3) - }) - it('can apply a retain of the rest of the input', function () { const op = new RetainOp(10) const { length, inputCursor } = op.applyToLength({ @@ -282,14 +474,4 @@ describe('RemoveOp', function () { expect(length).to.equal(10) expect(inputCursor).to.equal(13) }) - - it('does not change the result and adds to the cursor when applied', function () { - const op = new RemoveOp(3) - const { result, inputCursor } = op.apply('abcdefghi', { - result: 'xyz', - inputCursor: 3, - }) - expect(result).to.equal('xyz') - expect(inputCursor).to.equal(6) - }) }) diff --git a/libraries/overleaf-editor-core/test/support/random.js b/libraries/overleaf-editor-core/test/support/random.js index 0a87a5f4f9..76fad2ef4c 100644 --- a/libraries/overleaf-editor-core/test/support/random.js +++ b/libraries/overleaf-editor-core/test/support/random.js @@ -9,10 +9,10 @@ function randomInt(n) { return Math.floor(Math.random() * n) } -function randomString(n) { +function randomString(n, newLine = true) { let str = '' while (n--) { - if (Math.random() < 0.15) { + if (newLine && Math.random() < 0.15) { str += '\n' } else { const chr = randomInt(26) + 97 @@ -32,7 +32,35 @@ function randomTest(numTrials, test) { } } +function randomSubset(arr) { + const n = randomInt(arr.length) + const subset = [] + const indices = [] + for (let i = 0; i < arr.length; i++) indices.push(i) + for (let i = 0; i < n; i++) { + const index = randomInt(indices.length) + subset.push(arr[indices[index]]) + indices.splice(index, 1) + } + return subset +} + +function randomComments(number) { + const ids = new Set() + const comments = [] + while (comments.length < number) { + const id = randomString(10, false) + if (!ids.has(id)) { + comments.push({ id, ranges: [], resolved: false }) + ids.add(id) + } + } + return { ids: Array.from(ids), comments } +} + exports.int = randomInt exports.string = randomString exports.element = randomElement exports.test = randomTest +exports.comments = randomComments +exports.subset = randomSubset diff --git a/libraries/overleaf-editor-core/test/support/random_text_operation.js b/libraries/overleaf-editor-core/test/support/random_text_operation.js index 8b02deba43..4caf49bda3 100644 --- a/libraries/overleaf-editor-core/test/support/random_text_operation.js +++ b/libraries/overleaf-editor-core/test/support/random_text_operation.js @@ -1,12 +1,14 @@ +const TrackingProps = require('../../lib/file_data/tracking_props') const TextOperation = require('../../lib/operation/text_operation') const random = require('./random') /** * * @param {string} str + * @param {string[]} [commentIds] * @returns {TextOperation} */ -function randomTextOperation(str) { +function randomTextOperation(str, commentIds) { const operation = new TextOperation() let left while (true) { @@ -14,12 +16,33 @@ function randomTextOperation(str) { if (left === 0) break const r = Math.random() const l = 1 + random.int(Math.min(left - 1, 20)) + const trackedChange = + Math.random() < 0.1 + ? new TrackingProps( + random.element(['insert', 'delete', 'none']), + random.element(['user1', 'user2', 'user3']), + new Date( + random.element([ + '2024-01-01T00:00:00.000Z', + '2023-01-01T00:00:00.000Z', + '2022-01-01T00:00:00.000Z', + ]) + ) + ) + : undefined if (r < 0.2) { - operation.insert(random.string(l)) + let operationCommentIds + if (commentIds?.length > 0 && Math.random() < 0.3) { + operationCommentIds = random.subset(commentIds) + } + operation.insert(random.string(l), { + tracking: trackedChange, + commentIds: operationCommentIds, + }) } else if (r < 0.4) { operation.remove(l) } else { - operation.retain(l) + operation.retain(l, { tracking: trackedChange }) } } if (Math.random() < 0.3) { diff --git a/libraries/overleaf-editor-core/test/text_operation.test.js b/libraries/overleaf-editor-core/test/text_operation.test.js index 5a44b34eb9..abb3753ad5 100644 --- a/libraries/overleaf-editor-core/test/text_operation.test.js +++ b/libraries/overleaf-editor-core/test/text_operation.test.js @@ -14,6 +14,7 @@ const ot = require('..') const TextOperation = ot.TextOperation const StringFileData = require('../lib/file_data/string_file_data') const { RetainOp, InsertOp, RemoveOp } = require('../lib/operation/scan_op') +const TrackingProps = require('../lib/file_data/tracking_props') describe('TextOperation', function () { const numTrials = 500 @@ -141,85 +142,27 @@ describe('TextOperation', function () { 'applies (randomised)', random.test(numTrials, () => { const str = random.string(50) - const o = randomOperation(str) + const comments = random.comments(6) + const o = randomOperation(str, comments.ids) expect(str.length).to.equal(o.baseLength) - const file = new StringFileData(str) + const file = new StringFileData(str, comments.comments) o.apply(file) const result = file.getContent() expect(result.length).to.equal(o.targetLength) }) ) - it( - 'inverts (randomised)', - random.test(numTrials, () => { - const str = random.string(50) - const o = randomOperation(str) - const p = o.invert(new StringFileData(str)) - expect(o.baseLength).to.equal(p.targetLength) - expect(o.targetLength).to.equal(p.baseLength) - const file = new StringFileData(str) - o.apply(file) - p.apply(file) - const result = file.getContent() - expect(result).to.equal(str) - }) - ) - it( 'converts to/from JSON (randomised)', random.test(numTrials, () => { const doc = random.string(50) - const operation = randomOperation(doc) + const comments = random.comments(2) + const operation = randomOperation(doc, comments.ids) const roundTripOperation = TextOperation.fromJSON(operation.toJSON()) expect(operation.equals(roundTripOperation)).to.be.true }) ) - it( - 'composes (randomised)', - random.test(numTrials, () => { - // invariant: apply(str, compose(a, b)) === apply(apply(str, a), b) - const str = random.string(20) - const a = randomOperation(str) - const file = new StringFileData(str) - a.apply(file) - const afterA = file.getContent() - expect(afterA.length).to.equal(a.targetLength) - const b = randomOperation(afterA) - b.apply(file) - const afterB = file.getContent() - expect(afterB.length).to.equal(b.targetLength) - const ab = a.compose(b) - expect(ab.targetLength).to.equal(b.targetLength) - ab.apply(new StringFileData(str)) - const afterAB = file.getContent() - expect(afterAB).to.equal(afterB) - }) - ) - - it( - 'transforms (randomised)', - random.test(numTrials, () => { - // invariant: compose(a, b') = compose(b, a') - // where (a', b') = transform(a, b) - const str = random.string(20) - const a = randomOperation(str) - const b = randomOperation(str) - const primes = TextOperation.transform(a, b) - const aPrime = primes[0] - const bPrime = primes[1] - const abPrime = a.compose(bPrime) - const baPrime = b.compose(aPrime) - const abFile = new StringFileData(str) - const baFile = new StringFileData(str) - abPrime.apply(abFile) - baPrime.apply(baFile) - expect(abPrime.equals(baPrime)).to.be.true - expect(abFile.getContent()).to.equal(baFile.getContent()) - }) - ) - it('throws when invalid operations are applied', function () { const operation = new TextOperation().retain(1) expect(() => { @@ -261,4 +204,647 @@ describe('TextOperation', function () { /inserted text contains non BMP characters/ ) }) + + describe('invert', function () { + it( + 'inverts (randomised)', + random.test(numTrials, () => { + const str = random.string(50) + const comments = random.comments(6) + const o = randomOperation(str, comments.ids) + const originalFile = new StringFileData(str, comments.comments) + const p = o.invert(originalFile) + expect(o.baseLength).to.equal(p.targetLength) + expect(o.targetLength).to.equal(p.baseLength) + const file = new StringFileData(str, comments.comments) + o.apply(file) + p.apply(file) + const result = file.toRaw() + expect(result).to.deep.equal(originalFile.toRaw()) + }) + ) + + it('re-inserts removed range and comment when inverting', function () { + expectInverseToLeadToInitialState( + new StringFileData( + 'foo bar baz', + [{ id: 'comment1', ranges: [{ pos: 4, length: 3 }] }], + [ + { + range: { pos: 4, length: 3 }, + tracking: { + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }, + }, + ] + ), + new TextOperation().retain(4).remove(4).retain(3) + ) + }) + + it('deletes inserted range and comment when inverting', function () { + expectInverseToLeadToInitialState( + new StringFileData('foo baz', [ + { id: 'comment1', ranges: [], resolved: false }, + ]), + new TextOperation() + .retain(4) + .insert('bar', { + commentIds: ['comment1'], + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }), + }) + .insert(' ') + .retain(3) + ) + }) + + it('removes a tracked delete', function () { + expectInverseToLeadToInitialState( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }), + }) + .retain(3) + ) + }) + + it('restores comments that were removed', function () { + expectInverseToLeadToInitialState( + new StringFileData('foo bar baz', [ + { + id: 'comment1', + ranges: [{ pos: 4, length: 3 }], + resolved: false, + }, + ]), + new TextOperation().retain(4).remove(4).retain(3) + ) + }) + + it('re-inserting removed part of comment restores original comment range', function () { + expectInverseToLeadToInitialState( + new StringFileData('foo bar baz', [ + { + id: 'comment1', + ranges: [{ pos: 0, length: 11 }], + resolved: false, + }, + ]), + new TextOperation().retain(4).remove(4).retain(3) + ) + }) + + it('re-inserting removed part of tracked change restores tracked change range', function () { + expectInverseToLeadToInitialState( + new StringFileData('foo bar baz', undefined, [ + { + range: { pos: 0, length: 11 }, + tracking: { + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }, + }, + ]), + new TextOperation().retain(4).remove(4).retain(3) + ) + }) + }) + + describe('compose', function () { + it( + 'composes (randomised)', + random.test(numTrials, () => { + // invariant: apply(str, compose(a, b)) === apply(apply(str, a), b) + const str = random.string(20) + const comments = random.comments(6) + const a = randomOperation(str, comments.ids) + const file = new StringFileData(str, comments.comments) + a.apply(file) + const afterA = file.toRaw() + expect(afterA.content.length).to.equal(a.targetLength) + const b = randomOperation(afterA.content, comments.ids) + b.apply(file) + const afterB = file.toRaw() + expect(afterB.content.length).to.equal(b.targetLength) + const ab = a.compose(b) + expect(ab.targetLength).to.equal(b.targetLength) + ab.apply(new StringFileData(str, comments.comments)) + const afterAB = file.toRaw() + expect(afterAB).to.deep.equal(afterB) + }) + ) + + it('composes two operations with comments', function () { + expect( + compose( + new StringFileData('foo baz', [ + { id: 'comment1', ranges: [], resolved: false }, + ]), + new TextOperation() + .retain(4) + .insert('bar', { + commentIds: ['comment1'], + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }), + }) + .insert(' ') + .retain(3), + new TextOperation().retain(4).remove(4).retain(3) + ) + ).to.deep.equal({ + content: 'foo baz', + comments: [{ id: 'comment1', ranges: [], resolved: false }], + }) + }) + + it('prioritizes tracked changes info from the latter operation', function () { + expect( + compose( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }), + }) + .retain(3), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user2', + }), + }) + .retain(3) + ) + ).to.deep.equal({ + content: 'foo bar baz', + trackedChanges: [ + { + range: { pos: 4, length: 4 }, + tracking: { + ts: '2024-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user2', + }, + }, + ], + }) + }) + + it('does not remove tracked change if not overriden by operation 2', function () { + expect( + compose( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }), + }) + .retain(3), + new TextOperation().retain(11) + ) + ).to.deep.equal({ + content: 'foo bar baz', + trackedChanges: [ + { + range: { pos: 4, length: 4 }, + tracking: { + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }, + }, + ], + }) + }) + + it('adds comment ranges from both operations', function () { + expect( + compose( + new StringFileData('foo bar baz', [ + { + id: 'comment1', + ranges: [{ pos: 4, length: 3 }], + resolved: false, + }, + { + id: 'comment2', + ranges: [{ pos: 8, length: 3 }], + resolved: false, + }, + ]), + new TextOperation() + .retain(5) + .insert('aa', { + commentIds: ['comment1'], + }) + .retain(6), + new TextOperation() + .retain(11) + .insert('bb', { commentIds: ['comment2'] }) + .retain(2) + ) + ).to.deep.equal({ + content: 'foo baaar bbbaz', + comments: [ + { id: 'comment1', ranges: [{ pos: 4, length: 5 }], resolved: false }, + { id: 'comment2', ranges: [{ pos: 10, length: 5 }], resolved: false }, + ], + }) + }) + + it('it removes the tracking range from a tracked delete if operation 2 resolves it', function () { + expect( + compose( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }), + }) + .retain(3), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'none', + userId: 'user2', + }), + }) + .retain(3) + ) + ).to.deep.equal({ + content: 'foo bar baz', + }) + }) + }) + + describe('transform', function () { + it( + 'transforms (randomised)', + random.test(numTrials, () => { + // invariant: compose(a, b') = compose(b, a') + // where (a', b') = transform(a, b) + const str = random.string(20) + const comments = random.comments(6) + const a = randomOperation(str, comments.ids) + const b = randomOperation(str, comments.ids) + const primes = TextOperation.transform(a, b) + const aPrime = primes[0] + const bPrime = primes[1] + const abPrime = a.compose(bPrime) + const baPrime = b.compose(aPrime) + const abFile = new StringFileData(str, comments.comments) + const baFile = new StringFileData(str, comments.comments) + abPrime.apply(abFile) + baPrime.apply(baFile) + expect(abPrime.equals(baPrime)).to.be.true + expect(abFile.toRaw()).to.deep.equal(baFile.toRaw()) + }) + ) + + it('adds a tracked change from operation 1', function () { + expect( + transform( + new StringFileData('foo baz'), + new TextOperation() + .retain(4) + .insert('bar', { + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }), + }) + .insert(' ') + .retain(3), + new TextOperation().retain(7).insert(' qux') + ) + ).to.deep.equal({ + content: 'foo bar baz qux', + trackedChanges: [ + { + range: { pos: 4, length: 3 }, + tracking: { + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }, + }, + ], + }) + }) + + it('prioritizes tracked change from the first operation', function () { + expect( + transform( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }), + }) + .retain(3), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user2', + }), + }) + .retain(3) + ) + ).to.deep.equal({ + content: 'foo bar baz', + trackedChanges: [ + { + range: { pos: 4, length: 4 }, + tracking: { + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }, + }, + ], + }) + }) + + it('splits a tracked change in two to resolve conflicts', function () { + expect( + transform( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .retain(4, { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }), + }) + .retain(3), + new TextOperation() + .retain(4) + .retain(5, { + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user2', + }), + }) + .retain(2) + ) + ).to.deep.equal({ + content: 'foo bar baz', + trackedChanges: [ + { + range: { pos: 4, length: 4 }, + tracking: { + ts: '2023-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user1', + }, + }, + { + range: { pos: 8, length: 1 }, + tracking: { + ts: '2024-01-01T00:00:00.000Z', + type: 'delete', + userId: 'user2', + }, + }, + ], + }) + }) + + it('inserts a tracked change from operation 2 after a tracked change from operation 1', function () { + expect( + transform( + new StringFileData('aaabbbccc'), + new TextOperation() + .retain(3) + .insert('xxx', { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }), + }) + .retain(6), + new TextOperation() + .retain(3) + .insert('yyy', { + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user2', + }), + }) + .retain(6) + ) + ).to.deep.equal({ + content: 'aaaxxxyyybbbccc', + trackedChanges: [ + { + range: { pos: 3, length: 3 }, + tracking: { + ts: '2023-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }, + }, + { + range: { pos: 6, length: 3 }, + tracking: { + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user2', + }, + }, + ], + }) + }) + + it('preserves a comment even if it is completely removed in one operation', function () { + expect( + transform( + new StringFileData('foo bar baz', [ + { + id: 'comment1', + ranges: [{ pos: 4, length: 3 }], + resolved: false, + }, + ]), + new TextOperation().retain(4).remove(4).retain(3), + new TextOperation() + .retain(7) + .insert('qux ', { + commentIds: ['comment1'], + }) + .retain(4) + ) + ).to.deep.equal({ + content: 'foo qux baz', + comments: [ + { id: 'comment1', ranges: [{ pos: 4, length: 4 }], resolved: false }, + ], + }) + }) + + it('extends a comment to both ranges if both operations add text in it', function () { + expect( + transform( + new StringFileData('foo bar baz', [ + { + id: 'comment1', + ranges: [{ pos: 4, length: 3 }], + resolved: false, + }, + ]), + new TextOperation() + .retain(4) + .insert('qux ', { + commentIds: ['comment1'], + }) + .retain(7), + new TextOperation() + .retain(4) + .insert('corge ', { commentIds: ['comment1'] }) + .retain(7) + ) + ).to.deep.equal({ + content: 'foo qux corge bar baz', + comments: [ + { id: 'comment1', ranges: [{ pos: 4, length: 13 }], resolved: false }, + ], + }) + }) + + it('adds a tracked change from both operations at different places', function () { + expect( + transform( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .insert('qux ', { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }), + }) + .retain(7), + new TextOperation() + .retain(8) + .insert('corge ', { + tracking: TrackingProps.fromRaw({ + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user2', + }), + }) + .retain(3) + ) + ).to.deep.equal({ + content: 'foo qux bar corge baz', + trackedChanges: [ + { + range: { pos: 4, length: 4 }, + tracking: { + ts: '2023-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }, + }, + { + range: { pos: 12, length: 6 }, + tracking: { + ts: '2024-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user2', + }, + }, + ], + }) + }) + }) }) + +function expectInverseToLeadToInitialState(fileData, operation) { + const initialState = fileData + const result = initialState.toRaw() + const invertedOperation = operation.invert(initialState) + operation.apply(initialState) + invertedOperation.apply(initialState) + const invertedResult = initialState.toRaw() + expect(invertedResult).to.deep.equal(result) +} + +function compose(fileData, op1, op2) { + const copy = StringFileData.fromRaw(fileData.toRaw()) + op1.apply(fileData) + op2.apply(fileData) + const result1 = fileData.toRaw() + + const composed = op1.compose(op2) + composed.apply(copy) + const result2 = copy.toRaw() + + expect(result1).to.deep.equal(result2) + return fileData.toRaw() +} + +function transform(fileData, a, b) { + const initialState = fileData + const aFileData = StringFileData.fromRaw(initialState.toRaw()) + const bFileData = StringFileData.fromRaw(initialState.toRaw()) + + const [aPrime, bPrime] = TextOperation.transform(a, b) + a.apply(aFileData) + bPrime.apply(aFileData) + b.apply(bFileData) + aPrime.apply(bFileData) + + const resultA = aFileData.toRaw() + const resultB = bFileData.toRaw() + expect(resultA).to.deep.equal(resultB) + + return aFileData.toRaw() +} diff --git a/libraries/overleaf-editor-core/test/tracked_change_list.test.js b/libraries/overleaf-editor-core/test/tracked_change_list.test.js index 5a93d8400d..a03113818a 100644 --- a/libraries/overleaf-editor-core/test/tracked_change_list.test.js +++ b/libraries/overleaf-editor-core/test/tracked_change_list.test.js @@ -841,5 +841,44 @@ describe('TrackedChangeList', function () { expect(trackedChanges.trackedChanges.length).to.equal(0) expect(trackedChanges.toRaw()).to.deep.equal([]) }) + + it('should append a new tracked change when retaining a range from another user with tracking info', function () { + const trackedChanges = TrackedChangeList.fromRaw([ + { + range: { pos: 4, length: 4 }, + tracking: { + type: 'delete', + userId: 'user1', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + ]) + trackedChanges.applyRetain(8, 1, { + tracking: TrackingProps.fromRaw({ + type: 'delete', + userId: 'user2', + ts: '2024-01-01T00:00:00.000Z', + }), + }) + expect(trackedChanges.trackedChanges.length).to.equal(2) + expect(trackedChanges.toRaw()).to.deep.equal([ + { + range: { pos: 4, length: 4 }, + tracking: { + type: 'delete', + userId: 'user1', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 8, length: 1 }, + tracking: { + type: 'delete', + userId: 'user2', + ts: '2024-01-01T00:00:00.000Z', + }, + }, + ]) + }) }) })