From 55edbe6606d5e2ca4d2e65d43c987870f8f2849e Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Thu, 18 Sep 2025 14:59:08 +0200 Subject: [PATCH] Use history snapshot when doing file/project restore (#28502) * Add getDocUpdaterCompatibleRanges utility function * use history snapshot for file/project restore * move overleaf-editor-core from devDependencies GitOrigin-RevId: 62481a5304ada9d931e018418be3c0719bccf1f3 --- libraries/overleaf-editor-core/index.js | 4 + .../lib/doc_updater_compatible_ranges.js | 150 +++++++ .../doc_updater_compatible_ranges.test.js | 397 ++++++++++++++++++ .../project-history/app/js/SnapshotManager.js | 122 +----- .../src/Features/History/RestoreManager.mjs | 64 +-- services/web/package.json | 2 +- .../unit/src/History/RestoreManager.test.mjs | 174 +++++--- 7 files changed, 707 insertions(+), 206 deletions(-) create mode 100644 libraries/overleaf-editor-core/lib/doc_updater_compatible_ranges.js create mode 100644 libraries/overleaf-editor-core/test/doc_updater_compatible_ranges.test.js diff --git a/libraries/overleaf-editor-core/index.js b/libraries/overleaf-editor-core/index.js index 33b3dcf5dc..3bbb510a5c 100644 --- a/libraries/overleaf-editor-core/index.js +++ b/libraries/overleaf-editor-core/index.js @@ -46,6 +46,9 @@ const CommentList = require('./lib/file_data/comment_list') const LazyStringFileData = require('./lib/file_data/lazy_string_file_data') const StringFileData = require('./lib/file_data/string_file_data') const EditOperationBuilder = require('./lib/operation/edit_operation_builder') +const { + getDocUpdaterCompatibleRanges, +} = require('./lib/doc_updater_compatible_ranges') exports.AddCommentOperation = AddCommentOperation exports.Author = Author @@ -93,3 +96,4 @@ exports.TrackedChange = TrackedChange exports.Range = Range exports.CommentList = CommentList exports.TrackingProps = TrackingProps +exports.getDocUpdaterCompatibleRanges = getDocUpdaterCompatibleRanges diff --git a/libraries/overleaf-editor-core/lib/doc_updater_compatible_ranges.js b/libraries/overleaf-editor-core/lib/doc_updater_compatible_ranges.js new file mode 100644 index 0000000000..5922d0b1b1 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/doc_updater_compatible_ranges.js @@ -0,0 +1,150 @@ +// @ts-check +'use strict' + +/** + * @import File from "./file" + */ + +/** + * Constructs tracked changes and comments in a document-updater compatible format. + * Positions will be relative to a document where tracked deletes have been + * removed from the string. This also means that if a tracked delete overlaps + * a comment range, the comment range will be truncated. + * + * @param {File} file + */ +function getDocUpdaterCompatibleRanges(file) { + if (!file.isEditable()) { + // A binary file has no tracked changes or comments + return { + changes: [], + comments: [], + } + } + + const content = file.getContent() + if (content == null) { + throw new Error('Unable to read file contents') + } + + const trackedChanges = file.getTrackedChanges().asSorted() + const comments = file.getComments().toArray() + const docUpdaterCompatibleTrackedChanges = [] + + let trackedDeletionOffset = 0 + for (const trackedChange of trackedChanges) { + const isTrackedDeletion = trackedChange.tracking.type === 'delete' + const trackedChangeContent = content.slice( + trackedChange.range.start, + trackedChange.range.end + ) + const tcContent = isTrackedDeletion + ? { d: trackedChangeContent } + : { i: trackedChangeContent } + docUpdaterCompatibleTrackedChanges.push({ + op: { + p: trackedChange.range.start - trackedDeletionOffset, + ...tcContent, + }, + metadata: { + ts: trackedChange.tracking.ts.toISOString(), + user_id: trackedChange.tracking.userId, + }, + }) + if (isTrackedDeletion) { + trackedDeletionOffset += trackedChange.range.length + } + } + + // Comments are shifted left by the length of any previous tracked deletions. + // If they overlap with a tracked deletion, they are truncated. + // + // Example: + // { } comment + // [ ] tracked deletion + // the quic[k {b]rown [fox] jum[ps} ove]r the lazy dog + // => rown jum + // starting at position 8 + const trackedDeletions = trackedChanges.filter( + tc => tc.tracking.type === 'delete' + ) + const docUpdaterCompatibleComments = [] + for (const comment of comments) { + let trackedDeletionIndex = 0 + if (comment.ranges.length === 0) { + // Translate detached comments into zero length comments at position 0 + docUpdaterCompatibleComments.push({ + op: { + p: 0, + c: '', + t: comment.id, + resolved: comment.resolved, + }, + }) + continue + } + + // Consider a multiple range comment as a single comment that joins all its + // ranges + const commentStart = comment.ranges[0].start + const commentEnd = comment.ranges[comment.ranges.length - 1].end + + let commentContent = '' + // Docupdater position + let position = commentStart + while (trackedDeletions[trackedDeletionIndex]?.range.end <= commentStart) { + // Skip over tracked deletions that are before the current comment range + position -= trackedDeletions[trackedDeletionIndex].range.length + trackedDeletionIndex++ + } + + if (trackedDeletions[trackedDeletionIndex]?.range.start < commentStart) { + // There's overlap with a tracked deletion, move the position left and + // truncate the overlap + position -= + commentStart - trackedDeletions[trackedDeletionIndex].range.start + } + + // Cursor in the history content + let cursor = commentStart + while (cursor < commentEnd) { + const trackedDeletion = trackedDeletions[trackedDeletionIndex] + if (!trackedDeletion || trackedDeletion.range.start >= commentEnd) { + // We've run out of relevant tracked changes + commentContent += content.slice(cursor, commentEnd) + break + } + if (trackedDeletion.range.start > cursor) { + // There's a gap between the current cursor and the tracked deletion + commentContent += content.slice(cursor, trackedDeletion.range.start) + } + + if (trackedDeletion.range.end <= commentEnd) { + // Skip to the end of the tracked delete + cursor = trackedDeletion.range.end + trackedDeletionIndex++ + } else { + // We're done with that comment + break + } + } + docUpdaterCompatibleComments.push({ + op: { + p: position, + c: commentContent, + t: comment.id, + resolved: comment.resolved, + }, + id: comment.id, + }) + } + + return { + changes: docUpdaterCompatibleTrackedChanges, + comments: docUpdaterCompatibleComments, + } +} + +module.exports = { + getDocUpdaterCompatibleRanges, +} diff --git a/libraries/overleaf-editor-core/test/doc_updater_compatible_ranges.test.js b/libraries/overleaf-editor-core/test/doc_updater_compatible_ranges.test.js new file mode 100644 index 0000000000..03e6b8b048 --- /dev/null +++ b/libraries/overleaf-editor-core/test/doc_updater_compatible_ranges.test.js @@ -0,0 +1,397 @@ +'use strict' + +const { expect } = require('chai') +const { + getDocUpdaterCompatibleRanges, +} = require('../lib/doc_updater_compatible_ranges.js') +const StringFileData = require('../lib/file_data/string_file_data.js') +const File = require('../lib/file.js') + +describe('getDocUpdaterCompatibleRanges', function () { + describe('with tracked deletes', function () { + beforeEach(function () { + this.content = 'the quick brown fox jumps over the lazy dog' + this.trackedChanges = [ + { + range: { pos: 4, length: 6 }, // 'quick ' + tracking: { + type: 'delete', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 16, length: 4 }, // 'fox ' + tracking: { + type: 'delete', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 35, length: 5 }, // 'lazy ' + tracking: { + type: 'insert', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 40, length: 3 }, // 'dog' + tracking: { + type: 'delete', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + ] + }) + + it("doesn't shift the tracked delete by itself", function () { + const fileData = new StringFileData(this.content, [], this.trackedChanges) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.changes[0].op.p).to.eq(4) + }) + + it('should move subsequent tracked changes by the length of previous deletes', function () { + const fileData = new StringFileData(this.content, [], this.trackedChanges) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.changes[1].op.p).to.eq(16 - 6) + expect(result.changes[2].op.p).to.eq(35 - 6 - 4) + }) + + it("shouldn't move subsequent tracked changes by previous inserts", function () { + const fileData = new StringFileData(this.content, [], this.trackedChanges) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.changes[3].op.p).to.eq(40 - 6 - 4) + }) + }) + + describe('with comments and tracked deletes', function () { + beforeEach(function () { + this.content = 'the quick brown fox jumps over the lazy dog' + this.trackedChanges = [ + { + range: { pos: 2, length: 5 }, // 'e qui' + tracking: { + type: 'delete', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 11, length: 1 }, // 'r' + tracking: { + type: 'delete', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 28, length: 9 }, // 'er the la' + tracking: { + type: 'delete', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + ] + }) + + it('should move the comment to the start of the tracked delete and remove overlapping text', function () { + const comments = [ + { + id: 'comment-1', + ranges: [ + { pos: 4, length: 5 }, // 'quick' + { pos: 10, length: 5 }, // 'brown' + { pos: 26, length: 4 }, // 'over' + { pos: 35, length: 4 }, // 'lazy' + ], + resolved: false, + }, + ] + + const fileData = new StringFileData( + this.content, + comments, + this.trackedChanges + ) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.comments[0].op.p).to.eq(2) + expect(result.comments[0].op.c).to.eq('ck bown fox jumps ovzy') + }) + + it('should put resolved status in op', function () { + const comments = [ + { + id: 'comment-1', + ranges: [ + { pos: 4, length: 5 }, // 'quick' + { pos: 10, length: 5 }, // 'brown' + { pos: 26, length: 4 }, // 'over' + { pos: 35, length: 4 }, // 'lazy' + ], + resolved: false, + }, + { id: 'comment-2', ranges: [], resolved: true }, + { + id: 'comment-3', + ranges: [{ pos: 4, length: 1 }], // 'q' + resolved: true, + }, + ] + + const fileData = new StringFileData( + this.content, + comments, + this.trackedChanges + ) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.comments[0].op.resolved).to.be.false + expect(result.comments[1].op.resolved).to.be.true + expect(result.comments[2].op.resolved).to.be.true + }) + + it('should include thread id', function () { + const comments = [ + { + id: 'comment-1', + ranges: [ + { pos: 4, length: 5 }, // 'quick' + { pos: 10, length: 5 }, // 'brown' + { pos: 26, length: 4 }, // 'over' + { pos: 35, length: 4 }, // 'lazy' + ], + resolved: false, + }, + { id: 'comment-2', ranges: [], resolved: true }, + { + id: 'comment-3', + ranges: [{ pos: 4, length: 1 }], // 'q' + resolved: true, + }, + ] + + const fileData = new StringFileData( + this.content, + comments, + this.trackedChanges + ) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.comments[0].op.t).to.eq('comment-1') + expect(result.comments[1].op.t).to.eq('comment-2') + expect(result.comments[2].op.t).to.eq('comment-3') + }) + + it('should translate detached comment to zero length op', function () { + const comments = [ + { + id: 'comment-1', + ranges: [ + { pos: 4, length: 5 }, // 'quick' + { pos: 10, length: 5 }, // 'brown' + { pos: 26, length: 4 }, // 'over' + { pos: 35, length: 4 }, // 'lazy' + ], + resolved: false, + }, + { id: 'comment-2', ranges: [], resolved: true }, // detached comment + { + id: 'comment-3', + ranges: [{ pos: 4, length: 1 }], // 'q' + resolved: true, + }, + ] + + const fileData = new StringFileData( + this.content, + comments, + this.trackedChanges + ) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.comments[1].op.p).to.eq(0) + expect(result.comments[1].op.c).to.eq('') + }) + + it('should position a comment entirely in a tracked delete next to the tracked delete', function () { + const comments = [ + { + id: 'comment-1', + ranges: [ + { pos: 4, length: 5 }, // 'quick' + { pos: 10, length: 5 }, // 'brown' + { pos: 26, length: 4 }, // 'over' + { pos: 35, length: 4 }, // 'lazy' + ], + resolved: false, + }, + { id: 'comment-2', ranges: [], resolved: true }, + { + id: 'comment-3', + ranges: [{ pos: 4, length: 1 }], // 'q' - entirely in tracked delete + resolved: true, + }, + ] + + const fileData = new StringFileData( + this.content, + comments, + this.trackedChanges + ) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result.comments[2].op.p).to.eq(2) + expect(result.comments[2].op.c).to.eq('') + }) + }) + + describe('with multiple tracked changes and comments', function () { + it('returns the ranges with content and adjusted positions to ignore tracked deletes', function () { + const content = 'the quick brown fox jumps over the lazy dog' + const trackedChanges = [ + { + range: { pos: 4, length: 6 }, // 'quick ' + tracking: { + type: 'delete', + userId: '31', + ts: '2023-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 10, length: 6 }, // 'brown ' + tracking: { + type: 'insert', + userId: '31', + ts: '2024-01-01T00:00:00.000Z', + }, + }, + { + range: { pos: 35, length: 5 }, // 'lazy ' + tracking: { + type: 'delete', + userId: '31', + ts: '2024-01-01T00:00:00.000Z', + }, + }, + ] + + const comments = [ + { + id: 'comment-1', + ranges: [ + { pos: 4, length: 5 }, // 'quick' + { pos: 10, length: 5 }, // 'brown' + { pos: 35, length: 4 }, // 'lazy' + ], + resolved: false, + }, + { + id: 'comment-2', + ranges: [ + { pos: 0, length: 3 }, // 'the' + { pos: 31, length: 3 }, // 'the' + ], + resolved: true, + }, + ] + + const fileData = new StringFileData(content, comments, trackedChanges) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result).to.deep.equal({ + changes: [ + { + metadata: { + ts: '2023-01-01T00:00:00.000Z', + user_id: '31', + }, + op: { + d: 'quick ', + p: 4, + }, + }, + { + metadata: { + ts: '2024-01-01T00:00:00.000Z', + user_id: '31', + }, + op: { + i: 'brown ', + p: 4, + }, + }, + { + metadata: { + ts: '2024-01-01T00:00:00.000Z', + user_id: '31', + }, + op: { + d: 'lazy ', + p: 29, + }, + }, + ], + comments: [ + { + op: { + c: 'brown fox jumps over the ', + p: 4, + t: 'comment-1', + resolved: false, + }, + id: 'comment-1', + }, + { + op: { + c: 'the brown fox jumps over the', + p: 0, + t: 'comment-2', + resolved: true, + }, + id: 'comment-2', + }, + ], + }) + }) + }) + + describe('with an empty file', function () { + it('should return empty comments and changes', function () { + const fileData = new StringFileData('', []) + const file = new File(fileData) + + const result = getDocUpdaterCompatibleRanges(file) + + expect(result).to.deep.equal({ + changes: [], + comments: [], + }) + }) + }) +}) diff --git a/services/project-history/app/js/SnapshotManager.js b/services/project-history/app/js/SnapshotManager.js index ed316743cf..a43e004fa4 100644 --- a/services/project-history/app/js/SnapshotManager.js +++ b/services/project-history/app/js/SnapshotManager.js @@ -81,126 +81,10 @@ async function getRangesSnapshot(projectId, version, pathname) { } const historyId = await WebApiManager.promises.getHistoryId(projectId) await file.load('eager', HistoryStoreManager.getBlobStore(historyId)) - const content = file.getContent() - if (content == null) { - throw new Error('Unable to read file contents') - } - const trackedChanges = file.getTrackedChanges().asSorted() - const comments = file.getComments().toArray() - const docUpdaterCompatibleTrackedChanges = [] - let trackedDeletionOffset = 0 - for (const trackedChange of trackedChanges) { - const isTrackedDeletion = trackedChange.tracking.type === 'delete' - const trackedChangeContent = content.slice( - trackedChange.range.start, - trackedChange.range.end - ) - const tcContent = isTrackedDeletion - ? { d: trackedChangeContent } - : { i: trackedChangeContent } - docUpdaterCompatibleTrackedChanges.push({ - op: { - p: trackedChange.range.start - trackedDeletionOffset, - ...tcContent, - }, - metadata: { - ts: trackedChange.tracking.ts.toISOString(), - user_id: trackedChange.tracking.userId, - }, - }) - if (isTrackedDeletion) { - trackedDeletionOffset += trackedChange.range.length - } - } - - // Comments are shifted left by the length of any previous tracked deletions. - // If they overlap with a tracked deletion, they are truncated. - // - // Example: - // { } comment - // [ ] tracked deletion - // the quic[k {b]rown [fox] jum[ps} ove]r the lazy dog - // => rown jum - // starting at position 8 - const trackedDeletions = trackedChanges.filter( - tc => tc.tracking.type === 'delete' - ) - const docUpdaterCompatibleComments = [] - for (const comment of comments) { - let trackedDeletionIndex = 0 - if (comment.ranges.length === 0) { - // Translate detached comments into zero length comments at position 0 - docUpdaterCompatibleComments.push({ - op: { - p: 0, - c: '', - t: comment.id, - resolved: comment.resolved, - }, - }) - continue - } - - // Consider a multiple range comment as a single comment that joins all its - // ranges - const commentStart = comment.ranges[0].start - const commentEnd = comment.ranges[comment.ranges.length - 1].end - - let commentContent = '' - // Docupdater position - let position = commentStart - while (trackedDeletions[trackedDeletionIndex]?.range.end <= commentStart) { - // Skip over tracked deletions that are before the current comment range - position -= trackedDeletions[trackedDeletionIndex].range.length - trackedDeletionIndex++ - } - - if (trackedDeletions[trackedDeletionIndex]?.range.start < commentStart) { - // There's overlap with a tracked deletion, move the position left and - // truncate the overlap - position -= - commentStart - trackedDeletions[trackedDeletionIndex].range.start - } - - // Cursor in the history content - let cursor = commentStart - while (cursor < commentEnd) { - const trackedDeletion = trackedDeletions[trackedDeletionIndex] - if (!trackedDeletion || trackedDeletion.range.start >= commentEnd) { - // We've run out of relevant tracked changes - commentContent += content.slice(cursor, commentEnd) - break - } - if (trackedDeletion.range.start > cursor) { - // There's a gap between the current cursor and the tracked deletion - commentContent += content.slice(cursor, trackedDeletion.range.start) - } - - if (trackedDeletion.range.end <= commentEnd) { - // Skip to the end of the tracked delete - cursor = trackedDeletion.range.end - trackedDeletionIndex++ - } else { - // We're done with that comment - break - } - } - docUpdaterCompatibleComments.push({ - op: { - p: position, - c: commentContent, - t: comment.id, - resolved: comment.resolved, - }, - id: comment.id, - }) - } - - return { - changes: docUpdaterCompatibleTrackedChanges, - comments: docUpdaterCompatibleComments, - } + // Use the utility function from overleaf-editor-core + const { changes, comments } = Core.getDocUpdaterCompatibleRanges(file) + return { changes, comments } } /** diff --git a/services/web/app/src/Features/History/RestoreManager.mjs b/services/web/app/src/Features/History/RestoreManager.mjs index bcd175838c..3a0859cdd4 100644 --- a/services/web/app/src/Features/History/RestoreManager.mjs +++ b/services/web/app/src/Features/History/RestoreManager.mjs @@ -18,6 +18,8 @@ import ChatManager from '../Chat/ChatManager.js' import OError from '@overleaf/o-error' import ProjectGetter from '../Project/ProjectGetter.js' import ProjectEntityHandler from '../Project/ProjectEntityHandler.js' +import HistoryManager from './HistoryManager.js' +import { Snapshot, getDocUpdaterCompatibleRanges } from 'overleaf-editor-core' async function getCommentThreadIds(projectId) { await DocumentUpdaterHandler.promises.flushProjectToMongo(projectId) @@ -60,22 +62,41 @@ const RestoreManager = { async revertFile(userId, projectId, version, pathname, options = {}) { const threadIds = await getCommentThreadIds(projectId) + + const snapshotRaw = await HistoryManager.promises.getContentAtVersion( + projectId, + version + ) + const snapshot = Snapshot.fromRaw(snapshotRaw) + return await RestoreManager._revertSingleFile( userId, projectId, version, pathname, threadIds, + snapshot, options ) }, + /** + * + * @param {string} userId + * @param {string} projectId + * @param {string} version + * @param {string} pathname + * @param {Set} threadIds + * @param {Snapshot} projectSnapshotAtVersion + * @param {object} options + */ async _revertSingleFile( userId, projectId, version, pathname, threadIds, + projectSnapshotAtVersion, options = {} ) { const endTimer = Metrics.revertFileDurationSeconds.startTimer() @@ -156,16 +177,16 @@ const RestoreManager = { threadIds.delete(file.element._id.toString()) } - const { metadata } = await RestoreManager._getMetadataFromHistory( - projectId, - version, - pathname - ) + const snapshotFile = projectSnapshotAtVersion.getFile(pathname) + if (!snapshotFile) { + throw new OError('file not found in snapshot', { pathname }) + } // Look for metadata indicating a linked file. - const isFileMetadata = metadata && 'provider' in metadata + const fileMetadata = snapshotFile.getMetadata() + const isFileMetadata = fileMetadata && 'provider' in fileMetadata - logger.debug({ metadata }, 'metadata from history') + logger.debug({ fileMetadata }, 'metadata from history') if (importInfo.type === 'file' || isFileMetadata) { const newFile = await EditorController.promises.upsertFile( @@ -173,7 +194,7 @@ const RestoreManager = { parentFolderId, basename, fsPath, - metadata, + fileMetadata, origin, userId ) @@ -185,11 +206,7 @@ const RestoreManager = { } } - const ranges = await RestoreManager._getRangesFromHistory( - projectId, - version, - pathname - ) + const ranges = getDocUpdaterCompatibleRanges(snapshotFile) const documentCommentIds = new Set( ranges.comments?.map(({ op: { t } }) => t) @@ -364,6 +381,12 @@ const RestoreManager = { } const threadIds = await getCommentThreadIds(projectId) + const snapshotRaw = await HistoryManager.promises.getContentAtVersion( + projectId, + version + ) + const snapshot = Snapshot.fromRaw(snapshotRaw) + const reverted = [] for (const pathname of pathsAtPastVersion) { const res = await RestoreManager._revertSingleFile( @@ -372,6 +395,7 @@ const RestoreManager = { version, pathname, threadIds, + snapshot, { origin } ) reverted.push({ @@ -414,20 +438,6 @@ const RestoreManager = { return await FileWriter.promises.writeUrlToDisk(projectId, url) }, - async _getRangesFromHistory(projectId, version, pathname) { - const url = `${ - Settings.apis.project_history.url - }/project/${projectId}/ranges/version/${version}/${encodeURIComponent(pathname)}` - return await fetchJson(url) - }, - - async _getMetadataFromHistory(projectId, version, pathname) { - const url = `${ - Settings.apis.project_history.url - }/project/${projectId}/metadata/version/${version}/${encodeURIComponent(pathname)}` - return await fetchJson(url) - }, - async _getUpdatesFromHistory(projectId, version) { const url = `${Settings.apis.project_history.url}/project/${projectId}/updates?before=${version}&min_count=1` const res = await fetchJson(url) diff --git a/services/web/package.json b/services/web/package.json index 4bdcbca0de..95b9b3d4d5 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -158,6 +158,7 @@ "nodemailer": "^6.7.0", "on-headers": "^1.0.2", "otplib": "^12.0.1", + "overleaf-editor-core": "*", "p-limit": "^2.3.0", "p-props": "4.0.0", "p-queue": "^8.1.0", @@ -344,7 +345,6 @@ "mock-fs": "^5.1.2", "nock": "^13.5.6", "nvd3": "^1.8.6", - "overleaf-editor-core": "*", "p-reflect": "^3.1.0", "pdfjs-dist": "5.1.91", "pirates": "^4.0.1", diff --git a/services/web/test/unit/src/History/RestoreManager.test.mjs b/services/web/test/unit/src/History/RestoreManager.test.mjs index 22b45ed261..5b5756acbd 100644 --- a/services/web/test/unit/src/History/RestoreManager.test.mjs +++ b/services/web/test/unit/src/History/RestoreManager.test.mjs @@ -20,6 +20,60 @@ describe('RestoreManager', function () { default: Errors, })) + vi.doMock('../../../../app/src/Features/History/HistoryManager.js', () => ({ + default: (ctx.HistoryManager = { + promises: { + getContentAtVersion: sinon.stub().resolves({ + // Raw snapshot data that will be passed to Snapshot.fromRaw + files: { + 'main.tex': { + hash: 'abcdef1234567890abcdef1234567890abcdef12', + stringLength: 100, + metadata: { + editorId: 'test-editor', + }, + }, + 'foo.tex': { + hash: 'abcdef1234567890abcdef1234567890abcdef12', + stringLength: 100, + metadata: { + editorId: 'test-editor', + }, + }, + 'folder/file.tex': { + hash: 'abcdef1234567890abcdef1234567890abcdef12', + stringLength: 100, + metadata: { + editorId: 'test-editor', + }, + }, + 'foo.png': { + hash: 'abcdef1234567890abcdef1234567890abcdef12', + stringLength: 100, + metadata: { + provider: 'bar', + }, + }, + 'linkedFile.bib': { + hash: 'abcdef1234567890abcdef1234567890abcdef12', + stringLength: 100, + metadata: { + provider: 'mendeley', + }, + }, + 'withMainTrue.tex': { + hash: 'abcdef1234567890abcdef1234567890abcdef12', + stringLength: 100, + metadata: { + main: true, + }, + }, + }, + }), + }, + }), + })) + vi.doMock('../../../../app/src/infrastructure/Metrics.js', () => ({ default: { revertFileDurationSeconds: { @@ -101,10 +155,46 @@ describe('RestoreManager', function () { }) ) + vi.doMock('overleaf-editor-core', () => ({ + Snapshot: { + fromRaw: sinon.stub().callsFake(snapshotData => ({ + getFile: pathname => ({ + getStringLength: sinon.stub().returns(100), + getByteLength: sinon.stub().returns(100), + getContent: sinon.stub().returns('file content'), + isEditable: sinon.stub().returns(true), + getMetadata: sinon + .stub() + .returns(snapshotData?.files?.[pathname]?.metadata), + }), + })), + }, + getDocUpdaterCompatibleRanges: (ctx.getDocUpdaterCompatibleRanges = sinon + .stub() + .returns({ + changes: ctx.tracked_changes || [], + comments: ctx.comments || [], + })), + })) + ctx.RestoreManager = (await import(modulePath)).default ctx.user_id = 'mock-user-id' ctx.project_id = 'mock-project-id' ctx.version = 42 + + // Add missing method mocks to RestoreManager + ctx.RestoreManager.promises._getUpdatesFromHistory = sinon.stub().resolves([ + { + toV: ctx.version, + meta: { end_ts: new Date('2024-01-01T00:00:00.000Z') }, + }, + ]) + ctx.RestoreManager.promises._getProjectPathsAtVersion = sinon + .stub() + .resolves([]) + ctx.RestoreManager.promises._writeFileVersionToDisk = sinon + .stub() + .resolves('/tmp/mock-file-path') }) afterEach(function () { @@ -290,10 +380,6 @@ describe('RestoreManager', function () { ctx.FileSystemImportManager.promises.addEntity = sinon .stub() .resolves((ctx.entity = 'mock-entity')) - ctx.RestoreManager.promises._getRangesFromHistory = sinon.stub().rejects() - ctx.RestoreManager.promises._getMetadataFromHistory = sinon - .stub() - .resolves({ metadata: undefined }) }) describe('reverting a project without ranges support', function () { @@ -395,12 +481,10 @@ describe('RestoreManager', function () { ctx.FileSystemImportManager.promises.importFile = sinon .stub() .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] }) - ctx.RestoreManager.promises._getRangesFromHistory = sinon - .stub() - .resolves({ - changes: ctx.tracked_changes, - comments: ctx.comments, - }) + ctx.getDocUpdaterCompatibleRanges.returns({ + changes: ctx.tracked_changes, + comments: ctx.comments, + }) ctx.RestoreManager.promises._getUpdatesFromHistory = sinon .stub() .resolves([ @@ -444,12 +528,6 @@ describe('RestoreManager', function () { it('should return the created entity', function (ctx) { expect(ctx.data).to.deep.equal(ctx.addedFile) }) - - it('should look up ranges', function (ctx) { - expect( - ctx.RestoreManager.promises._getRangesFromHistory - ).to.have.been.calledWith(ctx.project_id, ctx.version, ctx.pathname) - }) }) describe('with an existing file in the current project', function () { @@ -560,12 +638,6 @@ describe('RestoreManager', function () { it('should return the created entity', function (ctx) { expect(ctx.data).to.deep.equal(ctx.addedFile) }) - - it('should look up ranges', function (ctx) { - expect( - ctx.RestoreManager.promises._getRangesFromHistory - ).to.have.been.calledWith(ctx.project_id, ctx.version, ctx.pathname) - }) }) describe('with comments in same doc', function () { @@ -623,7 +695,14 @@ describe('RestoreManager', function () { ctx.project_id, ctx.version, ctx.pathname, - ctx.threadIds + ctx.threadIds, + { + getFile: sinon.stub().returns({ + getMetadata: sinon.stub().returns(undefined), + getContent: sinon.stub().returns('file content'), + isEditable: sinon.stub().returns(true), + }), + } ) }) @@ -687,7 +766,14 @@ describe('RestoreManager', function () { ctx.project_id, ctx.version, ctx.pathname, - ctx.threadIds + ctx.threadIds, + { + getFile: sinon.stub().returns({ + getMetadata: sinon.stub().returns(undefined), + getContent: sinon.stub().returns('file content'), + isEditable: sinon.stub().returns(true), + }), + } ) }) @@ -803,12 +889,6 @@ describe('RestoreManager', function () { ctx.EditorController.promises.upsertFile = sinon .stub() .resolves({ _id: 'mock-file-id', type: 'file' }) - ctx.RestoreManager.promises._getRangesFromHistory = sinon - .stub() - .resolves({ - changes: [], - comments: [], - }) ctx.EditorController.promises.addDocWithRanges = sinon .stub() .resolves((ctx.addedFile = { _id: 'mock-doc-id', type: 'doc' })) @@ -831,9 +911,6 @@ describe('RestoreManager', function () { ctx.FileSystemImportManager.promises.importFile = sinon .stub() .resolves({ type: 'file' }) - ctx.RestoreManager.promises._getMetadataFromHistory = sinon - .stub() - .resolves({ metadata: { provider: 'bar' } }) ctx.result = await ctx.RestoreManager.promises.revertFile( ctx.user_id, ctx.project_id, @@ -868,11 +945,6 @@ describe('RestoreManager', function () { ) }) - it('should not look up ranges', function (ctx) { - expect(ctx.RestoreManager.promises._getRangesFromHistory).to.not.have - .been.called - }) - it('should not try to add a document', function (ctx) { expect(ctx.EditorController.promises.addDocWithRanges).to.not.have .been.called @@ -881,13 +953,10 @@ describe('RestoreManager', function () { describe('when reverting a linked document with provider', function () { beforeEach(async function (ctx) { - ctx.pathname = 'foo.tex' + ctx.pathname = 'linkedFile.bib' ctx.FileSystemImportManager.promises.importFile = sinon .stub() .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] }) - ctx.RestoreManager.promises._getMetadataFromHistory = sinon - .stub() - .resolves({ metadata: { provider: 'bar' } }) ctx.result = await ctx.RestoreManager.promises.revertFile( ctx.user_id, ctx.project_id, @@ -909,9 +978,9 @@ describe('RestoreManager', function () { ).to.have.been.calledWith( ctx.project_id, 'mock-folder-id', - 'foo.tex', + 'linkedFile.bib', ctx.fsPath, - { provider: 'bar' }, + { provider: 'mendeley' }, { kind: 'file-restore', path: ctx.pathname, @@ -922,11 +991,6 @@ describe('RestoreManager', function () { ) }) - it('should not look up ranges', function (ctx) { - expect(ctx.RestoreManager.promises._getRangesFromHistory).to.not.have - .been.called - }) - it('should not try to add a document', function (ctx) { expect(ctx.EditorController.promises.addDocWithRanges).to.not.have .been.called @@ -935,13 +999,10 @@ describe('RestoreManager', function () { describe('when reverting a linked document with { main: true }', function () { beforeEach(async function (ctx) { - ctx.pathname = 'foo.tex' + ctx.pathname = 'withMainTrue.tex' ctx.FileSystemImportManager.promises.importFile = sinon .stub() .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] }) - ctx.RestoreManager.promises._getMetadataFromHistory = sinon - .stub() - .resolves({ metadata: { main: true } }) ctx.result = await ctx.RestoreManager.promises.revertFile( ctx.user_id, ctx.project_id, @@ -962,18 +1023,13 @@ describe('RestoreManager', function () { .called }) - it('should look up ranges', function (ctx) { - expect(ctx.RestoreManager.promises._getRangesFromHistory).to.have.been - .called - }) - it('should add the document', function (ctx) { expect( ctx.EditorController.promises.addDocWithRanges ).to.have.been.calledWith( ctx.project_id, ctx.folder_id, - 'foo.tex', + 'withMainTrue.tex', ['foo', 'bar', 'baz'], { changes: [], comments: [] } )