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

GitOrigin-RevId: 08ddd1f953b0dbae0541a60b45fec0f88e0a1d06

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

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

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,

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

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

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

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

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
           )

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
         )