[project-history] fetch most recent chunk once when processing updates (#23261)

GitOrigin-RevId: 08ddd1f953b0dbae0541a60b45fec0f88e0a1d06
2026-05-30 12:24:25 +02:00 · 2025-01-30 11:47:02 +00:00
parent 97385ffa77
commit 268a1fbeda
9 changed files with 132 additions and 25 deletions
--- a/libraries/overleaf-editor-core/lib/snapshot.js
+++ b/libraries/overleaf-editor-core/lib/snapshot.js
@@ -224,7 +224,7 @@ class Snapshot {
   *
   * @param {string} kind see {File#load}
   * @param {ReadonlyBlobStore} blobStore
-   * @return {Promise<Object>} an object where keys are the pathnames and
+   * @return {Promise<Record<string, File>>} an object where keys are the pathnames and
   * values are the files in the snapshot
   */
  async loadFiles(kind, blobStore) {
--- a/services/project-history/app/js/HistoryStoreManager.js
+++ b/services/project-history/app/js/HistoryStoreManager.js
@@ -78,7 +78,8 @@ export function getMostRecentVersion(projectId, historyId, callback) {
          err1 || err2,
          mostRecentVersion,
          projectStructureAndDocVersions,
-          lastChange
+          lastChange,
+          chunk
        )
      })
    )
@@ -572,6 +573,7 @@ function _requestHistoryService(options, callback) {
 }

 export const promises = {
+  /** @type {(projectId: string, historyId: string) => Promise<{chunk: import('overleaf-editor-core/lib/types.js').RawChunk}>} */
  getMostRecentChunk: promisify(getMostRecentChunk),
  getChunkAtVersion: promisify(getChunkAtVersion),
  getMostRecentVersion: promisify(getMostRecentVersion),
--- a/services/project-history/app/js/SnapshotManager.js
+++ b/services/project-history/app/js/SnapshotManager.js
@@ -277,8 +277,26 @@ async function _getSnapshotAtVersion(projectId, version) {
  return snapshot
 }

+/**
+ * @param {string} projectId
+ * @param {string} historyId
+ * @return {Promise<Record<string, import('overleaf-editor-core').File>>}
+ */
 async function getLatestSnapshotFiles(projectId, historyId) {
-  const { snapshot } = await getLatestSnapshot(projectId, historyId)
+  const data = await HistoryStoreManager.promises.getMostRecentChunk(
+    projectId,
+    historyId
+  )
+  return await getLatestSnapshotFilesForChunk(historyId, data)
+}
+
+/**
+ * @param {string} historyId
+ * @param {{chunk: import('overleaf-editor-core/lib/types.js').RawChunk}} chunk
+ * @return {Promise<Record<string, import('overleaf-editor-core').File>>}
+ */
+async function getLatestSnapshotFilesForChunk(historyId, chunk) {
+  const { snapshot } = getLatestSnapshotFromChunk(chunk)
  const snapshotFiles = await snapshot.loadFiles(
    'lazy',
    HistoryStoreManager.getBlobStore(historyId)
@@ -286,11 +304,24 @@ async function getLatestSnapshotFiles(projectId, historyId) {
  return snapshotFiles
 }

+/**
+ * @param {string} projectId
+ * @param {string} historyId
+ * @return {Promise<{version: number, snapshot: import('overleaf-editor-core').Snapshot}>}
+ */
 async function getLatestSnapshot(projectId, historyId) {
  const data = await HistoryStoreManager.promises.getMostRecentChunk(
    projectId,
    historyId
  )
+  return getLatestSnapshotFromChunk(data)
+}
+
+/**
+ * @param {{chunk: import('overleaf-editor-core/lib/types.js').RawChunk}} data
+ * @return {{version: number, snapshot: import('overleaf-editor-core').Snapshot}}
+ */
+function getLatestSnapshotFromChunk(data) {
  if (data == null || data.chunk == null) {
    throw new OError('undefined chunk')
  }
@@ -397,11 +428,15 @@ const getFileSnapshotStreamCb = callbackify(getFileSnapshotStream)
 const getProjectSnapshotCb = callbackify(getProjectSnapshot)
 const getLatestSnapshotCb = callbackify(getLatestSnapshot)
 const getLatestSnapshotFilesCb = callbackify(getLatestSnapshotFiles)
+const getLatestSnapshotFilesForChunkCb = callbackify(
+  getLatestSnapshotFilesForChunk
+)
 const getRangesSnapshotCb = callbackify(getRangesSnapshot)
 const getFileMetadataSnapshotCb = callbackify(getFileMetadataSnapshot)
 const getPathsAtVersionCb = callbackify(getPathsAtVersion)

 export {
+  getLatestSnapshotFromChunk,
  getChangesSinceCb as getChangesSince,
  getChangesInChunkSinceCb as getChangesInChunkSince,
  getFileSnapshotStreamCb as getFileSnapshotStream,
@@ -409,6 +444,7 @@ export {
  getFileMetadataSnapshotCb as getFileMetadataSnapshot,
  getLatestSnapshotCb as getLatestSnapshot,
  getLatestSnapshotFilesCb as getLatestSnapshotFiles,
+  getLatestSnapshotFilesForChunkCb as getLatestSnapshotFilesForChunk,
  getRangesSnapshotCb as getRangesSnapshot,
  getPathsAtVersionCb as getPathsAtVersion,
 }
@@ -420,6 +456,7 @@ export const promises = {
  getProjectSnapshot,
  getLatestSnapshot,
  getLatestSnapshotFiles,
+  getLatestSnapshotFilesForChunk,
  getRangesSnapshot,
  getPathsAtVersion,
  getFileMetadataSnapshot,
--- a/services/project-history/app/js/SyncManager.js
+++ b/services/project-history/app/js/SyncManager.js
@@ -178,9 +178,18 @@ async function skipUpdatesDuringSync(projectId, updates) {
  return { updates: filteredUpdates, syncState }
 }

+/**
+ * @param {string} projectId
+ * @param {string} projectHistoryId
+ * @param {{chunk: import('overleaf-editor-core/lib/types.js').RawChunk}} mostRecentChunk
+ * @param {Array<Update>} updates
+ * @param {() => Promise<void>} extendLock
+ * @return {Promise<Array<Update>>}
+ */
 async function expandSyncUpdates(
  projectId,
  projectHistoryId,
+  mostRecentChunk,
  updates,
  extendLock
 ) {
@@ -195,10 +204,11 @@ async function expandSyncUpdates(
  const syncState = await _getResyncState(projectId)

  // compute the current snapshot from the most recent chunk
-  const snapshotFiles = await SnapshotManager.promises.getLatestSnapshotFiles(
-    projectId,
-    projectHistoryId
-  )
+  const snapshotFiles =
+    await SnapshotManager.promises.getLatestSnapshotFilesForChunk(
+      projectHistoryId,
+      mostRecentChunk
+    )

  // check if snapshot files are valid
  const invalidFiles = _.pickBy(
@@ -1102,15 +1112,31 @@ const skipUpdatesDuringSyncCb = callbackifyMultiResult(skipUpdatesDuringSync, [
  'updates',
  'syncState',
 ])
+
+/**
+ * @param {string} projectId
+ * @param {string} projectHistoryId
+ * @param {{chunk: import('overleaf-editor-core/lib/types.js').RawChunk}} mostRecentChunk
+ * @param {Array<Update>} updates
+ * @param {() => void} extendLock
+ * @param {(err: Error | null, updates?: Array<Update>) => void} callback
+ */
 const expandSyncUpdatesCb = (
  projectId,
  projectHistoryId,
+  mostRecentChunk,
  updates,
  extendLock,
  callback
 ) => {
  const extendLockPromises = promisify(extendLock)
-  expandSyncUpdates(projectId, projectHistoryId, updates, extendLockPromises)
+  expandSyncUpdates(
+    projectId,
+    projectHistoryId,
+    mostRecentChunk,
+    updates,
+    extendLockPromises
+  )
    .then(result => {
      callback(null, result)
    })
--- a/services/project-history/app/js/UpdatesProcessor.js
+++ b/services/project-history/app/js/UpdatesProcessor.js
@@ -362,7 +362,13 @@ export function _processUpdates(
      _getMostRecentVersionWithDebug(
        projectId,
        projectHistoryId,
-        (error, baseVersion, projectStructureAndDocVersions) => {
+        (
+          error,
+          baseVersion,
+          projectStructureAndDocVersions,
+          _lastChange,
+          mostRecentChunk
+        ) => {
          if (projectStructureAndDocVersions == null) {
            projectStructureAndDocVersions = { project: null, docs: {} }
          }
@@ -377,6 +383,7 @@ export function _processUpdates(
                SyncManager.expandSyncUpdates(
                  projectId,
                  projectHistoryId,
+                  mostRecentChunk,
                  filteredUpdates,
                  extendLock,
                  cb
--- a/services/project-history/scripts/debug_translate_updates.js
+++ b/services/project-history/scripts/debug_translate_updates.js
@@ -25,6 +25,7 @@ function expandResyncProjectStructure(chunk, update) {
  SyncManager.expandSyncUpdates(
    projectId,
    99999, // dummy history id
+    chunk,
    [update],
    cb => cb(), // extend lock
    (err, result) => {
--- a/services/project-history/test/acceptance/js/SyncTests.js
+++ b/services/project-history/test/acceptance/js/SyncTests.js
@@ -53,16 +53,6 @@ describe('Syncing with web and doc-updater', function () {
            },
          },
        })
-      MockHistoryStore()
-        .get(`/api/projects/${historyId}/latest/history`)
-        .reply(200, {
-          chunk: {
-            startVersion: 0,
-            history: {
-              changes: [],
-            },
-          },
-        })
      ProjectHistoryClient.initializeProject(historyId, done)
    })
  })
--- a/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js
+++ b/services/project-history/test/unit/js/SyncManager/SyncManagerTests.js
@@ -114,7 +114,7 @@ describe('SyncManager', function () {

    this.SnapshotManager = {
      promises: {
-        getLatestSnapshotFiles: sinon.stub(),
+        getLatestSnapshotFilesForChunk: sinon.stub(),
      },
    }

@@ -492,6 +492,7 @@ describe('SyncManager', function () {
        _hash: 'abcde',
      }
      this.loadedSnapshotDoc = File.fromString(this.persistedDocContent)
+      this.mostRecentChunk = 'fake chunk'
      this.fileMap = {
        'main.tex': {
          isEditable: sinon.stub().returns(true),
@@ -517,7 +518,7 @@ describe('SyncManager', function () {
        .returns('another.tex')
      this.UpdateTranslator._convertPathname.withArgs('1.png').returns('1.png')
      this.UpdateTranslator._convertPathname.withArgs('2.png').returns('2.png')
-      this.SnapshotManager.promises.getLatestSnapshotFiles.resolves(
+      this.SnapshotManager.promises.getLatestSnapshotFilesForChunk.resolves(
        this.fileMap
      )
    })
@@ -527,13 +528,14 @@ describe('SyncManager', function () {
      const expandedUpdates = await this.SyncManager.promises.expandSyncUpdates(
        this.projectId,
        this.historyId,
+        this.mostRecentChunk,
        updates,
        this.extendLock
      )

      expect(expandedUpdates).to.equal(updates)
-      expect(this.SnapshotManager.promises.getLatestSnapshotFiles).to.not.have
-        .been.called
+      expect(this.SnapshotManager.promises.getLatestSnapshotFilesForChunk).to
+        .not.have.been.called
      expect(this.extendLock).to.not.have.been.called
    })

@@ -549,6 +551,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -562,6 +565,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -586,6 +590,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -621,6 +626,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -658,6 +664,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -699,6 +706,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -738,6 +746,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -774,6 +783,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -824,6 +834,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -875,6 +886,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -921,6 +933,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -964,6 +977,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -998,6 +1012,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1026,6 +1041,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1051,6 +1067,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1097,6 +1114,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1139,6 +1157,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1163,6 +1182,7 @@ describe('SyncManager', function () {
          this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1176,6 +1196,7 @@ describe('SyncManager', function () {
          this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1195,6 +1216,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1221,6 +1243,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1259,6 +1282,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1303,6 +1327,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1328,6 +1353,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1364,6 +1390,7 @@ describe('SyncManager', function () {
            await this.SyncManager.promises.expandSyncUpdates(
              this.projectId,
              this.historyId,
+              this.mostRecentChunk,
              updates,
              this.extendLock
            )
@@ -1418,6 +1445,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1440,6 +1468,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1483,6 +1512,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1517,6 +1547,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1560,6 +1591,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1614,6 +1646,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1643,6 +1676,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1733,6 +1767,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1754,6 +1789,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1795,6 +1831,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1836,6 +1873,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
@@ -1911,6 +1949,7 @@ describe('SyncManager', function () {
          await this.SyncManager.promises.expandSyncUpdates(
            this.projectId,
            this.historyId,
+            this.mostRecentChunk,
            updates,
            this.extendLock
          )
--- a/services/project-history/test/unit/js/UpdatesManager/UpdatesProcessorTests.js
+++ b/services/project-history/test/unit/js/UpdatesManager/UpdatesProcessorTests.js
@@ -289,10 +289,14 @@ describe('UpdatesProcessor', function () {
      this.newSyncState = { resyncProjectStructure: false }

      this.extendLock = sinon.stub().yields()
+      this.mostRecentChunk = 'fake-chunk'

      this.HistoryStoreManager.getMostRecentVersion.yields(
        null,
-        this.mostRecentVersionInfo
+        this.mostRecentVersionInfo,
+        null,
+        '_lastChange',
+        this.mostRecentChunk
      )
      this.SyncManager.skipUpdatesDuringSync.yields(
        null,
@@ -300,7 +304,7 @@ describe('UpdatesProcessor', function () {
        this.newSyncState
      )
      this.SyncManager.expandSyncUpdates.callsArgWith(
-        4,
+        5,
        null,
        this.expandedUpdates
      )
@@ -345,6 +349,7 @@ describe('UpdatesProcessor', function () {
        return this.SyncManager.expandSyncUpdates.should.have.been.calledWith(
          this.project_id,
          this.ol_project_id,
+          this.mostRecentChunk,
          this.filteredUpdates,
          this.extendLock
        )