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

Promisify ClsiFormatChecker and tests

GitOrigin-RevId: 89196708b97ff936660cc50a0b31f549d9723c4b

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

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

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

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'