Jelajahi Sumber

[web] generate clsi buildId ahead of fetching project content (#24337)

* [web] generate clsi buildId ahead of fetching project content

The buildIds timestamp component will be used for cache invalidation.

* [clsi] strict validation for buildId

* [clsi] validate buildId parameter

GitOrigin-RevId: 88e8b2d48e78fa137b6dca7f2e6b93bbcf88a777
Jakob Ackermann 1 tahun lalu
induk
melakukan
13bf214a3c

+ 8 - 5
services/clsi/app/js/OutputCacheManager.js

@@ -98,10 +98,9 @@ module.exports = OutputCacheManager = {
   CONTENT_SUBDIR: 'content',
   CACHE_SUBDIR: 'generated-files',
   ARCHIVE_SUBDIR: 'archived-logs',
-  // build id is HEXDATE-HEXRANDOM from Date.now()and RandomBytes
-  // for backwards compatibility, make the randombytes part optional
-  BUILD_REGEX: /^[0-9a-f]+(-[0-9a-f]+)?$/,
-  CONTENT_REGEX: /^[0-9a-f]+(-[0-9a-f]+)?$/,
+  // build id is HEXDATE-HEXRANDOM from Date.now() and RandomBytes
+  BUILD_REGEX: /^[0-9a-f]+-[0-9a-f]+$/,
+  CONTENT_REGEX: /^[0-9a-f]+-[0-9a-f]+$/,
   CACHE_LIMIT: 2, // maximum number of cache directories
   CACHE_AGE: 90 * 60 * 1000, // up to 90 minutes old
 
@@ -137,7 +136,11 @@ module.exports = OutputCacheManager = {
     outputDir,
     callback
   ) {
-    OutputCacheManager.generateBuildId(function (err, buildId) {
+    const getBuildId = cb => {
+      if (request.buildId) return cb(null, request.buildId)
+      OutputCacheManager.generateBuildId(cb)
+    }
+    getBuildId(function (err, buildId) {
       if (err) {
         return callback(err)
       }

+ 13 - 0
services/clsi/app/js/RequestParser.js

@@ -1,4 +1,5 @@
 const settings = require('@overleaf/settings')
+const OutputCacheManager = require('./OutputCacheManager')
 
 const VALID_COMPILERS = ['pdflatex', 'latex', 'xelatex', 'lualatex']
 const MAX_TIMEOUT = 600
@@ -135,6 +136,11 @@ function parse(body, callback) {
       }
     )
     response.rootResourcePath = _checkPath(rootResourcePath)
+
+    response.buildId = _parseAttribute('buildId', compile.options.buildId, {
+      type: 'string',
+      regex: OutputCacheManager.BUILD_REGEX,
+    })
   } catch (error1) {
     const error = error1
     return callback(error)
@@ -199,6 +205,13 @@ function _parseAttribute(name, attribute, options) {
         throw new Error(`${name} attribute should be a ${options.type}`)
       }
     }
+    if (options.type === 'string' && options.regex instanceof RegExp) {
+      if (!options.regex.test(attribute)) {
+        throw new Error(
+          `${name} attribute does not match regex ${options.regex}`
+        )
+      }
+    }
   } else {
     if (options.default != null) {
       return options.default

+ 1 - 1
services/clsi/test/unit/js/DockerLockManagerTests.js

@@ -16,7 +16,7 @@ const modulePath = require('node:path').join(
   '../../../app/js/DockerLockManager'
 )
 
-describe('LockManager', function () {
+describe('DockerLockManager', function () {
   beforeEach(function () {
     return (this.LockManager = SandboxedModule.require(modulePath, {
       requires: {

+ 1 - 0
services/clsi/test/unit/js/LockManagerTests.js

@@ -21,6 +21,7 @@ describe('LockManager', function () {
           compileConcurrencyLimit: 5,
         }),
         './Errors': (this.Erros = Errors),
+        './RequestParser': { MAX_TIMEOUT: 600 },
       },
     })
   })

+ 32 - 0
services/clsi/test/unit/js/RequestParserTests.js

@@ -30,6 +30,7 @@ describe('RequestParser', function () {
     this.RequestParser = SandboxedModule.require(modulePath, {
       requires: {
         '@overleaf/settings': (this.settings = {}),
+        './OutputCacheManager': { BUILD_REGEX: /^[0-9a-f]+-[0-9a-f]+$/ },
       },
     })
   })
@@ -274,6 +275,37 @@ describe('RequestParser', function () {
     })
   })
 
+  describe('with a valid buildId', function () {
+    beforeEach(function (done) {
+      this.validRequest.compile.options.buildId = '195a4869176-a4ad60bee7bf35e4'
+      this.RequestParser.parse(this.validRequest, (error, data) => {
+        if (error) return done(error)
+        this.data = data
+        done()
+      })
+    })
+
+    it('should return an error', function () {
+      this.data.buildId.should.equal('195a4869176-a4ad60bee7bf35e4')
+    })
+  })
+
+  describe('with a bad buildId', function () {
+    beforeEach(function () {
+      this.validRequest.compile.options.buildId = 'foo/bar'
+      this.RequestParser.parse(this.validRequest, this.callback)
+    })
+
+    it('should return an error', function () {
+      this.callback
+        .calledWithMatch({
+          message:
+            'buildId attribute does not match regex /^[0-9a-f]+-[0-9a-f]+$/',
+        })
+        .should.equal(true)
+    })
+  })
+
   describe('with a resource with a valid date', function () {
     beforeEach(function () {
       this.date = '12:00 01/02/03'

+ 1 - 0
services/web/app/src/Features/Compile/ClsiManager.js

@@ -765,6 +765,7 @@ function _finaliseRequest(projectId, options, project, docs, files) {
   return {
     compile: {
       options: {
+        buildId: options.buildId,
         compiler: project.compiler,
         timeout: options.timeout,
         imageName: project.imageName,

+ 8 - 0
services/web/app/src/Features/Compile/CompileManager.js

@@ -1,4 +1,5 @@
 let CompileManager
+const Crypto = require('crypto')
 const Settings = require('@overleaf/settings')
 const RedisWrapper = require('../../infrastructure/RedisWrapper')
 const rclient = RedisWrapper.client('clsi_recently_compiled')
@@ -25,6 +26,10 @@ function instrumentWithTimer(fn, key) {
   }
 }
 
+function generateBuildId() {
+  return `${Date.now().toString(16)}-${Crypto.randomBytes(8).toString('hex')}`
+}
+
 async function compile(projectId, userId, options = {}) {
   const recentlyCompiled = await CompileManager._checkIfRecentlyCompiled(
     projectId,
@@ -67,6 +72,9 @@ 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 {

+ 1 - 0
services/web/test/unit/src/Compile/CompileManagerTests.js

@@ -122,6 +122,7 @@ describe('CompileManager', function () {
           .calledWith(this.project_id, this.user_id, {
             timeout: this.limits.timeout,
             compileGroup: 'standard',
+            buildId: sinon.match(/[a-f0-9]+-[a-f0-9]+/),
           })
           .should.equal(true)
       })