diff --git a/libraries/overleaf-editor-core/lib/change.js b/libraries/overleaf-editor-core/lib/change.js index 35183b9159..cfc3447251 100644 --- a/libraries/overleaf-editor-core/lib/change.js +++ b/libraries/overleaf-editor-core/lib/change.js @@ -294,8 +294,6 @@ class Change { if (this.v2DocVersions) { snapshot.updateV2DocVersions(this.v2DocVersions) } - - snapshot.setTimestamp(this.timestamp) } /** diff --git a/libraries/overleaf-editor-core/lib/snapshot.js b/libraries/overleaf-editor-core/lib/snapshot.js index ac037dca21..c33b48f829 100644 --- a/libraries/overleaf-editor-core/lib/snapshot.js +++ b/libraries/overleaf-editor-core/lib/snapshot.js @@ -36,8 +36,7 @@ class Snapshot { return new Snapshot( FileMap.fromRaw(raw.files), raw.projectVersion, - V2DocVersions.fromRaw(raw.v2DocVersions), - raw.timestamp ? new Date(raw.timestamp) : undefined + V2DocVersions.fromRaw(raw.v2DocVersions) ) } @@ -46,15 +45,8 @@ class Snapshot { const raw = { files: this.fileMap.toRaw(), } - if (this.projectVersion) { - raw.projectVersion = this.projectVersion - } - if (this.v2DocVersions) { - raw.v2DocVersions = this.v2DocVersions.toRaw() - } - if (this.timestamp != null) { - raw.timestamp = this.timestamp.toISOString() - } + if (this.projectVersion) raw.projectVersion = this.projectVersion + if (this.v2DocVersions) raw.v2DocVersions = this.v2DocVersions.toRaw() return raw } @@ -62,15 +54,13 @@ class Snapshot { * @param {FileMap} [fileMap] * @param {string} [projectVersion] * @param {V2DocVersions} [v2DocVersions] - * @param {Date} [timestamp] */ - constructor(fileMap, projectVersion, v2DocVersions, timestamp) { + constructor(fileMap, projectVersion, v2DocVersions) { assert.maybe.instance(fileMap, FileMap, 'bad fileMap') this.fileMap = fileMap || new FileMap({}) this.projectVersion = projectVersion this.v2DocVersions = v2DocVersions - this.timestamp = timestamp ?? null } /** @@ -119,17 +109,6 @@ class Snapshot { v2DocVersions.applyTo(this) } - getTimestamp() { - return this.timestamp - } - - /** - * @param {Date} timestamp - */ - setTimestamp(timestamp) { - this.timestamp = timestamp - } - /** * The underlying file map. * @return {FileMap} @@ -289,7 +268,6 @@ class Snapshot { files: rawFiles, projectVersion, v2DocVersions: rawV2DocVersions, - timestamp: this.getTimestamp() ?? undefined, } } diff --git a/libraries/overleaf-editor-core/lib/types.ts b/libraries/overleaf-editor-core/lib/types.ts index a393e7fcaa..53bf0622c4 100644 --- a/libraries/overleaf-editor-core/lib/types.ts +++ b/libraries/overleaf-editor-core/lib/types.ts @@ -74,7 +74,6 @@ export type RawSnapshot = { files: RawFileMap projectVersion?: string v2DocVersions?: RawV2DocVersions | null - timestamp?: string } export type RawHistory = { diff --git a/libraries/overleaf-editor-core/test/change.test.js b/libraries/overleaf-editor-core/test/change.test.js index 47eeb7d45a..b4704fa25b 100644 --- a/libraries/overleaf-editor-core/test/change.test.js +++ b/libraries/overleaf-editor-core/test/change.test.js @@ -1,15 +1,10 @@ 'use strict' const { expect } = require('chai') -const { - Change, - File, - Operation, - AddFileOperation, - Snapshot, - Origin, - RestoreFileOrigin, -} = require('..') +const core = require('..') +const Change = core.Change +const File = core.File +const Operation = core.Operation describe('Change', function () { describe('findBlobHashes', function () { @@ -42,9 +37,9 @@ describe('Change', function () { describe('RestoreFileOrigin', function () { it('should convert to and from raw', function () { - const origin = new RestoreFileOrigin(1, 'path', new Date()) + const origin = new core.RestoreFileOrigin(1, 'path', new Date()) const raw = origin.toRaw() - const newOrigin = Origin.fromRaw(raw) + const newOrigin = core.Origin.fromRaw(raw) expect(newOrigin).to.eql(origin) }) @@ -61,19 +56,7 @@ describe('Change', function () { }, }) - expect(change.getOrigin()).to.be.an.instanceof(RestoreFileOrigin) - }) - }) - - describe('applyTo', function () { - it('sets the timestamp on the snapshot', function () { - const snapshot = new Snapshot() - snapshot.addFile('main.tex', File.fromString('')) - const operation = new AddFileOperation('main.tex', File.fromString('')) - const now = new Date() - const change = new Change([operation], now) - change.applyTo(snapshot) - expect(snapshot.getTimestamp().toISOString()).to.equal(now.toISOString()) + expect(change.getOrigin()).to.be.an.instanceof(core.RestoreFileOrigin) }) }) }) diff --git a/services/history-v1/api/controllers/projects.js b/services/history-v1/api/controllers/projects.js index 455cfeafb5..2ebe5d1771 100644 --- a/services/history-v1/api/controllers/projects.js +++ b/services/history-v1/api/controllers/projects.js @@ -28,7 +28,6 @@ const withTmpDir = require('./with_tmp_dir') const StreamSizeLimit = require('./stream_size_limit') const { getProjectBlobsBatch } = require('../../storage/lib/blob_store') const assert = require('../../storage/lib/assert') -const { getChunkMetadataForVersion } = require('../../storage/lib/chunk_store') const pipeline = promisify(Stream.pipeline) @@ -397,28 +396,7 @@ async function getSnapshotAtVersion(projectId, version) { chunk.getChanges(), chunk.getEndVersion() - version ) - - if (changes.length > 0) { - snapshot.applyAll(changes) - } else { - // There are no changes in this chunk; we need to look at the previous chunk - // to get the snapshot's timestamp - let chunkMetadata - try { - chunkMetadata = await getChunkMetadataForVersion(projectId, version) - } catch (err) { - if (err instanceof Chunk.VersionNotFoundError) { - // The snapshot is the first snapshot of the first chunk, so we can't - // find a timestamp. This shouldn't happen often. Ignore the error and - // leave the timestamp empty. - } else { - throw err - } - } - - snapshot.setTimestamp(chunkMetadata.endTimestamp) - } - + snapshot.applyAll(changes) return snapshot } diff --git a/services/history-v1/test/acceptance/js/storage/persist_changes.test.js b/services/history-v1/test/acceptance/js/storage/persist_changes.test.js index b1ad7c48e0..4c491db4a3 100644 --- a/services/history-v1/test/acceptance/js/storage/persist_changes.test.js +++ b/services/history-v1/test/acceptance/js/storage/persist_changes.test.js @@ -108,7 +108,6 @@ describe('persistChanges', function () { content: '', }, }, - timestamp: thirdChange.getTimestamp().toISOString(), }) const history = new History(snapshot, [thirdChange]) const currentChunk = new Chunk(history, 2) diff --git a/services/project-history/test/acceptance/js/LatestSnapshotTests.js b/services/project-history/test/acceptance/js/LatestSnapshotTests.js index e93fcd091c..6e989ffda3 100644 --- a/services/project-history/test/acceptance/js/LatestSnapshotTests.js +++ b/services/project-history/test/acceptance/js/LatestSnapshotTests.js @@ -1,7 +1,6 @@ import { expect } from 'chai' import mongodb from 'mongodb-legacy' import nock from 'nock' -import { readFileSync } from 'node:fs' import * as ProjectHistoryClient from './helpers/ProjectHistoryClient.js' import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js' const { ObjectId } = mongodb @@ -51,12 +50,6 @@ describe('LatestSnapshot', function () { .get(`/api/projects/${this.historyId}/latest/history`) .replyWithFile(200, fixture('chunks/0-3.json')) - const fixtureData = JSON.parse( - readFileSync(fixture('chunks/0-3.json'), 'utf8') - ) - const changes = fixtureData.chunk.history.changes - const lastTimestamp = changes[changes.length - 1].timestamp - ProjectHistoryClient.getLatestSnapshot(this.projectId, (error, body) => { if (error) { throw error @@ -76,7 +69,6 @@ describe('LatestSnapshot', function () { operations: [{ textOperation: [26, '\n\nFour five six'] }], }, }, - timestamp: lastTimestamp, }, version: 3, }) diff --git a/services/web/app/src/Features/History/RestoreManager.mjs b/services/web/app/src/Features/History/RestoreManager.mjs index b7a3f5c60a..938a883d27 100644 --- a/services/web/app/src/Features/History/RestoreManager.mjs +++ b/services/web/app/src/Features/History/RestoreManager.mjs @@ -7,6 +7,7 @@ import EditorController from '../Editor/EditorController.js' import Errors from '../Errors/Errors.js' import moment from 'moment' import { callbackifyAll } from '@overleaf/promise-utils' +import { fetchJson } from '@overleaf/fetch-utils' import ProjectLocator from '../Project/ProjectLocator.js' import DocumentUpdaterHandler from '../DocumentUpdater/DocumentUpdaterHandler.js' import ChatApiHandler from '../Chat/ChatApiHandler.js' @@ -68,13 +69,6 @@ const RestoreManager = { ) const snapshot = Snapshot.fromRaw(snapshotRaw) - const origin = options.origin ?? { - kind: 'file-restore', - path: pathname, - version, - timestamp: snapshot.getTimestamp()?.toISOString(), - } - return await RestoreManager._revertSingleFile( userId, projectId, @@ -82,7 +76,7 @@ const RestoreManager = { pathname, threadIds, snapshot, - { origin } + options ) }, @@ -137,9 +131,17 @@ const RestoreManager = { }) .catch(() => null) - const snapshotFile = projectSnapshotAtVersion.getFile(pathname) - if (!snapshotFile) { - throw new OError('file not found in snapshot', { pathname }) + const updates = await RestoreManager._getUpdatesFromHistory( + projectId, + version + ) + const updateAtVersion = updates.find(update => update.toV === version) + + const origin = options.origin || { + kind: 'file-restore', + path: pathname, + version, + timestamp: new Date(updateAtVersion.meta.end_ts).toISOString(), } const importInfo = await FileSystemImportManager.promises.importFile( @@ -160,7 +162,7 @@ const RestoreManager = { projectId, file.element._id, file.type, - options.origin, + origin, userId ) @@ -175,6 +177,11 @@ const RestoreManager = { threadIds.delete(file.element._id.toString()) } + const snapshotFile = projectSnapshotAtVersion.getFile(pathname) + if (!snapshotFile) { + throw new OError('file not found in snapshot', { pathname }) + } + // Look for metadata indicating a linked file. const fileMetadata = snapshotFile.getMetadata() const isFileMetadata = fileMetadata && 'provider' in fileMetadata @@ -188,7 +195,7 @@ const RestoreManager = { basename, fsPath, fileMetadata, - options.origin, + origin, userId ) @@ -289,21 +296,13 @@ const RestoreManager = { newCommentThreadData ) - // At this point, we know that the file is editable in web. We assume it's - // also editable in history because the rules for editable files in history - // are less strict than the rules in web. Because of that, we can call - // getContent() on the snapshot file. - const lines = snapshotFile - .getContent({ filterTrackedDeletes: true }) - .split('\n') - const { _id } = await EditorController.promises.addDocWithRanges( projectId, parentFolderId, basename, - lines, + importInfo.lines, newRanges, - options.origin, + origin, userId ) @@ -363,21 +362,31 @@ const RestoreManager = { throw new OError('project does not have ranges support', { projectId }) } + // Get project paths at version + const pathsAtPastVersion = await RestoreManager._getProjectPathsAtVersion( + projectId, + version + ) + + const updates = await RestoreManager._getUpdatesFromHistory( + projectId, + version + ) + const updateAtVersion = updates.find(update => update.toV === version) + + const origin = { + kind: 'project-restore', + version, + timestamp: new Date(updateAtVersion.meta.end_ts).toISOString(), + } + const threadIds = await getCommentThreadIds(projectId) + const snapshotRaw = await HistoryManager.promises.getContentAtVersion( projectId, version ) const snapshot = Snapshot.fromRaw(snapshotRaw) - const pathsAtPastVersion = snapshot.getFilePathnames() - - const origin = { - kind: 'project-restore', - version, - timestamp: snapshot.getTimestamp()?.toISOString(), - } - const threadIds = await getCommentThreadIds(projectId) - const reverted = [] for (const pathname of pathsAtPastVersion) { const res = await RestoreManager._revertSingleFile( @@ -428,6 +437,18 @@ const RestoreManager = { }/project/${projectId}/version/${version}/${encodeURIComponent(pathname)}` return await FileWriter.promises.writeUrlToDisk(projectId, 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) + return res.updates + }, + + async _getProjectPathsAtVersion(projectId, version) { + const url = `${Settings.apis.project_history.url}/project/${projectId}/paths/version/${version}` + const res = await fetchJson(url) + return res.paths + }, } export default { ...callbackifyAll(RestoreManager), promises: RestoreManager } diff --git a/services/web/test/unit/src/History/RestoreManager.test.mjs b/services/web/test/unit/src/History/RestoreManager.test.mjs index 003731eb78..5b5756acbd 100644 --- a/services/web/test/unit/src/History/RestoreManager.test.mjs +++ b/services/web/test/unit/src/History/RestoreManager.test.mjs @@ -69,7 +69,6 @@ describe('RestoreManager', function () { }, }, }, - timestamp: new Date().toISOString(), }), }, }), @@ -162,22 +161,12 @@ describe('RestoreManager', function () { getFile: pathname => ({ getStringLength: sinon.stub().returns(100), getByteLength: sinon.stub().returns(100), - getContent: sinon.stub().returns('foo\nbar\nbaz'), + getContent: sinon.stub().returns('file content'), isEditable: sinon.stub().returns(true), getMetadata: sinon .stub() .returns(snapshotData?.files?.[pathname]?.metadata), }), - getFilePathnames: sinon - .stub() - .returns(Object.keys(snapshotData.files || {})), - getTimestamp: sinon - .stub() - .returns( - snapshotData?.timestamp - ? new Date(snapshotData.timestamp) - : new Date() - ), })), }, getDocUpdaterCompatibleRanges: (ctx.getDocUpdaterCompatibleRanges = sinon @@ -200,7 +189,9 @@ describe('RestoreManager', function () { 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') @@ -708,17 +699,9 @@ describe('RestoreManager', function () { { getFile: sinon.stub().returns({ getMetadata: sinon.stub().returns(undefined), - getContent: sinon.stub().returns('foo\nbar\nbaz'), + getContent: sinon.stub().returns('file content'), isEditable: sinon.stub().returns(true), }), - }, - { - origin: { - kind: 'file-restore', - path: 'foo.tex', - timestamp: new Date(ctx.endTs).toISOString(), - version: 42, - }, } ) }) @@ -787,17 +770,9 @@ describe('RestoreManager', function () { { getFile: sinon.stub().returns({ getMetadata: sinon.stub().returns(undefined), - getContent: sinon.stub().returns('foo\nbar\nbaz'), + getContent: sinon.stub().returns('file content'), isEditable: sinon.stub().returns(true), }), - }, - { - origin: { - kind: 'file-restore', - path: 'foo.tex', - timestamp: new Date(ctx.endTs).toISOString(), - version: 42, - }, } ) }) @@ -1153,41 +1128,15 @@ describe('RestoreManager', function () { describe('for a project with overlap in current files and old files', function () { beforeEach(async function (ctx) { - ctx.HistoryManager.promises.getContentAtVersion = sinon - .stub() - .resolves({ - files: { - 'main.tex': { - hash: 'abcdef1234567890abcdef1234567890abcdef12', - stringLength: 100, - metadata: { - editorId: 'test-editor', - }, - }, - 'figures/image.png': { - hash: 'abcdef1234567890abcdef1234567890abcdef12', - stringLength: 100, - metadata: { - provider: 'bar', - }, - }, - 'since-deleted.tex': { - hash: 'abcdef1234567890abcdef1234567890abcdef12', - stringLength: 100, - metadata: { - editorId: 'test-editor', - }, - }, - }, - timestamp: new Date().toISOString(), - }) - ctx.ProjectEntityHandler.promises.getAllEntities = sinon .stub() .resolves({ docs: [{ path: '/main.tex' }, { path: '/new-file.tex' }], files: [{ path: '/figures/image.png' }], }) + ctx.RestoreManager.promises._getProjectPathsAtVersion = sinon + .stub() + .resolves(['main.tex', 'figures/image.png', 'since-deleted.tex']) await ctx.RestoreManager.promises.revertProject( ctx.user_id,