Merge pull request #18375 from overleaf/em-promisify-chat-api-handler

Promisify ChatApiHandler

GitOrigin-RevId: 83cedb14b5e2b187fb2cb02fcbf888ada5a599b1

services/web/app/src/Features/Chat/ChatApiHandler.js

@@ -1,190 +1,120 @@
-/* eslint-disable
-    n/handle-callback-err,
-    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 ChatApiHandler
-const OError = require('@overleaf/o-error')
-const request = require('request')
+// @ts-check
+
+const { fetchJson, fetchNothing } = require('@overleaf/fetch-utils')
 const settings = require('@overleaf/settings')
-const { promisify } = require('util')
+const { callbackify } = require('util')
 
-function getThreads(projectId, callback) {
-  if (callback == null) {
-    callback = function () {}
-  }
-  return ChatApiHandler._apiRequest(
-    {
-      url: `${settings.apis.chat.internal_url}/project/${projectId}/threads`,
-      method: 'GET',
-      json: true,
-    },
-    callback
-  )
+async function getThreads(projectId) {
+  return await fetchJson(chatApiUrl(`/project/${projectId}/threads`))
 }
 
-function destroyProject(projectId, callback) {
-  if (callback == null) {
-    callback = function () {}
-  }
-  return ChatApiHandler._apiRequest(
-    {
-      url: `${settings.apis.chat.internal_url}/project/${projectId}`,
-      method: 'DELETE',
-    },
-    callback
-  )
+async function destroyProject(projectId) {
+  await fetchNothing(chatApiUrl(`/project/${projectId}`), { method: 'DELETE' })
 }
 
-module.exports = ChatApiHandler = {
-  _apiRequest(opts, callback) {
-    if (callback == null) {
-      callback = function () {}
+async function sendGlobalMessage(projectId, userId, content) {
+  const message = await fetchJson(
+    chatApiUrl(`/project/${projectId}/messages`),
+    {
+      method: 'POST',
+      json: { user_id: userId, content },
     }
-    return request(opts, function (error, response, data) {
-      if (error != null) {
-        return callback(error)
-      }
-      if (response.statusCode >= 200 && response.statusCode < 300) {
-        return callback(null, data)
-      } else {
-        error = new OError(
-          `chat api returned non-success code: ${response.statusCode}`,
-          opts
-        )
-        error.statusCode = response.statusCode
-        return callback(error)
-      }
-    })
-  },
-
-  sendGlobalMessage(projectId, userId, content, callback) {
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/messages`,
-        method: 'POST',
-        json: { user_id: userId, content },
-      },
-      callback
-    )
-  },
+  )
+  return message
+}
 
-  getGlobalMessages(projectId, limit, before, callback) {
-    const qs = {}
-    if (limit != null) {
-      qs.limit = limit
-    }
-    if (before != null) {
-      qs.before = before
-    }
+async function getGlobalMessages(projectId, limit, before) {
+  const url = chatApiUrl(`/project/${projectId}/messages`)
+  if (limit != null) {
+    url.searchParams.set('limit', limit)
+  }
+  if (before != null) {
+    url.searchParams.set('before', before)
+  }
 
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/messages`,
-        method: 'GET',
-        qs,
-        json: true,
-      },
-      callback
-    )
-  },
+  return await fetchJson(url)
+}
 
-  sendComment(projectId, threadId, userId, content, callback) {
-    if (callback == null) {
-      callback = function () {}
+async function sendComment(projectId, threadId, userId, content) {
+  const comment = await fetchJson(
+    chatApiUrl(`/project/${projectId}/thread/${threadId}/messages`),
+    {
+      method: 'POST',
+      json: { user_id: userId, content },
     }
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/thread/${threadId}/messages`,
-        method: 'POST',
-        json: { user_id: userId, content },
-      },
-      callback
-    )
-  },
+  )
+  return comment
+}
 
-  resolveThread(projectId, threadId, userId, callback) {
-    if (callback == null) {
-      callback = function () {}
+async function resolveThread(projectId, threadId, userId) {
+  await fetchNothing(
+    chatApiUrl(`/project/${projectId}/thread/${threadId}/resolve`),
+    {
+      method: 'POST',
+      json: { user_id: userId },
     }
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/thread/${threadId}/resolve`,
-        method: 'POST',
-        json: { user_id: userId },
-      },
-      callback
-    )
-  },
+  )
+}
 
-  reopenThread(projectId, threadId, callback) {
-    if (callback == null) {
-      callback = function () {}
-    }
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/thread/${threadId}/reopen`,
-        method: 'POST',
-      },
-      callback
-    )
-  },
+async function reopenThread(projectId, threadId) {
+  await fetchNothing(
+    chatApiUrl(`/project/${projectId}/thread/${threadId}/reopen`),
+    { method: 'POST' }
+  )
+}
 
-  deleteThread(projectId, threadId, callback) {
-    if (callback == null) {
-      callback = function () {}
-    }
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/thread/${threadId}`,
-        method: 'DELETE',
-      },
-      callback
-    )
-  },
+async function deleteThread(projectId, threadId) {
+  await fetchNothing(chatApiUrl(`/project/${projectId}/thread/${threadId}`), {
+    method: 'DELETE',
+  })
+}
 
-  editMessage(projectId, threadId, messageId, userId, content, callback) {
-    if (callback == null) {
-      callback = function () {}
+async function editMessage(projectId, threadId, messageId, userId, content) {
+  await fetchNothing(
+    chatApiUrl(
+      `/project/${projectId}/thread/${threadId}/messages/${messageId}/edit`
+    ),
+    {
+      method: 'POST',
+      json: { content, userId },
     }
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/thread/${threadId}/messages/${messageId}/edit`,
-        method: 'POST',
-        json: {
-          content,
-          userId,
-        },
-      },
-      callback
-    )
-  },
+  )
+}
 
-  deleteMessage(projectId, threadId, messageId, callback) {
-    if (callback == null) {
-      callback = function () {}
-    }
-    return ChatApiHandler._apiRequest(
-      {
-        url: `${settings.apis.chat.internal_url}/project/${projectId}/thread/${threadId}/messages/${messageId}`,
-        method: 'DELETE',
-      },
-      callback
-    )
-  },
+async function deleteMessage(projectId, threadId, messageId) {
+  await fetchNothing(
+    chatApiUrl(
+      `/project/${projectId}/thread/${threadId}/messages/${messageId}`
+    ),
+    { method: 'DELETE' }
+  )
+}
 
-  getThreads,
-  destroyProject,
+function chatApiUrl(path) {
+  return new URL(path, settings.apis.chat.internal_url)
+}
 
+module.exports = {
+  getThreads: callbackify(getThreads),
+  destroyProject: callbackify(destroyProject),
+  sendGlobalMessage: callbackify(sendGlobalMessage),
+  getGlobalMessages: callbackify(getGlobalMessages),
+  sendComment: callbackify(sendComment),
+  resolveThread: callbackify(resolveThread),
+  reopenThread: callbackify(reopenThread),
+  deleteThread: callbackify(deleteThread),
+  editMessage: callbackify(editMessage),
+  deleteMessage: callbackify(deleteMessage),
   promises: {
-    getThreads: promisify(getThreads),
-    destroyProject: promisify(destroyProject),
+    getThreads,
+    destroyProject,
+    sendGlobalMessage,
+    getGlobalMessages,
+    sendComment,
+    resolveThread,
+    reopenThread,
+    deleteThread,
+    editMessage,
+    deleteMessage,
   },
 }

services/web/test/unit/src/Chat/ChatApiHandlerTests.js

@@ -1,93 +1,83 @@
-/* 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 path = require('path')
 const sinon = require('sinon')
-const modulePath = path.join(
+const { expect } = require('chai')
+const { RequestFailedError } = require('@overleaf/fetch-utils')
+
+const MODULE_PATH = path.join(
   __dirname,
   '../../../../app/src/Features/Chat/ChatApiHandler'
 )
-const { expect } = require('chai')
 
 describe('ChatApiHandler', function () {
   beforeEach(function () {
     this.settings = {
       apis: {
         chat: {
-          internal_url: 'chat.overleaf.env',
+          internal_url: 'http://chat.overleaf.env',
         },
       },
     }
-    this.request = sinon.stub()
-    this.ChatApiHandler = SandboxedModule.require(modulePath, {
+    this.FetchUtils = {
+      fetchJson: sinon.stub(),
+      fetchNothing: sinon.stub().resolves(),
+    }
+    this.ChatApiHandler = SandboxedModule.require(MODULE_PATH, {
       requires: {
         '@overleaf/settings': this.settings,
-        request: this.request,
+        '@overleaf/fetch-utils': this.FetchUtils,
       },
     })
     this.project_id = '3213213kl12j'
     this.user_id = '2k3jlkjs9'
     this.content = 'my message here'
-    return (this.callback = sinon.stub())
   })
 
   describe('sendGlobalMessage', function () {
     describe('successfully', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.message = { mock: 'message' }
-        this.request.callsArgWith(1, null, { statusCode: 200 }, this.message)
-        return this.ChatApiHandler.sendGlobalMessage(
+        this.FetchUtils.fetchJson.resolves(this.message)
+        this.result = await this.ChatApiHandler.promises.sendGlobalMessage(
           this.project_id,
           this.user_id,
-          this.content,
-          this.callback
+          this.content
         )
       })
 
       it('should post the data to the chat api', function () {
-        return this.request
-          .calledWith({
-            url: `${this.settings.apis.chat.internal_url}/project/${this.project_id}/messages`,
+        this.FetchUtils.fetchJson.should.have.been.calledWith(
+          sinon.match(
+            url =>
+              url.toString() ===
+              `${this.settings.apis.chat.internal_url}/project/${this.project_id}/messages`
+          ),
+          {
             method: 'POST',
             json: {
               content: this.content,
               user_id: this.user_id,
             },
-          })
-          .should.equal(true)
+          }
+        )
       })
 
       it('should return the message from the post', function () {
-        return this.callback.calledWith(null, this.message).should.equal(true)
+        expect(this.result).to.deep.equal(this.message)
       })
     })
 
     describe('with a non-success status code', function () {
-      beforeEach(function () {
-        this.request.callsArgWith(1, null, { statusCode: 500 })
-        return this.ChatApiHandler.sendGlobalMessage(
-          this.project_id,
-          this.user_id,
-          this.content,
-          this.callback
-        )
-      })
-
-      it('should return an error', function () {
-        expect(this.callback).to.have.been.calledWith(
-          sinon.match.instanceOf(Error).and(sinon.match.has('statusCode', 500))
-        )
+      beforeEach(async function () {
+        this.error = new RequestFailedError('some-url', {}, { status: 500 })
+        this.FetchUtils.fetchJson.rejects(this.error)
+        await expect(
+          this.ChatApiHandler.promises.sendGlobalMessage(
+            this.project_id,
+            this.user_id,
+            this.content
+          )
+        ).to.be.rejectedWith(this.error)
       })
     })
   })
@@ -96,54 +86,45 @@ describe('ChatApiHandler', function () {
     beforeEach(function () {
       this.messages = [{ mock: 'message' }]
       this.limit = 30
-      return (this.before = '1234')
+      this.before = '1234'
     })
 
     describe('successfully', function () {
-      beforeEach(function () {
-        this.request.callsArgWith(1, null, { statusCode: 200 }, this.messages)
-        return this.ChatApiHandler.getGlobalMessages(
+      beforeEach(async function () {
+        this.FetchUtils.fetchJson.resolves(this.messages)
+        this.result = await this.ChatApiHandler.promises.getGlobalMessages(
           this.project_id,
           this.limit,
-          this.before,
-          this.callback
+          this.before
         )
       })
 
       it('should make get request for room to chat api', function () {
-        return this.request
-          .calledWith({
-            method: 'GET',
-            url: `${this.settings.apis.chat.internal_url}/project/${this.project_id}/messages`,
-            qs: {
-              limit: this.limit,
-              before: this.before,
-            },
-            json: true,
-          })
-          .should.equal(true)
+        this.FetchUtils.fetchJson.should.have.been.calledWith(
+          sinon.match(
+            url =>
+              url.toString() ===
+              `${this.settings.apis.chat.internal_url}/project/${this.project_id}/messages?limit=${this.limit}&before=${this.before}`
+          )
+        )
       })
 
       it('should return the messages from the request', function () {
-        return this.callback.calledWith(null, this.messages).should.equal(true)
+        expect(this.result).to.deep.equal(this.messages)
       })
     })
 
     describe('with failure error code', function () {
-      beforeEach(function () {
-        this.request.callsArgWith(1, null, { statusCode: 500 }, null)
-        return this.ChatApiHandler.getGlobalMessages(
-          this.project_id,
-          this.limit,
-          this.before,
-          this.callback
-        )
-      })
-
-      it('should return an error', function () {
-        expect(this.callback).to.have.been.calledWith(
-          sinon.match.instanceOf(Error).and(sinon.match.has('statusCode', 500))
-        )
+      beforeEach(async function () {
+        this.error = new RequestFailedError('some-url', {}, { status: 500 })
+        this.FetchUtils.fetchJson.rejects(this.error)
+        await expect(
+          this.ChatApiHandler.getGlobalMessages(
+            this.project_id,
+            this.limit,
+            this.before
+          )
+        ).to.be.rejectedWith(this.error)
       })
     })
   })