Revert "File restore optimization - using snapshot timestamp and file paths (#28546)"

This reverts commit 376a53e1f927cb56544e6782b47d80345655038c.

GitOrigin-RevId: 8e4ab1d4042fec8df1b62ab1c5418d873dc6f5eb

libraries/overleaf-editor-core/lib/change.js

@@ -294,8 +294,6 @@ class Change {
     if (this.v2DocVersions) {
       snapshot.updateV2DocVersions(this.v2DocVersions)
     }
-
-    snapshot.setTimestamp(this.timestamp)
   }
 
   /**

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

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

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

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
 }
 

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)

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

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 })
     }
 
-    const snapshotRaw = await HistoryManager.promises.getContentAtVersion(
+    // Get project paths at version
+    const pathsAtPastVersion = await RestoreManager._getProjectPathsAtVersion(
       projectId,
       version
     )
-    const snapshot = Snapshot.fromRaw(snapshotRaw)
 
-    const pathsAtPastVersion = snapshot.getFilePathnames()
+    const updates = await RestoreManager._getUpdatesFromHistory(
+      projectId,
+      version
+    )
+    const updateAtVersion = updates.find(update => update.toV === version)
 
     const origin = {
       kind: 'project-restore',
       version,
-      timestamp: snapshot.getTimestamp()?.toISOString(),
+      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 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 }

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,