[web] determine rootResourcePath if not set in compile request (#34398)

* [web] determine rootResourcePath if not set in compile request

This affects the following compile requests:
- compile request from "Download PDF" button on project dashboard
  Pre-resolving the rootResourcePath for all projects is too expensive.
  Adding an endpoint to resolve it is too slow and doubles project auth
  check.
- custom frontends
  Can't do much about these. I tried blocking them, requests still come
  in after weeks of returning 400s.

The rootDoc maintenance methods now return {rootResourcePath,rootDocId}
rather than just the rootDocId. This avoids an extra project lookup when
the rootDocId was updated.

GitOrigin-RevId: ab7c539b7ffd162bde0903fe1040a48834a245f5

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

@@ -217,24 +217,6 @@ const _CompileController = {
       options.pdfCachingMinChunkSize = pdfCachingMinChunkSize
     }
 
-    if (!options.rootResourcePath) {
-      const agent = (req.headers['user-agent'] || '').toLowerCase()
-      const isKnownOtherFrontend = agent.includes('node-fetch')
-      logger.warn(
-        { isKnownOtherFrontend, req, projectId, userId, options },
-        'rootResourcePath is missing in request body'
-      )
-      if (isKnownOtherFrontend) {
-        // Reject malformed compile request from "known" other frontend.
-        res.status(400).json({
-          error: 'rootResourcePath is missing in request body',
-        })
-        return
-      }
-      // All others: Log for now and fall back to old compile mode.
-      options.compileFromHistory = false
-    }
-
     const {
       status,
       outputFiles,

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

@@ -48,7 +48,20 @@ async function compile(projectId, userId, options = {}) {
     return { status: 'autocompile-backoff', outputFiles: [] }
   }
 
-  await ProjectRootDocManager.promises.ensureRootDocumentIsSet(projectId)
+  if (!options.rootResourcePath) {
+    const result =
+      await ProjectRootDocManager.promises.ensureRootDocumentIsValid(projectId)
+    if (result) {
+      options.rootDoc_id = result.rootDocId
+      options.rootResourcePath = result.rootResourcePath.replace(/^\//, '')
+    } else {
+      return {
+        status: 'validation-problems',
+        validationProblems: { mainFile: 'no main file specified' },
+        outputFiles: [],
+      }
+    }
+  }
 
   const limits =
     await CompileManager.promises.getProjectCompileLimits(projectId)

services/web/app/src/Features/Exports/ExportsHandler.mjs

@@ -1,7 +1,6 @@
 import OError from '@overleaf/o-error'
 import ProjectGetter from '../Project/ProjectGetter.mjs'
 import ProjectHistoryHandler from '../Project/ProjectHistoryHandler.mjs'
-import ProjectLocator from '../Project/ProjectLocator.mjs'
 import ProjectRootDocManager from '../Project/ProjectRootDocManager.mjs'
 import UserGetter from '../User/UserGetter.mjs'
 import logger from '@overleaf/logger'
@@ -37,18 +36,26 @@ export default ExportsHandler = {
       show_source: showSource,
     } = exportParams
 
-    let project, rootDoc, user, historyVersion
+    let project, rootResourcePath, user, historyVersion
 
     try {
-      project = await ProjectGetter.promises.getProject(projectId)
-
-      await ProjectRootDocManager.promises.ensureRootDocumentIsValid(projectId)
-
-      rootDoc = await ProjectLocator.promises.findRootDoc({
-        project,
-        project_id: projectId,
+      project = await ProjectGetter.promises.getProject(projectId, {
+        overleaf: 1,
+        compiler: 1,
+        imageName: 1,
       })
 
+      const result =
+        await ProjectRootDocManager.promises.ensureRootDocumentIsValid(
+          projectId
+        )
+      if (!result) {
+        throw new OError('cannot export project without root doc', {
+          project_id: projectId,
+        })
+      }
+      rootResourcePath = result.rootResourcePath
+
       user = await UserGetter.promises.getUser(userId, {
         first_name: 1,
         last_name: 1,
@@ -67,12 +74,6 @@ export default ExportsHandler = {
       })
     }
 
-    if (!rootDoc || !rootDoc.path) {
-      throw new OError('cannot export project without root doc', {
-        project_id: projectId,
-      })
-    }
-
     if (exportParams.first_name && exportParams.last_name) {
       user.first_name = exportParams.first_name
       user.last_name = exportParams.last_name
@@ -81,7 +82,7 @@ export default ExportsHandler = {
     return {
       project: {
         id: projectId,
-        rootDocPath: rootDoc.path ? rootDoc.path.fileSystem : undefined,
+        rootDocPath: rootResourcePath,
         historyId: project.overleaf?.history?.id,
         historyVersion,
         v1ProjectId: project.overleaf != null ? project.overleaf.id : undefined,

services/web/app/src/Features/Project/ProjectEntityUpdateHandler.mjs

@@ -201,27 +201,27 @@ async function updateDocLines(
   return { rev, modified }
 }
 
-async function setRootDoc(projectId, newRootDocID) {
-  logger.debug({ projectId, rootDocId: newRootDocID }, 'setting root doc')
-  if (projectId == null || newRootDocID == null) {
+async function setRootDoc(projectId, rootDocId) {
+  logger.debug({ projectId, rootDocId }, 'setting root doc')
+  if (projectId == null || rootDocId == null) {
     throw new Errors.InvalidError('missing arguments (project or doc)')
   }
-  const docPath =
+  const rootResourcePath =
     await ProjectEntityHandler.promises.getDocPathByProjectIdAndDocId(
       projectId,
-      newRootDocID
+      rootDocId
     )
-  if (ProjectEntityUpdateHandler.isPathValidForRootDoc(docPath)) {
+  if (ProjectEntityUpdateHandler.isPathValidForRootDoc(rootResourcePath)) {
     await Project.updateOne(
       { _id: projectId },
-      { rootDoc_id: newRootDocID }
+      { rootDoc_id: rootDocId }
     ).exec()
   } else {
     throw new Errors.UnsupportedFileTypeError(
       'invalid file extension for root doc'
     )
   }
-  return newRootDocID
+  return { rootDocId, rootResourcePath }
 }
 
 async function unsetRootDoc(projectId) {

services/web/app/src/Features/Project/ProjectRootDocManager.mjs

@@ -129,19 +129,6 @@ async function setRootDocFromName(projectId, rootDocName) {
   }
 }
 
-async function ensureRootDocumentIsSet(projectId) {
-  const project = await ProjectGetter.promises.getProject(projectId, {
-    rootDoc_id: 1,
-  })
-  if (!project) {
-    throw new Error('project not found')
-  }
-  if (project.rootDoc_id != null) {
-    return
-  }
-  return await ProjectRootDocManager.promises.setRootDocAutomatically(projectId)
-}
-
 /**
  * @param {ObjectId | string} projectId
  */
@@ -154,13 +141,13 @@ async function ensureRootDocumentIsValid(projectId) {
     throw new Error('project not found')
   }
   if (project.rootDoc_id != null) {
-    const docPath =
+    const rootResourcePath =
       await ProjectEntityHandler.promises.getDocPathFromProjectByDocId(
         project,
         project.rootDoc_id
       )
-    if (docPath) {
-      return
+    if (rootResourcePath) {
+      return { rootDocId: project.rootDoc_id, rootResourcePath }
     }
     await ProjectEntityUpdateHandler.promises.unsetRootDoc(projectId)
   }
@@ -214,13 +201,11 @@ const ProjectRootDocManager = {
     ['path', 'content']
   ),
   setRootDocFromName: callbackify(setRootDocFromName),
-  ensureRootDocumentIsSet: callbackify(ensureRootDocumentIsSet),
   ensureRootDocumentIsValid: callbackify(ensureRootDocumentIsValid),
   promises: {
     setRootDocAutomatically,
     findRootDocFileFromDirectory,
     setRootDocFromName,
-    ensureRootDocumentIsSet,
     ensureRootDocumentIsValid,
   },
 }

services/web/app/src/Features/Templates/TemplatesManager.mjs

@@ -116,11 +116,11 @@ const TemplatesManager = {
     if (mainFile == null) {
       return
     }
-    const rootDocId = await ProjectRootDocManager.setRootDocFromName(
+    const result = await ProjectRootDocManager.setRootDocFromName(
       project._id,
       mainFile
     )
-    if (rootDocId) project.rootDoc_id = rootDocId
+    if (result) project.rootDoc_id = result.rootDocId
   },
 
   async fetchFromV1(templateId) {

services/web/app/src/Features/Uploads/ProjectUploadManager.mjs

@@ -91,11 +91,11 @@ async function createProjectFromZipArchiveWithName(
     try {
       const { fileEntries, docEntries } =
         await _initializeProjectWithZipContents(ownerId, project, contentsPath)
-      const rootDocId =
+      const result =
         await ProjectRootDocManager.promises.setRootDocAutomatically(
           project._id
         )
-      if (rootDocId) project.rootDoc_id = rootDocId
+      if (result) project.rootDoc_id = result.rootDocId
       return { fileEntries, docEntries, project }
     } catch (err) {
       // no need to wait for the cleanup here

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

@@ -95,9 +95,12 @@ describe('CompileManager', function () {
   describe('compile', function () {
     beforeEach(function (ctx) {
       ctx.CompileManager._checkIfRecentlyCompiled = sinon.stub().resolves(false)
-      ctx.ProjectRootDocManager.promises.ensureRootDocumentIsSet = sinon
+      ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid = sinon
         .stub()
-        .resolves()
+        .resolves({
+          rootDocId: 'mock-root-doc-id-123',
+          rootResourcePath: '/main.tex',
+        })
       ctx.CompileManager.promises.getProjectCompileLimits = sinon
         .stub()
         .resolves(ctx.limits)
@@ -108,7 +111,7 @@ describe('CompileManager', function () {
       })
     })
 
-    describe('succesfully', function () {
+    describe('successfully', function () {
       let result
       beforeEach(async function (ctx) {
         ctx.CompileManager._checkIfAutoCompileLimitHasBeenHit = async (
@@ -140,7 +143,7 @@ describe('CompileManager', function () {
       })
 
       it('should ensure that the root document is set', function (ctx) {
-        ctx.ProjectRootDocManager.promises.ensureRootDocumentIsSet
+        ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid
           .calledWith(ctx.project_id)
           .should.equal(true)
       })
@@ -152,13 +155,17 @@ describe('CompileManager', function () {
       })
 
       it('should run the compile with the compile limits', function (ctx) {
-        ctx.ClsiManager.promises.sendRequest
-          .calledWith(ctx.project_id, ctx.user_id, {
+        ctx.ClsiManager.promises.sendRequest.should.have.been.calledWith(
+          ctx.project_id,
+          ctx.user_id,
+          {
             timeout: ctx.limits.timeout,
             compileGroup: 'standard',
             buildId: sinon.match(/[a-f0-9]+-[a-f0-9]+/),
-          })
-          .should.equal(true)
+            rootResourcePath: 'main.tex',
+            rootDoc_id: 'mock-root-doc-id-123',
+          }
+        )
       })
 
       it('should resolve with the output', function (ctx) {
@@ -172,6 +179,37 @@ describe('CompileManager', function () {
       })
     })
 
+    describe('with rootResourcePath given', function () {
+      it('should not check the root doc', async function (ctx) {
+        const { status } = await ctx.CompileManager.promises.compile(
+          ctx.project_id,
+          ctx.user_id,
+          { rootResourcePath: 'main.tex' }
+        )
+        ctx.ClsiManager.promises.sendRequest.should.have.been.called
+        status.should.equal('mock-status')
+
+        ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid.should.not
+          .have.been.called
+      })
+    })
+
+    describe('without a root doc', function () {
+      it('should return early', async function (ctx) {
+        ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid.resolves()
+        const { status, validationProblems } =
+          await ctx.CompileManager.promises.compile(
+            ctx.project_id,
+            ctx.user_id,
+            {}
+          )
+        status.should.equal('validation-problems')
+        validationProblems.should.deep.equal({
+          mainFile: 'no main file specified',
+        })
+      })
+    })
+
     describe('when the project has been recently compiled', function () {
       it('should return', async function (ctx) {
         ctx.CompileManager._checkIfAutoCompileLimitHasBeenHit = async (

services/web/test/unit/src/Exports/ExportsHandler.test.mjs

@@ -27,12 +27,6 @@ describe('ExportsHandler', function () {
 
     vi.doMock('@overleaf/fetch-utils', () => ctx.fetchUtils)
 
-    vi.doMock('../../../../app/src/Features/Project/ProjectLocator', () => ({
-      default: (ctx.ProjectLocator = {
-        promises: {},
-      }),
-    }))
-
     vi.doMock(
       '../../../../app/src/Features/Project/ProjectRootDocManager',
       () => ({
@@ -174,12 +168,9 @@ describe('ExportsHandler', function () {
       ctx.ProjectHistoryHandler.promises.ensureHistoryExistsForProject = sinon
         .stub()
         .resolves()
-      ctx.ProjectLocator.promises.findRootDoc = sinon
-        .stub()
-        .resolves({ element: null, path: { fileSystem: 'main.tex' } })
       ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid = sinon
         .stub()
-        .resolves()
+        .resolves({ rootResourcePath: 'main.tex' })
       ctx.UserGetter.promises.getUser = sinon.stub().resolves(ctx.user)
       ctx.ExportsHandler._requestVersion = sinon
         .stub()
@@ -305,9 +296,9 @@ describe('ExportsHandler', function () {
         let exportData
         beforeEach(async function (ctx) {
           ctx.project.rootDoc_id = null
-          ctx.ProjectLocator.promises.findRootDoc = sinon
+          ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid = sinon
             .stub()
-            .resolves({ path: { fileSystem: 'other.tex' } })
+            .resolves({ rootResourcePath: 'other.tex' })
           exportData = await ctx.ExportsHandler._buildExport(ctx.export_params)
         })
 
@@ -361,9 +352,9 @@ describe('ExportsHandler', function () {
         beforeEach(async function (ctx) {
           ctx.fakeDoc_id = '1a2b3c4d5e6f'
           ctx.project.rootDoc_id = ctx.fakeDoc_id
-          ctx.ProjectLocator.promises.findRootDoc = sinon
+          ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid = sinon
             .stub()
-            .resolves({ path: { fileSystem: 'other.tex' } })
+            .resolves({ rootResourcePath: 'other.tex' })
           exportData = await ctx.ExportsHandler._buildExport(ctx.export_params)
         })
 
@@ -412,9 +403,9 @@ describe('ExportsHandler', function () {
 
       describe('when no root doc can be identified', function () {
         beforeEach(function (ctx) {
-          ctx.ProjectLocator.promises.findRootDoc = sinon
+          ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid = sinon
             .stub()
-            .resolves({ element: null, path: null })
+            .resolves()
         })
 
         it('should return an error', async function (ctx) {

services/web/test/unit/src/Project/ProjectRootDocManager.test.mjs

@@ -517,72 +517,6 @@ describe('ProjectRootDocManager', function () {
     })
   })
 
-  describe('ensureRootDocumentIsSet', function () {
-    beforeEach(function (ctx) {
-      ctx.project = {}
-      ctx.ProjectGetter.promises.getProject = sinon.stub().resolves(ctx.project)
-      ctx.ProjectRootDocManager.promises.setRootDocAutomatically = sinon
-        .stub()
-        .resolves()
-    })
-
-    describe('when the root doc is set', function () {
-      beforeEach(async function (ctx) {
-        ctx.project.rootDoc_id = ctx.docId2
-
-        await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsSet(
-          ctx.project_id
-        )
-      })
-
-      it('should find the project fetching only the rootDoc_id field', function (ctx) {
-        ctx.ProjectGetter.promises.getProject
-          .calledWith(ctx.project_id, { rootDoc_id: 1 })
-          .should.equal(true)
-      })
-
-      it('should not try to update the project rootDoc_id', function (ctx) {
-        ctx.ProjectRootDocManager.promises.setRootDocAutomatically.called.should.equal(
-          false
-        )
-      })
-    })
-
-    describe('when the root doc is not set', function () {
-      beforeEach(async function (ctx) {
-        await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsSet(
-          ctx.project_id
-        )
-      })
-
-      it('should find the project with only the rootDoc_id field', function (ctx) {
-        ctx.ProjectGetter.promises.getProject
-          .calledWith(ctx.project_id, { rootDoc_id: 1 })
-          .should.equal(true)
-      })
-
-      it('should update the project rootDoc_id', function (ctx) {
-        ctx.ProjectRootDocManager.promises.setRootDocAutomatically
-          .calledWith(ctx.project_id)
-          .should.equal(true)
-      })
-    })
-
-    describe('when the project does not exist', function () {
-      beforeEach(function (ctx) {
-        ctx.ProjectGetter.promises.getProject = sinon.stub().resolves(null)
-      })
-
-      it('should reject', async function (ctx) {
-        await expect(
-          ctx.ProjectRootDocManager.promises.ensureRootDocumentIsSet(
-            ctx.project_id
-          )
-        ).to.be.rejectedWith('project not found')
-      })
-    })
-  })
-
   describe('ensureRootDocumentIsValid', function () {
     beforeEach(function (ctx) {
       ctx.project = {}
@@ -601,14 +535,23 @@ describe('ProjectRootDocManager', function () {
 
     describe('when the root doc is set', function () {
       describe('when the root doc is valid', function () {
+        let result
         beforeEach(async function (ctx) {
           ctx.project.rootDoc_id = ctx.docId2
           ctx.ProjectEntityHandler.promises.getDocPathFromProjectByDocId = sinon
             .stub()
             .resolves(ctx.docPaths[ctx.docId2])
-          await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid(
-            ctx.project_id
-          )
+          result =
+            await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid(
+              ctx.project_id
+            )
+        })
+
+        it('should return the path and id', ctx => {
+          result.should.deep.equal({
+            rootDocId: ctx.docId2,
+            rootResourcePath: ctx.docPaths[ctx.docId2],
+          })
         })
 
         it('should find the project without doc lines', function (ctx) {
@@ -625,14 +568,27 @@ describe('ProjectRootDocManager', function () {
       })
 
       describe('when the root doc is not valid', function () {
+        let result
         beforeEach(async function (ctx) {
           ctx.project.rootDoc_id = new ObjectId()
           ctx.ProjectEntityHandler.promises.getDocPathFromProjectByDocId = sinon
             .stub()
             .resolves(null)
-          await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid(
-            ctx.project_id
+          ctx.automaticRootDoc = {
+            rootDocId: ctx.docId4,
+            rootResourcePath: ctx.docPaths[ctx.docId4],
+          }
+          ctx.ProjectRootDocManager.promises.setRootDocAutomatically.resolves(
+            ctx.automaticRootDoc
           )
+          result =
+            await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid(
+              ctx.project_id
+            )
+        })
+
+        it('should return the path and id', ctx => {
+          result.should.deep.equal(ctx.automaticRootDoc)
         })
 
         it('should find the project without doc lines', function (ctx) {
@@ -656,10 +612,23 @@ describe('ProjectRootDocManager', function () {
     })
 
     describe('when the root doc is not set', function () {
+      let result
       beforeEach(async function (ctx) {
-        await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid(
-          ctx.project_id
+        ctx.automaticRootDoc = {
+          rootDocId: ctx.docId4,
+          rootResourcePath: ctx.docPaths[ctx.docId4],
+        }
+        ctx.ProjectRootDocManager.promises.setRootDocAutomatically.resolves(
+          ctx.automaticRootDoc
         )
+        result =
+          await ctx.ProjectRootDocManager.promises.ensureRootDocumentIsValid(
+            ctx.project_id
+          )
+      })
+
+      it('should return the path and id', ctx => {
+        result.should.deep.equal(ctx.automaticRootDoc)
       })
 
       it('should find the project without doc lines', async function (ctx) {