Merge pull request #17065 from overleaf/dp-mongoose-callback-inactive-project-manager

Promisify InactiveProjectManager and InactiveProjectManagerTests

GitOrigin-RevId: 985d0d4c80bfd1e46fa3c85c98203432459bdc84

services/web/app/src/Features/InactiveData/InactiveProjectManager.js

@@ -1,17 +1,4 @@
-/* eslint-disable
-    max-len,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let InactiveProjectManager
 const OError = require('@overleaf/o-error')
-const async = require('async')
-const _ = require('lodash')
 const logger = require('@overleaf/logger')
 const DocstoreManager = require('../Docstore/DocstoreManager')
 const ProjectGetter = require('../Project/ProjectGetter')
@@ -19,43 +6,45 @@ const ProjectUpdateHandler = require('../Project/ProjectUpdateHandler')
 const { Project } = require('../../models/Project')
 const { ObjectId } = require('mongodb')
 const { READ_PREFERENCE_SECONDARY } = require('../../infrastructure/mongodb')
+const { callbackifyAll } = require('@overleaf/promise-utils')
 
 const MILISECONDS_IN_DAY = 86400000
-module.exports = InactiveProjectManager = {
-  reactivateProjectIfRequired(projectId, callback) {
-    ProjectGetter.getProject(
-      projectId,
-      { active: true },
-      function (err, project) {
-        if (err != null) {
-          OError.tag(err, 'error getting project', {
-            project_id: projectId,
-          })
-          return callback(err)
-        }
-        logger.debug(
-          { projectId, active: project.active },
-          'seeing if need to reactivate project'
-        )
-
-        if (project.active) {
-          return callback()
-        }
+const InactiveProjectManager = {
+  async reactivateProjectIfRequired(projectId) {
+    let project
+    try {
+      project = await ProjectGetter.promises.getProject(projectId, {
+        active: true,
+      })
+    } catch (err) {
+      OError.tag(err, 'error getting project', {
+        project_id: projectId,
+      })
+      throw err
+    }
 
-        DocstoreManager.unarchiveProject(projectId, function (err) {
-          if (err != null) {
-            OError.tag(err, 'error reactivating project in docstore', {
-              project_id: projectId,
-            })
-            return callback(err)
-          }
-          ProjectUpdateHandler.markAsActive(projectId, callback)
-        })
-      }
+    logger.debug(
+      { projectId, active: project.active },
+      'seeing if need to reactivate project'
     )
+
+    if (project.active) {
+      return
+    }
+
+    try {
+      await DocstoreManager.promises.unarchiveProject(projectId)
+    } catch (err) {
+      OError.tag(err, 'error reactivating project in docstore', {
+        project_id: projectId,
+      })
+      throw err
+    }
+
+    await ProjectUpdateHandler.promises.markAsActive(projectId)
   },
 
-  deactivateOldProjects(limit, daysOld, callback) {
+  async deactivateOldProjects(limit, daysOld) {
     if (limit == null) {
       limit = 10
     }
@@ -63,60 +52,59 @@ module.exports = InactiveProjectManager = {
       daysOld = 360
     }
     const oldProjectDate = new Date() - MILISECONDS_IN_DAY * daysOld
-    // use $not $gt to catch non-opened projects where lastOpened is null
-    Project.find({ lastOpened: { $not: { $gt: oldProjectDate } } })
-      .where('_id')
-      .lt(ObjectId.createFromTime(oldProjectDate / 1000))
-      .where('active')
-      .equals(true)
-      .select('_id')
-      .sort({ _id: 1 })
-      .limit(limit)
-      .read(READ_PREFERENCE_SECONDARY)
-      .exec(function (err, projects) {
-        if (err != null) {
-          logger.err({ err }, 'could not get projects for deactivating')
-        }
-        const jobs = _.map(
-          projects,
-          project => cb =>
-            InactiveProjectManager.deactivateProject(
-              project._id,
-              function (err) {
-                if (err) {
-                  logger.err(
-                    { projectId: project._id, err },
-                    'unable to deactivate project'
-                  )
-                }
-                cb()
-              }
-            )
-        )
-        logger.debug(
-          { numberOfProjects: projects && projects.length },
-          'deactivating projects'
-        )
-        async.series(jobs, function (err) {
-          if (err != null) {
-            logger.warn({ err }, 'error deactivating projects')
-          }
-          callback(err, projects)
-        })
+
+    let projects
+    try {
+      // use $not $gt to catch non-opened projects where lastOpened is null
+      projects = await Project.find({
+        lastOpened: { $not: { $gt: oldProjectDate } },
       })
+        .where('_id')
+        .lt(ObjectId.createFromTime(oldProjectDate / 1000))
+        .where('active')
+        .equals(true)
+        .select('_id')
+        .sort({ _id: 1 })
+        .limit(limit)
+        .read(READ_PREFERENCE_SECONDARY)
+        .exec()
+    } catch (err) {
+      logger.err({ err }, 'could not get projects for deactivating')
+    }
+
+    logger.debug(
+      { numberOfProjects: projects && projects.length },
+      'deactivating projects'
+    )
+
+    for (const project of projects) {
+      try {
+        await InactiveProjectManager.deactivateProject(project._id)
+      } catch (err) {
+        logger.err(
+          { projectId: project._id, err },
+          'unable to deactivate project'
+        )
+      }
+    }
+
+    return projects
   },
 
-  deactivateProject(projectId, callback) {
+  async deactivateProject(projectId) {
     logger.debug({ projectId }, 'deactivating inactive project')
-    const jobs = [
-      cb => DocstoreManager.archiveProject(projectId, cb),
-      cb => ProjectUpdateHandler.markAsInactive(projectId, cb),
-    ]
-    async.series(jobs, function (err) {
-      if (err != null) {
-        logger.warn({ err, projectId }, 'error deactivating project')
-      }
-      callback(err)
-    })
+
+    try {
+      await DocstoreManager.promises.archiveProject(projectId)
+      await ProjectUpdateHandler.promises.markAsInactive(projectId)
+    } catch (err) {
+      logger.warn({ err, projectId }, 'error deactivating project')
+      throw err
+    }
   },
 }
+
+module.exports = {
+  ...callbackifyAll(InactiveProjectManager),
+  promises: InactiveProjectManager,
+}

services/web/test/unit/src/InactiveData/InactiveProjectManagerTests.js

@@ -1,39 +1,29 @@
-/* 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 SandboxedModule = require('sandboxed-module')
-const assert = require('assert')
 const path = require('path')
 const sinon = require('sinon')
 const modulePath = path.join(
   __dirname,
   '../../../../app/src/Features/InactiveData/InactiveProjectManager'
 )
-const { expect } = require('chai')
 const { ObjectId, ReadPreference } = require('mongodb')
+const { expect } = require('chai')
 
 describe('InactiveProjectManager', function () {
   beforeEach(function () {
     this.settings = {}
     this.DocstoreManager = {
-      unarchiveProject: sinon.stub(),
-      archiveProject: sinon.stub(),
+      promises: {
+        unarchiveProject: sinon.stub(),
+        archiveProject: sinon.stub(),
+      },
     }
     this.ProjectUpdateHandler = {
-      markAsActive: sinon.stub(),
-      markAsInactive: sinon.stub(),
+      promises: {
+        markAsActive: sinon.stub(),
+        markAsInactive: sinon.stub(),
+      },
     }
-    this.ProjectGetter = { getProject: sinon.stub() }
+    this.ProjectGetter = { promises: { getProject: sinon.stub() } }
     this.InactiveProjectManager = SandboxedModule.require(modulePath, {
       requires: {
         mongodb: { ObjectId },
@@ -48,106 +38,90 @@ describe('InactiveProjectManager', function () {
         },
       },
     })
-    return (this.project_id = '1234')
+    this.project_id = '1234'
   })
 
   describe('reactivateProjectIfRequired', function () {
     beforeEach(function () {
       this.project = { active: false }
-      this.ProjectGetter.getProject.callsArgWith(2, null, this.project)
-      return this.ProjectUpdateHandler.markAsActive.callsArgWith(1)
+      this.ProjectGetter.promises.getProject.resolves(this.project)
+      this.ProjectUpdateHandler.promises.markAsActive.resolves()
     })
 
-    it('should call unarchiveProject', function (done) {
-      this.DocstoreManager.unarchiveProject.callsArgWith(1)
-      return this.InactiveProjectManager.reactivateProjectIfRequired(
-        this.project_id,
-        err => {
-          this.DocstoreManager.unarchiveProject
-            .calledWith(this.project_id)
-            .should.equal(true)
-          this.ProjectUpdateHandler.markAsActive
-            .calledWith(this.project_id)
-            .should.equal(true)
-          return done()
-        }
+    it('should call unarchiveProject', async function () {
+      this.DocstoreManager.promises.unarchiveProject.resolves()
+      await this.InactiveProjectManager.promises.reactivateProjectIfRequired(
+        this.project_id
       )
+
+      this.DocstoreManager.promises.unarchiveProject
+        .calledWith(this.project_id)
+        .should.equal(true)
+      this.ProjectUpdateHandler.promises.markAsActive
+        .calledWith(this.project_id)
+        .should.equal(true)
     })
 
-    it('should not mark project as active if error with unarchiving', function (done) {
-      const error = new Error('error')
-      this.DocstoreManager.unarchiveProject.callsArgWith(1, error)
-      return this.InactiveProjectManager.reactivateProjectIfRequired(
-        this.project_id,
-        err => {
-          err.should.equal(error)
-          this.DocstoreManager.unarchiveProject
-            .calledWith(this.project_id)
-            .should.equal(true)
-          this.ProjectUpdateHandler.markAsActive
-            .calledWith(this.project_id)
-            .should.equal(false)
-          return done()
-        }
-      )
+    it('should not mark project as active if error with unarchiving', async function () {
+      this.DocstoreManager.promises.unarchiveProject.rejects()
+      await expect(
+        this.InactiveProjectManager.promises.reactivateProjectIfRequired(
+          this.project_id
+        )
+      ).to.be.rejected
+
+      this.DocstoreManager.promises.unarchiveProject
+        .calledWith(this.project_id)
+        .should.equal(true)
+      this.ProjectUpdateHandler.promises.markAsActive
+        .calledWith(this.project_id)
+        .should.equal(false)
     })
 
-    it('should not call unarchiveProject if it is active', function (done) {
+    it('should not call unarchiveProject if it is active', async function () {
       this.project.active = true
-      this.DocstoreManager.unarchiveProject.callsArgWith(1)
-      return this.InactiveProjectManager.reactivateProjectIfRequired(
-        this.project_id,
-        err => {
-          this.DocstoreManager.unarchiveProject
-            .calledWith(this.project_id)
-            .should.equal(false)
-          this.ProjectUpdateHandler.markAsActive
-            .calledWith(this.project_id)
-            .should.equal(false)
-          return done()
-        }
+      this.DocstoreManager.promises.unarchiveProject.resolves()
+      await this.InactiveProjectManager.promises.reactivateProjectIfRequired(
+        this.project_id
       )
+      this.DocstoreManager.promises.unarchiveProject
+        .calledWith(this.project_id)
+        .should.equal(false)
+      this.ProjectUpdateHandler.promises.markAsActive
+        .calledWith(this.project_id)
+        .should.equal(false)
     })
   })
 
   describe('deactivateProject', function () {
-    it('should call unarchiveProject and markAsInactive', function (done) {
-      this.DocstoreManager.archiveProject.callsArgWith(1)
-
-      this.ProjectUpdateHandler.markAsInactive.callsArgWith(1)
+    it('should call unarchiveProject and markAsInactive', async function () {
+      this.DocstoreManager.promises.archiveProject.resolves()
+      this.ProjectUpdateHandler.promises.markAsInactive.resolves()
 
-      return this.InactiveProjectManager.deactivateProject(
-        this.project_id,
-        err => {
-          this.DocstoreManager.archiveProject
-            .calledWith(this.project_id)
-            .should.equal(true)
-          this.ProjectUpdateHandler.markAsInactive
-            .calledWith(this.project_id)
-            .should.equal(true)
-          return done()
-        }
+      await this.InactiveProjectManager.promises.deactivateProject(
+        this.project_id
       )
+      this.DocstoreManager.promises.archiveProject
+        .calledWith(this.project_id)
+        .should.equal(true)
+      this.ProjectUpdateHandler.promises.markAsInactive
+        .calledWith(this.project_id)
+        .should.equal(true)
     })
 
-    it('should not call markAsInactive if there was a problem archiving in docstore', function (done) {
-      this.DocstoreManager.archiveProject.callsArgWith(1, 'errorrr')
+    it('should not call markAsInactive if there was a problem archiving in docstore', async function () {
+      this.DocstoreManager.promises.archiveProject.rejects()
+      this.ProjectUpdateHandler.promises.markAsInactive.resolves()
 
-      this.ProjectUpdateHandler.markAsInactive.callsArgWith(1)
-
-      return this.InactiveProjectManager.deactivateProject(
-        this.project_id,
-        err => {
-          err.should.equal('errorrr')
-          this.DocstoreManager.archiveProject
-            .calledWith(this.project_id)
-            .should.equal(true)
-          this.ProjectUpdateHandler.markAsInactive
-            .calledWith(this.project_id)
-            .should.equal(false)
-          return done()
-        }
-      )
+      await expect(
+        this.InactiveProjectManager.promises.deactivateProject(this.project_id)
+      ).to.be.rejected
+      this.DocstoreManager.promises.archiveProject
+        .calledWith(this.project_id)
+        .should.equal(true)
+      this.ProjectUpdateHandler.promises.markAsInactive
+        .calledWith(this.project_id)
+        .should.equal(false)
     })
   })
 })