Merge pull request #17084 from overleaf/dp-mongoose-callback-system-message-manager

Promisify SystemMessageManager and SystemMessageManagerTests

GitOrigin-RevId: b8fafdfdba817160c1b18cf7eb0270a27adf114c

libraries/promise-utils/index.js

@@ -143,13 +143,18 @@ function promisifyMultiResult(fn, resultNames) {
  *
  * @param {Object} module - The module to callbackify
  * @param {Object} opts - Options
+ * @param {Array<string>} opts.without - Array of method names to exclude from
+ *                                       being callbackified
  * @param {Object} opts.multiResult - Spec of methods to be callbackified with
  *                                    callbackifyMultiResult()
  */
 function callbackifyAll(module, opts = {}) {
-  const { multiResult = {} } = opts
+  const { without = [], multiResult = {} } = opts
   const callbacks = {}
   for (const propName of Object.getOwnPropertyNames(module)) {
+    if (without.includes(propName)) {
+      continue
+    }
     const propValue = module[propName]
     if (typeof propValue === 'function') {
       if (propValue.constructor.name === 'AsyncFunction') {

libraries/promise-utils/test/unit/PromiseUtilsTests.js

@@ -295,4 +295,32 @@ describe('callbackifyAll', function () {
       })
     })
   })
+
+  describe('without option', function () {
+    before(function () {
+      this.module = {
+        async asyncAdd(a, b) {
+          return a + b
+        },
+        async asyncArithmetic(a, b) {
+          return { sum: a + b, product: a * b }
+        },
+      }
+      this.callbackified = callbackifyAll(this.module, {
+        without: ['asyncAdd'],
+      })
+    })
+
+    it('does not callbackify excluded functions', function () {
+      expect(this.callbackified.asyncAdd).not.to.exist
+    })
+
+    it('callbackifies other functions', async function () {
+      this.callbackified.asyncArithmetic(5, 6, (err, { sum, product }) => {
+        expect(err).not.to.exist
+        expect(sum).to.equal(11)
+        expect(product).to.equal(30)
+      })
+    })
+  })
 })

services/web/app/src/Features/SystemMessages/SystemMessageController.js

@@ -8,23 +8,19 @@ const ProjectController = {
       // gracefully handle requests from anonymous users
       return res.json([])
     }
-    SystemMessageManager.getMessages((err, messages) => {
+    let messages = SystemMessageManager.getMessages()
-      if (err) {
+
-        next(err)
+    if (!Settings.siteIsOpen) {
-      } else {
+      // Override all messages with notice for admins when site is closed.
-        if (!Settings.siteIsOpen) {
+      messages = [
-          // Override all messages with notice for admins when site is closed.
+        {
-          messages = [
+          content:
-            {
+            'SITE IS CLOSED TO PUBLIC. OPEN ONLY FOR SITE ADMINS. DO NOT EDIT PROJECTS.',
-              content:
+          _id: 'protected', // prevents hiding message in frontend
-                'SITE IS CLOSED TO PUBLIC. OPEN ONLY FOR SITE ADMINS. DO NOT EDIT PROJECTS.',
+        },
-              _id: 'protected', // prevents hiding message in frontend
+      ]
-            },
+    }
-          ]
+    res.json(messages || [])
-        }
-        res.json(messages || [])
-      }
-    })
   },
 }
 

services/web/app/src/Features/SystemMessages/SystemMessageManager.js

@@ -1,55 +1,30 @@
-/* eslint-disable
-    n/handle-callback-err,
-*/
-// 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 SystemMessageManager
 const { SystemMessage } = require('../../models/SystemMessage')
 const {
   addRequiredCleanupHandlerBeforeDrainingConnections,
 } = require('../../infrastructure/GracefulShutdown')
+const { callbackifyAll } = require('@overleaf/promise-utils')
 
-module.exports = SystemMessageManager = {
+const SystemMessageManager = {
-  getMessages(callback) {
+  getMessages() {
-    if (callback == null) {
+    return this._cachedMessages
-      callback = function () {}
-    }
-    callback(null, this._cachedMessages)
   },
 
-  getMessagesFromDB(callback) {
+  async getMessagesFromDB() {
-    if (callback == null) {
+    return await SystemMessage.find({}).exec()
-      callback = function () {}
-    }
-    SystemMessage.find({}, callback)
   },
 
-  clearMessages(callback) {
+  async clearMessages() {
-    if (callback == null) {
+    await SystemMessage.deleteMany({}).exec()
-      callback = function () {}
-    }
-    SystemMessage.deleteMany({}, callback)
   },
 
-  createMessage(content, callback) {
+  async createMessage(content) {
-    if (callback == null) {
-      callback = function () {}
-    }
     const message = new SystemMessage({ content })
-    message.save(callback)
+    await message.save()
   },
 
-  refreshCache() {
+  async refreshCache() {
-    this.getMessagesFromDB((error, messages) => {
+    const messages = await this.getMessagesFromDB()
-      if (!error) {
+    this._cachedMessages = messages
-        this._cachedMessages = messages
-      }
-    })
   },
 }
 
@@ -66,3 +41,9 @@ addRequiredCleanupHandlerBeforeDrainingConnections(
     clearInterval(intervalHandle)
   }
 )
+
+module.exports = {
+  getMessages: SystemMessageManager.getMessages.bind(SystemMessageManager),
+  ...callbackifyAll(SystemMessageManager, { without: ['getMessages'] }),
+  promises: SystemMessageManager,
+}

services/web/test/unit/src/SystemMessages/SystemMessageManagerTests.js

@@ -1,17 +1,4 @@
-/* eslint-disable
-    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 sinon = require('sinon')
 const modulePath = require('path').join(
   __dirname,
@@ -22,14 +9,15 @@ describe('SystemMessageManager', function () {
   beforeEach(function () {
     this.messages = ['messages-stub']
     this.SystemMessage = {
-      find: sinon.stub().yields(null, this.messages),
+      find: sinon.stub().returns({
+        exec: sinon.stub().resolves(this.messages),
+      }),
     }
     this.SystemMessageManager = SandboxedModule.require(modulePath, {
       requires: {
         '../../models/SystemMessage': { SystemMessage: this.SystemMessage },
       },
     })
-    return (this.callback = sinon.stub())
   })
 
   it('should look the messages up in the database on import', function () {
@@ -39,26 +27,24 @@ describe('SystemMessageManager', function () {
   describe('getMessage', function () {
     beforeEach(function () {
       this.SystemMessageManager._cachedMessages = this.messages
-      return this.SystemMessageManager.getMessages(this.callback)
+      this.result = this.SystemMessageManager.getMessages()
     })
 
     it('should return the messages', function () {
-      return this.callback.calledWith(null, this.messages).should.equal(true)
+      this.result.should.equal(this.messages)
     })
   })
 
   describe('clearMessages', function () {
     beforeEach(function () {
-      this.SystemMessage.deleteMany = sinon.stub().callsArg(1)
+      this.SystemMessage.deleteMany = sinon.stub().returns({
-      return this.SystemMessageManager.clearMessages(this.callback)
+        exec: sinon.stub().resolves(),
+      })
+      this.SystemMessageManager.promises.clearMessages()
     })
 
     it('should remove the messages from the database', function () {
-      return this.SystemMessage.deleteMany.calledWith({}).should.equal(true)
+      this.SystemMessage.deleteMany.calledWith({}).should.equal(true)
-    })
-
-    it('should return the callback', function () {
-      return this.callback.called.should.equal(true)
     })
   })
 })