Merge pull request #30539 from overleaf/mfb-promisify-cooldown-manager

promisify cooldown manager

GitOrigin-RevId: 88c8c7d7e306042524ecd400fb15527206472cb1

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

services/web/app/src/Features/Cooldown/CooldownMiddleware.mjs

@@ -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()
-      }
-    )
-  },
-}

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

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

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

services/web/test/unit/src/Cooldown/CooldownMiddleware.test.mjs

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

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