diff --git a/services/project-history/app/js/HttpController.js b/services/project-history/app/js/HttpController.js index d49fbd39d0..af2f4c36a8 100644 --- a/services/project-history/app/js/HttpController.js +++ b/services/project-history/app/js/HttpController.js @@ -219,6 +219,21 @@ export function getRangesSnapshot(req, res, next) { ) } +export function getFileMetadataSnapshot(req, res, next) { + const { project_id: projectId, version, pathname } = req.params + SnapshotManager.getFileMetadataSnapshot( + projectId, + version, + pathname, + (err, data) => { + if (err) { + return next(OError.tag(err)) + } + res.json(data) + } + ) +} + export function getLatestSnapshot(req, res, next) { const { project_id: projectId } = req.params WebApiManager.getHistoryId(projectId, (error, historyId) => { diff --git a/services/project-history/app/js/Router.js b/services/project-history/app/js/Router.js index cdf550ee73..08d97cd04d 100644 --- a/services/project-history/app/js/Router.js +++ b/services/project-history/app/js/Router.js @@ -163,6 +163,11 @@ export function initialize(app) { HttpController.getRangesSnapshot ) + app.get( + '/project/:project_id/metadata/version/:version/:pathname', + HttpController.getFileMetadataSnapshot + ) + app.get( '/project/:project_id/version/:version', HttpController.getProjectSnapshot diff --git a/services/project-history/app/js/SnapshotManager.js b/services/project-history/app/js/SnapshotManager.js index 1f5ee7df20..1049d074a3 100644 --- a/services/project-history/app/js/SnapshotManager.js +++ b/services/project-history/app/js/SnapshotManager.js @@ -7,6 +7,7 @@ import OError from '@overleaf/o-error' import * as HistoryStoreManager from './HistoryStoreManager.js' import * as WebApiManager from './WebApiManager.js' import * as Errors from './Errors.js' +import _ from 'lodash' /** * @typedef {import('stream').Readable} ReadableStream @@ -209,6 +210,30 @@ async function getRangesSnapshot(projectId, version, pathname) { } } +/** + * Gets the file metadata at a specific version. + * + * @param {string} projectId + * @param {number} version + * @param {string} pathname + * @returns {Promise<{metadata: any}>} + */ +async function getFileMetadataSnapshot(projectId, version, pathname) { + const snapshot = await _getSnapshotAtVersion(projectId, version) + const file = snapshot.getFile(pathname) + if (!file) { + throw new Errors.NotFoundError(`${pathname} not found`, { + projectId, + version, + pathname, + }) + } + const rawMetadata = file.getMetadata() + const metadata = _.isEmpty(rawMetadata) ? undefined : rawMetadata + + return { metadata } +} + // Returns project snapshot containing the document content for files with // text operations in the relevant chunk, and hashes for unmodified/binary // files. Used by git bridge to get the state of the project. @@ -350,12 +375,14 @@ const getProjectSnapshotCb = callbackify(getProjectSnapshot) const getLatestSnapshotCb = callbackify(getLatestSnapshot) const getLatestSnapshotFilesCb = callbackify(getLatestSnapshotFiles) const getRangesSnapshotCb = callbackify(getRangesSnapshot) +const getFileMetadataSnapshotCb = callbackify(getFileMetadataSnapshot) const getPathsAtVersionCb = callbackify(getPathsAtVersion) export { getChangesSinceCb as getChangesSince, getFileSnapshotStreamCb as getFileSnapshotStream, getProjectSnapshotCb as getProjectSnapshot, + getFileMetadataSnapshotCb as getFileMetadataSnapshot, getLatestSnapshotCb as getLatestSnapshot, getLatestSnapshotFilesCb as getLatestSnapshotFiles, getRangesSnapshotCb as getRangesSnapshot, @@ -370,4 +397,5 @@ export const promises = { getLatestSnapshotFiles, getRangesSnapshot, getPathsAtVersion, + getFileMetadataSnapshot, } diff --git a/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js b/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js index dbc51464e2..7a10d33b2b 100644 --- a/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js +++ b/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js @@ -999,6 +999,78 @@ Four five six\ }) }) + describe('getFileMetadataSnapshot', function () { + beforeEach(function () { + this.WebApiManager.promises.getHistoryId.resolves(this.historyId) + this.HistoryStoreManager.promises.getChunkAtVersion.resolves({ + chunk: (this.chunk = { + history: { + snapshot: { + files: { + 'main.tex': { + hash: '5d2781d78fa5a97b7bafa849fe933dfc9dc93eba', + metadata: { + importer_id: 'test-user-id', + imported_at: '2024-01-01T00:00:00.000Z', + }, + stringLength: 41, + }, + 'other.tex': { + hash: '5d2781d78fa5a97b7bafa849fe933dfc9dc93eba', + stringLength: 41, + }, + }, + }, + changes: [], + }, + startVersion: 1, + authors: [ + { + id: 31, + email: 'author@example.com', + name: 'Author', + }, + ], + }), + }) + }) + + it('should return the metadata for the file', async function () { + const result = + await this.SnapshotManager.promises.getFileMetadataSnapshot( + this.projectId, + 1, + 'main.tex' + ) + expect(result).to.deep.equal({ + metadata: { + importer_id: 'test-user-id', + imported_at: '2024-01-01T00:00:00.000Z', + }, + }) + }) + + it('should return undefined when file does not have metadata', async function () { + const result = + await this.SnapshotManager.promises.getFileMetadataSnapshot( + this.projectId, + 1, + 'other.tex' + ) + expect(result).to.deep.equal({ metadata: undefined }) + }) + + it('throw an error when file does not exist', async function () { + await expect( + this.SnapshotManager.promises.getFileMetadataSnapshot( + this.projectId, + 1, + 'does-not-exist.tex' + ) + ).to.be.rejectedWith(Error) + }) + }) + describe('getPathsAtVersion', function () { beforeEach(function () { this.WebApiManager.promises.getHistoryId.resolves(this.historyId) diff --git a/services/web/app/src/Features/History/RestoreManager.js b/services/web/app/src/Features/History/RestoreManager.js index 7d5cb60769..5b006c237e 100644 --- a/services/web/app/src/Features/History/RestoreManager.js +++ b/services/web/app/src/Features/History/RestoreManager.js @@ -97,24 +97,11 @@ const RestoreManager = { fsPath, pathname ) - if (importInfo.type === 'file') { - const newFile = await EditorController.promises.upsertFile( - projectId, - parentFolderId, - basename, - fsPath, - file?.element?.linkedFileData, - origin, - userId - ) - - return { - _id: newFile._id, - type: importInfo.type, - } - } if (file) { + if (file.type !== 'doc' && file.type !== 'file') { + throw new OError('unexpected file type', { type: file.type }) + } logger.debug( { projectId, fileId: file.element._id, type: importInfo.type }, 'deleting entity before reverting it' @@ -122,12 +109,37 @@ const RestoreManager = { await EditorController.promises.deleteEntity( projectId, file.element._id, - importInfo.type, + file.type, origin, userId ) } + const { metadata } = await RestoreManager._getMetadataFromHistory( + projectId, + version, + pathname + ) + + logger.debug({ metadata }, 'metadata from history') + + if (importInfo.type === 'file' || metadata) { + const newFile = await EditorController.promises.upsertFile( + projectId, + parentFolderId, + basename, + fsPath, + metadata, + origin, + userId + ) + + return { + _id: newFile._id, + type: 'file', + } + } + const ranges = await RestoreManager._getRangesFromHistory( projectId, version, @@ -309,6 +321,13 @@ const RestoreManager = { 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/test/unit/src/History/RestoreManagerTests.js b/services/web/test/unit/src/History/RestoreManagerTests.js index 69a6d53ad4..a142f1409e 100644 --- a/services/web/test/unit/src/History/RestoreManagerTests.js +++ b/services/web/test/unit/src/History/RestoreManagerTests.js @@ -231,6 +231,9 @@ describe('RestoreManager', function () { this.RestoreManager.promises._getRangesFromHistory = sinon .stub() .rejects() + this.RestoreManager.promises._getMetadataFromHistory = sinon + .stub() + .resolves({ metadata: undefined }) }) describe('reverting a project without ranges support', function () { @@ -252,7 +255,7 @@ describe('RestoreManager', function () { }) }) - describe('reverting a document', function () { + describe('reverting a document with ranges', function () { beforeEach(function () { this.pathname = 'foo.tex' this.comments = [ @@ -315,7 +318,9 @@ describe('RestoreManager', function () { }) this.RestoreManager.promises._getUpdatesFromHistory = sinon .stub() - .resolves([{ toV: this.version, meta: { end_ts: Date.now() } }]) + .resolves([ + { toV: this.version, meta: { end_ts: (this.endTs = new Date()) } }, + ]) this.EditorController.promises.addDocWithRanges = sinon .stub() .resolves((this.addedFile = { _id: 'mock-doc', type: 'doc' })) @@ -367,6 +372,39 @@ describe('RestoreManager', function () { }) describe('with an existing file in the current project', function () { + beforeEach(async function () { + this.ProjectLocator.promises.findElementByPath = sinon + .stub() + .resolves({ type: 'file', element: { _id: 'mock-file-id' } }) + this.EditorController.promises.deleteEntity = sinon.stub().resolves() + + this.data = await this.RestoreManager.promises.revertFile( + this.user_id, + this.project_id, + this.version, + this.pathname + ) + }) + + it('should delete the existing file', async function () { + expect( + this.EditorController.promises.deleteEntity + ).to.have.been.calledWith( + this.project_id, + 'mock-file-id', + 'file', + { + kind: 'file-restore', + path: this.pathname, + version: this.version, + timestamp: new Date(this.endTs).toISOString(), + }, + this.user_id + ) + }) + }) + + describe('with an existing document in the current project', function () { beforeEach(async function () { this.ProjectLocator.promises.findElementByPath = sinon .stub() @@ -392,7 +430,7 @@ describe('RestoreManager', function () { kind: 'file-restore', path: this.pathname, version: this.version, - timestamp: new Date().toISOString(), + timestamp: new Date(this.endTs).toISOString(), }, this.user_id ) @@ -427,7 +465,7 @@ describe('RestoreManager', function () { kind: 'file-restore', path: this.pathname, version: this.version, - timestamp: new Date().toISOString(), + timestamp: new Date(this.endTs).toISOString(), } ) }) @@ -448,6 +486,130 @@ describe('RestoreManager', function () { }) }) + describe('reverting a document with metadata', function () { + beforeEach(async function () { + this.pathname = 'foo.tex' + this.ProjectLocator.promises.findElementByPath = sinon.stub().rejects() + this.EditorController.promises.addDocWithRanges = sinon.stub() + this.FileSystemImportManager.promises.importFile = sinon + .stub() + .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] }) + this.RestoreManager.promises._getUpdatesFromHistory = sinon + .stub() + .resolves([ + { toV: this.version, meta: { end_ts: (this.endTs = new Date()) } }, + ]) + + this.EditorController.promises.upsertFile = sinon + .stub() + .resolves({ _id: 'mock-file-id', type: 'file' }) + this.RestoreManager.promises._getMetadataFromHistory = sinon + .stub() + .resolves({ metadata: { foo: 'bar' } }) + this.result = await this.RestoreManager.promises.revertFile( + this.user_id, + this.project_id, + this.version, + this.pathname + ) + }) + + it('should revert it as a file', function () { + expect(this.result).to.deep.equal({ _id: 'mock-file-id', type: 'file' }) + }) + + it('should upload to the project as a file', function () { + expect( + this.EditorController.promises.upsertFile + ).to.have.been.calledWith( + this.project_id, + 'mock-folder-id', + 'foo.tex', + this.fsPath, + { foo: 'bar' }, + { + kind: 'file-restore', + path: this.pathname, + version: this.version, + timestamp: new Date(this.endTs).toISOString(), + }, + this.user_id + ) + }) + + it('should not look up ranges', function () { + expect(this.RestoreManager.promises._getRangesFromHistory).to.not.have + .been.called + }) + + it('should not try to add a file', function () { + expect(this.EditorController.promises.addDocWithRanges).to.not.have.been + .called + }) + }) + + describe('reverting a file with metadata', function () { + beforeEach(async function () { + this.pathname = 'foo.png' + this.ProjectLocator.promises.findElementByPath = sinon.stub().rejects() + this.EditorController.promises.addDocWithRanges = sinon.stub() + this.FileSystemImportManager.promises.importFile = sinon + .stub() + .resolves({ type: 'file' }) + this.RestoreManager.promises._getUpdatesFromHistory = sinon + .stub() + .resolves([ + { toV: this.version, meta: { end_ts: (this.endTs = new Date()) } }, + ]) + + this.EditorController.promises.upsertFile = sinon + .stub() + .resolves({ _id: 'mock-file-id', type: 'file' }) + this.RestoreManager.promises._getMetadataFromHistory = sinon + .stub() + .resolves({ metadata: { foo: 'bar' } }) + this.result = await this.RestoreManager.promises.revertFile( + this.user_id, + this.project_id, + this.version, + this.pathname + ) + }) + + it('should revert it as a file', function () { + expect(this.result).to.deep.equal({ _id: 'mock-file-id', type: 'file' }) + }) + + it('should upload to the project as a file', function () { + expect( + this.EditorController.promises.upsertFile + ).to.have.been.calledWith( + this.project_id, + 'mock-folder-id', + 'foo.png', + this.fsPath, + { foo: 'bar' }, + { + kind: 'file-restore', + path: this.pathname, + version: this.version, + timestamp: new Date(this.endTs).toISOString(), + }, + this.user_id + ) + }) + + it('should not look up ranges', function () { + expect(this.RestoreManager.promises._getRangesFromHistory).to.not.have + .been.called + }) + + it('should not try to add a file', function () { + expect(this.EditorController.promises.addDocWithRanges).to.not.have.been + .called + }) + }) + describe('when reverting a binary file', function () { beforeEach(async function () { this.pathname = 'foo.png' @@ -457,6 +619,7 @@ describe('RestoreManager', function () { this.EditorController.promises.upsertFile = sinon .stub() .resolves({ _id: 'mock-file-id', type: 'file' }) + this.EditorController.promises.deleteEntity = sinon.stub().resolves() this.RestoreManager.promises._getUpdatesFromHistory = sinon .stub() .resolves([{ toV: this.version, meta: { end_ts: Date.now() } }]) @@ -465,7 +628,7 @@ describe('RestoreManager', function () { it('should return the created entity if file exists', async function () { this.ProjectLocator.promises.findElementByPath = sinon .stub() - .resolves({ type: 'file' }) + .resolves({ type: 'file', element: { _id: 'existing-file-id' } }) const revertRes = await this.RestoreManager.promises.revertFile( this.user_id,