Browse Source

[web] inline contacts service into web (#33546)

GitOrigin-RevId: d5e84d4f80f5ad4e951934d6dcdc332b0d26f3d0
Jakob Ackermann 2 months ago
parent
commit
293d89a4cb

+ 6 - 3
services/web/app/src/Features/Contacts/ContactController.mjs

@@ -14,12 +14,15 @@ function _formatContact(contact) {
   }
 }
 
+const MAX_CONTACTS = 50
+
 async function getContacts(req, res) {
   const userId = SessionManager.getLoggedInUserId(req.session)
 
-  const contactIds = await ContactManager.promises.getContactIds(userId, {
-    limit: 50,
-  })
+  const contactIds = await ContactManager.promises.getContactIds(
+    userId,
+    MAX_CONTACTS
+  )
 
   let contacts = await UserGetter.promises.getUsers(contactIds, {
     email: 1,

+ 33 - 33
services/web/app/src/Features/Contacts/ContactManager.mjs

@@ -1,44 +1,44 @@
 import { callbackify } from 'node:util'
-import OError from '@overleaf/o-error'
-import { fetchJson } from '@overleaf/fetch-utils'
-import settings from '@overleaf/settings'
-
-async function getContactIds(userId, options) {
-  options = options ?? { limit: 50 }
-
-  const url = new URL(`${settings.apis.contacts.url}/user/${userId}/contacts`)
-
-  for (const [key, val] of Object.entries(options)) {
-    url.searchParams.set(key, val)
-  }
+import { db, ObjectId } from '../../infrastructure/mongodb.mjs'
+
+async function touchContact(userId, contactId) {
+  await db.contacts.updateOne(
+    { user_id: new ObjectId(userId.toString()) },
+    {
+      $inc: { [`contacts.${contactId}.n`]: 1 },
+      $set: { [`contacts.${contactId}.ts`]: new Date() },
+    },
+    { upsert: true }
+  )
+}
 
-  let body
-  try {
-    body = await fetchJson(url)
-  } catch (err) {
-    throw OError.tag(err, 'failed request to contacts API', { userId })
-  }
+async function getContactIds(userId, limit) {
+  const user = await db.contacts.findOne({
+    user_id: new ObjectId(userId.toString()),
+  })
 
-  return body?.contact_ids || []
+  return buildContactIds(user?.contacts, limit)
 }
 
 async function addContact(userId, contactId) {
-  const url = new URL(`${settings.apis.contacts.url}/user/${userId}/contacts`)
+  await Promise.all([
+    touchContact(userId, contactId),
+    touchContact(contactId, userId),
+  ])
+}
 
-  let body
-  try {
-    body = await fetchJson(url, {
-      method: 'POST',
-      json: { contact_id: contactId },
-    })
-  } catch (err) {
-    throw OError.tag(err, 'failed request to contacts API', {
-      userId,
-      contactId,
-    })
-  }
+// sort by decreasing count, decreasing timestamp.
+// i.e. highest count, most recent first.
+function sortContacts(a, b) {
+  return a.n === b.n ? b.ts - a.ts : b.n - a.n
+}
 
-  return body?.contact_ids || []
+function buildContactIds(contacts, limit) {
+  return Object.entries(contacts || {})
+    .map(([id, { n, ts }]) => ({ id, n, ts }))
+    .sort(sortContacts)
+    .slice(0, limit)
+    .map(contact => contact.id)
 }
 
 export default {

+ 0 - 3
services/web/config/settings.defaults.js

@@ -264,9 +264,6 @@ module.exports = {
     realTime: {
       url: `http://${process.env.REALTIME_HOST || '127.0.0.1'}:3026`,
     },
-    contacts: {
-      url: `http://${process.env.CONTACTS_HOST || '127.0.0.1'}:3036`,
-    },
     notifications: {
       url: `http://${process.env.NOTIFICATIONS_HOST || '127.0.0.1'}:3042`,
     },

+ 1 - 1
services/web/test/unit/src/Contact/ContactController.test.mjs

@@ -83,7 +83,7 @@ describe('ContactController', function () {
       ctx.res.callback = () => {
         expect(
           ctx.ContactManager.promises.getContactIds
-        ).to.have.been.calledWith(ctx.user_id, { limit: 50 })
+        ).to.have.been.calledWith(ctx.user_id, 50)
       }
       ctx.ContactController.getContacts(ctx.req, ctx.res)
     })

+ 73 - 0
services/web/test/unit/src/Contact/ContactManager.sequential.test.mjs

@@ -0,0 +1,73 @@
+import sinon from 'sinon'
+import {
+  connectionPromise,
+  cleanupTestDatabase,
+} from '../../../../app/src/infrastructure/mongodb.mjs'
+import ContactManager from '../../../../app/src/Features/Contacts/ContactManager.mjs'
+
+describe('ContactManager', function () {
+  beforeAll(async function () {
+    await connectionPromise
+  })
+  beforeEach(cleanupTestDatabase)
+
+  const userId = 'aaaaaaaaaaaaaaaaaaaaaaaa'
+  const contactId = 'bbbbbbbbbbbbbbbbbbbbbbbb'
+  const otherId1 = 'cccccccccccccccccccccccc'
+  const otherId2 = 'dddddddddddddddddddddddd'
+  const otherId3 = 'eeeeeeeeeeeeeeeeeeeeeeee'
+
+  describe('addContact', function () {
+    beforeEach(async function () {
+      await ContactManager.promises.addContact(userId, contactId)
+    })
+
+    it('should record the contact under the user', async function () {
+      const ids = await ContactManager.promises.getContactIds(userId, 50)
+      expect(ids).to.deep.equal([contactId])
+    })
+
+    it('should record the user under the contact', async function () {
+      const ids = await ContactManager.promises.getContactIds(contactId, 50)
+      expect(ids).to.deep.equal([userId])
+    })
+  })
+
+  describe('getContactIds', function () {
+    beforeEach(async function (ctx) {
+      ctx.clock = sinon.useFakeTimers(new Date('2026-01-01'))
+
+      // otherId3: touched once at T → count 1, ts = T
+      await ContactManager.promises.addContact(userId, otherId3)
+
+      // otherId2: touched twice at T → count 2, ts = T
+      await ContactManager.promises.addContact(userId, otherId2)
+      await ContactManager.promises.addContact(userId, otherId2)
+
+      // otherId1: touched once at T+1s → count 1, ts = T+1s
+      ctx.clock.tick(1000)
+      await ContactManager.promises.addContact(userId, otherId1)
+    })
+
+    afterEach(function (ctx) {
+      ctx.clock.restore()
+    })
+
+    it('should sort by count descending then timestamp descending', async function () {
+      const ids = await ContactManager.promises.getContactIds(userId, 50)
+      expect(ids).to.deep.equal([otherId2, otherId1, otherId3])
+    })
+
+    it('should respect the limit', async function () {
+      const ids = await ContactManager.promises.getContactIds(userId, 2)
+      expect(ids).to.deep.equal([otherId2, otherId1])
+    })
+  })
+
+  describe('with no contacts in the database', function () {
+    it('should return an empty array', async function () {
+      const ids = await ContactManager.promises.getContactIds(userId, 50)
+      expect(ids).to.deep.equal([])
+    })
+  })
+})

+ 0 - 105
services/web/test/unit/src/Contact/ContactManager.test.mjs

@@ -1,105 +0,0 @@
-import { vi, expect } from 'vitest'
-import sinon from 'sinon'
-const modulePath = '../../../../app/src/Features/Contacts/ContactManager'
-
-describe('ContactManager', function () {
-  beforeEach(async function (ctx) {
-    ctx.user_id = 'user-id-123'
-    ctx.contact_id = 'contact-id-123'
-    ctx.contact_ids = ['mock', 'contact_ids']
-    ctx.FetchUtils = {
-      fetchJson: sinon.stub(),
-    }
-
-    vi.doMock('@overleaf/fetch-utils', () => ctx.FetchUtils)
-
-    vi.doMock('@overleaf/settings', () => ({
-      default: (ctx.settings = {
-        apis: {
-          contacts: {
-            url: 'http://contacts.overleaf.com',
-          },
-        },
-      }),
-    }))
-
-    ctx.ContactManager = (await import(modulePath)).default
-  })
-
-  describe('getContacts', function () {
-    describe('with a successful response code', function () {
-      beforeEach(async function (ctx) {
-        ctx.FetchUtils.fetchJson.resolves({ contact_ids: ctx.contact_ids })
-
-        ctx.result = await ctx.ContactManager.promises.getContactIds(
-          ctx.user_id,
-          { limit: 42 }
-        )
-      })
-
-      it('should get the contacts from the contacts api', function (ctx) {
-        ctx.FetchUtils.fetchJson.should.have.been.calledWithMatch(
-          sinon.match(
-            url =>
-              url.toString() ===
-              `${ctx.settings.apis.contacts.url}/user/${ctx.user_id}/contacts?limit=42`
-          )
-        )
-      })
-
-      it('should return the contacts', function (ctx) {
-        ctx.result.should.equal(ctx.contact_ids)
-      })
-    })
-
-    describe('when an error occurs', function () {
-      beforeEach(async function (ctx) {
-        ctx.response = {
-          ok: false,
-          statusCode: 500,
-          json: sinon.stub().resolves({ contact_ids: ctx.contact_ids }),
-        }
-        ctx.FetchUtils.fetchJson.rejects(new Error('request error'))
-      })
-
-      it('should reject the promise', async function (ctx) {
-        await expect(
-          ctx.ContactManager.promises.getContactIds(ctx.user_id, {
-            limit: 42,
-          })
-        ).to.be.rejected
-      })
-    })
-  })
-
-  describe('addContact', function () {
-    describe('with a successful response code', function () {
-      beforeEach(async function (ctx) {
-        ctx.FetchUtils.fetchJson.resolves({ contact_ids: ctx.contact_ids })
-
-        ctx.result = await ctx.ContactManager.promises.addContact(
-          ctx.user_id,
-          ctx.contact_id
-        )
-      })
-
-      it('should add the contacts for the user in the contacts api', function (ctx) {
-        ctx.FetchUtils.fetchJson.should.have.been.calledWithMatch(
-          sinon.match(
-            url =>
-              url.toString() ===
-              `${ctx.settings.apis.contacts.url}/user/${ctx.user_id}/contacts`
-          ),
-          sinon.match({
-            method: 'POST',
-            json: { contact_id: ctx.contact_id },
-          })
-        )
-      })
-
-      it('should call the callback', function (ctx) {
-        ctx.result.should.equal(ctx.contact_ids)
-      })
-    })
-  })
-})