diff --git a/services/web/app/src/Features/Contacts/ContactController.mjs b/services/web/app/src/Features/Contacts/ContactController.mjs index 5673d0be2d..3a71a510ed 100644 --- a/services/web/app/src/Features/Contacts/ContactController.mjs +++ b/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, diff --git a/services/web/app/src/Features/Contacts/ContactManager.mjs b/services/web/app/src/Features/Contacts/ContactManager.mjs index 1afb40470c..6a851ec2cc 100644 --- a/services/web/app/src/Features/Contacts/ContactManager.mjs +++ b/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' +import { db, ObjectId } from '../../infrastructure/mongodb.mjs' -async function getContactIds(userId, options) { - options = options ?? { limit: 50 } +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 } + ) +} - const url = new URL(`${settings.apis.contacts.url}/user/${userId}/contacts`) +async function getContactIds(userId, limit) { + const user = await db.contacts.findOne({ + user_id: new ObjectId(userId.toString()), + }) - for (const [key, val] of Object.entries(options)) { - url.searchParams.set(key, val) - } - - let body - try { - body = await fetchJson(url) - } catch (err) { - throw OError.tag(err, 'failed request to contacts API', { userId }) - } - - 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 { diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index 0db3d6b8ae..5375871cc7 100644 --- a/services/web/config/settings.defaults.js +++ b/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`, }, diff --git a/services/web/test/unit/src/Contact/ContactController.test.mjs b/services/web/test/unit/src/Contact/ContactController.test.mjs index de1c61bb21..84857c5b07 100644 --- a/services/web/test/unit/src/Contact/ContactController.test.mjs +++ b/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) }) diff --git a/services/web/test/unit/src/Contact/ContactManager.sequential.test.mjs b/services/web/test/unit/src/Contact/ContactManager.sequential.test.mjs new file mode 100644 index 0000000000..f5fe0f84e4 --- /dev/null +++ b/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([]) + }) + }) +}) diff --git a/services/web/test/unit/src/Contact/ContactManager.test.mjs b/services/web/test/unit/src/Contact/ContactManager.test.mjs deleted file mode 100644 index 359b3bc9d8..0000000000 --- a/services/web/test/unit/src/Contact/ContactManager.test.mjs +++ /dev/null @@ -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) - }) - }) - }) -})