Merge pull request #20266 from overleaf/mj-revert-linked-file

[project-history+web] Restore metadata when reverting file

GitOrigin-RevId: dbfa8202a2fe0bf077d8eedb51a2a13b9f1e8a83

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) => {

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

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,
 }

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)

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)

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,