From bafb7ec56dcb97d5d48dc2ea31e0c000ba1816b2 Mon Sep 17 00:00:00 2001 From: MoxAmber Date: Thu, 16 Oct 2025 12:16:27 +0100 Subject: [PATCH] Merge pull request #28895 from overleaf/as-promisify-clsiformatchecker Promisify ClsiFormatChecker and tests GitOrigin-RevId: 89196708b97ff936660cc50a0b31f549d9723c4b --- .../src/Features/Compile/ClsiFormatChecker.js | 60 ++++---------- .../app/src/Features/Compile/ClsiManager.mjs | 7 +- .../src/Compile/ClsiFormatCheckerTests.js | 81 +++++++------------ .../unit/src/Compile/ClsiManager.test.mjs | 8 +- 4 files changed, 48 insertions(+), 108 deletions(-) diff --git a/services/web/app/src/Features/Compile/ClsiFormatChecker.js b/services/web/app/src/Features/Compile/ClsiFormatChecker.js index 1828f2f417..74b2d55725 100644 --- 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 diff --git a/services/web/app/src/Features/Compile/ClsiManager.mjs b/services/web/app/src/Features/Compile/ClsiManager.mjs index 49a994483c..ed4b98e688 100644 --- 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 }, diff --git a/services/web/test/unit/src/Compile/ClsiFormatCheckerTests.js b/services/web/test/unit/src/Compile/ClsiFormatCheckerTests.js index aa19732b9e..52eb029223 100644 --- 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 }) }) diff --git a/services/web/test/unit/src/Compile/ClsiManager.test.mjs b/services/web/test/unit/src/Compile/ClsiManager.test.mjs index c677f08bad..edf3903932 100644 --- 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'