diff --git a/services/web/app/src/Features/Cooldown/CooldownManager.mjs b/services/web/app/src/Features/Cooldown/CooldownManager.mjs index 0d7eb28f29..7a414fe542 100644 --- a/services/web/app/src/Features/Cooldown/CooldownManager.mjs +++ b/services/web/app/src/Features/Cooldown/CooldownManager.mjs @@ -1,6 +1,5 @@ import RedisWrapper from '../../infrastructure/RedisWrapper.mjs' import logger from '@overleaf/logger' -import { promisify } from '@overleaf/promise-utils' const rclient = RedisWrapper.client('cooldown') const COOLDOWN_IN_SECONDS = 60 * 10 @@ -10,39 +9,23 @@ const CooldownManager = { return `Cooldown:{${projectId}}` }, - putProjectOnCooldown(projectId, callback) { - if (callback == null) { - callback = function () {} - } + async putProjectOnCooldown(projectId) { logger.debug( { projectId }, `[Cooldown] putting project on cooldown for ${COOLDOWN_IN_SECONDS} seconds` ) - rclient.set( + await rclient.set( CooldownManager._buildKey(projectId), '1', 'EX', - COOLDOWN_IN_SECONDS, - callback + COOLDOWN_IN_SECONDS ) }, - isProjectOnCooldown(projectId, callback) { - if (callback == null) { - callback = function () {} - } - rclient.get(CooldownManager._buildKey(projectId), function (err, result) { - if (err != null) { - return callback(err) - } - return callback(null, result === '1') - }) + async isProjectOnCooldown(projectId) { + const result = await rclient.get(CooldownManager._buildKey(projectId)) + return result === '1' }, } -CooldownManager.promises = { - putProjectOnCooldown: promisify(CooldownManager.putProjectOnCooldown), - isProjectOnCooldown: promisify(CooldownManager.isProjectOnCooldown), -} - export default CooldownManager diff --git a/services/web/app/src/Features/Cooldown/CooldownMiddleware.mjs b/services/web/app/src/Features/Cooldown/CooldownMiddleware.mjs deleted file mode 100644 index d2d1cb01e3..0000000000 --- a/services/web/app/src/Features/Cooldown/CooldownMiddleware.mjs +++ /dev/null @@ -1,41 +0,0 @@ -/* eslint-disable - max-len, - 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 - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -import CooldownManager from './CooldownManager.mjs' -import logger from '@overleaf/logger' - -let CooldownMiddleware - -export default CooldownMiddleware = { - freezeProject(req, res, next) { - const projectId = req.params.Project_id - if (projectId == null) { - return next(new Error('[Cooldown] No projectId parameter on route')) - } - return CooldownManager.isProjectOnCooldown( - projectId, - function (err, projectIsOnCooldown) { - if (err != null) { - return next(err) - } - if (projectIsOnCooldown) { - logger.debug( - { projectId }, - '[Cooldown] project is on cooldown, denying request' - ) - return res.sendStatus(429) - } - return next() - } - ) - }, -} diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.mjs b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.mjs index d526fa5202..50d8d8f32d 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.mjs +++ b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.mjs @@ -554,7 +554,7 @@ async function _putElement(project, folderId, element, type, userId) { { projectId: project._id }, 'project too big, stopping insertions' ) - CooldownManager.putProjectOnCooldown(project._id) + await CooldownManager.putProjectOnCooldown(project._id) throw new Error('project_has_too_many_files') } diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.mjs b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.mjs index 57e1a7e0fc..6de01be624 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.mjs +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.mjs @@ -25,8 +25,9 @@ async function newUpdate( return null } - const projectIsOnCooldown = - await CooldownManager.promises.isProjectOnCooldown(project._id) + const projectIsOnCooldown = await CooldownManager.isProjectOnCooldown( + project._id + ) if (projectIsOnCooldown) { throw new Errors.TooManyRequestsError('project on cooldown') } @@ -173,8 +174,9 @@ async function createFolder(userId, projectId, projectName, path) { return null } - const projectIsOnCooldown = - await CooldownManager.promises.isProjectOnCooldown(project._id) + const projectIsOnCooldown = await CooldownManager.isProjectOnCooldown( + project._id + ) if (projectIsOnCooldown) { throw new Errors.TooManyRequestsError('project on cooldown') } diff --git a/services/web/test/unit/src/Cooldown/CooldownManager.sequential.test.mjs b/services/web/test/unit/src/Cooldown/CooldownManager.sequential.test.mjs index 5c6640c8f4..3dc0c08428 100644 --- a/services/web/test/unit/src/Cooldown/CooldownManager.sequential.test.mjs +++ b/services/web/test/unit/src/Cooldown/CooldownManager.sequential.test.mjs @@ -22,35 +22,27 @@ describe('CooldownManager', function () { describe('isProjectOnCooldown', function () { describe('when no project is on cooldown', function () { it('returns false for project 1', async function (ctx) { - const result = await CooldownManager.promises.isProjectOnCooldown( - ctx.project1Id - ) + const result = await CooldownManager.isProjectOnCooldown(ctx.project1Id) expect(result).to.be.false }) it('returns false for project 2', async function (ctx) { - const result = await CooldownManager.promises.isProjectOnCooldown( - ctx.project2Id - ) + const result = await CooldownManager.isProjectOnCooldown(ctx.project2Id) expect(result).to.be.false }) }) describe('when project 1 is on cooldown', function () { beforeEach(async function (ctx) { - await CooldownManager.promises.putProjectOnCooldown(ctx.project1Id) + await CooldownManager.putProjectOnCooldown(ctx.project1Id) }) it('returns true for project 1', async function (ctx) { - const result = await CooldownManager.promises.isProjectOnCooldown( - ctx.project1Id - ) + const result = await CooldownManager.isProjectOnCooldown(ctx.project1Id) expect(result).to.be.true }) it('returns false for project 2', async function (ctx) { - const result = await CooldownManager.promises.isProjectOnCooldown( - ctx.project2Id - ) + const result = await CooldownManager.isProjectOnCooldown(ctx.project2Id) expect(result).to.be.false }) }) diff --git a/services/web/test/unit/src/Cooldown/CooldownMiddleware.test.mjs b/services/web/test/unit/src/Cooldown/CooldownMiddleware.test.mjs deleted file mode 100644 index f7294c3b08..0000000000 --- a/services/web/test/unit/src/Cooldown/CooldownMiddleware.test.mjs +++ /dev/null @@ -1,125 +0,0 @@ -import { expect, vi } from 'vitest' -import sinon from 'sinon' -const modulePath = new URL( - '../../../../app/src/Features/Cooldown/CooldownMiddleware.mjs', - import.meta.url -).pathname - -describe('CooldownMiddleware', function () { - beforeEach(async function (ctx) { - ctx.CooldownManager = { isProjectOnCooldown: sinon.stub() } - - vi.doMock( - '../../../../app/src/Features/Cooldown/CooldownManager.mjs', - () => ({ - default: ctx.CooldownManager, - }) - ) - - ctx.CooldownMiddleware = (await import(modulePath)).default - }) - - describe('freezeProject', function () { - describe('when project is on cooldown', function () { - beforeEach(function (ctx) { - ctx.CooldownManager.isProjectOnCooldown = sinon - .stub() - .callsArgWith(1, null, true) - ctx.req = { params: { Project_id: 'abc' } } - ctx.res = { sendStatus: sinon.stub() } - return (ctx.next = sinon.stub()) - }) - - it('should call CooldownManager.isProjectOnCooldown', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - ctx.CooldownManager.isProjectOnCooldown.callCount.should.equal(1) - return ctx.CooldownManager.isProjectOnCooldown - .calledWith('abc') - .should.equal(true) - }) - - it('should not produce an error', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - return ctx.next.callCount.should.equal(0) - }) - - it('should send a 429 status', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - ctx.res.sendStatus.callCount.should.equal(1) - return ctx.res.sendStatus.calledWith(429).should.equal(true) - }) - }) - - describe('when project is not on cooldown', function () { - beforeEach(function (ctx) { - ctx.CooldownManager.isProjectOnCooldown = sinon - .stub() - .callsArgWith(1, null, false) - ctx.req = { params: { Project_id: 'abc' } } - ctx.res = { sendStatus: sinon.stub() } - return (ctx.next = sinon.stub()) - }) - - it('should call CooldownManager.isProjectOnCooldown', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - ctx.CooldownManager.isProjectOnCooldown.callCount.should.equal(1) - return ctx.CooldownManager.isProjectOnCooldown - .calledWith('abc') - .should.equal(true) - }) - - it('call next with no arguments', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - ctx.next.callCount.should.equal(1) - return expect(ctx.next.lastCall.args.length).to.equal(0) - }) - }) - - describe('when isProjectOnCooldown produces an error', function () { - beforeEach(function (ctx) { - ctx.CooldownManager.isProjectOnCooldown = sinon - .stub() - .callsArgWith(1, new Error('woops')) - ctx.req = { params: { Project_id: 'abc' } } - ctx.res = { sendStatus: sinon.stub() } - return (ctx.next = sinon.stub()) - }) - - it('should call CooldownManager.isProjectOnCooldown', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - ctx.CooldownManager.isProjectOnCooldown.callCount.should.equal(1) - return ctx.CooldownManager.isProjectOnCooldown - .calledWith('abc') - .should.equal(true) - }) - - it('call next with an error', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - ctx.next.callCount.should.equal(1) - return expect(ctx.next.lastCall.args[0]).to.be.instanceof(Error) - }) - }) - - describe('when projectId is not part of route', function () { - beforeEach(function (ctx) { - ctx.CooldownManager.isProjectOnCooldown = sinon - .stub() - .callsArgWith(1, null, true) - ctx.req = { params: { lol: 'abc' } } - ctx.res = { sendStatus: sinon.stub() } - return (ctx.next = sinon.stub()) - }) - - it('call next with an error', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - ctx.next.callCount.should.equal(1) - return expect(ctx.next.lastCall.args[0]).to.be.instanceof(Error) - }) - - it('should not call CooldownManager.isProjectOnCooldown', function (ctx) { - ctx.CooldownMiddleware.freezeProject(ctx.req, ctx.res, ctx.next) - return ctx.CooldownManager.isProjectOnCooldown.callCount.should.equal(0) - }) - }) - }) -}) diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandler.test.mjs b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandler.test.mjs index eb0a79b4e7..9330afb7cb 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandler.test.mjs +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandler.test.mjs @@ -45,9 +45,7 @@ describe('TpdsUpdateHandler', function () { } ctx.CooldownManager = { - promises: { - isProjectOnCooldown: sinon.stub().resolves(false), - }, + isProjectOnCooldown: sinon.stub().resolves(false), } ctx.FileTypeManager = { shouldIgnore: sinon.stub().returns(false), @@ -487,7 +485,7 @@ function setupMatchingProjects(projectKeys) { function setupProjectOnCooldown() { beforeEach(function (ctx) { - ctx.CooldownManager.promises.isProjectOnCooldown + ctx.CooldownManager.isProjectOnCooldown .withArgs(ctx.projects.active1._id) .resolves(true) })