Merge pull request #28895 from overleaf/as-promisify-clsiformatchecker

Promisify ClsiFormatChecker and tests GitOrigin-RevId: 89196708b97ff936660cc50a0b31f549d9723c4b
2026-05-27 02:51:57 +02:00 · 2025-10-16 12:16:27 +01:00
parent 89898b0e9a
commit bafb7ec56d
4 changed files with 48 additions and 108 deletions
--- a/services/web/app/src/Features/Compile/ClsiFormatChecker.js
+++ b/services/web/app/src/Features/Compile/ClsiFormatChecker.js
@@ -1,48 +1,20 @@
-/* eslint-disable
-    max-len,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let ClsiFormatChecker
 const _ = require('lodash')
-const async = require('async')
 const settings = require('@overleaf/settings')
-const { promisifyAll } = require('@overleaf/promise-utils')

-module.exports = ClsiFormatChecker = {
-  checkRecoursesForProblems(resources, callback) {
-    const jobs = {
-      conflictedPaths(cb) {
-        return ClsiFormatChecker._checkForConflictingPaths(resources, cb)
-      },
-
-      sizeCheck(cb) {
-        return ClsiFormatChecker._checkDocsAreUnderSizeLimit(resources, cb)
-      },
+const ClsiFormatChecker = {
+  checkRecoursesForProblems(resources) {
+    let problems = {
+      conflictedPaths: ClsiFormatChecker._checkForConflictingPaths(resources),
+      sizeCheck: ClsiFormatChecker._checkDocsAreUnderSizeLimit(resources),
    }

-    return async.series(jobs, function (err, problems) {
-      if (err != null) {
-        return callback(err)
-      }
-
-      problems = _.omitBy(problems, _.isEmpty)
-
-      if (_.isEmpty(problems)) {
-        return callback()
-      } else {
-        return callback(null, problems)
-      }
-    })
+    problems = _.omitBy(problems, _.isEmpty)
+    if (!_.isEmpty(problems)) {
+      return problems
+    }
  },

-  _checkForConflictingPaths(resources, callback) {
+  _checkForConflictingPaths(resources) {
    const paths = resources.map(resource => resource.path)

    const conflicts = _.filter(paths, function (path) {
@@ -56,10 +28,10 @@ module.exports = ClsiFormatChecker = {

    const conflictObjects = conflicts.map(conflict => ({ path: conflict }))

-    return callback(null, conflictObjects)
+    return conflictObjects
  },

-  _checkDocsAreUnderSizeLimit(resources, callback) {
+  _checkDocsAreUnderSizeLimit(resources) {
    const sizeLimit = 1000 * 1000 * settings.compileBodySizeLimitMb

    let totalSize = 0
@@ -77,13 +49,11 @@ module.exports = ClsiFormatChecker = {
    })

    const tooLarge = totalSize > sizeLimit
-    if (!tooLarge) {
-      return callback()
-    } else {
+    if (tooLarge) {
      sizedResources = _.sortBy(sizedResources, 'size').reverse().slice(0, 10)
-      return callback(null, { resources: sizedResources, totalSize })
+      return { resources: sizedResources, totalSize }
    }
  },
 }

-module.exports.promises = promisifyAll(module.exports)
+module.exports = ClsiFormatChecker
--- a/services/web/app/src/Features/Compile/ClsiManager.mjs
+++ b/services/web/app/src/Features/Compile/ClsiManager.mjs
@@ -185,10 +185,9 @@ async function _sendBuiltRequest(projectId, userId, req, options) {
  if (options.forceNewClsiServer) {
    await clearClsiServerId(projectId, userId)
  }
-  const validationProblems =
-    await ClsiFormatChecker.promises.checkRecoursesForProblems(
-      req.compile?.resources
-    )
+  const validationProblems = ClsiFormatChecker.checkRecoursesForProblems(
+    req.compile?.resources
+  )
  if (validationProblems != null) {
    logger.debug(
      { projectId, validationProblems },
--- a/services/web/test/unit/src/Compile/ClsiFormatCheckerTests.js
+++ b/services/web/test/unit/src/Compile/ClsiFormatCheckerTests.js
@@ -1,16 +1,3 @@
-/* eslint-disable
-    n/handle-callback-err,
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const sinon = require('sinon')
 const { expect } = require('chai')
 const modulePath = '../../../../app/src/Features/Compile/ClsiFormatChecker.js'
@@ -50,14 +37,11 @@ describe('ClsiFormatChecker', function () {
    it('should call _checkDocsAreUnderSizeLimit and _checkForConflictingPaths', async function () {
      this.ClsiFormatChecker._checkForConflictingPaths = sinon
        .stub()
-        .callsArgWith(1, null)
+        .returns(null)
      this.ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon
        .stub()
-        .callsArgWith(1)
-      const problems =
-        await this.ClsiFormatChecker.promises.checkRecoursesForProblems(
-          this.resources
-        )
+        .returns(null)
+      this.ClsiFormatChecker.checkRecoursesForProblems(this.resources)
      this.ClsiFormatChecker._checkForConflictingPaths.called.should.equal(true)
      this.ClsiFormatChecker._checkDocsAreUnderSizeLimit.called.should.equal(
        true
@@ -67,28 +51,26 @@ describe('ClsiFormatChecker', function () {
    it('should remove undefined errors', async function () {
      this.ClsiFormatChecker._checkForConflictingPaths = sinon
        .stub()
-        .callsArgWith(1, null, [])
+        .returns([])
      this.ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon
        .stub()
-        .callsArgWith(1, null, {})
-      const problems =
-        await this.ClsiFormatChecker.promises.checkRecoursesForProblems(
-          this.resources
-        )
+        .returns({})
+      const problems = this.ClsiFormatChecker.checkRecoursesForProblems(
+        this.resources
+      )
      expect(problems).to.not.exist
    })

    it('should keep populated arrays', async function () {
      this.ClsiFormatChecker._checkForConflictingPaths = sinon
        .stub()
-        .callsArgWith(1, null, [{ path: 'somewhere/main.tex' }])
+        .returns([{ path: 'somewhere/main.tex' }])
      this.ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon
        .stub()
-        .callsArgWith(1, null, {})
-      const problems =
-        await this.ClsiFormatChecker.promises.checkRecoursesForProblems(
-          this.resources
-        )
+        .returns({})
+      const problems = this.ClsiFormatChecker.checkRecoursesForProblems(
+        this.resources
+      )
      problems.conflictedPaths[0].path.should.equal('somewhere/main.tex')
      expect(problems.sizeCheck).to.not.exist
    })
@@ -96,17 +78,16 @@ describe('ClsiFormatChecker', function () {
    it('should keep populated object', async function () {
      this.ClsiFormatChecker._checkForConflictingPaths = sinon
        .stub()
-        .callsArgWith(1, null, [])
+        .returns([])
      this.ClsiFormatChecker._checkDocsAreUnderSizeLimit = sinon
        .stub()
-        .callsArgWith(1, null, {
+        .returns({
          resources: [{ 'a.tex': 'a.tex' }, { 'b.tex': 'b.tex' }],
          totalSize: 1000000,
        })
-      const problems =
-        await this.ClsiFormatChecker.promises.checkRecoursesForProblems(
-          this.resources
-        )
+      const problems = this.ClsiFormatChecker.checkRecoursesForProblems(
+        this.resources
+      )
      problems.sizeCheck.resources.length.should.equal(2)
      problems.sizeCheck.totalSize.should.equal(1000000)
      expect(problems.conflictedPaths).to.not.exist
@@ -132,9 +113,7 @@ describe('ClsiFormatChecker', function () {
        })

        const conflictPathErrors =
-          await this.ClsiFormatChecker.promises._checkForConflictingPaths(
-            this.resources
-          )
+          this.ClsiFormatChecker._checkForConflictingPaths(this.resources)
        conflictPathErrors.length.should.equal(1)
        conflictPathErrors[0].path.should.equal('stuff/image')
      })
@@ -146,9 +125,7 @@ describe('ClsiFormatChecker', function () {
        })

        const conflictPathErrors =
-          await this.ClsiFormatChecker.promises._checkForConflictingPaths(
-            this.resources
-          )
+          this.ClsiFormatChecker._checkForConflictingPaths(this.resources)
        conflictPathErrors.length.should.equal(1)
        conflictPathErrors[0].path.should.equal('stuff')
      })
@@ -160,9 +137,7 @@ describe('ClsiFormatChecker', function () {
        })

        const conflictPathErrors =
-          await this.ClsiFormatChecker.promises._checkForConflictingPaths(
-            this.resources
-          )
+          this.ClsiFormatChecker._checkForConflictingPaths(this.resources)
        conflictPathErrors.length.should.equal(0)
      })
    })
@@ -181,10 +156,9 @@ describe('ClsiFormatChecker', function () {
          })
        }

-        const sizeError =
-          await this.ClsiFormatChecker.promises._checkDocsAreUnderSizeLimit(
-            this.resources
-          )
+        const sizeError = this.ClsiFormatChecker._checkDocsAreUnderSizeLimit(
+          this.resources
+        )
        sizeError.totalSize.should.equal(16 + 833333 * 11) // 16 is for earlier resources
        sizeError.resources.length.should.equal(10)
        sizeError.resources[0].path.should.equal('massive.tex')
@@ -204,10 +178,9 @@ describe('ClsiFormatChecker', function () {
          })
        }

-        const sizeError =
-          await this.ClsiFormatChecker.promises._checkDocsAreUnderSizeLimit(
-            this.resources
-          )
+        const sizeError = this.ClsiFormatChecker._checkDocsAreUnderSizeLimit(
+          this.resources
+        )
        expect(sizeError).to.not.exist
      })
    })
--- a/services/web/test/unit/src/Compile/ClsiManager.test.mjs
+++ b/services/web/test/unit/src/Compile/ClsiManager.test.mjs
@@ -108,9 +108,7 @@ describe('ClsiManager', function () {
      computeHash: sinon.stub().returns('01234567890abcdef'),
    }
    ctx.ClsiFormatChecker = {
-      promises: {
-        checkRecoursesForProblems: sinon.stub().resolves(),
-      },
+      checkRecoursesForProblems: sinon.stub().returns(),
    }
    ctx.Project = {}
    ctx.ProjectEntityHandler = {
@@ -835,7 +833,7 @@ describe('ClsiManager', function () {

    describe('when the resources fail the precompile check', function () {
      beforeEach(function (ctx) {
-        ctx.ClsiFormatChecker.promises.checkRecoursesForProblems.rejects(
+        ctx.ClsiFormatChecker.checkRecoursesForProblems.throws(
          new Error('failed')
        )
      })
@@ -983,7 +981,7 @@ describe('ClsiManager', function () {

    describe('when the resources fail the precompile check', function () {
      beforeEach(async function (ctx) {
-        ctx.ClsiFormatChecker.promises.checkRecoursesForProblems.rejects(
+        ctx.ClsiFormatChecker.checkRecoursesForProblems.throws(
          new Error('failed')
        )
        ctx.responseBody.compile.status = 'failure'