[web] reduce mongo lookups for compile requests (#34890)

* [web] _getUserCompileLimits: use owner.analyticsId directly

* [web] deduplicate project lookups for compile requests

* [web] add test case for prefetched project

GitOrigin-RevId: 0e140619c874accf8673153ed90af2ce70cf9234

services/web/app/src/Features/Compile/ClsiManager.mjs

@@ -150,25 +150,22 @@ function collectMetricsOnBlgFiles(outputFiles) {
   Metrics.count('blg_output_file', nested, 1, { path: 'nested' })
 }
 
-async function sendRequest(projectId, userId, options) {
-  if (options == null) {
-    options = {}
-  }
-  let result = await sendRequestOnce(projectId, userId, options)
+async function sendRequest(project, projectId, userId, options) {
+  let result = await sendRequestOnce(project, projectId, userId, options)
   if (result.status === 'missing-updates') {
     // try again with updated baseline
-    result = await sendRequestOnce(projectId, userId, {
+    result = await sendRequestOnce(project, projectId, userId, {
       ...options,
       baseHistoryVersion: result.baseHistoryVersion,
     })
   } else if (result.status === 'conflict') {
     // Try again, with a full compile
-    result = await sendRequestOnce(projectId, userId, {
+    result = await sendRequestOnce(null, projectId, userId, {
       ...options,
       syncType: 'full',
     })
   } else if (result.status === 'unavailable') {
-    result = await sendRequestOnce(projectId, userId, {
+    result = await sendRequestOnce(null, projectId, userId, {
       ...options,
       syncType: 'full',
       forceNewClsiServer: true,
@@ -177,10 +174,10 @@ async function sendRequest(projectId, userId, options) {
   return result
 }
 
-async function sendRequestOnce(projectId, userId, options) {
+async function sendRequestOnce(project, projectId, userId, options) {
   let req
   try {
-    req = await _buildRequest(projectId, userId, options)
+    req = await _buildRequest(project, projectId, userId, options)
   } catch (err) {
     if (err.message === 'no main file specified') {
       return {
@@ -735,13 +732,15 @@ function _parseOutputFiles(projectId, rawOutputFiles = []) {
   return outputFiles
 }
 
-async function _buildRequest(projectId, userId, options) {
-  const project = await ProjectGetter.promises.getProject(projectId, {
-    compiler: 1,
-    imageName: 1,
-    'overleaf.history.id': 1,
-    ...(options.compileFromHistory ? {} : { rootDoc_id: 1, rootFolder: 1 }),
-  })
+async function _buildRequest(project, projectId, userId, options) {
+  if (project === null) {
+    project = await ProjectGetter.promises.getProject(projectId, {
+      compiler: 1,
+      imageName: 1,
+      'overleaf.history.id': 1,
+      ...(options.compileFromHistory ? {} : { rootDoc_id: 1, rootFolder: 1 }),
+    })
+  }
   if (project == null) {
     throw new Errors.NotFoundError(`project does not exist: ${projectId}`)
   }
@@ -770,7 +769,7 @@ async function _buildRequest(projectId, userId, options) {
         'failed to compose history-full request'
       )
       // fall back to old compile mode
-      return await _buildRequest(projectId, userId, {
+      return await _buildRequest(null, projectId, userId, {
         ...options,
         compileFromHistory: false,
       })
@@ -791,7 +790,7 @@ async function _buildRequest(projectId, userId, options) {
         'failed to compose history-incremental request'
       )
       // fall back to old compile mode
-      return await _buildRequest(projectId, userId, {
+      return await _buildRequest(null, projectId, userId, {
         ...options,
         compileFromHistory: false,
       })
@@ -1175,7 +1174,7 @@ function _finaliseRequest(projectId, options, project, docs, files) {
 }
 
 async function buildDocumentConversionRequest(projectId, userId, options) {
-  return await _buildRequest(projectId, userId, {
+  return await _buildRequest(null, projectId, userId, {
     ...options,
     // Use the history snapshot as populated on clsi-cache.
     populateClsiCache: true,
@@ -1186,7 +1185,7 @@ async function buildDocumentConversionRequest(projectId, userId, options) {
 
 async function wordCount(projectId, userId, file, limits, clsiserverid) {
   const { compileBackendClass, compileGroup } = limits
-  const req = await _buildRequest(projectId, userId, limits)
+  const req = await _buildRequest(null, projectId, userId, limits)
   const filename = file || req.compile.rootResourcePath
   const url = _getCompilerUrl(
     compileBackendClass,

services/web/app/src/Features/Compile/CompileManager.mjs

@@ -7,8 +7,8 @@ import UserGetter from '../User/UserGetter.mjs'
 import ClsiManager from './ClsiManager.mjs'
 import Metrics from '@overleaf/metrics'
 import { RateLimiter } from '../../infrastructure/RateLimiter.mjs'
-import UserAnalyticsDataCache from '../Analytics/UserAnalyticsDataCache.mjs'
 import { callbackify, callbackifyMultiResult } from '@overleaf/promise-utils'
+import Errors from '../Errors/Errors.js'
 let CompileManager
 const rclient = RedisWrapper.client('clsi_recently_compiled')
 
@@ -63,8 +63,23 @@ async function compile(projectId, userId, options = {}) {
     }
   }
 
-  const limits =
-    await CompileManager.promises.getProjectCompileLimits(projectId)
+  // Generate the buildId ahead of fetching the project content from redis/mongo so that the buildId's timestamp is before any lastUpdated date.
+  options.buildId = generateBuildId()
+
+  const project = await ProjectGetter.promises.getProject(projectId, {
+    // _getProjectCompileLimits
+    owner_ref: 1,
+    fromV1TemplateId: 1,
+    // _build_request
+    compiler: 1,
+    imageName: 1,
+    'overleaf.history.id': 1,
+    ...(options.compileFromHistory ? {} : { rootDoc_id: 1, rootFolder: 1 }),
+  })
+  if (project == null) {
+    throw new Errors.NotFoundError(`project does not exist: ${projectId}`)
+  }
+  const limits = await _getProjectCompileLimits(project)
   for (const key in limits) {
     const value = limits[key]
     options[key] = value
@@ -82,9 +97,6 @@ async function compile(projectId, userId, options = {}) {
     return { message: 'autocompile-backoff', outputFiles: [] }
   }
 
-  // Generate the buildId ahead of fetching the project content from redis/mongo so that the buildId's timestamp is before any lastUpdated date.
-  options.buildId = generateBuildId()
-
   // only pass userId down to clsi if this is a per-user compile
   const compileAsUser = Settings.disablePerUserCompiles ? undefined : userId
   const {
@@ -99,7 +111,12 @@ async function compile(projectId, userId, options = {}) {
     clsiCacheShard,
     baseHistoryVersion,
     instanceType,
-  } = await ClsiManager.promises.sendRequest(projectId, compileAsUser, options)
+  } = await ClsiManager.promises.sendRequest(
+    project,
+    projectId,
+    compileAsUser,
+    options
+  )
 
   return {
     status,
@@ -153,11 +170,6 @@ async function _getUserCompileLimits(userId) {
     ownerFeatures.compileGroup = 'alpha'
   }
 
-  const analyticsId = await UserAnalyticsDataCache.getAnalyticsId(
-    owner._id,
-    '_getUserCompileLimits'
-  )
-
   const compileGroup =
     ownerFeatures.compileGroup || Settings.defaultFeatures.compileGroup
   const limits = {
@@ -168,7 +180,7 @@ async function _getUserCompileLimits(userId) {
       compileGroup === 'standard'
         ? Settings.apis.clsi.standardCompileBackendClass
         : Settings.apis.clsi.priorityCompileBackendClass,
-    ownerAnalyticsId: analyticsId,
+    ownerAnalyticsId: owner.analyticsId,
   }
 
   return limits

services/web/test/unit/src/Compile/ClsiManager.test.mjs

@@ -332,6 +332,7 @@ describe('ClsiManager', function () {
         ctx.responseBody.compile.buildId = buildId
         ctx.timeout = 100
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {
@@ -435,6 +436,87 @@ describe('ClsiManager', function () {
       })
     })
 
+    describe('with the project prefetched', function () {
+      const buildId = '18fbe9e7564-30dcb2f71250c690'
+
+      beforeEach(async function (ctx) {
+        ctx.outputFiles = [
+          {
+            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.pdf`,
+            path: 'output.pdf',
+            type: 'pdf',
+            build: buildId,
+          },
+          {
+            url: `/project/${ctx.project_id}/user/${ctx.user_id}/build/${buildId}/output/output.log`,
+            path: 'output.log',
+            type: 'log',
+            build: buildId,
+          },
+        ]
+        ctx.responseBody.compile.outputFiles = ctx.outputFiles.map(
+          outputFile => ({
+            ...outputFile,
+            url: `http://${CLSI_HOST}${outputFile.url}`,
+          })
+        )
+        ctx.responseBody.compile.buildId = buildId
+        ctx.timeout = 100
+        ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          ctx.project,
+          ctx.project._id,
+          ctx.user_id,
+          {
+            compileBackendClass: 'free',
+            compileGroup: 'standard',
+            timeout: ctx.timeout,
+          }
+        )
+      })
+
+      it('should send the request to the CLSI', function (ctx) {
+        ctx.FetchUtils.fetchStringWithResponse.should.have.been.calledWith(
+          sinon.match(
+            url =>
+              url.host === CLSI_HOST &&
+              url.pathname ===
+                `/project/${ctx.project._id}/user/${ctx.user_id}/compile` &&
+              url.searchParams.get('compileBackendClass') === 'free' &&
+              url.searchParams.get('compileGroup') === 'standard'
+          ),
+          {
+            method: 'POST',
+            json: sinon.match({
+              compile: {
+                options: {
+                  compiler: ctx.project.compiler,
+                  imageName: ctx.project.imageName,
+                  timeout: ctx.timeout,
+                  draft: false,
+                  compileGroup: 'standard',
+                  metricsMethod: 'standard',
+                  stopOnFirstError: false,
+                  syncType: undefined,
+                },
+                rootResourcePath: 'main.tex',
+                resources: _makeResources(ctx.project, ctx.docs, ctx.files),
+              },
+            }),
+            headers: {
+              Accept: 'application/json',
+              'Content-Type': 'application/json',
+              Cookie: `${ctx.clsiCookieKey}=${ctx.clsiServerId}`,
+            },
+            signal: sinon.match.instanceOf(AbortSignal),
+          }
+        )
+      })
+
+      it('should get the project with the required fields', function (ctx) {
+        ctx.ProjectGetter.promises.getProject.should.not.have.been.called
+      })
+    })
+
     describe('with compile from history fallback to incremental', function () {
       const buildId = '18fbe9e7564-30dcb2f71250c690'
 
@@ -470,6 +552,7 @@ describe('ClsiManager', function () {
           'mock-doc-id-1': 'main.tex',
         })
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {
@@ -619,6 +702,7 @@ describe('ClsiManager', function () {
         ctx.responseBody.compile.stats = ctx.stats
         ctx.responseBody.compile.timings = ctx.timings
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           { compileBackendClass: 'free', compileGroup: 'standard' }
@@ -651,6 +735,7 @@ describe('ClsiManager', function () {
           'mock-doc-id-1': 'main.tex',
         })
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {
@@ -762,6 +847,7 @@ describe('ClsiManager', function () {
           'mock-doc-id-2': '/chapters/chapter1.tex',
         })
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {
@@ -785,6 +871,7 @@ describe('ClsiManager', function () {
     describe('when root doc override is valid', function () {
       beforeEach(async function (ctx) {
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           { rootDoc_id: 'mock-doc-id-2' }
@@ -804,6 +891,7 @@ describe('ClsiManager', function () {
     describe('when root doc override is invalid', function () {
       beforeEach(async function (ctx) {
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           { rootDoc_id: 'invalid-id' }
@@ -824,6 +912,7 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.project.compiler = 'context'
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {}
@@ -844,6 +933,7 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.project.rootDoc_id = 'not-valid'
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {}
@@ -877,6 +967,7 @@ describe('ClsiManager', function () {
         }
         ctx.ProjectEntityHandler.promises.getAllDocs.resolves(ctx.docs)
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {}
@@ -900,6 +991,7 @@ describe('ClsiManager', function () {
         }
         ctx.ProjectEntityHandler.promises.getAllDocs.resolves(ctx.docs)
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {}
@@ -919,6 +1011,7 @@ describe('ClsiManager', function () {
     describe('with the draft option', function () {
       beforeEach(async function (ctx) {
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {
@@ -942,6 +1035,7 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.responseBody.compile.status = 'failure'
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {}
@@ -970,6 +1064,7 @@ describe('ClsiManager', function () {
             response: ctx.response,
           })
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {}
@@ -1006,6 +1101,7 @@ describe('ClsiManager', function () {
           response: ctx.response,
         })
         ctx.result = await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           { compileBackendClass: 'free' }
@@ -1050,7 +1146,12 @@ describe('ClsiManager', function () {
 
       it('should throw an error', async function (ctx) {
         await expect(
-          ctx.ClsiManager.promises.sendRequest(ctx.project._id, ctx.user_id, {})
+          ctx.ClsiManager.promises.sendRequest(
+            null,
+            ctx.project._id,
+            ctx.user_id,
+            {}
+          )
         ).to.be.rejected
       })
     })
@@ -1059,6 +1160,7 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.Settings.apis.clsi_new.url = 'https://compiles.somewhere.test'
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {
@@ -1113,6 +1215,7 @@ describe('ClsiManager', function () {
         ctx.Settings.apis.clsi_new.url = 'https://compiles.somewhere.test'
         ctx.Settings.apis.clsi_new.doubleCompileFree.sample = 0
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {
@@ -1143,6 +1246,7 @@ describe('ClsiManager', function () {
       beforeEach(async function (ctx) {
         ctx.Settings.apis.clsi_new.url = 'https://compiles.somewhere.test'
         await ctx.ClsiManager.promises.sendRequest(
+          null,
           ctx.project._id,
           ctx.user_id,
           {

services/web/test/unit/src/Compile/CompileManager.test.mjs

@@ -1,5 +1,6 @@
 import { vi, expect } from 'vitest'
 import sinon from 'sinon'
+import _ from 'lodash'
 
 const MODULE_PATH = '../../../../app/src/Features/Compile/CompileManager.mjs'
 
@@ -16,6 +17,45 @@ describe('CompileManager', function () {
       inc: sinon.stub(),
     }
 
+    ctx.project = {
+      _id: 'project-id',
+      owner_ref: 'owner-id',
+      compiler: 'latex',
+      rootDoc_id: 'mock-doc-id-1',
+      imageName: 'mock-image-name',
+      overleaf: { history: { id: 42 } },
+      fromV1TemplateId: 1337,
+      rootFolder: [
+        {
+          docs: [],
+          files: [],
+          folders: [],
+        },
+      ],
+    }
+
+    ctx.user = {
+      _id: 'owner-id',
+      features: { compileTimeout: 42, compileGroup: 'standard' },
+      analyticsId: 'abc',
+    }
+
+    ctx.ProjectGetter = {
+      promises: {
+        getProject: sinon.stub().callsFake((projectId, projection) => {
+          const result = { _id: ctx.project._id }
+          for (const [field, v] of Object.entries(projection)) {
+            if (v) {
+              _.set(result, field, _.get(ctx.project, field))
+            } else {
+              _.unset(result, field)
+            }
+          }
+          return result
+        }),
+      },
+    }
+
     vi.doMock('@overleaf/settings', () => ({
       default: (ctx.settings = {
         apis: {
@@ -49,11 +89,15 @@ describe('CompileManager', function () {
     )
 
     vi.doMock('../../../../app/src/Features/Project/ProjectGetter', () => ({
-      default: (ctx.ProjectGetter = { promises: {} }),
+      default: ctx.ProjectGetter,
     }))
 
     vi.doMock('../../../../app/src/Features/User/UserGetter', () => ({
-      default: (ctx.UserGetter = { promises: {} }),
+      default: (ctx.UserGetter = {
+        promises: {
+          getUser: sinon.stub().resolves(ctx.user),
+        },
+      }),
     }))
 
     vi.doMock('../../../../app/src/Features/Compile/ClsiManager', () => ({
@@ -105,9 +149,6 @@ describe('CompileManager', function () {
           rootDocId: 'mock-root-doc-id-123',
           rootResourcePath: '/main.tex',
         })
-      ctx.CompileManager.promises.getProjectCompileLimits = sinon
-        .stub()
-        .resolves(ctx.limits)
       ctx.ClsiManager.promises.sendRequest = sinon.stub().resolves({
         status: (ctx.status = 'mock-status'),
         outputFiles: (ctx.outputFiles = []),
@@ -122,17 +163,6 @@ describe('CompileManager', function () {
           isAutoCompile,
           compileGroup
         ) => true
-        ctx.ProjectGetter.promises.getProject = sinon
-          .stub()
-          .resolves(
-            (ctx.project = { owner_ref: (ctx.owner_id = 'owner-id-123') })
-          )
-        ctx.UserGetter.promises.getUser = sinon.stub().resolves(
-          (ctx.user = {
-            features: { compileTimeout: '20s', compileGroup: 'standard' },
-            analyticsId: 'abc',
-          })
-        )
         result = await ctx.CompileManager.promises.compile(
           ctx.project_id,
           ctx.user_id,
@@ -153,13 +183,23 @@ describe('CompileManager', function () {
       })
 
       it('should get the project compile limits', function (ctx) {
-        ctx.CompileManager.promises.getProjectCompileLimits
-          .calledWith(ctx.project_id)
+        ctx.UserGetter.promises.getUser
+          .calledWith(ctx.project.owner_ref)
           .should.equal(true)
       })
 
       it('should run the compile with the compile limits', function (ctx) {
         ctx.ClsiManager.promises.sendRequest.should.have.been.calledWith(
+          {
+            _id: 'project-id',
+            compiler: 'latex',
+            fromV1TemplateId: 1337,
+            imageName: 'mock-image-name',
+            overleaf: { history: { id: 42 } },
+            owner_ref: 'owner-id',
+            rootDoc_id: 'mock-doc-id-1',
+            rootFolder: [{ docs: [], files: [], folders: [] }],
+          },
           ctx.project_id,
           ctx.user_id,
           {
@@ -168,6 +208,8 @@ describe('CompileManager', function () {
             buildId: sinon.match(/[a-f0-9]+-[a-f0-9]+/),
             rootResourcePath: 'main.tex',
             rootDoc_id: 'mock-root-doc-id-123',
+            compileBackendClass: 'free',
+            ownerAnalyticsId: 'abc',
           }
         )
       })