From 222943ac55da10f1c7cedc3587afec6cd481fc0c Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Tue, 18 Nov 2025 14:56:49 -0300 Subject: [PATCH 01/23] fix: d --- ee/apps/ddp-streamer/src/DDPStreamer.ts | 5 ++ ee/packages/presence/src/Presence.ts | 8 ++++ .../src/lib/processConnectionStatus.ts | 5 +- .../tests/lib/processConnectionStatus.test.ts | 47 +++++++++++++++++++ packages/core-services/src/types/IPresence.ts | 1 + packages/core-typings/src/IUserSession.ts | 1 + .../src/models/IUsersSessionsModel.ts | 3 +- packages/models/src/models/UsersSessions.ts | 19 +++++++- 8 files changed, 86 insertions(+), 3 deletions(-) diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index 1a6a1046ae776..f57c81cb0b724 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -215,6 +215,11 @@ export class DDPStreamer extends ServiceClass { server.on(DDP_EVENTS.CONNECTED, ({ connection }) => { this.api?.broadcast('socket.connected', connection); }); + + server.on(DDP_EVENTS.PING, (client: Client): void => { + const { connection } = client; + Presence.renewConnection(connection.id); + }); } async started(): Promise { diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index 5b537857698c9..dc27e893aa96a 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -128,6 +128,7 @@ export class Presence extends ServiceClass implements IPresence { id: session, instanceId: nodeId, status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() + 5 * 60 * 1000), }); await this.updateUserPresence(uid); @@ -180,6 +181,13 @@ export class Presence extends ServiceClass implements IPresence { return affectedUsers.map(({ _id }) => _id); } + async renewConnection(connectionId: string): Promise { + const result = await UsersSessions.renewConnectionByConnectionId(connectionId, new Date(Date.now() + 5 * 60 * 1000)); + if (result.modifiedCount === 0) { + throw new Error('Connection not found'); + } + } + async setStatus(uid: string, statusDefault: UserStatus, statusText?: string): Promise { const userSessions = (await UsersSessions.findOneById(uid)) || { connections: [] }; diff --git a/ee/packages/presence/src/lib/processConnectionStatus.ts b/ee/packages/presence/src/lib/processConnectionStatus.ts index a14aa45219e8a..8c2dd866e148f 100644 --- a/ee/packages/presence/src/lib/processConnectionStatus.ts +++ b/ee/packages/presence/src/lib/processConnectionStatus.ts @@ -36,7 +36,10 @@ export const processPresenceAndStatus = ( userSessions: IUserSessionConnection[] = [], statusDefault = UserStatus.ONLINE, ): { status: UserStatus; statusConnection: UserStatus } => { - const statusConnection = userSessions.map((s) => s.status).reduce(processConnectionStatus, UserStatus.OFFLINE); + const statusConnection = userSessions + .filter((s) => s.expiresAt > new Date()) + .map((s) => s.status) + .reduce(processConnectionStatus, UserStatus.OFFLINE); const status = processStatus(statusConnection, statusDefault); diff --git a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts index 1646636d92b2e..7455d46c06e5d 100644 --- a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts +++ b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts @@ -49,6 +49,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -64,6 +65,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.AWAY, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -79,6 +81,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -94,6 +97,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -109,6 +113,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.AWAY, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -124,6 +129,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -139,6 +145,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.AWAY, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -156,6 +163,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -163,6 +171,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.AWAY, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -178,6 +187,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.AWAY, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -185,6 +195,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -200,6 +211,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.AWAY, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -207,6 +219,7 @@ describe('Presence micro service', () => { id: 'random', instanceId: 'random', status: UserStatus.AWAY, + expiresAt: new Date(Date.now() + 1000 * 60), _createdAt: new Date(), _updatedAt: new Date(), }, @@ -237,4 +250,38 @@ describe('Presence micro service', () => { statusConnection: UserStatus.OFFLINE, }); }); + + test('should return correct status and statusConnection when there are expired connections', () => { + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + expiresAt: new Date(Date.now() - 1000 * 60), + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + expiresAt: new Date(Date.now() - 1000 * 60), + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); + }); }); diff --git a/packages/core-services/src/types/IPresence.ts b/packages/core-services/src/types/IPresence.ts index 5f7c57d679955..43a5f3f20053e 100644 --- a/packages/core-services/src/types/IPresence.ts +++ b/packages/core-services/src/types/IPresence.ts @@ -13,6 +13,7 @@ export interface IPresence extends IServiceClass { session: string | undefined, nodeId: string, ): Promise<{ uid: string; session: string } | undefined>; + renewConnection(connectionId: string): Promise; removeLostConnections(nodeID: string): Promise; setStatus(uid: string, status: UserStatus, statusText?: string): Promise; setConnectionStatus(uid: string, status: UserStatus, session: string): Promise; diff --git a/packages/core-typings/src/IUserSession.ts b/packages/core-typings/src/IUserSession.ts index 48844fab6e278..b1f7f0f9beb72 100644 --- a/packages/core-typings/src/IUserSession.ts +++ b/packages/core-typings/src/IUserSession.ts @@ -4,6 +4,7 @@ export interface IUserSessionConnection { id: string; instanceId: string; status: UserStatus; + expiresAt: Date; _createdAt: Date; _updatedAt: Date; } diff --git a/packages/model-typings/src/models/IUsersSessionsModel.ts b/packages/model-typings/src/models/IUsersSessionsModel.ts index 75800af233bcc..3e7e64959964e 100644 --- a/packages/model-typings/src/models/IUsersSessionsModel.ts +++ b/packages/model-typings/src/models/IUsersSessionsModel.ts @@ -8,10 +8,11 @@ export interface IUsersSessionsModel extends IBaseModel { updateConnectionStatusById(uid: string, connectionId: string, status: string): ReturnType['updateOne']>; removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']>; removeConnectionByConnectionId(connectionId: string): ReturnType['updateMany']>; + renewConnectionByConnectionId(connectionId: string, expiresAt: Date): ReturnType['updateMany']>; findByInstanceId(instanceId: string): FindCursor; addConnectionById( userId: string, - { id, instanceId, status }: Pick, + { id, instanceId, status, expiresAt }: Pick, ): ReturnType['updateOne']>; findByOtherInstanceIds(instanceIds: string[], options?: FindOptions): FindCursor; removeConnectionsFromOtherInstanceIds(instanceIds: string[]): ReturnType['updateMany']>; diff --git a/packages/models/src/models/UsersSessions.ts b/packages/models/src/models/UsersSessions.ts index 02cfd889eb277..cdb8167913bf0 100644 --- a/packages/models/src/models/UsersSessions.ts +++ b/packages/models/src/models/UsersSessions.ts @@ -90,6 +90,22 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes ); } + async renewConnectionByConnectionId(connectionId: string, expiresAt: Date): ReturnType['updateMany']> { + const result = await this.updateMany( + { + 'connections.id': connectionId, + }, + { + $set: { + 'connections.$.expiresAt': expiresAt, + 'connections.$._updatedAt': new Date(), + }, + }, + ); + + return result; + } + findByInstanceId(instanceId: string): FindCursor { return this.find({ 'connections.instanceId': instanceId, @@ -98,7 +114,7 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes addConnectionById( userId: string, - { id, instanceId, status }: Pick, + { id, instanceId, status, expiresAt }: Pick, ): ReturnType['updateOne']> { const now = new Date(); @@ -108,6 +124,7 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes id, instanceId, status, + expiresAt, _createdAt: now, _updatedAt: now, }, From b2818bdeae92c02beae2c71edd7d274d2bcd4442 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Tue, 18 Nov 2025 16:47:14 -0300 Subject: [PATCH 02/23] fix: renew with ddp over rest --- apps/meteor/client/meteor/overrides/ddpOverREST.ts | 4 ++++ apps/meteor/definition/externals/meteor/meteor.d.ts | 12 +++++++++++- apps/meteor/server/methods/userPresence.ts | 8 ++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/apps/meteor/client/meteor/overrides/ddpOverREST.ts b/apps/meteor/client/meteor/overrides/ddpOverREST.ts index 305b9d6b002a6..8e57c06dcf14e 100644 --- a/apps/meteor/client/meteor/overrides/ddpOverREST.ts +++ b/apps/meteor/client/meteor/overrides/ddpOverREST.ts @@ -29,6 +29,10 @@ const shouldBypass = ({ msg, method, params }: Meteor.IDDPMessage): boolean => { const withDDPOverREST = (_send: (this: Meteor.IMeteorConnection, message: Meteor.IDDPMessage, ...args: unknown[]) => void) => { return function _sendOverREST(this: Meteor.IMeteorConnection, message: Meteor.IDDPMessage, ...args: unknown[]): void { if (shouldBypass(message)) { + if (message.msg === 'ping') { + sdk.call('UserPresence:renew'); + } + return _send.call(this, message, ...args); } diff --git a/apps/meteor/definition/externals/meteor/meteor.d.ts b/apps/meteor/definition/externals/meteor/meteor.d.ts index cab110392c12c..43f31c1a23a57 100644 --- a/apps/meteor/definition/externals/meteor/meteor.d.ts +++ b/apps/meteor/definition/externals/meteor/meteor.d.ts @@ -48,7 +48,7 @@ declare module 'meteor/meteor' { } interface IDDPMessage { - msg: 'method'; + msg: 'method' | 'ping'; method: string; params: EJSON[]; id: string; @@ -65,6 +65,16 @@ declare module 'meteor/meteor' { clientAddress: string; _send(message: IDDPMessage): void; + _heartbeat: { + heartbeatInterval: number; + heartbeatTimeout: number; + _heartbeatIntervalHandle: number; + _heartbeatTimeoutHandle: number | null; + _onTimeout(): void; + _seenPacket: boolean; + _sendPing(): void; + }; + _methodInvokers: Record; _livedata_data(message: IDDPUpdatedMessage): void; diff --git a/apps/meteor/server/methods/userPresence.ts b/apps/meteor/server/methods/userPresence.ts index 21f3f795155ea..de9bf1e399c57 100644 --- a/apps/meteor/server/methods/userPresence.ts +++ b/apps/meteor/server/methods/userPresence.ts @@ -9,6 +9,7 @@ declare module '@rocket.chat/ddp-client' { 'UserPresence:setDefaultStatus'(status: UserStatus): boolean | undefined; 'UserPresence:online'(): boolean | undefined; 'UserPresence:away'(): boolean | undefined; + 'UserPresence:renew'(): void | undefined; } } @@ -34,4 +35,11 @@ Meteor.methods({ } return Presence.setConnectionStatus(userId, UserStatus.AWAY, connection.id); }, + 'UserPresence:renew'() { + const { connection } = this; + if (!connection) { + return; + } + return Presence.renewConnection(connection.id); + }, }); From 2c79e929502f3d462f347b6a8058f3a24547fdbb Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Tue, 18 Nov 2025 16:57:52 -0300 Subject: [PATCH 03/23] chore: improve types --- .../client/meteor/overrides/ddpOverREST.ts | 7 ++++--- .../definition/externals/meteor/meteor.d.ts | 20 ++++++++----------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/apps/meteor/client/meteor/overrides/ddpOverREST.ts b/apps/meteor/client/meteor/overrides/ddpOverREST.ts index 8e57c06dcf14e..54ebd8881febf 100644 --- a/apps/meteor/client/meteor/overrides/ddpOverREST.ts +++ b/apps/meteor/client/meteor/overrides/ddpOverREST.ts @@ -6,11 +6,13 @@ import { getUserId } from '../../lib/user'; const bypassMethods: string[] = ['setUserStatus', 'logout']; -const shouldBypass = ({ msg, method, params }: Meteor.IDDPMessage): boolean => { - if (msg !== 'method') { +const shouldBypass = (message: Meteor.IDDPMessage): message is Exclude => { + if (message.msg !== 'method') { return true; } + const { method, params } = message; + if (method === 'login' && params[0]?.resume) { return true; } @@ -32,7 +34,6 @@ const withDDPOverREST = (_send: (this: Meteor.IMeteorConnection, message: Meteor if (message.msg === 'ping') { sdk.call('UserPresence:renew'); } - return _send.call(this, message, ...args); } diff --git a/apps/meteor/definition/externals/meteor/meteor.d.ts b/apps/meteor/definition/externals/meteor/meteor.d.ts index 43f31c1a23a57..1fa8987cc52f1 100644 --- a/apps/meteor/definition/externals/meteor/meteor.d.ts +++ b/apps/meteor/definition/externals/meteor/meteor.d.ts @@ -47,13 +47,19 @@ declare module 'meteor/meteor' { twoFactorChecked: boolean | undefined; } - interface IDDPMessage { - msg: 'method' | 'ping'; + interface IDDPPingMessage { + msg: 'ping'; + } + + interface IDDPMethodMessage { + msg: 'method'; method: string; params: EJSON[]; id: string; } + type IDDPMessage = IDDPPingMessage | IDDPMethodMessage; + interface IDDPUpdatedMessage { msg: 'updated'; methods: string[]; @@ -65,16 +71,6 @@ declare module 'meteor/meteor' { clientAddress: string; _send(message: IDDPMessage): void; - _heartbeat: { - heartbeatInterval: number; - heartbeatTimeout: number; - _heartbeatIntervalHandle: number; - _heartbeatTimeoutHandle: number | null; - _onTimeout(): void; - _seenPacket: boolean; - _sendPing(): void; - }; - _methodInvokers: Record; _livedata_data(message: IDDPUpdatedMessage): void; From cb85c50cd30223c2341bb7698f7a0232c208def3 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Wed, 19 Nov 2025 13:53:27 -0300 Subject: [PATCH 04/23] refactor: take userId in renewConnection --- apps/meteor/server/methods/userPresence.ts | 6 ++-- ee/apps/ddp-streamer/src/DDPStreamer.ts | 7 ++-- ee/packages/presence/src/Presence.ts | 7 ++-- .../src/lib/processConnectionStatus.ts | 3 +- packages/core-services/src/types/IPresence.ts | 2 +- .../src/models/IUsersSessionsModel.ts | 2 +- packages/models/src/models/UsersSessions.ts | 32 +++++++++---------- 7 files changed, 30 insertions(+), 29 deletions(-) diff --git a/apps/meteor/server/methods/userPresence.ts b/apps/meteor/server/methods/userPresence.ts index de9bf1e399c57..ca53b9e9d2762 100644 --- a/apps/meteor/server/methods/userPresence.ts +++ b/apps/meteor/server/methods/userPresence.ts @@ -36,10 +36,10 @@ Meteor.methods({ return Presence.setConnectionStatus(userId, UserStatus.AWAY, connection.id); }, 'UserPresence:renew'() { - const { connection } = this; - if (!connection) { + const { connection, userId } = this; + if (!userId || !connection) { return; } - return Presence.renewConnection(connection.id); + return Presence.renewConnection(userId, connection.id); }, }); diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index f57c81cb0b724..5c325a0126586 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -217,8 +217,11 @@ export class DDPStreamer extends ServiceClass { }); server.on(DDP_EVENTS.PING, (client: Client): void => { - const { connection } = client; - Presence.renewConnection(connection.id); + const { connection, userId } = client; + if (!userId) { + return; + } + Presence.renewConnection(userId, connection.id); }); } diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index dc27e893aa96a..5f76a7a936d27 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -181,11 +181,8 @@ export class Presence extends ServiceClass implements IPresence { return affectedUsers.map(({ _id }) => _id); } - async renewConnection(connectionId: string): Promise { - const result = await UsersSessions.renewConnectionByConnectionId(connectionId, new Date(Date.now() + 5 * 60 * 1000)); - if (result.modifiedCount === 0) { - throw new Error('Connection not found'); - } + async renewConnection(uid: string, connectionId: string): Promise { + await UsersSessions.renewConnectionStatusById(uid, connectionId, new Date(Date.now() + 5 * 60 * 1000)); } async setStatus(uid: string, statusDefault: UserStatus, statusText?: string): Promise { diff --git a/ee/packages/presence/src/lib/processConnectionStatus.ts b/ee/packages/presence/src/lib/processConnectionStatus.ts index 8c2dd866e148f..0da737ed41ef2 100644 --- a/ee/packages/presence/src/lib/processConnectionStatus.ts +++ b/ee/packages/presence/src/lib/processConnectionStatus.ts @@ -35,9 +35,10 @@ export const processStatus = (statusConnection: UserStatus, statusDefault: UserS export const processPresenceAndStatus = ( userSessions: IUserSessionConnection[] = [], statusDefault = UserStatus.ONLINE, + currentDate = new Date(), ): { status: UserStatus; statusConnection: UserStatus } => { const statusConnection = userSessions - .filter((s) => s.expiresAt > new Date()) + .filter((s) => s.expiresAt > currentDate) .map((s) => s.status) .reduce(processConnectionStatus, UserStatus.OFFLINE); diff --git a/packages/core-services/src/types/IPresence.ts b/packages/core-services/src/types/IPresence.ts index 43a5f3f20053e..e49890daf0515 100644 --- a/packages/core-services/src/types/IPresence.ts +++ b/packages/core-services/src/types/IPresence.ts @@ -13,7 +13,7 @@ export interface IPresence extends IServiceClass { session: string | undefined, nodeId: string, ): Promise<{ uid: string; session: string } | undefined>; - renewConnection(connectionId: string): Promise; + renewConnection(uid: string, connectionId: string): Promise; removeLostConnections(nodeID: string): Promise; setStatus(uid: string, status: UserStatus, statusText?: string): Promise; setConnectionStatus(uid: string, status: UserStatus, session: string): Promise; diff --git a/packages/model-typings/src/models/IUsersSessionsModel.ts b/packages/model-typings/src/models/IUsersSessionsModel.ts index 3e7e64959964e..8d7c9ce646598 100644 --- a/packages/model-typings/src/models/IUsersSessionsModel.ts +++ b/packages/model-typings/src/models/IUsersSessionsModel.ts @@ -8,7 +8,7 @@ export interface IUsersSessionsModel extends IBaseModel { updateConnectionStatusById(uid: string, connectionId: string, status: string): ReturnType['updateOne']>; removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']>; removeConnectionByConnectionId(connectionId: string): ReturnType['updateMany']>; - renewConnectionByConnectionId(connectionId: string, expiresAt: Date): ReturnType['updateMany']>; + renewConnectionStatusById(uid: string, connectionId: string, expiresAt: Date): ReturnType['updateOne']>; findByInstanceId(instanceId: string): FindCursor; addConnectionById( userId: string, diff --git a/packages/models/src/models/UsersSessions.ts b/packages/models/src/models/UsersSessions.ts index cdb8167913bf0..3c16e5a76e766 100644 --- a/packages/models/src/models/UsersSessions.ts +++ b/packages/models/src/models/UsersSessions.ts @@ -45,6 +45,22 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes return this.updateOne(query, update); } + renewConnectionStatusById(uid: string, connectionId: string, expiresAt: Date): ReturnType['updateOne']> { + const query = { + '_id': uid, + 'connections.id': connectionId, + }; + + const update = { + $set: { + 'connections.$.expiresAt': expiresAt, + 'connections.$._updatedAt': new Date(), + }, + }; + + return this.updateOne(query, update); + } + async removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']> { return this.updateMany( { @@ -90,22 +106,6 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes ); } - async renewConnectionByConnectionId(connectionId: string, expiresAt: Date): ReturnType['updateMany']> { - const result = await this.updateMany( - { - 'connections.id': connectionId, - }, - { - $set: { - 'connections.$.expiresAt': expiresAt, - 'connections.$._updatedAt': new Date(), - }, - }, - ); - - return result; - } - findByInstanceId(instanceId: string): FindCursor { return this.find({ 'connections.instanceId': instanceId, From 818db849969085c91169561f2f6bdebf2d25f1f9 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Wed, 19 Nov 2025 16:06:57 -0300 Subject: [PATCH 05/23] fix: remove expired connections on renew --- packages/models/src/models/UsersSessions.ts | 39 +++++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/models/src/models/UsersSessions.ts b/packages/models/src/models/UsersSessions.ts index 3c16e5a76e766..43b20ce090a1a 100644 --- a/packages/models/src/models/UsersSessions.ts +++ b/packages/models/src/models/UsersSessions.ts @@ -47,16 +47,41 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes renewConnectionStatusById(uid: string, connectionId: string, expiresAt: Date): ReturnType['updateOne']> { const query = { - '_id': uid, - 'connections.id': connectionId, + _id: uid, }; - const update = { - $set: { - 'connections.$.expiresAt': expiresAt, - 'connections.$._updatedAt': new Date(), + const update = [ + { + $set: { + connections: { + $map: { + input: { + $filter: { + input: '$connections', + as: 'connection', + cond: { $gte: ['$$connection.expiresAt', new Date()] }, + }, + }, + as: 'connection', + in: { + $mergeObjects: [ + '$$connection', + { + expiresAt: { + $cond: { + if: { $eq: ['$$connection.id', connectionId] }, + then: expiresAt, + else: '$$connection.expiresAt', + }, + }, + }, + ], + }, + }, + }, + }, }, - }; + ]; return this.updateOne(query, update); } From bcda88d95443832781405bdb4f0a65669b4f8b6b Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 21 Nov 2025 19:04:38 -0300 Subject: [PATCH 06/23] refactor: simplify logic & separate tests --- .../client/meteor/overrides/ddpOverREST.ts | 2 +- apps/meteor/server/methods/userPresence.ts | 10 +- ee/apps/ddp-streamer/src/DDPStreamer.ts | 2 +- ee/apps/ddp-streamer/src/configureServer.ts | 11 +- ee/packages/presence/src/Presence.ts | 9 +- .../src/lib/processConnectionStatus.ts | 37 --- ee/packages/presence/src/lib/processStatus.ts | 16 + .../presence/src/processPresenceAndStatus.ts | 31 ++ .../tests/lib/processConnectionStatus.test.ts | 270 +--------------- .../lib/processPresenceAndStatus.test.ts | 287 ++++++++++++++++++ .../presence/tests/lib/processStatus.test.ts | 30 ++ packages/core-services/src/types/IPresence.ts | 3 +- packages/core-typings/src/IUserSession.ts | 1 - .../src/models/IUsersSessionsModel.ts | 5 +- packages/models/src/models/UsersSessions.ts | 48 +-- 15 files changed, 389 insertions(+), 373 deletions(-) create mode 100644 ee/packages/presence/src/lib/processStatus.ts create mode 100644 ee/packages/presence/src/processPresenceAndStatus.ts create mode 100644 ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts create mode 100644 ee/packages/presence/tests/lib/processStatus.test.ts diff --git a/apps/meteor/client/meteor/overrides/ddpOverREST.ts b/apps/meteor/client/meteor/overrides/ddpOverREST.ts index 54ebd8881febf..55ef7fcc3f7be 100644 --- a/apps/meteor/client/meteor/overrides/ddpOverREST.ts +++ b/apps/meteor/client/meteor/overrides/ddpOverREST.ts @@ -32,7 +32,7 @@ const withDDPOverREST = (_send: (this: Meteor.IMeteorConnection, message: Meteor return function _sendOverREST(this: Meteor.IMeteorConnection, message: Meteor.IDDPMessage, ...args: unknown[]): void { if (shouldBypass(message)) { if (message.msg === 'ping') { - sdk.call('UserPresence:renew'); + sdk.call('UserPresence:ping'); } return _send.call(this, message, ...args); } diff --git a/apps/meteor/server/methods/userPresence.ts b/apps/meteor/server/methods/userPresence.ts index ca53b9e9d2762..b265c0f54a9b2 100644 --- a/apps/meteor/server/methods/userPresence.ts +++ b/apps/meteor/server/methods/userPresence.ts @@ -9,7 +9,7 @@ declare module '@rocket.chat/ddp-client' { 'UserPresence:setDefaultStatus'(status: UserStatus): boolean | undefined; 'UserPresence:online'(): boolean | undefined; 'UserPresence:away'(): boolean | undefined; - 'UserPresence:renew'(): void | undefined; + 'UserPresence:ping'(): boolean | undefined; } } @@ -26,20 +26,20 @@ Meteor.methods({ if (!userId || !connection) { return; } - return Presence.setConnectionStatus(userId, UserStatus.ONLINE, connection.id); + return Presence.setConnectionStatus(userId, connection.id, UserStatus.ONLINE); }, 'UserPresence:away'() { const { userId, connection } = this; if (!userId || !connection) { return; } - return Presence.setConnectionStatus(userId, UserStatus.AWAY, connection.id); + return Presence.setConnectionStatus(userId, connection.id, UserStatus.AWAY); }, - 'UserPresence:renew'() { + 'UserPresence:ping'() { const { connection, userId } = this; if (!userId || !connection) { return; } - return Presence.renewConnection(userId, connection.id); + return Presence.setConnectionStatus(userId, connection.id); }, }); diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index 5c325a0126586..065b04f22cbed 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -221,7 +221,7 @@ export class DDPStreamer extends ServiceClass { if (!userId) { return; } - Presence.renewConnection(userId, connection.id); + Presence.setConnectionStatus(userId, connection.id); }); } diff --git a/ee/apps/ddp-streamer/src/configureServer.ts b/ee/apps/ddp-streamer/src/configureServer.ts index bf9bd15627cfa..2c2d776711cb1 100644 --- a/ee/apps/ddp-streamer/src/configureServer.ts +++ b/ee/apps/ddp-streamer/src/configureServer.ts @@ -121,14 +121,21 @@ server.methods({ if (!userId) { return; } - return Presence.setConnectionStatus(userId, UserStatus.ONLINE, session); + return Presence.setConnectionStatus(userId, session, UserStatus.ONLINE); }, 'UserPresence:away'() { const { userId, session } = this; if (!userId) { return; } - return Presence.setConnectionStatus(userId, UserStatus.AWAY, session); + return Presence.setConnectionStatus(userId, session, UserStatus.AWAY); + }, + 'UserPresence:ping'() { + const { userId, session } = this; + if (!userId) { + return; + } + return Presence.setConnectionStatus(userId, session); }, 'setUserStatus'(status, statusText) { const { userId } = this; diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index 5f76a7a936d27..a6639b3bf1615 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -4,7 +4,7 @@ import type { IUser } from '@rocket.chat/core-typings'; import { UserStatus } from '@rocket.chat/core-typings'; import { Settings, Users, UsersSessions } from '@rocket.chat/models'; -import { processPresenceAndStatus } from './lib/processConnectionStatus'; +import { processPresenceAndStatus } from './processPresenceAndStatus'; const MAX_CONNECTIONS = 200; @@ -128,7 +128,6 @@ export class Presence extends ServiceClass implements IPresence { id: session, instanceId: nodeId, status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() + 5 * 60 * 1000), }); await this.updateUserPresence(uid); @@ -181,10 +180,6 @@ export class Presence extends ServiceClass implements IPresence { return affectedUsers.map(({ _id }) => _id); } - async renewConnection(uid: string, connectionId: string): Promise { - await UsersSessions.renewConnectionStatusById(uid, connectionId, new Date(Date.now() + 5 * 60 * 1000)); - } - async setStatus(uid: string, statusDefault: UserStatus, statusText?: string): Promise { const userSessions = (await UsersSessions.findOneById(uid)) || { connections: [] }; @@ -208,7 +203,7 @@ export class Presence extends ServiceClass implements IPresence { return !!result.modifiedCount; } - async setConnectionStatus(uid: string, status: UserStatus, session: string): Promise { + async setConnectionStatus(uid: string, session: string, status?: UserStatus): Promise { const result = await UsersSessions.updateConnectionStatusById(uid, session, status); await this.updateUserPresence(uid); diff --git a/ee/packages/presence/src/lib/processConnectionStatus.ts b/ee/packages/presence/src/lib/processConnectionStatus.ts index 0da737ed41ef2..43bc77d88e444 100644 --- a/ee/packages/presence/src/lib/processConnectionStatus.ts +++ b/ee/packages/presence/src/lib/processConnectionStatus.ts @@ -1,4 +1,3 @@ -import type { IUserSessionConnection } from '@rocket.chat/core-typings'; import { UserStatus } from '@rocket.chat/core-typings'; /** @@ -13,39 +12,3 @@ export const processConnectionStatus = (current: UserStatus, status: UserStatus) } return current; }; - -/** - * Defines user's status based on presence and connection status - */ -export const processStatus = (statusConnection: UserStatus, statusDefault: UserStatus): UserStatus => { - if (statusConnection === UserStatus.OFFLINE) { - return statusConnection; - } - - if (statusDefault === UserStatus.ONLINE) { - return statusConnection; - } - - return statusDefault; -}; - -/** - * Defines user's status and connection status based on user's connections and default status - */ -export const processPresenceAndStatus = ( - userSessions: IUserSessionConnection[] = [], - statusDefault = UserStatus.ONLINE, - currentDate = new Date(), -): { status: UserStatus; statusConnection: UserStatus } => { - const statusConnection = userSessions - .filter((s) => s.expiresAt > currentDate) - .map((s) => s.status) - .reduce(processConnectionStatus, UserStatus.OFFLINE); - - const status = processStatus(statusConnection, statusDefault); - - return { - status, - statusConnection, - }; -}; diff --git a/ee/packages/presence/src/lib/processStatus.ts b/ee/packages/presence/src/lib/processStatus.ts new file mode 100644 index 0000000000000..1c9583d840434 --- /dev/null +++ b/ee/packages/presence/src/lib/processStatus.ts @@ -0,0 +1,16 @@ +import { UserStatus } from '@rocket.chat/core-typings'; + +/** + * Defines user's status based on presence and connection status + */ +export const processStatus = (statusConnection: UserStatus, statusDefault: UserStatus): UserStatus => { + if (statusConnection === UserStatus.OFFLINE) { + return statusConnection; + } + + if (statusDefault === UserStatus.ONLINE) { + return statusConnection; + } + + return statusDefault; +}; diff --git a/ee/packages/presence/src/processPresenceAndStatus.ts b/ee/packages/presence/src/processPresenceAndStatus.ts new file mode 100644 index 0000000000000..a97849cfdd40b --- /dev/null +++ b/ee/packages/presence/src/processPresenceAndStatus.ts @@ -0,0 +1,31 @@ +import type { IUserSessionConnection } from '@rocket.chat/core-typings'; +import { UserStatus } from '@rocket.chat/core-typings'; + +import { processConnectionStatus } from './lib/processConnectionStatus'; +import { processStatus } from './lib/processStatus'; + +const isAtMostFiveMinutesAgo = (userSession: IUserSessionConnection): boolean => { + const now = Date.now(); + const diff = now - userSession._updatedAt.getTime(); + return diff <= 300_000; // 5 minutes in milliseconds +}; + +/** + * Defines user's status and connection status based on user's connections and default status + */ +export const processPresenceAndStatus = ( + userSessions: IUserSessionConnection[] = [], + statusDefault = UserStatus.ONLINE, +): { status: UserStatus; statusConnection: UserStatus } => { + const statusConnection = userSessions + .filter(isAtMostFiveMinutesAgo) + .map((s) => s.status) + .reduce(processConnectionStatus, UserStatus.OFFLINE); + + const status = processStatus(statusConnection, statusDefault); + + return { + status, + statusConnection, + }; +}; diff --git a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts index 7455d46c06e5d..0d98cca47b8a4 100644 --- a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts +++ b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from '@jest/globals'; import { UserStatus } from '@rocket.chat/core-typings'; -import { processConnectionStatus, processStatus, processPresenceAndStatus } from '../../src/lib/processConnectionStatus'; +import { processConnectionStatus } from '../../src/lib/processConnectionStatus'; describe('Presence micro service', () => { test('should return connection as online when there is a connection online', () => { @@ -16,272 +16,4 @@ describe('Presence micro service', () => { expect(processConnectionStatus(UserStatus.ONLINE, UserStatus.OFFLINE)).toBe(UserStatus.ONLINE); expect(processConnectionStatus(UserStatus.AWAY, UserStatus.OFFLINE)).toBe(UserStatus.AWAY); }); - - test('should return the connection status when the default status is online', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.ONLINE)).toBe(UserStatus.ONLINE); - expect(processStatus(UserStatus.AWAY, UserStatus.ONLINE)).toBe(UserStatus.AWAY); - expect(processStatus(UserStatus.OFFLINE, UserStatus.ONLINE)).toBe(UserStatus.OFFLINE); - }); - - test('should return status busy when the default status is busy', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.BUSY)).toBe(UserStatus.BUSY); - expect(processStatus(UserStatus.AWAY, UserStatus.BUSY)).toBe(UserStatus.BUSY); - expect(processStatus(UserStatus.OFFLINE, UserStatus.BUSY)).toBe(UserStatus.OFFLINE); - }); - - test('should return status away when the default status is away', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.AWAY)).toBe(UserStatus.AWAY); - expect(processStatus(UserStatus.AWAY, UserStatus.AWAY)).toBe(UserStatus.AWAY); - expect(processStatus(UserStatus.OFFLINE, UserStatus.AWAY)).toBe(UserStatus.OFFLINE); - }); - - test('should return status offline when the default status is offline', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); - expect(processStatus(UserStatus.AWAY, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); - expect(processStatus(UserStatus.OFFLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); - }); - - test('should return correct status and statusConnection when connected once', () => { - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.BUSY, - ), - ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.AWAY, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.BUSY, - ), - ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.AWAY }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.OFFLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.OFFLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.AWAY }); - }); - - test('should return correct status and statusConnection when connected twice', () => { - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() + 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); - }); - - test('should return correct status and statusConnection when not connected', () => { - expect(processPresenceAndStatus([], UserStatus.ONLINE)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - - expect(processPresenceAndStatus([], UserStatus.BUSY)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - - expect(processPresenceAndStatus([], UserStatus.AWAY)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - - expect(processPresenceAndStatus([], UserStatus.OFFLINE)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - }); - - test('should return correct status and statusConnection when there are expired connections', () => { - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - expiresAt: new Date(Date.now() - 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - expiresAt: new Date(Date.now() - 1000 * 60), - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); - }); }); diff --git a/ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts b/ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts new file mode 100644 index 0000000000000..cec9e3f435dc2 --- /dev/null +++ b/ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts @@ -0,0 +1,287 @@ +import { describe, test, expect } from '@jest/globals'; +import { UserStatus } from '@rocket.chat/core-typings'; + +import { processPresenceAndStatus } from '../../src/processPresenceAndStatus'; + +describe('processPresenceAndStatus', () => { + test('should return correct status and statusConnection when connected once', () => { + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.BUSY, + ), + ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.AWAY, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.BUSY, + ), + ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.AWAY }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.OFFLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.OFFLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.AWAY }); + }); + + test('should return correct status and statusConnection when connected twice', () => { + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); + }); + + test('should return correct status and statusConnection when not connected', () => { + expect(processPresenceAndStatus([], UserStatus.ONLINE)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + + expect(processPresenceAndStatus([], UserStatus.BUSY)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + + expect(processPresenceAndStatus([], UserStatus.AWAY)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + + expect(processPresenceAndStatus([], UserStatus.OFFLINE)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + }); + + test('should ignore connections last updated more than 5 minutes ago', () => { + const now = new Date(); + const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); + const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); + + expect( + processPresenceAndStatus( + [ + { + id: 'random1', + instanceId: 'random1', + status: UserStatus.ONLINE, + _createdAt: sixMinutesAgo, + _updatedAt: sixMinutesAgo, + }, + { + id: 'random2', + instanceId: 'random2', + status: UserStatus.AWAY, + _createdAt: fourMinutesAgo, + _updatedAt: fourMinutesAgo, + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); + }); + + test('should return offline if all connections are stale', () => { + const now = new Date(); + const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); + const sevenMinutesAgo = new Date(now.getTime() - 7 * 60 * 1000); + + expect( + processPresenceAndStatus( + [ + { + id: 'random1', + instanceId: 'random1', + status: UserStatus.ONLINE, + _createdAt: sixMinutesAgo, + _updatedAt: sixMinutesAgo, + }, + { + id: 'random2', + instanceId: 'random2', + status: UserStatus.AWAY, + _createdAt: sevenMinutesAgo, + _updatedAt: sevenMinutesAgo, + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); + }); + + test('should consider all connections if they were updated within the last 5 minutes', () => { + const now = new Date(); + const threeMinutesAgo = new Date(now.getTime() - 3 * 60 * 1000); + const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); + + expect( + processPresenceAndStatus( + [ + { + id: 'random1', + instanceId: 'random1', + status: UserStatus.ONLINE, + _createdAt: threeMinutesAgo, + _updatedAt: threeMinutesAgo, + }, + { + id: 'random2', + instanceId: 'random2', + status: UserStatus.AWAY, + _createdAt: fourMinutesAgo, + _updatedAt: fourMinutesAgo, + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + }); +}); diff --git a/ee/packages/presence/tests/lib/processStatus.test.ts b/ee/packages/presence/tests/lib/processStatus.test.ts new file mode 100644 index 0000000000000..c9e63a988578a --- /dev/null +++ b/ee/packages/presence/tests/lib/processStatus.test.ts @@ -0,0 +1,30 @@ +import { describe, test, expect } from '@jest/globals'; +import { UserStatus } from '@rocket.chat/core-typings'; + +import { processStatus } from '../../src/lib/processStatus'; + +describe('processStatus', () => { + test('should return the connection status when the default status is online', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.ONLINE)).toBe(UserStatus.ONLINE); + expect(processStatus(UserStatus.AWAY, UserStatus.ONLINE)).toBe(UserStatus.AWAY); + expect(processStatus(UserStatus.OFFLINE, UserStatus.ONLINE)).toBe(UserStatus.OFFLINE); + }); + + test('should return status busy when the default status is busy', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.BUSY)).toBe(UserStatus.BUSY); + expect(processStatus(UserStatus.AWAY, UserStatus.BUSY)).toBe(UserStatus.BUSY); + expect(processStatus(UserStatus.OFFLINE, UserStatus.BUSY)).toBe(UserStatus.OFFLINE); + }); + + test('should return status away when the default status is away', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.AWAY)).toBe(UserStatus.AWAY); + expect(processStatus(UserStatus.AWAY, UserStatus.AWAY)).toBe(UserStatus.AWAY); + expect(processStatus(UserStatus.OFFLINE, UserStatus.AWAY)).toBe(UserStatus.OFFLINE); + }); + + test('should return status offline when the default status is offline', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); + expect(processStatus(UserStatus.AWAY, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); + expect(processStatus(UserStatus.OFFLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); + }); +}); diff --git a/packages/core-services/src/types/IPresence.ts b/packages/core-services/src/types/IPresence.ts index e49890daf0515..fb17a3bf3549e 100644 --- a/packages/core-services/src/types/IPresence.ts +++ b/packages/core-services/src/types/IPresence.ts @@ -13,10 +13,9 @@ export interface IPresence extends IServiceClass { session: string | undefined, nodeId: string, ): Promise<{ uid: string; session: string } | undefined>; - renewConnection(uid: string, connectionId: string): Promise; removeLostConnections(nodeID: string): Promise; setStatus(uid: string, status: UserStatus, statusText?: string): Promise; - setConnectionStatus(uid: string, status: UserStatus, session: string): Promise; + setConnectionStatus(uid: string, session: string, status?: UserStatus): Promise; updateUserPresence(uid: string): Promise; toggleBroadcast(enabled: boolean): void; getConnectionCount(): { current: number; max: number }; diff --git a/packages/core-typings/src/IUserSession.ts b/packages/core-typings/src/IUserSession.ts index b1f7f0f9beb72..48844fab6e278 100644 --- a/packages/core-typings/src/IUserSession.ts +++ b/packages/core-typings/src/IUserSession.ts @@ -4,7 +4,6 @@ export interface IUserSessionConnection { id: string; instanceId: string; status: UserStatus; - expiresAt: Date; _createdAt: Date; _updatedAt: Date; } diff --git a/packages/model-typings/src/models/IUsersSessionsModel.ts b/packages/model-typings/src/models/IUsersSessionsModel.ts index 8d7c9ce646598..03f1f03b78be5 100644 --- a/packages/model-typings/src/models/IUsersSessionsModel.ts +++ b/packages/model-typings/src/models/IUsersSessionsModel.ts @@ -5,14 +5,13 @@ import type { IBaseModel } from './IBaseModel'; export interface IUsersSessionsModel extends IBaseModel { clearConnectionsFromInstanceId(instanceId: string[]): ReturnType['updateMany']>; - updateConnectionStatusById(uid: string, connectionId: string, status: string): ReturnType['updateOne']>; + updateConnectionStatusById(uid: string, connectionId: string, status?: string): ReturnType['updateOne']>; removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']>; removeConnectionByConnectionId(connectionId: string): ReturnType['updateMany']>; - renewConnectionStatusById(uid: string, connectionId: string, expiresAt: Date): ReturnType['updateOne']>; findByInstanceId(instanceId: string): FindCursor; addConnectionById( userId: string, - { id, instanceId, status, expiresAt }: Pick, + { id, instanceId, status }: Pick, ): ReturnType['updateOne']>; findByOtherInstanceIds(instanceIds: string[], options?: FindOptions): FindCursor; removeConnectionsFromOtherInstanceIds(instanceIds: string[]): ReturnType['updateMany']>; diff --git a/packages/models/src/models/UsersSessions.ts b/packages/models/src/models/UsersSessions.ts index 43b20ce090a1a..acd8104197c38 100644 --- a/packages/models/src/models/UsersSessions.ts +++ b/packages/models/src/models/UsersSessions.ts @@ -29,7 +29,7 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes ); } - updateConnectionStatusById(uid: string, connectionId: string, status: string): ReturnType['updateOne']> { + updateConnectionStatusById(uid: string, connectionId: string, status?: string): ReturnType['updateOne']> { const query = { '_id': uid, 'connections.id': connectionId, @@ -37,7 +37,7 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes const update = { $set: { - 'connections.$.status': status, + ...(status && { 'connections.$.status': status }), 'connections.$._updatedAt': new Date(), }, }; @@ -45,47 +45,6 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes return this.updateOne(query, update); } - renewConnectionStatusById(uid: string, connectionId: string, expiresAt: Date): ReturnType['updateOne']> { - const query = { - _id: uid, - }; - - const update = [ - { - $set: { - connections: { - $map: { - input: { - $filter: { - input: '$connections', - as: 'connection', - cond: { $gte: ['$$connection.expiresAt', new Date()] }, - }, - }, - as: 'connection', - in: { - $mergeObjects: [ - '$$connection', - { - expiresAt: { - $cond: { - if: { $eq: ['$$connection.id', connectionId] }, - then: expiresAt, - else: '$$connection.expiresAt', - }, - }, - }, - ], - }, - }, - }, - }, - }, - ]; - - return this.updateOne(query, update); - } - async removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']> { return this.updateMany( { @@ -139,7 +98,7 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes addConnectionById( userId: string, - { id, instanceId, status, expiresAt }: Pick, + { id, instanceId, status }: Pick, ): ReturnType['updateOne']> { const now = new Date(); @@ -149,7 +108,6 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes id, instanceId, status, - expiresAt, _createdAt: now, _updatedAt: now, }, From 926642be40bfd08f81fbcaa65385f23fb596c68a Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Mon, 24 Nov 2025 10:30:19 -0300 Subject: [PATCH 07/23] chore: add changeset Fixes user status inaccuracy by refreshing active connections and filtering out the stale ones. --- .changeset/spicy-nails-design.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/spicy-nails-design.md diff --git a/.changeset/spicy-nails-design.md b/.changeset/spicy-nails-design.md new file mode 100644 index 0000000000000..0388b121763d1 --- /dev/null +++ b/.changeset/spicy-nails-design.md @@ -0,0 +1,10 @@ +--- +"@rocket.chat/meteor": patch +"@rocket.chat/core-services": patch +"@rocket.chat/model-typings": patch +"@rocket.chat/models": patch +"@rocket.chat/ddp-streamer": patch +"@rocket.chat/presence": patch +--- + +Fixes user status inaccuracy by refreshing active connections and filtering out the stale ones. From 6bde2cf0a3e09d54a1bbe1f00317630899318d55 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 28 Nov 2025 18:54:44 -0300 Subject: [PATCH 08/23] refactor: remove UserPresence:ping --- apps/meteor/client/meteor/overrides/ddpOverREST.ts | 9 ++------- apps/meteor/definition/externals/meteor/meteor.d.ts | 12 ++++-------- apps/meteor/ee/server/startup/presence.ts | 7 +++++++ apps/meteor/server/methods/userPresence.ts | 8 -------- ee/apps/ddp-streamer/src/configureServer.ts | 7 ------- 5 files changed, 13 insertions(+), 30 deletions(-) diff --git a/apps/meteor/client/meteor/overrides/ddpOverREST.ts b/apps/meteor/client/meteor/overrides/ddpOverREST.ts index 55ef7fcc3f7be..305b9d6b002a6 100644 --- a/apps/meteor/client/meteor/overrides/ddpOverREST.ts +++ b/apps/meteor/client/meteor/overrides/ddpOverREST.ts @@ -6,13 +6,11 @@ import { getUserId } from '../../lib/user'; const bypassMethods: string[] = ['setUserStatus', 'logout']; -const shouldBypass = (message: Meteor.IDDPMessage): message is Exclude => { - if (message.msg !== 'method') { +const shouldBypass = ({ msg, method, params }: Meteor.IDDPMessage): boolean => { + if (msg !== 'method') { return true; } - const { method, params } = message; - if (method === 'login' && params[0]?.resume) { return true; } @@ -31,9 +29,6 @@ const shouldBypass = (message: Meteor.IDDPMessage): message is Exclude void) => { return function _sendOverREST(this: Meteor.IMeteorConnection, message: Meteor.IDDPMessage, ...args: unknown[]): void { if (shouldBypass(message)) { - if (message.msg === 'ping') { - sdk.call('UserPresence:ping'); - } return _send.call(this, message, ...args); } diff --git a/apps/meteor/definition/externals/meteor/meteor.d.ts b/apps/meteor/definition/externals/meteor/meteor.d.ts index 1fa8987cc52f1..c0d3e599731fe 100644 --- a/apps/meteor/definition/externals/meteor/meteor.d.ts +++ b/apps/meteor/definition/externals/meteor/meteor.d.ts @@ -39,7 +39,9 @@ declare module 'meteor/meteor' { isDesktop: () => boolean; } - const server: any; + const server: { + sessions: Map void } }>; + }; const runAsUser: (userId: string, scope: () => T) => T; @@ -47,19 +49,13 @@ declare module 'meteor/meteor' { twoFactorChecked: boolean | undefined; } - interface IDDPPingMessage { - msg: 'ping'; - } - - interface IDDPMethodMessage { + interface IDDPMessage { msg: 'method'; method: string; params: EJSON[]; id: string; } - type IDDPMessage = IDDPPingMessage | IDDPMethodMessage; - interface IDDPUpdatedMessage { msg: 'updated'; methods: string[]; diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index e0756e4b4c59d..a762faeba4fce 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -40,6 +40,13 @@ Meteor.startup(() => { return; } + const originalSendPing = session.heartbeat._sendPing.bind(session.heartbeat); + + session.heartbeat._sendPing = function () { + originalSendPing(); + void Presence.setConnectionStatus(login.user._id, login.connection.id); + }; + void (async function () { await Presence.newConnection(login.user._id, login.connection.id, nodeId); updateConns(); diff --git a/apps/meteor/server/methods/userPresence.ts b/apps/meteor/server/methods/userPresence.ts index b265c0f54a9b2..b7a32df6a294c 100644 --- a/apps/meteor/server/methods/userPresence.ts +++ b/apps/meteor/server/methods/userPresence.ts @@ -9,7 +9,6 @@ declare module '@rocket.chat/ddp-client' { 'UserPresence:setDefaultStatus'(status: UserStatus): boolean | undefined; 'UserPresence:online'(): boolean | undefined; 'UserPresence:away'(): boolean | undefined; - 'UserPresence:ping'(): boolean | undefined; } } @@ -35,11 +34,4 @@ Meteor.methods({ } return Presence.setConnectionStatus(userId, connection.id, UserStatus.AWAY); }, - 'UserPresence:ping'() { - const { connection, userId } = this; - if (!userId || !connection) { - return; - } - return Presence.setConnectionStatus(userId, connection.id); - }, }); diff --git a/ee/apps/ddp-streamer/src/configureServer.ts b/ee/apps/ddp-streamer/src/configureServer.ts index 2c2d776711cb1..c22c303c46c7c 100644 --- a/ee/apps/ddp-streamer/src/configureServer.ts +++ b/ee/apps/ddp-streamer/src/configureServer.ts @@ -130,13 +130,6 @@ server.methods({ } return Presence.setConnectionStatus(userId, session, UserStatus.AWAY); }, - 'UserPresence:ping'() { - const { userId, session } = this; - if (!userId) { - return; - } - return Presence.setConnectionStatus(userId, session); - }, 'setUserStatus'(status, statusText) { const { userId } = this; if (!userId) { From d842b11552f0c181391b6d2030279da1ef057cce Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Mon, 1 Dec 2025 10:48:08 -0300 Subject: [PATCH 09/23] fix: meteor types --- apps/meteor/definition/externals/meteor/meteor.d.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/meteor/definition/externals/meteor/meteor.d.ts b/apps/meteor/definition/externals/meteor/meteor.d.ts index c0d3e599731fe..68021c40a4426 100644 --- a/apps/meteor/definition/externals/meteor/meteor.d.ts +++ b/apps/meteor/definition/externals/meteor/meteor.d.ts @@ -41,6 +41,9 @@ declare module 'meteor/meteor' { const server: { sessions: Map void } }>; + publish_handlers: { + meteor_autoupdate_clientVersions(): void; + }; }; const runAsUser: (userId: string, scope: () => T) => T; From 992a691e538f698e621add9e1c048d4de8bbf813 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Mon, 1 Dec 2025 13:40:02 -0300 Subject: [PATCH 10/23] chore: updateConnectionStatus throttle --- .../externals/meteor/ddp-common.d.ts | 64 +++++++++++++++++++ .../definition/externals/meteor/meteor.d.ts | 4 +- apps/meteor/ee/server/startup/presence.ts | 27 ++++++-- 3 files changed, 87 insertions(+), 8 deletions(-) diff --git a/apps/meteor/definition/externals/meteor/ddp-common.d.ts b/apps/meteor/definition/externals/meteor/ddp-common.d.ts index 3a7f1538ab407..cbe96a9722968 100644 --- a/apps/meteor/definition/externals/meteor/ddp-common.d.ts +++ b/apps/meteor/definition/externals/meteor/ddp-common.d.ts @@ -14,5 +14,69 @@ declare module 'meteor/ddp-common' { userId?: string; }); } + + /** + * Heartbeat options + */ + type HeartbeatOptions = { + /** + * interval to send pings, in milliseconds + */ + heartbeatInterval: number; + /** + * timeout to close the connection if a reply isn't received, in milliseconds. + */ + heartbeatTimeout: number; + /** + * function to call to send a ping on the connection. + */ + sendPing: () => void; + /** + * function to call to close the connection + */ + onTimeout: () => void; + }; + + class Heartbeat { + heartbeatInterval: number; + + heartbeatTimeout: number; + + _sendPing: () => void; + + _onTimeout: () => void; + + _seenPacket: boolean; + + _heartbeatIntervalHandle: ReturnType | null; + + _heartbeatTimeoutHandle: ReturnType | null; + + constructor(options: HeartbeatOptions); + + stop(): void; + + start(): void; + + _startHeartbeatIntervalTimer(): void; + + _startHeartbeatTimeoutTimer(): void; + + _clearHeartbeatIntervalTimer(): void; + + _clearHeartbeatTimeoutTimer(): void; + + /** + * The heartbeat interval timer is fired when we should send a ping. + */ + _heartbeatIntervalFired(): void; + + /** + * The heartbeat timeout timer is fired when we sent a ping, but we timed out waiting for the pong. + */ + _heartbeatTimeoutFired(): void; + + messageReceived(): void; + } } } diff --git a/apps/meteor/definition/externals/meteor/meteor.d.ts b/apps/meteor/definition/externals/meteor/meteor.d.ts index 68021c40a4426..e93445a891a36 100644 --- a/apps/meteor/definition/externals/meteor/meteor.d.ts +++ b/apps/meteor/definition/externals/meteor/meteor.d.ts @@ -1,6 +1,6 @@ import 'meteor/meteor'; import type { ServerMethods } from '@rocket.chat/ddp-client'; -import type { IStreamerConstructor, IStreamer } from 'meteor/rocketchat:streamer'; +import type { DDPCommon, IStreamerConstructor, IStreamer } from 'meteor/ddp-common'; type StringifyBuffers = { [P in keyof T]: T[P] extends Buffer ? string : T[P]; @@ -40,7 +40,7 @@ declare module 'meteor/meteor' { } const server: { - sessions: Map void } }>; + sessions: Map; publish_handlers: { meteor_autoupdate_clientVersions(): void; }; diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index a762faeba4fce..b25afb76e7c58 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -4,6 +4,22 @@ import { Accounts } from 'meteor/accounts-base'; import { Meteor } from 'meteor/meteor'; import { throttle } from 'underscore'; +const lastWrite = new Map(); + +// throttle presence updates to avoid excessive writes +const updateConnectionStatus = (userId: string, connectionId: string) => { + const key = `${userId}-${connectionId}`; + const now = Date.now(); + const last = lastWrite.get(key) || 0; + + if (now - last > 60000) { + lastWrite.set(key, now); + return Presence.setConnectionStatus(userId, connectionId); + } + + return Promise.resolve(); +}; + // update connections count every 30 seconds const updateConns = throttle(function _updateConns() { void InstanceStatus.updateConnections(Meteor.server.sessions.size); @@ -29,7 +45,7 @@ Meteor.startup(() => { await Presence.removeLostConnections(nodeId); }); - Accounts.onLogin((login: any): void => { + Accounts.onLogin((login: { connection: { id: string }; user: { _id: string } }): void => { if (!login.connection.id) { return; } @@ -40,11 +56,10 @@ Meteor.startup(() => { return; } - const originalSendPing = session.heartbeat._sendPing.bind(session.heartbeat); - - session.heartbeat._sendPing = function () { - originalSendPing(); - void Presence.setConnectionStatus(login.user._id, login.connection.id); + const _messageReceived = session.heartbeat.messageReceived.bind(session.heartbeat); + session.heartbeat.messageReceived = function messageReceived() { + void updateConnectionStatus(login.user._id, login.connection.id); + return _messageReceived(); }; void (async function () { From 7d3d039d28c9796d0ee33a6e2582fcd0337f851e Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Tue, 2 Dec 2025 18:38:11 -0300 Subject: [PATCH 11/23] chore: normalize monolith & microservice behavior --- apps/meteor/ee/server/startup/presence.ts | 23 +++++++++++++-------- ee/apps/ddp-streamer/src/Client.ts | 1 + ee/apps/ddp-streamer/src/DDPStreamer.ts | 25 +++++++++++++++++++++-- ee/apps/ddp-streamer/src/constants.ts | 1 + 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index b25afb76e7c58..d88aef6f0ad03 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -4,20 +4,25 @@ import { Accounts } from 'meteor/accounts-base'; import { Meteor } from 'meteor/meteor'; import { throttle } from 'underscore'; -const lastWrite = new Map(); +const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; +const lastConnectionStatusUpdate = new Map(); -// throttle presence updates to avoid excessive writes -const updateConnectionStatus = (userId: string, connectionId: string) => { +const shouldUpdateConnectionStatus = (userId: string, connectionId: string): boolean => { const key = `${userId}-${connectionId}`; const now = Date.now(); - const last = lastWrite.get(key) || 0; - - if (now - last > 60000) { - lastWrite.set(key, now); - return Presence.setConnectionStatus(userId, connectionId); + const last = lastConnectionStatusUpdate.get(key) ?? 0; + if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { + return false; } + lastConnectionStatusUpdate.set(key, now); + return true; +}; - return Promise.resolve(); +const updateConnectionStatus = async (userId: string, connectionId: string): Promise => { + if (!shouldUpdateConnectionStatus(userId, connectionId)) { + return; + } + await Presence.setConnectionStatus(userId, connectionId); }; // update connections count every 30 seconds diff --git a/ee/apps/ddp-streamer/src/Client.ts b/ee/apps/ddp-streamer/src/Client.ts index fb75af64b3da0..661c1d2931125 100644 --- a/ee/apps/ddp-streamer/src/Client.ts +++ b/ee/apps/ddp-streamer/src/Client.ts @@ -201,6 +201,7 @@ export class Client extends EventEmitter { try { const packet = server.parse(payload, isBinary); this.emit('message', packet); + server.emit(DDP_EVENTS.MESSAGE, this); if (this.wait) { return new Promise((resolve) => this.once(DDP_EVENTS.LOGGED, () => resolve(this.process(packet.msg, packet)))); } diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index 065b04f22cbed..d510c53f343f5 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -18,6 +18,27 @@ import { StreamerCentral } from '../../../../apps/meteor/server/modules/streamer const { PORT = 4000 } = process.env; +const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; +const lastConnectionStatusUpdate = new Map(); + +const shouldUpdateConnectionStatus = (userId: string, connectionId: string): boolean => { + const key = `${userId}-${connectionId}`; + const now = Date.now(); + const last = lastConnectionStatusUpdate.get(key) ?? 0; + if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { + return false; + } + lastConnectionStatusUpdate.set(key, now); + return true; +}; + +const updateConnectionStatus = async (userId: string, connectionId: string): Promise => { + if (!shouldUpdateConnectionStatus(userId, connectionId)) { + return; + } + await Presence.setConnectionStatus(userId, connectionId); +}; + export class DDPStreamer extends ServiceClass { protected name = 'streamer'; @@ -216,12 +237,12 @@ export class DDPStreamer extends ServiceClass { this.api?.broadcast('socket.connected', connection); }); - server.on(DDP_EVENTS.PING, (client: Client): void => { + server.on(DDP_EVENTS.MESSAGE, (client: Client): void => { const { connection, userId } = client; if (!userId) { return; } - Presence.setConnectionStatus(userId, connection.id); + void updateConnectionStatus(userId, connection.id); }); } diff --git a/ee/apps/ddp-streamer/src/constants.ts b/ee/apps/ddp-streamer/src/constants.ts index 398ae16ce5d9a..23dd4d9e563ec 100644 --- a/ee/apps/ddp-streamer/src/constants.ts +++ b/ee/apps/ddp-streamer/src/constants.ts @@ -16,6 +16,7 @@ export const DDP_EVENTS = { CHANGED: 'changed', REMOVED: 'removed', RESUME: 'resume', + MESSAGE: 'message', RESULT: 'result', METHOD: 'method', UPDATED: 'updated', From 628b508c676bd345271a6bdcd0f80c46687adcfb Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Wed, 3 Dec 2025 11:43:33 -0300 Subject: [PATCH 12/23] fix: clean up throttle tracking for connections --- apps/meteor/ee/server/startup/presence.ts | 11 ++++++----- ee/apps/ddp-streamer/src/DDPStreamer.ts | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index d88aef6f0ad03..2e928d2e86b2e 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -7,19 +7,18 @@ import { throttle } from 'underscore'; const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; const lastConnectionStatusUpdate = new Map(); -const shouldUpdateConnectionStatus = (userId: string, connectionId: string): boolean => { - const key = `${userId}-${connectionId}`; +const shouldUpdateConnectionStatus = (connectionId: string): boolean => { const now = Date.now(); - const last = lastConnectionStatusUpdate.get(key) ?? 0; + const last = lastConnectionStatusUpdate.get(connectionId) ?? 0; if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { return false; } - lastConnectionStatusUpdate.set(key, now); + lastConnectionStatusUpdate.set(connectionId, now); return true; }; const updateConnectionStatus = async (userId: string, connectionId: string): Promise => { - if (!shouldUpdateConnectionStatus(userId, connectionId)) { + if (!shouldUpdateConnectionStatus(connectionId)) { return; } await Presence.setConnectionStatus(userId, connectionId); @@ -41,6 +40,7 @@ Meteor.startup(() => { return; } + lastConnectionStatusUpdate.delete(connection.id); await Presence.removeConnection(session.userId, connection.id, nodeId); updateConns(); }); @@ -74,6 +74,7 @@ Meteor.startup(() => { }); Accounts.onLogout((login): void => { + lastConnectionStatusUpdate.delete(login.connection.id); void Presence.removeConnection(login.user?._id, login.connection.id, nodeId); updateConns(); diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index d510c53f343f5..95584196f3a84 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -21,19 +21,18 @@ const { PORT = 4000 } = process.env; const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; const lastConnectionStatusUpdate = new Map(); -const shouldUpdateConnectionStatus = (userId: string, connectionId: string): boolean => { - const key = `${userId}-${connectionId}`; +const shouldUpdateConnectionStatus = (connectionId: string): boolean => { const now = Date.now(); - const last = lastConnectionStatusUpdate.get(key) ?? 0; + const last = lastConnectionStatusUpdate.get(connectionId) ?? 0; if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { return false; } - lastConnectionStatusUpdate.set(key, now); + lastConnectionStatusUpdate.set(connectionId, now); return true; }; const updateConnectionStatus = async (userId: string, connectionId: string): Promise => { - if (!shouldUpdateConnectionStatus(userId, connectionId)) { + if (!shouldUpdateConnectionStatus(connectionId)) { return; } await Presence.setConnectionStatus(userId, connectionId); @@ -217,6 +216,7 @@ export class DDPStreamer extends ServiceClass { if (!userId) { return; } + lastConnectionStatusUpdate.delete(connection.id); Presence.removeConnection(userId, connection.id, nodeID); }); @@ -230,6 +230,7 @@ export class DDPStreamer extends ServiceClass { if (!userId) { return; } + lastConnectionStatusUpdate.delete(connection.id); Presence.removeConnection(userId, connection.id, nodeID); }); From a65f79b7c24c61cf1e813bd03af0694a963eecae Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Wed, 3 Dec 2025 17:08:10 -0300 Subject: [PATCH 13/23] fix: periodically remove stale connections --- apps/meteor/ee/server/startup/presence.ts | 24 +-------- ee/apps/ddp-streamer/src/DDPStreamer.ts | 24 +-------- ee/packages/presence/src/Presence.ts | 61 +++++++++++++++++++++-- 3 files changed, 60 insertions(+), 49 deletions(-) diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index 2e928d2e86b2e..2ce1dc491a927 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -4,26 +4,6 @@ import { Accounts } from 'meteor/accounts-base'; import { Meteor } from 'meteor/meteor'; import { throttle } from 'underscore'; -const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; -const lastConnectionStatusUpdate = new Map(); - -const shouldUpdateConnectionStatus = (connectionId: string): boolean => { - const now = Date.now(); - const last = lastConnectionStatusUpdate.get(connectionId) ?? 0; - if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { - return false; - } - lastConnectionStatusUpdate.set(connectionId, now); - return true; -}; - -const updateConnectionStatus = async (userId: string, connectionId: string): Promise => { - if (!shouldUpdateConnectionStatus(connectionId)) { - return; - } - await Presence.setConnectionStatus(userId, connectionId); -}; - // update connections count every 30 seconds const updateConns = throttle(function _updateConns() { void InstanceStatus.updateConnections(Meteor.server.sessions.size); @@ -40,7 +20,6 @@ Meteor.startup(() => { return; } - lastConnectionStatusUpdate.delete(connection.id); await Presence.removeConnection(session.userId, connection.id, nodeId); updateConns(); }); @@ -63,7 +42,7 @@ Meteor.startup(() => { const _messageReceived = session.heartbeat.messageReceived.bind(session.heartbeat); session.heartbeat.messageReceived = function messageReceived() { - void updateConnectionStatus(login.user._id, login.connection.id); + void Presence.setConnectionStatus(login.user._id, login.connection.id); return _messageReceived(); }; @@ -74,7 +53,6 @@ Meteor.startup(() => { }); Accounts.onLogout((login): void => { - lastConnectionStatusUpdate.delete(login.connection.id); void Presence.removeConnection(login.user?._id, login.connection.id, nodeId); updateConns(); diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index 95584196f3a84..b80aa020d7ff0 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -18,26 +18,6 @@ import { StreamerCentral } from '../../../../apps/meteor/server/modules/streamer const { PORT = 4000 } = process.env; -const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; -const lastConnectionStatusUpdate = new Map(); - -const shouldUpdateConnectionStatus = (connectionId: string): boolean => { - const now = Date.now(); - const last = lastConnectionStatusUpdate.get(connectionId) ?? 0; - if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { - return false; - } - lastConnectionStatusUpdate.set(connectionId, now); - return true; -}; - -const updateConnectionStatus = async (userId: string, connectionId: string): Promise => { - if (!shouldUpdateConnectionStatus(connectionId)) { - return; - } - await Presence.setConnectionStatus(userId, connectionId); -}; - export class DDPStreamer extends ServiceClass { protected name = 'streamer'; @@ -216,7 +196,6 @@ export class DDPStreamer extends ServiceClass { if (!userId) { return; } - lastConnectionStatusUpdate.delete(connection.id); Presence.removeConnection(userId, connection.id, nodeID); }); @@ -230,7 +209,6 @@ export class DDPStreamer extends ServiceClass { if (!userId) { return; } - lastConnectionStatusUpdate.delete(connection.id); Presence.removeConnection(userId, connection.id, nodeID); }); @@ -243,7 +221,7 @@ export class DDPStreamer extends ServiceClass { if (!userId) { return; } - void updateConnectionStatus(userId, connection.id); + void Presence.setConnectionStatus(userId, connection.id); }); } diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index a6639b3bf1615..be0d9abeb3e09 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -7,6 +7,18 @@ import { Settings, Users, UsersSessions } from '@rocket.chat/models'; import { processPresenceAndStatus } from './processPresenceAndStatus'; const MAX_CONNECTIONS = 200; +const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; +const lastConnectionStatusUpdate = new Map(); + +const shouldUpdateConnectionStatus = (connectionId: string): boolean => { + const now = Date.now(); + const last = lastConnectionStatusUpdate.get(connectionId) ?? 0; + if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { + return false; + } + lastConnectionStatusUpdate.set(connectionId, now); + return true; +}; export class Presence extends ServiceClass implements IPresence { protected name = 'presence'; @@ -21,6 +33,8 @@ export class Presence extends ServiceClass implements IPresence { private lostConTimeout?: NodeJS.Timeout; + private staleConInterval?: NodeJS.Timeout; + private connsPerInstance = new Map(); private peakConnections = 0; @@ -78,6 +92,11 @@ export class Presence extends ServiceClass implements IPresence { return affectedUsers.forEach((uid) => this.updateUserPresence(uid)); }, 10000); + this.staleConInterval = setInterval(async () => { + const affectedUsers = await this.removeStaleConnections(); + return affectedUsers.forEach((uid) => this.updateUserPresence(uid)); + }, 10000); + try { await Settings.updateValueById('Presence_broadcast_disabled', false); @@ -90,10 +109,12 @@ export class Presence extends ServiceClass implements IPresence { } async stopped(): Promise { - if (!this.lostConTimeout) { - return; + if (this.lostConTimeout) { + clearTimeout(this.lostConTimeout); + } + if (this.staleConInterval) { + clearInterval(this.staleConInterval); } - clearTimeout(this.lostConTimeout); } async toggleBroadcast(enabled: boolean): Promise { @@ -141,6 +162,9 @@ export class Presence extends ServiceClass implements IPresence { if (!uid || !session) { return; } + + lastConnectionStatusUpdate.delete(session); + await UsersSessions.removeConnectionByConnectionId(session); await this.updateUserPresence(uid); @@ -151,6 +175,34 @@ export class Presence extends ServiceClass implements IPresence { }; } + async removeStaleConnections(): Promise { + const cutoff = new Date(Date.now() - CONNECTION_STATUS_UPDATE_INTERVAL); + const users = UsersSessions.find({ 'connections._updatedAt': { $lt: cutoff } }); + const affectedUsers = new Set(); + for await (const userSession of users) { + const staleConnectionIds = userSession.connections.filter((conn) => conn._updatedAt < cutoff).map((conn) => conn.id); + if (staleConnectionIds.length === 0) { + continue; + } + const result = await UsersSessions.updateOne( + { _id: userSession._id }, + { + $pull: { + connections: { id: { $in: staleConnectionIds } }, + }, + }, + ); + if (result.modifiedCount > 0) { + for (const id of staleConnectionIds) { + lastConnectionStatusUpdate.delete(id); + } + affectedUsers.add(userSession._id); + } + } + + return Array.from(affectedUsers); + } + async removeLostConnections(nodeID?: string): Promise { if (nodeID) { const affectedUsers = await UsersSessions.findByInstanceId(nodeID).toArray(); @@ -204,6 +256,9 @@ export class Presence extends ServiceClass implements IPresence { } async setConnectionStatus(uid: string, session: string, status?: UserStatus): Promise { + if (!status && !shouldUpdateConnectionStatus(session)) { + return false; + } const result = await UsersSessions.updateConnectionStatusById(uid, session, status); await this.updateUserPresence(uid); From 3f106f3f39fc364fd02c6e2dbd24e8ca18e7d0fb Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Thu, 4 Dec 2025 13:41:39 -0300 Subject: [PATCH 14/23] refactor: move throttling logic to presence service --- apps/meteor/ee/server/startup/presence.ts | 4 +- apps/meteor/server/methods/userPresence.ts | 4 +- ee/apps/ddp-streamer/src/DDPStreamer.ts | 2 +- ee/apps/ddp-streamer/src/configureServer.ts | 4 +- ee/packages/presence/src/Presence.ts | 170 ++++++---- ee/packages/presence/src/lib/constants.ts | 14 + .../src/lib/processConnectionStatus.ts | 43 ++- ee/packages/presence/src/lib/processStatus.ts | 16 - .../presence/src/processPresenceAndStatus.ts | 31 -- .../tests/lib/processConnectionStatus.test.ts | 307 +++++++++++++++++- .../lib/processPresenceAndStatus.test.ts | 287 ---------------- .../presence/tests/lib/processStatus.test.ts | 30 -- packages/core-services/src/types/IPresence.ts | 3 +- .../src/models/IUsersSessionsModel.ts | 4 +- packages/models/src/models/UsersSessions.ts | 43 ++- 15 files changed, 527 insertions(+), 435 deletions(-) create mode 100644 ee/packages/presence/src/lib/constants.ts delete mode 100644 ee/packages/presence/src/lib/processStatus.ts delete mode 100644 ee/packages/presence/src/processPresenceAndStatus.ts delete mode 100644 ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts delete mode 100644 ee/packages/presence/tests/lib/processStatus.test.ts diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index 2ce1dc491a927..966916498ae59 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -29,7 +29,7 @@ Meteor.startup(() => { await Presence.removeLostConnections(nodeId); }); - Accounts.onLogin((login: { connection: { id: string }; user: { _id: string } }): void => { + Accounts.onLogin((login: any): void => { if (!login.connection.id) { return; } @@ -42,7 +42,7 @@ Meteor.startup(() => { const _messageReceived = session.heartbeat.messageReceived.bind(session.heartbeat); session.heartbeat.messageReceived = function messageReceived() { - void Presence.setConnectionStatus(login.user._id, login.connection.id); + void Presence.updateConnection(login.user._id, login.connection.id); return _messageReceived(); }; diff --git a/apps/meteor/server/methods/userPresence.ts b/apps/meteor/server/methods/userPresence.ts index b7a32df6a294c..21f3f795155ea 100644 --- a/apps/meteor/server/methods/userPresence.ts +++ b/apps/meteor/server/methods/userPresence.ts @@ -25,13 +25,13 @@ Meteor.methods({ if (!userId || !connection) { return; } - return Presence.setConnectionStatus(userId, connection.id, UserStatus.ONLINE); + return Presence.setConnectionStatus(userId, UserStatus.ONLINE, connection.id); }, 'UserPresence:away'() { const { userId, connection } = this; if (!userId || !connection) { return; } - return Presence.setConnectionStatus(userId, connection.id, UserStatus.AWAY); + return Presence.setConnectionStatus(userId, UserStatus.AWAY, connection.id); }, }); diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index b80aa020d7ff0..44517b3dbf4ac 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -221,7 +221,7 @@ export class DDPStreamer extends ServiceClass { if (!userId) { return; } - void Presence.setConnectionStatus(userId, connection.id); + void Presence.updateConnection(userId, connection.id); }); } diff --git a/ee/apps/ddp-streamer/src/configureServer.ts b/ee/apps/ddp-streamer/src/configureServer.ts index c22c303c46c7c..bf9bd15627cfa 100644 --- a/ee/apps/ddp-streamer/src/configureServer.ts +++ b/ee/apps/ddp-streamer/src/configureServer.ts @@ -121,14 +121,14 @@ server.methods({ if (!userId) { return; } - return Presence.setConnectionStatus(userId, session, UserStatus.ONLINE); + return Presence.setConnectionStatus(userId, UserStatus.ONLINE, session); }, 'UserPresence:away'() { const { userId, session } = this; if (!userId) { return; } - return Presence.setConnectionStatus(userId, session, UserStatus.AWAY); + return Presence.setConnectionStatus(userId, UserStatus.AWAY, session); }, 'setUserStatus'(status, statusText) { const { userId } = this; diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index be0d9abeb3e09..d9ab22b2ffa58 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -1,24 +1,12 @@ import type { IPresence, IBrokerNode } from '@rocket.chat/core-services'; import { License, ServiceClass } from '@rocket.chat/core-services'; -import type { IUser } from '@rocket.chat/core-typings'; +import type { IUser, IUserSession } from '@rocket.chat/core-typings'; import { UserStatus } from '@rocket.chat/core-typings'; import { Settings, Users, UsersSessions } from '@rocket.chat/models'; +import type { AnyBulkWriteOperation } from 'mongodb'; -import { processPresenceAndStatus } from './processPresenceAndStatus'; - -const MAX_CONNECTIONS = 200; -const CONNECTION_STATUS_UPDATE_INTERVAL = 60000; -const lastConnectionStatusUpdate = new Map(); - -const shouldUpdateConnectionStatus = (connectionId: string): boolean => { - const now = Date.now(); - const last = lastConnectionStatusUpdate.get(connectionId) ?? 0; - if (now - last < CONNECTION_STATUS_UPDATE_INTERVAL) { - return false; - } - lastConnectionStatusUpdate.set(connectionId, now); - return true; -}; +import { UPDATE_INTERVAL, STALE_THRESHOLD, MAX_CONNECTIONS } from './lib/constants'; +import { processPresenceAndStatus } from './lib/processConnectionStatus'; export class Presence extends ServiceClass implements IPresence { protected name = 'presence'; @@ -39,6 +27,21 @@ export class Presence extends ServiceClass implements IPresence { private peakConnections = 0; + private lastUpdate = new Map(); + + shouldUpdateConnectionStatus(session: string): boolean { + const lastUpdated = this.lastUpdate.get(session); + if (!lastUpdated) { + this.lastUpdate.set(session, Date.now()); + return true; + } + if (Date.now() - lastUpdated > UPDATE_INTERVAL) { + this.lastUpdate.set(session, Date.now()); + return true; + } + return false; + } + constructor() { super(); @@ -95,7 +98,7 @@ export class Presence extends ServiceClass implements IPresence { this.staleConInterval = setInterval(async () => { const affectedUsers = await this.removeStaleConnections(); return affectedUsers.forEach((uid) => this.updateUserPresence(uid)); - }, 10000); + }, UPDATE_INTERVAL); try { await Settings.updateValueById('Presence_broadcast_disabled', false); @@ -109,12 +112,8 @@ export class Presence extends ServiceClass implements IPresence { } async stopped(): Promise { - if (this.lostConTimeout) { - clearTimeout(this.lostConTimeout); - } - if (this.staleConInterval) { - clearInterval(this.staleConInterval); - } + clearTimeout(this.lostConTimeout); + clearInterval(this.staleConInterval); } async toggleBroadcast(enabled: boolean): Promise { @@ -158,49 +157,107 @@ export class Presence extends ServiceClass implements IPresence { }; } - async removeConnection(uid: string | undefined, session: string | undefined): Promise<{ uid: string; session: string } | undefined> { - if (!uid || !session) { + async updateConnection(uid: string, session: string): Promise<{ uid: string; session: string } | undefined> { + if (!this.shouldUpdateConnectionStatus(session)) { + return; + } + console.debug(`Updating connection for user ${uid} and session ${session}`); + const result = await UsersSessions.updateConnectionById(uid, session); + if (result.modifiedCount === 0) { return; } - - lastConnectionStatusUpdate.delete(session); - - await UsersSessions.removeConnectionByConnectionId(session); await this.updateUserPresence(uid); - return { - uid, - session, - }; + return { uid, session }; } - async removeStaleConnections(): Promise { - const cutoff = new Date(Date.now() - CONNECTION_STATUS_UPDATE_INTERVAL); - const users = UsersSessions.find({ 'connections._updatedAt': { $lt: cutoff } }); - const affectedUsers = new Set(); - for await (const userSession of users) { - const staleConnectionIds = userSession.connections.filter((conn) => conn._updatedAt < cutoff).map((conn) => conn.id); - if (staleConnectionIds.length === 0) { - continue; - } - const result = await UsersSessions.updateOne( - { _id: userSession._id }, - { - $pull: { - connections: { id: { $in: staleConnectionIds } }, + /** + * Runs the cleanup job to remove stale connections and sync user status. + */ + async removeStaleConnections() { + console.debug('[Cleanup] Starting stale connections cleanup job.'); + const cutoffDate = new Date(Date.now() - STALE_THRESHOLD); + + // STEP 1: Find users who have AT LEAST one stale connection + // We project the whole connections array because we need to inspect it in memory + const cursor = UsersSessions.find({ 'connections._updatedAt': { $lte: cutoffDate } }, { projection: { _id: 1, connections: 1 } }); + + const bulkSessionOps: AnyBulkWriteOperation[] = []; + const bulkUserOps: AnyBulkWriteOperation[] = []; + const processedUserIds = []; + + // STEP 2: Iterate and Calculate + for await (const sessionDoc of cursor) { + const userId = sessionDoc._id; + const allConnections = sessionDoc.connections || []; + + // Separate valid vs stale based on the cutoff + const staleConnections = allConnections.filter((c) => c._updatedAt <= cutoffDate); + const validConnections = allConnections.filter((c) => c._updatedAt > cutoffDate); + + if (staleConnections.length === 0) continue; // Should not happen due to query, but safe to check + + // Collect the IDs of the connections we want to remove + const staleConnectionIds = staleConnections.map((c) => c.id); + + // OPERATION A: Remove specific connections from usersSessions + // We use the unique IDs to be surgically precise + bulkSessionOps.push({ + updateOne: { + filter: { _id: userId }, + update: { + $pull: { + connections: { id: { $in: staleConnectionIds } }, + }, }, }, - ); - if (result.modifiedCount > 0) { - for (const id of staleConnectionIds) { - lastConnectionStatusUpdate.delete(id); - } - affectedUsers.add(userSession._id); + }); + + // OPERATION B: Update user status if they will have NO connections left + if (validConnections.length === 0) { + bulkUserOps.push({ + updateOne: { + filter: { _id: userId }, + update: { $set: { status: UserStatus.OFFLINE } }, + }, + }); + processedUserIds.push(userId); } } - return Array.from(affectedUsers); + // STEP 3: Execute the operations + if (bulkSessionOps.length > 0) { + await UsersSessions.col.bulkWrite(bulkSessionOps); + console.log(`[Cleanup] Removed stale connections for ${bulkSessionOps.length} users.`); + } + + if (bulkUserOps.length > 0) { + await Users.col.bulkWrite(bulkUserOps); + console.log(`[Cleanup] Marked ${bulkUserOps.length} users as OFFLINE.`); + } + + console.debug(`[Cleanup] Finished stale connections cleanup job.`); + + return processedUserIds; + } + + async removeConnection(uid: string | undefined, session: string | undefined): Promise<{ uid: string; session: string } | undefined> { + if (uid === 'rocketchat.internal.admin.test') { + console.log('Admin detected, skipping removal of connection for testing purposes.'); + return; + } + if (!uid || !session) { + return; + } + await UsersSessions.removeConnectionByConnectionId(session); + + await this.updateUserPresence(uid); + + return { + uid, + session, + }; } async removeLostConnections(nodeID?: string): Promise { @@ -255,10 +312,7 @@ export class Presence extends ServiceClass implements IPresence { return !!result.modifiedCount; } - async setConnectionStatus(uid: string, session: string, status?: UserStatus): Promise { - if (!status && !shouldUpdateConnectionStatus(session)) { - return false; - } + async setConnectionStatus(uid: string, status: UserStatus, session: string): Promise { const result = await UsersSessions.updateConnectionStatusById(uid, session, status); await this.updateUserPresence(uid); diff --git a/ee/packages/presence/src/lib/constants.ts b/ee/packages/presence/src/lib/constants.ts new file mode 100644 index 0000000000000..e5b4c70ba459b --- /dev/null +++ b/ee/packages/presence/src/lib/constants.ts @@ -0,0 +1,14 @@ +/** + * Maximum number of connections allowed. + */ +export const MAX_CONNECTIONS = 200; + +/** + * Interval to update connection status in milliseconds. + */ +export const UPDATE_INTERVAL = 60_000; + +/** + * Threshold to consider a connection as stale in milliseconds. + */ +export const STALE_THRESHOLD = 300_000; diff --git a/ee/packages/presence/src/lib/processConnectionStatus.ts b/ee/packages/presence/src/lib/processConnectionStatus.ts index 43bc77d88e444..18f802e904817 100644 --- a/ee/packages/presence/src/lib/processConnectionStatus.ts +++ b/ee/packages/presence/src/lib/processConnectionStatus.ts @@ -1,4 +1,6 @@ -import { UserStatus } from '@rocket.chat/core-typings'; +import { UserStatus, type IUserSessionConnection } from '@rocket.chat/core-typings'; + +import { STALE_THRESHOLD } from './constants'; /** * Defines new connection status compared to a previous connection status @@ -12,3 +14,42 @@ export const processConnectionStatus = (current: UserStatus, status: UserStatus) } return current; }; + +/** + * Defines user's status based on presence and connection status + */ +export const processStatus = (statusConnection: UserStatus, statusDefault: UserStatus): UserStatus => { + if (statusConnection === UserStatus.OFFLINE) { + return statusConnection; + } + + if (statusDefault === UserStatus.ONLINE) { + return statusConnection; + } + + return statusDefault; +}; + +const isFresh = (updatedAt: Date): boolean => { + return Date.now() - updatedAt.getTime() <= STALE_THRESHOLD; +}; + +/** + * Defines user's status and connection status based on user's connections and default status + */ +export const processPresenceAndStatus = ( + userSessions: IUserSessionConnection[] = [], + statusDefault = UserStatus.ONLINE, +): { status: UserStatus; statusConnection: UserStatus } => { + const statusConnection = userSessions + .filter((c) => isFresh(c._updatedAt)) + .map((s) => s.status) + .reduce(processConnectionStatus, UserStatus.OFFLINE); + + const status = processStatus(statusConnection, statusDefault); + + return { + status, + statusConnection, + }; +}; diff --git a/ee/packages/presence/src/lib/processStatus.ts b/ee/packages/presence/src/lib/processStatus.ts deleted file mode 100644 index 1c9583d840434..0000000000000 --- a/ee/packages/presence/src/lib/processStatus.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { UserStatus } from '@rocket.chat/core-typings'; - -/** - * Defines user's status based on presence and connection status - */ -export const processStatus = (statusConnection: UserStatus, statusDefault: UserStatus): UserStatus => { - if (statusConnection === UserStatus.OFFLINE) { - return statusConnection; - } - - if (statusDefault === UserStatus.ONLINE) { - return statusConnection; - } - - return statusDefault; -}; diff --git a/ee/packages/presence/src/processPresenceAndStatus.ts b/ee/packages/presence/src/processPresenceAndStatus.ts deleted file mode 100644 index a97849cfdd40b..0000000000000 --- a/ee/packages/presence/src/processPresenceAndStatus.ts +++ /dev/null @@ -1,31 +0,0 @@ -import type { IUserSessionConnection } from '@rocket.chat/core-typings'; -import { UserStatus } from '@rocket.chat/core-typings'; - -import { processConnectionStatus } from './lib/processConnectionStatus'; -import { processStatus } from './lib/processStatus'; - -const isAtMostFiveMinutesAgo = (userSession: IUserSessionConnection): boolean => { - const now = Date.now(); - const diff = now - userSession._updatedAt.getTime(); - return diff <= 300_000; // 5 minutes in milliseconds -}; - -/** - * Defines user's status and connection status based on user's connections and default status - */ -export const processPresenceAndStatus = ( - userSessions: IUserSessionConnection[] = [], - statusDefault = UserStatus.ONLINE, -): { status: UserStatus; statusConnection: UserStatus } => { - const statusConnection = userSessions - .filter(isAtMostFiveMinutesAgo) - .map((s) => s.status) - .reduce(processConnectionStatus, UserStatus.OFFLINE); - - const status = processStatus(statusConnection, statusDefault); - - return { - status, - statusConnection, - }; -}; diff --git a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts index 0d98cca47b8a4..1431b1b5b0a41 100644 --- a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts +++ b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from '@jest/globals'; import { UserStatus } from '@rocket.chat/core-typings'; -import { processConnectionStatus } from '../../src/lib/processConnectionStatus'; +import { processConnectionStatus, processStatus, processPresenceAndStatus } from '../../src/lib/processConnectionStatus'; describe('Presence micro service', () => { test('should return connection as online when there is a connection online', () => { @@ -16,4 +16,309 @@ describe('Presence micro service', () => { expect(processConnectionStatus(UserStatus.ONLINE, UserStatus.OFFLINE)).toBe(UserStatus.ONLINE); expect(processConnectionStatus(UserStatus.AWAY, UserStatus.OFFLINE)).toBe(UserStatus.AWAY); }); + + test('should return the connection status when the default status is online', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.ONLINE)).toBe(UserStatus.ONLINE); + expect(processStatus(UserStatus.AWAY, UserStatus.ONLINE)).toBe(UserStatus.AWAY); + expect(processStatus(UserStatus.OFFLINE, UserStatus.ONLINE)).toBe(UserStatus.OFFLINE); + }); + + test('should return status busy when the default status is busy', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.BUSY)).toBe(UserStatus.BUSY); + expect(processStatus(UserStatus.AWAY, UserStatus.BUSY)).toBe(UserStatus.BUSY); + expect(processStatus(UserStatus.OFFLINE, UserStatus.BUSY)).toBe(UserStatus.OFFLINE); + }); + + test('should return status away when the default status is away', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.AWAY)).toBe(UserStatus.AWAY); + expect(processStatus(UserStatus.AWAY, UserStatus.AWAY)).toBe(UserStatus.AWAY); + expect(processStatus(UserStatus.OFFLINE, UserStatus.AWAY)).toBe(UserStatus.OFFLINE); + }); + + test('should return status offline when the default status is offline', () => { + expect(processStatus(UserStatus.ONLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); + expect(processStatus(UserStatus.AWAY, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); + expect(processStatus(UserStatus.OFFLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); + }); + + test('should return correct status and statusConnection when connected once', () => { + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.BUSY, + ), + ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.AWAY, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.BUSY, + ), + ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.AWAY }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.OFFLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.OFFLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.AWAY }); + }); + + test('should return correct status and statusConnection when connected twice', () => { + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + { + id: 'random', + instanceId: 'random', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + + expect( + processPresenceAndStatus( + [ + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + { + id: 'random', + instanceId: 'random', + status: UserStatus.AWAY, + _createdAt: new Date(), + _updatedAt: new Date(), + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); + }); + + test('should return correct status and statusConnection when not connected', () => { + expect(processPresenceAndStatus([], UserStatus.ONLINE)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + + expect(processPresenceAndStatus([], UserStatus.BUSY)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + + expect(processPresenceAndStatus([], UserStatus.AWAY)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + + expect(processPresenceAndStatus([], UserStatus.OFFLINE)).toStrictEqual({ + status: UserStatus.OFFLINE, + statusConnection: UserStatus.OFFLINE, + }); + }); + + test('should ignore connections last updated more than 5 minutes ago', () => { + const now = new Date(); + const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); + const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); + + expect( + processPresenceAndStatus( + [ + { + id: 'random1', + instanceId: 'random1', + status: UserStatus.ONLINE, + _createdAt: sixMinutesAgo, + _updatedAt: sixMinutesAgo, + }, + { + id: 'random2', + instanceId: 'random2', + status: UserStatus.AWAY, + _createdAt: fourMinutesAgo, + _updatedAt: fourMinutesAgo, + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); + }); + + test('should return offline if all connections are stale', () => { + const now = new Date(); + const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); + const sevenMinutesAgo = new Date(now.getTime() - 7 * 60 * 1000); + + expect( + processPresenceAndStatus( + [ + { + id: 'random1', + instanceId: 'random1', + status: UserStatus.ONLINE, + _createdAt: sixMinutesAgo, + _updatedAt: sixMinutesAgo, + }, + { + id: 'random2', + instanceId: 'random2', + status: UserStatus.AWAY, + _createdAt: sevenMinutesAgo, + _updatedAt: sevenMinutesAgo, + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); + }); + + test('should consider all connections if they were updated within the last 5 minutes', () => { + const now = new Date(); + const threeMinutesAgo = new Date(now.getTime() - 3 * 60 * 1000); + const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); + + expect( + processPresenceAndStatus( + [ + { + id: 'random1', + instanceId: 'random1', + status: UserStatus.ONLINE, + _createdAt: threeMinutesAgo, + _updatedAt: threeMinutesAgo, + }, + { + id: 'random2', + instanceId: 'random2', + status: UserStatus.AWAY, + _createdAt: fourMinutesAgo, + _updatedAt: fourMinutesAgo, + }, + ], + UserStatus.ONLINE, + ), + ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); + }); }); diff --git a/ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts b/ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts deleted file mode 100644 index cec9e3f435dc2..0000000000000 --- a/ee/packages/presence/tests/lib/processPresenceAndStatus.test.ts +++ /dev/null @@ -1,287 +0,0 @@ -import { describe, test, expect } from '@jest/globals'; -import { UserStatus } from '@rocket.chat/core-typings'; - -import { processPresenceAndStatus } from '../../src/processPresenceAndStatus'; - -describe('processPresenceAndStatus', () => { - test('should return correct status and statusConnection when connected once', () => { - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.BUSY, - ), - ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.AWAY, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.BUSY, - ), - ).toStrictEqual({ status: UserStatus.BUSY, statusConnection: UserStatus.AWAY }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.OFFLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.OFFLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.AWAY }); - }); - - test('should return correct status and statusConnection when connected twice', () => { - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - { - id: 'random', - instanceId: 'random', - status: UserStatus.ONLINE, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - - expect( - processPresenceAndStatus( - [ - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - { - id: 'random', - instanceId: 'random', - status: UserStatus.AWAY, - _createdAt: new Date(), - _updatedAt: new Date(), - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); - }); - - test('should return correct status and statusConnection when not connected', () => { - expect(processPresenceAndStatus([], UserStatus.ONLINE)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - - expect(processPresenceAndStatus([], UserStatus.BUSY)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - - expect(processPresenceAndStatus([], UserStatus.AWAY)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - - expect(processPresenceAndStatus([], UserStatus.OFFLINE)).toStrictEqual({ - status: UserStatus.OFFLINE, - statusConnection: UserStatus.OFFLINE, - }); - }); - - test('should ignore connections last updated more than 5 minutes ago', () => { - const now = new Date(); - const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); - const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); - - expect( - processPresenceAndStatus( - [ - { - id: 'random1', - instanceId: 'random1', - status: UserStatus.ONLINE, - _createdAt: sixMinutesAgo, - _updatedAt: sixMinutesAgo, - }, - { - id: 'random2', - instanceId: 'random2', - status: UserStatus.AWAY, - _createdAt: fourMinutesAgo, - _updatedAt: fourMinutesAgo, - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); - }); - - test('should return offline if all connections are stale', () => { - const now = new Date(); - const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); - const sevenMinutesAgo = new Date(now.getTime() - 7 * 60 * 1000); - - expect( - processPresenceAndStatus( - [ - { - id: 'random1', - instanceId: 'random1', - status: UserStatus.ONLINE, - _createdAt: sixMinutesAgo, - _updatedAt: sixMinutesAgo, - }, - { - id: 'random2', - instanceId: 'random2', - status: UserStatus.AWAY, - _createdAt: sevenMinutesAgo, - _updatedAt: sevenMinutesAgo, - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); - }); - - test('should consider all connections if they were updated within the last 5 minutes', () => { - const now = new Date(); - const threeMinutesAgo = new Date(now.getTime() - 3 * 60 * 1000); - const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); - - expect( - processPresenceAndStatus( - [ - { - id: 'random1', - instanceId: 'random1', - status: UserStatus.ONLINE, - _createdAt: threeMinutesAgo, - _updatedAt: threeMinutesAgo, - }, - { - id: 'random2', - instanceId: 'random2', - status: UserStatus.AWAY, - _createdAt: fourMinutesAgo, - _updatedAt: fourMinutesAgo, - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - }); -}); diff --git a/ee/packages/presence/tests/lib/processStatus.test.ts b/ee/packages/presence/tests/lib/processStatus.test.ts deleted file mode 100644 index c9e63a988578a..0000000000000 --- a/ee/packages/presence/tests/lib/processStatus.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { describe, test, expect } from '@jest/globals'; -import { UserStatus } from '@rocket.chat/core-typings'; - -import { processStatus } from '../../src/lib/processStatus'; - -describe('processStatus', () => { - test('should return the connection status when the default status is online', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.ONLINE)).toBe(UserStatus.ONLINE); - expect(processStatus(UserStatus.AWAY, UserStatus.ONLINE)).toBe(UserStatus.AWAY); - expect(processStatus(UserStatus.OFFLINE, UserStatus.ONLINE)).toBe(UserStatus.OFFLINE); - }); - - test('should return status busy when the default status is busy', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.BUSY)).toBe(UserStatus.BUSY); - expect(processStatus(UserStatus.AWAY, UserStatus.BUSY)).toBe(UserStatus.BUSY); - expect(processStatus(UserStatus.OFFLINE, UserStatus.BUSY)).toBe(UserStatus.OFFLINE); - }); - - test('should return status away when the default status is away', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.AWAY)).toBe(UserStatus.AWAY); - expect(processStatus(UserStatus.AWAY, UserStatus.AWAY)).toBe(UserStatus.AWAY); - expect(processStatus(UserStatus.OFFLINE, UserStatus.AWAY)).toBe(UserStatus.OFFLINE); - }); - - test('should return status offline when the default status is offline', () => { - expect(processStatus(UserStatus.ONLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); - expect(processStatus(UserStatus.AWAY, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); - expect(processStatus(UserStatus.OFFLINE, UserStatus.OFFLINE)).toBe(UserStatus.OFFLINE); - }); -}); diff --git a/packages/core-services/src/types/IPresence.ts b/packages/core-services/src/types/IPresence.ts index fb17a3bf3549e..42b87ec50d0eb 100644 --- a/packages/core-services/src/types/IPresence.ts +++ b/packages/core-services/src/types/IPresence.ts @@ -13,9 +13,10 @@ export interface IPresence extends IServiceClass { session: string | undefined, nodeId: string, ): Promise<{ uid: string; session: string } | undefined>; + updateConnection(uid: string, session: string): Promise<{ uid: string; session: string } | undefined>; removeLostConnections(nodeID: string): Promise; setStatus(uid: string, status: UserStatus, statusText?: string): Promise; - setConnectionStatus(uid: string, session: string, status?: UserStatus): Promise; + setConnectionStatus(uid: string, status: UserStatus, session: string): Promise; updateUserPresence(uid: string): Promise; toggleBroadcast(enabled: boolean): void; getConnectionCount(): { current: number; max: number }; diff --git a/packages/model-typings/src/models/IUsersSessionsModel.ts b/packages/model-typings/src/models/IUsersSessionsModel.ts index 03f1f03b78be5..cc6c7e6b5a3cb 100644 --- a/packages/model-typings/src/models/IUsersSessionsModel.ts +++ b/packages/model-typings/src/models/IUsersSessionsModel.ts @@ -5,7 +5,9 @@ import type { IBaseModel } from './IBaseModel'; export interface IUsersSessionsModel extends IBaseModel { clearConnectionsFromInstanceId(instanceId: string[]): ReturnType['updateMany']>; - updateConnectionStatusById(uid: string, connectionId: string, status?: string): ReturnType['updateOne']>; + updateConnectionStatusById(uid: string, connectionId: string, status: string): ReturnType['updateOne']>; + updateConnectionById(uid: string, connectionId: string): ReturnType['updateOne']>; + findStaleConnections(cutoff: Date): FindCursor; removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']>; removeConnectionByConnectionId(connectionId: string): ReturnType['updateMany']>; findByInstanceId(instanceId: string): FindCursor; diff --git a/packages/models/src/models/UsersSessions.ts b/packages/models/src/models/UsersSessions.ts index acd8104197c38..5fa4e8ebb5243 100644 --- a/packages/models/src/models/UsersSessions.ts +++ b/packages/models/src/models/UsersSessions.ts @@ -29,7 +29,7 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes ); } - updateConnectionStatusById(uid: string, connectionId: string, status?: string): ReturnType['updateOne']> { + updateConnectionStatusById(uid: string, connectionId: string, status: string): ReturnType['updateOne']> { const query = { '_id': uid, 'connections.id': connectionId, @@ -37,7 +37,7 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes const update = { $set: { - ...(status && { 'connections.$.status': status }), + 'connections.$.status': status, 'connections.$._updatedAt': new Date(), }, }; @@ -45,6 +45,45 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes return this.updateOne(query, update); } + updateConnectionById(uid: string, connectionId: string): ReturnType['updateOne']> { + const query = { + '_id': uid, + 'connections.id': connectionId, + }; + + const update = { + $set: { + 'connections.$._updatedAt': new Date(), + }, + }; + + return this.updateOne(query, update); + } + + findStaleConnections(cutoff: Date): FindCursor { + return this.find( + { + 'connections._updatedAt': { $lt: cutoff }, + }, + { projection: { _id: 1 } }, + ); + } + + removeConnectionsByConnectionIds(connectionIds: string[]): ReturnType['updateMany']> { + return this.updateMany( + { + 'connections.id': { $in: connectionIds }, + }, + { + $pull: { + connections: { + id: { $in: connectionIds }, + }, + }, + }, + ); + } + async removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']> { return this.updateMany( { From 8db5df22089db25185437a0939c07ae4d9c91ac8 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Thu, 4 Dec 2025 19:00:19 -0300 Subject: [PATCH 15/23] refactor: separate cleaning logic into its own class --- ee/packages/presence/src/Presence.ts | 171 +++++++++--------- .../presence/src/lib/PresenceReaper.spec.ts | 134 ++++++++++++++ .../presence/src/lib/PresenceReaper.ts | 131 ++++++++++++++ ee/packages/presence/src/lib/constants.ts | 14 -- .../src/lib/processConnectionStatus.ts | 11 +- .../tests/lib/processConnectionStatus.test.ts | 84 --------- 6 files changed, 347 insertions(+), 198 deletions(-) create mode 100644 ee/packages/presence/src/lib/PresenceReaper.spec.ts create mode 100644 ee/packages/presence/src/lib/PresenceReaper.ts delete mode 100644 ee/packages/presence/src/lib/constants.ts diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index d9ab22b2ffa58..e51f35b3446de 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -1,13 +1,17 @@ import type { IPresence, IBrokerNode } from '@rocket.chat/core-services'; import { License, ServiceClass } from '@rocket.chat/core-services'; -import type { IUser, IUserSession } from '@rocket.chat/core-typings'; +import type { IUser } from '@rocket.chat/core-typings'; import { UserStatus } from '@rocket.chat/core-typings'; import { Settings, Users, UsersSessions } from '@rocket.chat/models'; -import type { AnyBulkWriteOperation } from 'mongodb'; -import { UPDATE_INTERVAL, STALE_THRESHOLD, MAX_CONNECTIONS } from './lib/constants'; +import { PresenceReaper } from './lib/PresenceReaper'; import { processPresenceAndStatus } from './lib/processConnectionStatus'; +/** + * Maximum number of connections allowed. + */ +export const MAX_CONNECTIONS = 200; + export class Presence extends ServiceClass implements IPresence { protected name = 'presence'; @@ -27,20 +31,7 @@ export class Presence extends ServiceClass implements IPresence { private peakConnections = 0; - private lastUpdate = new Map(); - - shouldUpdateConnectionStatus(session: string): boolean { - const lastUpdated = this.lastUpdate.get(session); - if (!lastUpdated) { - this.lastUpdate.set(session, Date.now()); - return true; - } - if (Date.now() - lastUpdated > UPDATE_INTERVAL) { - this.lastUpdate.set(session, Date.now()); - return true; - } - return false; - } + private reaper: PresenceReaper; constructor() { super(); @@ -95,10 +86,8 @@ export class Presence extends ServiceClass implements IPresence { return affectedUsers.forEach((uid) => this.updateUserPresence(uid)); }, 10000); - this.staleConInterval = setInterval(async () => { - const affectedUsers = await this.removeStaleConnections(); - return affectedUsers.forEach((uid) => this.updateUserPresence(uid)); - }, UPDATE_INTERVAL); + this.reaper = new PresenceReaper(UsersSessions, (userIds) => this.handleReaperUpdates(userIds)); + this.reaper.start(); try { await Settings.updateValueById('Presence_broadcast_disabled', false); @@ -111,6 +100,16 @@ export class Presence extends ServiceClass implements IPresence { } } + private async handleReaperUpdates(userIds: string[]): Promise { + if (userIds.length === 0) { + return; + } + + console.log(`[PresenceReaper] Updating presence for ${userIds.length} users due to stale connections.`); + + await Promise.all(userIds.map((uid) => this.updateUserPresence(uid))); + } + async stopped(): Promise { clearTimeout(this.lostConTimeout); clearInterval(this.staleConInterval); @@ -158,9 +157,6 @@ export class Presence extends ServiceClass implements IPresence { } async updateConnection(uid: string, session: string): Promise<{ uid: string; session: string } | undefined> { - if (!this.shouldUpdateConnectionStatus(session)) { - return; - } console.debug(`Updating connection for user ${uid} and session ${session}`); const result = await UsersSessions.updateConnectionById(uid, session); if (result.modifiedCount === 0) { @@ -175,72 +171,67 @@ export class Presence extends ServiceClass implements IPresence { /** * Runs the cleanup job to remove stale connections and sync user status. */ - async removeStaleConnections() { - console.debug('[Cleanup] Starting stale connections cleanup job.'); - const cutoffDate = new Date(Date.now() - STALE_THRESHOLD); - - // STEP 1: Find users who have AT LEAST one stale connection - // We project the whole connections array because we need to inspect it in memory - const cursor = UsersSessions.find({ 'connections._updatedAt': { $lte: cutoffDate } }, { projection: { _id: 1, connections: 1 } }); - - const bulkSessionOps: AnyBulkWriteOperation[] = []; - const bulkUserOps: AnyBulkWriteOperation[] = []; - const processedUserIds = []; - - // STEP 2: Iterate and Calculate - for await (const sessionDoc of cursor) { - const userId = sessionDoc._id; - const allConnections = sessionDoc.connections || []; - - // Separate valid vs stale based on the cutoff - const staleConnections = allConnections.filter((c) => c._updatedAt <= cutoffDate); - const validConnections = allConnections.filter((c) => c._updatedAt > cutoffDate); - - if (staleConnections.length === 0) continue; // Should not happen due to query, but safe to check - - // Collect the IDs of the connections we want to remove - const staleConnectionIds = staleConnections.map((c) => c.id); - - // OPERATION A: Remove specific connections from usersSessions - // We use the unique IDs to be surgically precise - bulkSessionOps.push({ - updateOne: { - filter: { _id: userId }, - update: { - $pull: { - connections: { id: { $in: staleConnectionIds } }, - }, - }, - }, - }); - - // OPERATION B: Update user status if they will have NO connections left - if (validConnections.length === 0) { - bulkUserOps.push({ - updateOne: { - filter: { _id: userId }, - update: { $set: { status: UserStatus.OFFLINE } }, - }, - }); - processedUserIds.push(userId); - } - } - - // STEP 3: Execute the operations - if (bulkSessionOps.length > 0) { - await UsersSessions.col.bulkWrite(bulkSessionOps); - console.log(`[Cleanup] Removed stale connections for ${bulkSessionOps.length} users.`); - } - - if (bulkUserOps.length > 0) { - await Users.col.bulkWrite(bulkUserOps); - console.log(`[Cleanup] Marked ${bulkUserOps.length} users as OFFLINE.`); - } - - console.debug(`[Cleanup] Finished stale connections cleanup job.`); - - return processedUserIds; - } + // async removeStaleConnections() { + // console.debug('[Cleanup] Starting stale connections cleanup job.'); + // const cutoffDate = new Date(Date.now() - this.staleThreshold); + + // // STEP 1: Find users who have AT LEAST one stale connection + // // We project the whole connections array because we need to inspect it in memory + // const cursor = UsersSessions.find({ 'connections._updatedAt': { $lte: cutoffDate } }, { projection: { _id: 1, connections: 1 } }); + + // const bulkSessionOps: AnyBulkWriteOperation[] = []; + // // const bulkUserOps: AnyBulkWriteOperation[] = []; + // const processedUserIds = []; + + // // STEP 2: Iterate and Calculate + // for await (const sessionDoc of cursor) { + // const userId = sessionDoc._id; + // const allConnections = sessionDoc.connections || []; + + // // Separate valid vs stale based on the cutoff + // const staleConnections = allConnections.filter((c) => c._updatedAt <= cutoffDate); + // const validConnections = allConnections.filter((c) => c._updatedAt > cutoffDate); + + // if (staleConnections.length === 0) continue; // Should not happen due to query, but safe to check + + // // Collect the IDs of the connections we want to remove + // const staleConnectionIds = staleConnections.map((c) => c.id); + + // // OPERATION A: Remove specific connections from usersSessions + // // We use the unique IDs to be surgically precise + // bulkSessionOps.push({ + // updateOne: { + // filter: { _id: userId }, + // update: { + // $pull: { + // connections: { id: { $in: staleConnectionIds }, _updatedAt: { $lte: cutoffDate } }, + // }, + // }, + // }, + // }); + + // // OPERATION B: Update user status if they will have NO connections left + // if (validConnections.length === 0) { + // // bulkUserOps.push({ + // // updateOne: { + // // filter: { _id: userId }, + // // update: { $set: { status: UserStatus.OFFLINE } }, + // // }, + // // }); + // processedUserIds.push(userId); + // } + // } + + // // STEP 3: Execute the operations + // if (bulkSessionOps.length > 0) { + // await UsersSessions.col.bulkWrite(bulkSessionOps); + // console.log(`[Cleanup] Removed stale connections for ${bulkSessionOps.length} users.`); + // } + + // console.debug(`[Cleanup] Finished stale connections cleanup job.`); + + // return processedUserIds; + // } async removeConnection(uid: string | undefined, session: string | undefined): Promise<{ uid: string; session: string } | undefined> { if (uid === 'rocketchat.internal.admin.test') { diff --git a/ee/packages/presence/src/lib/PresenceReaper.spec.ts b/ee/packages/presence/src/lib/PresenceReaper.spec.ts new file mode 100644 index 0000000000000..90198acda5655 --- /dev/null +++ b/ee/packages/presence/src/lib/PresenceReaper.spec.ts @@ -0,0 +1,134 @@ +import type { IUserSession } from '@rocket.chat/core-typings'; +import type { IUsersSessionsModel } from '@rocket.chat/model-typings'; +import type { Collection } from 'mongodb'; + +import { PresenceReaper } from './PresenceReaper'; + +// Define a simplified interface for our mock docs +type MockSession = { + _id: string; + connections: { id: string; _updatedAt: Date }[]; +}; + +describe('PresenceReaper', () => { + let reaper: PresenceReaper; + let mockSessionCollection: Omit, 'col'> & { + col: jest.Mocked>; + }; + let mockOnUpdate: jest.Mock; + + beforeEach(() => { + // 1. Mock the Collections + mockSessionCollection = { + find: jest.fn(), + col: { + bulkWrite: jest.fn().mockResolvedValue({ modifiedCount: 1 }), + }, + } as any; + + // 2. Mock the onUpdate callback + mockOnUpdate = jest.fn(); + + // 3. Instantiate Reaper + reaper = new PresenceReaper(mockSessionCollection, mockOnUpdate); + }); + + describe('processDocument (Business Logic)', () => { + it('should identify stale connections by "id" and preserve valid ones', () => { + const now = new Date(); + const staleTime = new Date(now.getTime() - 10 * 60 * 1000); // 10 mins ago + const activeTime = new Date(now.getTime() - 1 * 60 * 1000); // 1 min ago + const cutoff = new Date(now.getTime() - 5 * 60 * 1000); // 5 mins ago + + const doc: MockSession = { + _id: 'user-123', + connections: [ + { id: 'conn-stale', _updatedAt: staleTime }, // Should be removed + { id: 'conn-active', _updatedAt: activeTime }, // Should stay + ], + }; + + const changeMap = new Map(); + + // @ts-expect-error - testing private method + reaper.processDocument(doc, cutoff, changeMap); + + const result = changeMap.get('user-123'); + + // Assertions + expect(result).toBeDefined(); + expect(result?.removeIds).toContain('conn-stale'); // Found the stale ID + expect(result?.removeIds).not.toContain('conn-active'); // Ignored the active ID + expect(result?.shouldMarkOffline).toBe(false); // User still has 1 active connection + }); + + it('should mark user offline only if ALL connections are stale', () => { + const now = new Date(); + const staleTime = new Date(now.getTime() - 10000); + const cutoff = new Date(now); // Cutoff is now, so everything before is stale + + const doc: MockSession = { + _id: 'user-456', + connections: [ + { id: 'conn-1', _updatedAt: staleTime }, + { id: 'conn-2', _updatedAt: staleTime }, + ], + }; + + const changeMap = new Map(); + // @ts-expect-error - testing private method + reaper.processDocument(doc, cutoff, changeMap); + + const result = changeMap.get('user-456'); + + expect(result).toBeDefined(); + expect(result?.removeIds).toHaveLength(2); + expect(result?.shouldMarkOffline).toBe(true); // No valid connections left + }); + }); + + describe('run (Integration Flow)', () => { + it('should generate correct bulkWrite operations', async () => { + const now = new Date(); + const staleTime = new Date(now.getTime() - 6 * 60 * 1000); // 6 mins ago (Stale) + + // Mock Data from DB Cursor + const mockCursor = { + async *[Symbol.asyncIterator]() { + yield { + _id: 'user-789', + connections: [{ id: 'zombie-conn', _updatedAt: staleTime }], + }; + }, + } as any; + mockSessionCollection.find.mockReturnValue(mockCursor); + + // Execute Run + await reaper.run(); + + // Verify 'usersSessions' Update + expect(mockSessionCollection.col.bulkWrite).toHaveBeenCalledTimes(1); + expect(mockSessionCollection.col.bulkWrite).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + updateOne: expect.objectContaining({ + filter: { _id: 'user-789' }, + update: { + $pull: { + connections: { + id: { $in: ['zombie-conn'] }, + _updatedAt: { $lte: expect.any(Date) }, + }, + }, + }, + }), + }), + ]), + ); + + // Verify 'users' Update (Status Offline) + expect(mockOnUpdate).toHaveBeenCalledTimes(1); + expect(mockOnUpdate).toHaveBeenCalledWith(['user-789']); + }); + }); +}); diff --git a/ee/packages/presence/src/lib/PresenceReaper.ts b/ee/packages/presence/src/lib/PresenceReaper.ts new file mode 100644 index 0000000000000..9974b4c5db2d1 --- /dev/null +++ b/ee/packages/presence/src/lib/PresenceReaper.ts @@ -0,0 +1,131 @@ +import type { IUserSession } from '@rocket.chat/core-typings'; +import type { IUsersSessionsModel } from '@rocket.chat/model-typings'; + +type ReaperPlan = { + userId: string; + removeIds: string[]; + shouldMarkOffline: boolean; + cutoffDate: Date; +}; + +type ReaperCallback = (userIds: string[]) => void; + +export class PresenceReaper { + private usersSessions: IUsersSessionsModel; + + private staleThresholdMs: number; + + private batchSize: number; + + private running: boolean; + + private onUpdate: ReaperCallback; + + constructor(usersSessions: IUsersSessionsModel, onUpdate: ReaperCallback) { + this.usersSessions = usersSessions; + this.onUpdate = onUpdate; + + // Configuration: 5 minutes stale, process 500 users at a time + this.staleThresholdMs = 5 * 60 * 1000; + this.batchSize = 500; + this.running = false; + } + + public start() { + if (this.running) return; + this.running = true; + + // Run every 1 minute + setInterval(() => { + this.run().catch((err) => console.error('[PresenceReaper] Error:', err)); + }, 60 * 1000); + + console.log('[PresenceReaper] Service started.'); + } + + public async run(): Promise { + console.log('[PresenceReaper] Running presence reaper job...'); + const cutoffDate = new Date(Date.now() - this.staleThresholdMs); + + // 1. Find users with potentially stale connections + const cursor = this.usersSessions.find( + { 'connections._updatedAt': { $lte: cutoffDate } }, + { + projection: { _id: 1, connections: 1 }, + }, + ); + + const userChangeSet = new Map(); + + for await (const sessionDoc of cursor) { + this.processDocument(sessionDoc, cutoffDate, userChangeSet); + + if (userChangeSet.size >= this.batchSize) { + await this.flushBatch(userChangeSet); + userChangeSet.clear(); + } + } + + if (userChangeSet.size > 0) { + await this.flushBatch(userChangeSet); + } + console.log('[PresenceReaper] Presence reaper job completed.'); + } + + private processDocument(sessionDoc: IUserSession, cutoffDate: Date, changeMap: Map): void { + const userId = sessionDoc._id; + const allConnections = sessionDoc.connections || []; + + // Filter connections based on the cutoff + const staleConnections = allConnections.filter((c) => c._updatedAt <= cutoffDate); + const validConnections = allConnections.filter((c) => c._updatedAt > cutoffDate); + + if (staleConnections.length === 0) return; + + changeMap.set(userId, { + userId, + removeIds: staleConnections.map((c) => c.id), + cutoffDate, // Keep reference for race-condition check + shouldMarkOffline: validConnections.length === 0, + }); + } + + private async flushBatch(changeMap: Map): Promise { + const sessionOps = []; + const usersToUpdate: string[] = []; + + for (const plan of changeMap.values()) { + // 1. Prepare DB Cleanup + if (plan.removeIds.length > 0) { + sessionOps.push({ + updateOne: { + filter: { _id: plan.userId }, + update: { + $pull: { + connections: { + id: { $in: plan.removeIds }, + _updatedAt: { $lte: plan.cutoffDate }, + }, + }, + }, + }, + }); + } + + // 2. Identify potential offline users + if (plan.shouldMarkOffline) { + usersToUpdate.push(plan.userId); + } + } + + // Step A: Clean the Database + if (sessionOps.length > 0) { + await this.usersSessions.col.bulkWrite(sessionOps); + } + + // Step B: EMIT the event instead of calling a method directly + if (usersToUpdate.length > 0) { + this.onUpdate(usersToUpdate); + } + } +} diff --git a/ee/packages/presence/src/lib/constants.ts b/ee/packages/presence/src/lib/constants.ts deleted file mode 100644 index e5b4c70ba459b..0000000000000 --- a/ee/packages/presence/src/lib/constants.ts +++ /dev/null @@ -1,14 +0,0 @@ -/** - * Maximum number of connections allowed. - */ -export const MAX_CONNECTIONS = 200; - -/** - * Interval to update connection status in milliseconds. - */ -export const UPDATE_INTERVAL = 60_000; - -/** - * Threshold to consider a connection as stale in milliseconds. - */ -export const STALE_THRESHOLD = 300_000; diff --git a/ee/packages/presence/src/lib/processConnectionStatus.ts b/ee/packages/presence/src/lib/processConnectionStatus.ts index 18f802e904817..61790ce332927 100644 --- a/ee/packages/presence/src/lib/processConnectionStatus.ts +++ b/ee/packages/presence/src/lib/processConnectionStatus.ts @@ -1,7 +1,5 @@ import { UserStatus, type IUserSessionConnection } from '@rocket.chat/core-typings'; -import { STALE_THRESHOLD } from './constants'; - /** * Defines new connection status compared to a previous connection status */ @@ -30,10 +28,6 @@ export const processStatus = (statusConnection: UserStatus, statusDefault: UserS return statusDefault; }; -const isFresh = (updatedAt: Date): boolean => { - return Date.now() - updatedAt.getTime() <= STALE_THRESHOLD; -}; - /** * Defines user's status and connection status based on user's connections and default status */ @@ -41,10 +35,7 @@ export const processPresenceAndStatus = ( userSessions: IUserSessionConnection[] = [], statusDefault = UserStatus.ONLINE, ): { status: UserStatus; statusConnection: UserStatus } => { - const statusConnection = userSessions - .filter((c) => isFresh(c._updatedAt)) - .map((s) => s.status) - .reduce(processConnectionStatus, UserStatus.OFFLINE); + const statusConnection = userSessions.map((s) => s.status).reduce(processConnectionStatus, UserStatus.OFFLINE); const status = processStatus(statusConnection, statusDefault); diff --git a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts index 1431b1b5b0a41..1646636d92b2e 100644 --- a/ee/packages/presence/tests/lib/processConnectionStatus.test.ts +++ b/ee/packages/presence/tests/lib/processConnectionStatus.test.ts @@ -237,88 +237,4 @@ describe('Presence micro service', () => { statusConnection: UserStatus.OFFLINE, }); }); - - test('should ignore connections last updated more than 5 minutes ago', () => { - const now = new Date(); - const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); - const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); - - expect( - processPresenceAndStatus( - [ - { - id: 'random1', - instanceId: 'random1', - status: UserStatus.ONLINE, - _createdAt: sixMinutesAgo, - _updatedAt: sixMinutesAgo, - }, - { - id: 'random2', - instanceId: 'random2', - status: UserStatus.AWAY, - _createdAt: fourMinutesAgo, - _updatedAt: fourMinutesAgo, - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.AWAY, statusConnection: UserStatus.AWAY }); - }); - - test('should return offline if all connections are stale', () => { - const now = new Date(); - const sixMinutesAgo = new Date(now.getTime() - 6 * 60 * 1000); - const sevenMinutesAgo = new Date(now.getTime() - 7 * 60 * 1000); - - expect( - processPresenceAndStatus( - [ - { - id: 'random1', - instanceId: 'random1', - status: UserStatus.ONLINE, - _createdAt: sixMinutesAgo, - _updatedAt: sixMinutesAgo, - }, - { - id: 'random2', - instanceId: 'random2', - status: UserStatus.AWAY, - _createdAt: sevenMinutesAgo, - _updatedAt: sevenMinutesAgo, - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.OFFLINE, statusConnection: UserStatus.OFFLINE }); - }); - - test('should consider all connections if they were updated within the last 5 minutes', () => { - const now = new Date(); - const threeMinutesAgo = new Date(now.getTime() - 3 * 60 * 1000); - const fourMinutesAgo = new Date(now.getTime() - 4 * 60 * 1000); - - expect( - processPresenceAndStatus( - [ - { - id: 'random1', - instanceId: 'random1', - status: UserStatus.ONLINE, - _createdAt: threeMinutesAgo, - _updatedAt: threeMinutesAgo, - }, - { - id: 'random2', - instanceId: 'random2', - status: UserStatus.AWAY, - _createdAt: fourMinutesAgo, - _updatedAt: fourMinutesAgo, - }, - ], - UserStatus.ONLINE, - ), - ).toStrictEqual({ status: UserStatus.ONLINE, statusConnection: UserStatus.ONLINE }); - }); }); From 5112f8a3e1eb3d1b3ad6e899e40d0aea85305dd1 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Thu, 4 Dec 2025 19:25:09 -0300 Subject: [PATCH 16/23] fix: only update connection if no packet was seen --- apps/meteor/ee/server/startup/presence.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index 966916498ae59..28f121fe396ae 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -42,7 +42,9 @@ Meteor.startup(() => { const _messageReceived = session.heartbeat.messageReceived.bind(session.heartbeat); session.heartbeat.messageReceived = function messageReceived() { - void Presence.updateConnection(login.user._id, login.connection.id); + if (this._seenPacket === false) { + void Presence.updateConnection(login.user._id, login.connection.id); + } return _messageReceived(); }; From ff983de58d6f5bb3b95851fc3f19f49355c37f1e Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 5 Dec 2025 09:33:49 -0300 Subject: [PATCH 17/23] chore: remove comments & unused properties --- ee/packages/presence/src/Presence.ts | 76 ++-------------------------- 1 file changed, 4 insertions(+), 72 deletions(-) diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index e51f35b3446de..53149d68b1442 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -7,10 +7,7 @@ import { Settings, Users, UsersSessions } from '@rocket.chat/models'; import { PresenceReaper } from './lib/PresenceReaper'; import { processPresenceAndStatus } from './lib/processConnectionStatus'; -/** - * Maximum number of connections allowed. - */ -export const MAX_CONNECTIONS = 200; +const MAX_CONNECTIONS = 200; export class Presence extends ServiceClass implements IPresence { protected name = 'presence'; @@ -25,8 +22,6 @@ export class Presence extends ServiceClass implements IPresence { private lostConTimeout?: NodeJS.Timeout; - private staleConInterval?: NodeJS.Timeout; - private connsPerInstance = new Map(); private peakConnections = 0; @@ -111,8 +106,10 @@ export class Presence extends ServiceClass implements IPresence { } async stopped(): Promise { + if (!this.lostConTimeout) { + return; + } clearTimeout(this.lostConTimeout); - clearInterval(this.staleConInterval); } async toggleBroadcast(enabled: boolean): Promise { @@ -168,71 +165,6 @@ export class Presence extends ServiceClass implements IPresence { return { uid, session }; } - /** - * Runs the cleanup job to remove stale connections and sync user status. - */ - // async removeStaleConnections() { - // console.debug('[Cleanup] Starting stale connections cleanup job.'); - // const cutoffDate = new Date(Date.now() - this.staleThreshold); - - // // STEP 1: Find users who have AT LEAST one stale connection - // // We project the whole connections array because we need to inspect it in memory - // const cursor = UsersSessions.find({ 'connections._updatedAt': { $lte: cutoffDate } }, { projection: { _id: 1, connections: 1 } }); - - // const bulkSessionOps: AnyBulkWriteOperation[] = []; - // // const bulkUserOps: AnyBulkWriteOperation[] = []; - // const processedUserIds = []; - - // // STEP 2: Iterate and Calculate - // for await (const sessionDoc of cursor) { - // const userId = sessionDoc._id; - // const allConnections = sessionDoc.connections || []; - - // // Separate valid vs stale based on the cutoff - // const staleConnections = allConnections.filter((c) => c._updatedAt <= cutoffDate); - // const validConnections = allConnections.filter((c) => c._updatedAt > cutoffDate); - - // if (staleConnections.length === 0) continue; // Should not happen due to query, but safe to check - - // // Collect the IDs of the connections we want to remove - // const staleConnectionIds = staleConnections.map((c) => c.id); - - // // OPERATION A: Remove specific connections from usersSessions - // // We use the unique IDs to be surgically precise - // bulkSessionOps.push({ - // updateOne: { - // filter: { _id: userId }, - // update: { - // $pull: { - // connections: { id: { $in: staleConnectionIds }, _updatedAt: { $lte: cutoffDate } }, - // }, - // }, - // }, - // }); - - // // OPERATION B: Update user status if they will have NO connections left - // if (validConnections.length === 0) { - // // bulkUserOps.push({ - // // updateOne: { - // // filter: { _id: userId }, - // // update: { $set: { status: UserStatus.OFFLINE } }, - // // }, - // // }); - // processedUserIds.push(userId); - // } - // } - - // // STEP 3: Execute the operations - // if (bulkSessionOps.length > 0) { - // await UsersSessions.col.bulkWrite(bulkSessionOps); - // console.log(`[Cleanup] Removed stale connections for ${bulkSessionOps.length} users.`); - // } - - // console.debug(`[Cleanup] Finished stale connections cleanup job.`); - - // return processedUserIds; - // } - async removeConnection(uid: string | undefined, session: string | undefined): Promise<{ uid: string; session: string } | undefined> { if (uid === 'rocketchat.internal.admin.test') { console.log('Admin detected, skipping removal of connection for testing purposes.'); From 96b25ceb3121622b23c8af39c1abd40552a1d9f3 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 5 Dec 2025 13:02:02 -0300 Subject: [PATCH 18/23] improve: presence reaper robustness and connection updates Adds resilience and configurability to presence reaping and connection updates to prevent dangling/stale connections and unhandled errors. - Ensure async connection updates on heartbeat are caught and logged to avoid unhandled rejections. - Refactor presence reaper to accept options (usersSessions, onUpdate, stale threshold, batch size), use a typed non-empty callback, and manage its interval lifecycle (start/stop). - Make reaper notify via batched callbacks and treat empty batches safely. - Update Presence service to instantiate and start/stop the reaper correctly and adapt to the new callback signature. - Simplify updateConnection to use a direct updateOne query and return connectionId for clarity. - Expand and harden tests for PresenceReaper (empty cursor handling, batching behavior). These changes reduce dangling connections, improve testability, and make reaper behavior configurable and safer in production. --- apps/meteor/ee/server/startup/presence.ts | 4 +- ee/packages/presence/src/Presence.ts | 37 ++++++--- .../presence/src/lib/PresenceReaper.spec.ts | 83 ++++++++++++++----- .../presence/src/lib/PresenceReaper.ts | 45 +++++++--- packages/core-services/src/types/IPresence.ts | 2 +- .../src/models/IUsersSessionsModel.ts | 2 - packages/models/src/models/UsersSessions.ts | 39 --------- 7 files changed, 124 insertions(+), 88 deletions(-) diff --git a/apps/meteor/ee/server/startup/presence.ts b/apps/meteor/ee/server/startup/presence.ts index 28f121fe396ae..6bf587b500ba6 100644 --- a/apps/meteor/ee/server/startup/presence.ts +++ b/apps/meteor/ee/server/startup/presence.ts @@ -43,7 +43,9 @@ Meteor.startup(() => { const _messageReceived = session.heartbeat.messageReceived.bind(session.heartbeat); session.heartbeat.messageReceived = function messageReceived() { if (this._seenPacket === false) { - void Presence.updateConnection(login.user._id, login.connection.id); + void Presence.updateConnection(login.user._id, login.connection.id).catch((err) => { + console.error('Error updating connection presence on heartbeat:', err); + }); } return _messageReceived(); }; diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index 53149d68b1442..0333fa6005f69 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -31,6 +31,13 @@ export class Presence extends ServiceClass implements IPresence { constructor() { super(); + this.reaper = new PresenceReaper({ + usersSessions: UsersSessions, + batchSize: 500, + staleThresholdMs: 5 * 60 * 1000, // 5 minutes + onUpdate: (userIds) => this.handleReaperUpdates(userIds), + }); + this.onEvent('watch.instanceStatus', async ({ clientAction, id, diff }): Promise => { if (clientAction === 'removed') { this.connsPerInstance.delete(id); @@ -76,14 +83,12 @@ export class Presence extends ServiceClass implements IPresence { } async started(): Promise { + this.reaper.start(); this.lostConTimeout = setTimeout(async () => { const affectedUsers = await this.removeLostConnections(); return affectedUsers.forEach((uid) => this.updateUserPresence(uid)); }, 10000); - this.reaper = new PresenceReaper(UsersSessions, (userIds) => this.handleReaperUpdates(userIds)); - this.reaper.start(); - try { await Settings.updateValueById('Presence_broadcast_disabled', false); @@ -96,16 +101,12 @@ export class Presence extends ServiceClass implements IPresence { } private async handleReaperUpdates(userIds: string[]): Promise { - if (userIds.length === 0) { - return; - } - console.log(`[PresenceReaper] Updating presence for ${userIds.length} users due to stale connections.`); - await Promise.all(userIds.map((uid) => this.updateUserPresence(uid))); } async stopped(): Promise { + this.reaper.stop(); if (!this.lostConTimeout) { return; } @@ -153,16 +154,28 @@ export class Presence extends ServiceClass implements IPresence { }; } - async updateConnection(uid: string, session: string): Promise<{ uid: string; session: string } | undefined> { - console.debug(`Updating connection for user ${uid} and session ${session}`); - const result = await UsersSessions.updateConnectionById(uid, session); + async updateConnection(uid: string, connectionId: string): Promise<{ uid: string; connectionId: string } | undefined> { + console.debug(`Updating connection for user ${uid} and connection ${connectionId}`); + + const query = { + '_id': uid, + 'connections.id': connectionId, + }; + + const update = { + $set: { + 'connections.$._updatedAt': new Date(), + }, + }; + + const result = await UsersSessions.updateOne(query, update); if (result.modifiedCount === 0) { return; } await this.updateUserPresence(uid); - return { uid, session }; + return { uid, connectionId }; } async removeConnection(uid: string | undefined, session: string | undefined): Promise<{ uid: string; session: string } | undefined> { diff --git a/ee/packages/presence/src/lib/PresenceReaper.spec.ts b/ee/packages/presence/src/lib/PresenceReaper.spec.ts index 90198acda5655..edc92648b8e85 100644 --- a/ee/packages/presence/src/lib/PresenceReaper.spec.ts +++ b/ee/packages/presence/src/lib/PresenceReaper.spec.ts @@ -1,6 +1,6 @@ import type { IUserSession } from '@rocket.chat/core-typings'; import type { IUsersSessionsModel } from '@rocket.chat/model-typings'; -import type { Collection } from 'mongodb'; +import type { Collection, FindCursor, WithId } from 'mongodb'; import { PresenceReaper } from './PresenceReaper'; @@ -30,7 +30,12 @@ describe('PresenceReaper', () => { mockOnUpdate = jest.fn(); // 3. Instantiate Reaper - reaper = new PresenceReaper(mockSessionCollection, mockOnUpdate); + reaper = new PresenceReaper({ + usersSessions: mockSessionCollection, + onUpdate: mockOnUpdate, + staleThresholdMs: 5 * 60 * 1000, // 5 minutes + batchSize: 2, // small batch size for testing + }); }); describe('processDocument (Business Logic)', () => { @@ -88,6 +93,22 @@ describe('PresenceReaper', () => { }); describe('run (Integration Flow)', () => { + it('should handle empty collections without errors', async () => { + // Mock empty cursor + const mockCursor = { + async *[Symbol.asyncIterator]() { + // No documents + }, + } as FindCursor>; + mockSessionCollection.find.mockReturnValue(mockCursor); + + // Execute Run + await reaper.run(); + + // Verify no updates were made + expect(mockOnUpdate).not.toHaveBeenCalled(); + }); + it('should generate correct bulkWrite operations', async () => { const now = new Date(); const staleTime = new Date(now.getTime() - 6 * 60 * 1000); // 6 mins ago (Stale) @@ -100,35 +121,51 @@ describe('PresenceReaper', () => { connections: [{ id: 'zombie-conn', _updatedAt: staleTime }], }; }, - } as any; + } as FindCursor>; mockSessionCollection.find.mockReturnValue(mockCursor); // Execute Run await reaper.run(); - // Verify 'usersSessions' Update - expect(mockSessionCollection.col.bulkWrite).toHaveBeenCalledTimes(1); - expect(mockSessionCollection.col.bulkWrite).toHaveBeenCalledWith( - expect.arrayContaining([ - expect.objectContaining({ - updateOne: expect.objectContaining({ - filter: { _id: 'user-789' }, - update: { - $pull: { - connections: { - id: { $in: ['zombie-conn'] }, - _updatedAt: { $lte: expect.any(Date) }, - }, - }, - }, - }), - }), - ]), - ); - // Verify 'users' Update (Status Offline) expect(mockOnUpdate).toHaveBeenCalledTimes(1); expect(mockOnUpdate).toHaveBeenCalledWith(['user-789']); }); }); + + describe('end-to-end Presence Reaping', () => { + it('should process multiple users and batch updates correctly', async () => { + const now = new Date(); + const staleTime = new Date(now.getTime() - 10 * 60 * 1000); // 10 mins ago + + // Mock Data from DB Cursor + const mockCursor = { + async *[Symbol.asyncIterator]() { + yield { + _id: 'user-1', + + connections: [{ id: 'conn-1', _updatedAt: staleTime }], + }; + yield { + _id: 'user-2', + connections: [{ id: 'conn-2', _updatedAt: staleTime }], + }; + yield { + _id: 'user-3', + + connections: [{ id: 'conn-3', _updatedAt: staleTime }], + }; + }, + }; + mockSessionCollection.find.mockReturnValue(mockCursor as FindCursor>); + + // Execute Run + await reaper.run(); + + // Verify 'users' Update called twice due to batch size of 2 + expect(mockOnUpdate).toHaveBeenCalledTimes(2); + expect(mockOnUpdate).toHaveBeenNthCalledWith(1, ['user-1', 'user-2']); + expect(mockOnUpdate).toHaveBeenNthCalledWith(2, ['user-3']); + }); + }); }); diff --git a/ee/packages/presence/src/lib/PresenceReaper.ts b/ee/packages/presence/src/lib/PresenceReaper.ts index 9974b4c5db2d1..40c1779f0108b 100644 --- a/ee/packages/presence/src/lib/PresenceReaper.ts +++ b/ee/packages/presence/src/lib/PresenceReaper.ts @@ -1,3 +1,5 @@ +import { setInterval } from 'node:timers'; + import type { IUserSession } from '@rocket.chat/core-typings'; import type { IUsersSessionsModel } from '@rocket.chat/model-typings'; @@ -8,7 +10,18 @@ type ReaperPlan = { cutoffDate: Date; }; -type ReaperCallback = (userIds: string[]) => void; +type NonEmptyArray = [T, ...T[]]; + +const isNonEmptyArray = (arr: T[]): arr is NonEmptyArray => arr.length > 0; + +type ReaperCallback = (userIds: NonEmptyArray) => void; + +type ReaperOptions = { + usersSessions: IUsersSessionsModel; + onUpdate: ReaperCallback; + staleThresholdMs: number; + batchSize: number; +}; export class PresenceReaper { private usersSessions: IUsersSessionsModel; @@ -21,13 +34,13 @@ export class PresenceReaper { private onUpdate: ReaperCallback; - constructor(usersSessions: IUsersSessionsModel, onUpdate: ReaperCallback) { - this.usersSessions = usersSessions; - this.onUpdate = onUpdate; + private intervalId?: NodeJS.Timeout; - // Configuration: 5 minutes stale, process 500 users at a time - this.staleThresholdMs = 5 * 60 * 1000; - this.batchSize = 500; + constructor(options: ReaperOptions) { + this.usersSessions = options.usersSessions; + this.onUpdate = options.onUpdate; + this.staleThresholdMs = options.staleThresholdMs; + this.batchSize = options.batchSize; this.running = false; } @@ -36,13 +49,25 @@ export class PresenceReaper { this.running = true; // Run every 1 minute - setInterval(() => { + this.intervalId = setInterval(() => { this.run().catch((err) => console.error('[PresenceReaper] Error:', err)); }, 60 * 1000); console.log('[PresenceReaper] Service started.'); } + public stop() { + if (!this.running) return; + this.running = false; + + if (this.intervalId) { + clearInterval(this.intervalId); + this.intervalId = undefined; + } + + console.log('[PresenceReaper] Service stopped.'); + } + public async run(): Promise { console.log('[PresenceReaper] Running presence reaper job...'); const cutoffDate = new Date(Date.now() - this.staleThresholdMs); @@ -123,8 +148,8 @@ export class PresenceReaper { await this.usersSessions.col.bulkWrite(sessionOps); } - // Step B: EMIT the event instead of calling a method directly - if (usersToUpdate.length > 0) { + // Step B: Notify Presence Service + if (isNonEmptyArray(usersToUpdate)) { this.onUpdate(usersToUpdate); } } diff --git a/packages/core-services/src/types/IPresence.ts b/packages/core-services/src/types/IPresence.ts index 42b87ec50d0eb..268fed8cd1e66 100644 --- a/packages/core-services/src/types/IPresence.ts +++ b/packages/core-services/src/types/IPresence.ts @@ -13,7 +13,7 @@ export interface IPresence extends IServiceClass { session: string | undefined, nodeId: string, ): Promise<{ uid: string; session: string } | undefined>; - updateConnection(uid: string, session: string): Promise<{ uid: string; session: string } | undefined>; + updateConnection(uid: string, connectionId: string): Promise<{ uid: string; connectionId: string } | undefined>; removeLostConnections(nodeID: string): Promise; setStatus(uid: string, status: UserStatus, statusText?: string): Promise; setConnectionStatus(uid: string, status: UserStatus, session: string): Promise; diff --git a/packages/model-typings/src/models/IUsersSessionsModel.ts b/packages/model-typings/src/models/IUsersSessionsModel.ts index cc6c7e6b5a3cb..75800af233bcc 100644 --- a/packages/model-typings/src/models/IUsersSessionsModel.ts +++ b/packages/model-typings/src/models/IUsersSessionsModel.ts @@ -6,8 +6,6 @@ import type { IBaseModel } from './IBaseModel'; export interface IUsersSessionsModel extends IBaseModel { clearConnectionsFromInstanceId(instanceId: string[]): ReturnType['updateMany']>; updateConnectionStatusById(uid: string, connectionId: string, status: string): ReturnType['updateOne']>; - updateConnectionById(uid: string, connectionId: string): ReturnType['updateOne']>; - findStaleConnections(cutoff: Date): FindCursor; removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']>; removeConnectionByConnectionId(connectionId: string): ReturnType['updateMany']>; findByInstanceId(instanceId: string): FindCursor; diff --git a/packages/models/src/models/UsersSessions.ts b/packages/models/src/models/UsersSessions.ts index 5fa4e8ebb5243..02cfd889eb277 100644 --- a/packages/models/src/models/UsersSessions.ts +++ b/packages/models/src/models/UsersSessions.ts @@ -45,45 +45,6 @@ export class UsersSessionsRaw extends BaseRaw implements IUsersSes return this.updateOne(query, update); } - updateConnectionById(uid: string, connectionId: string): ReturnType['updateOne']> { - const query = { - '_id': uid, - 'connections.id': connectionId, - }; - - const update = { - $set: { - 'connections.$._updatedAt': new Date(), - }, - }; - - return this.updateOne(query, update); - } - - findStaleConnections(cutoff: Date): FindCursor { - return this.find( - { - 'connections._updatedAt': { $lt: cutoff }, - }, - { projection: { _id: 1 } }, - ); - } - - removeConnectionsByConnectionIds(connectionIds: string[]): ReturnType['updateMany']> { - return this.updateMany( - { - 'connections.id': { $in: connectionIds }, - }, - { - $pull: { - connections: { - id: { $in: connectionIds }, - }, - }, - }, - ); - } - async removeConnectionsFromInstanceId(instanceId: string): ReturnType['updateMany']> { return this.updateMany( { From 3b0c9d06b064f9e2d12804cd184bd7d70d38bdd3 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 5 Dec 2025 15:09:14 -0300 Subject: [PATCH 19/23] fix(ddp-streamer): only update connection on heartbeat --- ee/apps/ddp-streamer/src/Client.ts | 20 +++++++++++++++++++- ee/apps/ddp-streamer/src/DDPStreamer.ts | 8 -------- ee/apps/ddp-streamer/src/constants.ts | 1 - 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/ee/apps/ddp-streamer/src/Client.ts b/ee/apps/ddp-streamer/src/Client.ts index 661c1d2931125..0ce8762948726 100644 --- a/ee/apps/ddp-streamer/src/Client.ts +++ b/ee/apps/ddp-streamer/src/Client.ts @@ -1,6 +1,7 @@ import { EventEmitter } from 'events'; import type { IncomingMessage } from 'http'; +import { Presence } from '@rocket.chat/core-services'; import type { ISocketConnection } from '@rocket.chat/core-typings'; import { v1 as uuidv1 } from 'uuid'; import type WebSocket from 'ws'; @@ -73,6 +74,8 @@ export class Client extends EventEmitter { public userToken?: string; + private _seenPacket = true; + constructor( public ws: WebSocket, public meteorClient = false, @@ -179,6 +182,18 @@ export class Client extends EventEmitter { this.ws.close(WS_ERRORS.TIMEOUT, WS_ERRORS_MESSAGES.TIMEOUT); }; + private messageReceived = (): void => { + if (this._seenPacket || !this.userId) { + this._seenPacket = true; + return; + } + + this._seenPacket = true; + void Presence.updateConnection(this.userId, this.connection.id).catch((err) => { + console.error('Error updating connection presence after heartbeat:', err); + }); + }; + ping(id?: string): void { this.send(server.serialize({ [DDP_EVENTS.MSG]: DDP_EVENTS.PING, ...(id && { [DDP_EVENTS.ID]: id }) })); } @@ -188,6 +203,9 @@ export class Client extends EventEmitter { } handleIdle = (): void => { + if (this.userId) { + this._seenPacket = false; + } this.ping(); this.timeout = setTimeout(this.closeTimeout, TIMEOUT); }; @@ -200,8 +218,8 @@ export class Client extends EventEmitter { handler = async (payload: WebSocket.Data, isBinary: boolean): Promise => { try { const packet = server.parse(payload, isBinary); + this.messageReceived(); this.emit('message', packet); - server.emit(DDP_EVENTS.MESSAGE, this); if (this.wait) { return new Promise((resolve) => this.once(DDP_EVENTS.LOGGED, () => resolve(this.process(packet.msg, packet)))); } diff --git a/ee/apps/ddp-streamer/src/DDPStreamer.ts b/ee/apps/ddp-streamer/src/DDPStreamer.ts index 44517b3dbf4ac..1a6a1046ae776 100644 --- a/ee/apps/ddp-streamer/src/DDPStreamer.ts +++ b/ee/apps/ddp-streamer/src/DDPStreamer.ts @@ -215,14 +215,6 @@ export class DDPStreamer extends ServiceClass { server.on(DDP_EVENTS.CONNECTED, ({ connection }) => { this.api?.broadcast('socket.connected', connection); }); - - server.on(DDP_EVENTS.MESSAGE, (client: Client): void => { - const { connection, userId } = client; - if (!userId) { - return; - } - void Presence.updateConnection(userId, connection.id); - }); } async started(): Promise { diff --git a/ee/apps/ddp-streamer/src/constants.ts b/ee/apps/ddp-streamer/src/constants.ts index 23dd4d9e563ec..398ae16ce5d9a 100644 --- a/ee/apps/ddp-streamer/src/constants.ts +++ b/ee/apps/ddp-streamer/src/constants.ts @@ -16,7 +16,6 @@ export const DDP_EVENTS = { CHANGED: 'changed', REMOVED: 'removed', RESUME: 'resume', - MESSAGE: 'message', RESULT: 'result', METHOD: 'method', UPDATED: 'updated', From 56332254e5c2c221aca044990d203a186a012d11 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Sat, 6 Dec 2025 18:30:01 -0300 Subject: [PATCH 20/23] chore: remove test code --- ee/packages/presence/src/Presence.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index 0333fa6005f69..963d695c8b4fc 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -179,10 +179,6 @@ export class Presence extends ServiceClass implements IPresence { } async removeConnection(uid: string | undefined, session: string | undefined): Promise<{ uid: string; session: string } | undefined> { - if (uid === 'rocketchat.internal.admin.test') { - console.log('Admin detected, skipping removal of connection for testing purposes.'); - return; - } if (!uid || !session) { return; } From 0882ac2f5b7a44a54024d4dec88815e04dfbe4f6 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Tue, 9 Dec 2025 15:36:58 -0300 Subject: [PATCH 21/23] fix(presence): reaper error handling and logic --- ee/packages/presence/src/Presence.ts | 17 +- .../presence/src/lib/PresenceReaper.spec.ts | 230 +++++++++++------- .../presence/src/lib/PresenceReaper.ts | 24 +- 3 files changed, 164 insertions(+), 107 deletions(-) diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index 484f07a0c05f0..83fd494e83f21 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -32,7 +32,6 @@ export class Presence extends ServiceClass implements IPresence { super(); this.reaper = new PresenceReaper({ - usersSessions: UsersSessions, batchSize: 500, staleThresholdMs: 5 * 60 * 1000, // 5 minutes onUpdate: (userIds) => this.handleReaperUpdates(userIds), @@ -101,8 +100,20 @@ export class Presence extends ServiceClass implements IPresence { } private async handleReaperUpdates(userIds: string[]): Promise { - console.log(`[PresenceReaper] Updating presence for ${userIds.length} users due to stale connections.`); - await Promise.all(userIds.map((uid) => this.updateUserPresence(uid))); + const results = await Promise.allSettled(userIds.map((uid) => this.updateUserPresence(uid))); + const fulfilled = results.filter((result) => result.status === 'fulfilled'); + const rejected = results.filter((result) => result.status === 'rejected'); + + if (fulfilled.length > 0) { + console.debug(`[PresenceReaper] Successfully updated presence for ${fulfilled.length} users.`); + } + + if (rejected.length > 0) { + console.error( + `[PresenceReaper] Failed to update presence for ${rejected.length} users:`, + rejected.map(({ reason }) => reason), + ); + } } override async stopped(): Promise { diff --git a/ee/packages/presence/src/lib/PresenceReaper.spec.ts b/ee/packages/presence/src/lib/PresenceReaper.spec.ts index edc92648b8e85..f0bba7150c1ce 100644 --- a/ee/packages/presence/src/lib/PresenceReaper.spec.ts +++ b/ee/packages/presence/src/lib/PresenceReaper.spec.ts @@ -1,38 +1,63 @@ -import type { IUserSession } from '@rocket.chat/core-typings'; -import type { IUsersSessionsModel } from '@rocket.chat/model-typings'; -import type { Collection, FindCursor, WithId } from 'mongodb'; +import { UserStatus, type IUserSession, type IUserSessionConnection } from '@rocket.chat/core-typings'; +import { registerModel } from '@rocket.chat/models'; +import type { FindCursor, WithId } from 'mongodb'; -import { PresenceReaper } from './PresenceReaper'; +import { PresenceReaper, type ReaperPlan } from './PresenceReaper'; -// Define a simplified interface for our mock docs -type MockSession = { - _id: string; - connections: { id: string; _updatedAt: Date }[]; +const createSession = (overrides: Partial = {}): IUserSession => ({ + _id: 'user-id', + connections: [], + ...overrides, +}); + +const createConnection = (overrides: Partial = {}): IUserSessionConnection => ({ + id: 'connection-id', + instanceId: 'instance-id', + status: UserStatus.ONLINE, + _createdAt: new Date(), + _updatedAt: new Date(), + ...overrides, +}); + +const createDates = () => { + const now = new Date(); + const stale = new Date(now.getTime() - 10 * 60 * 1000); // 10 mins ago + const active = new Date(now.getTime() - 1 * 60 * 1000); // 1 min ago + const cutoff = new Date(now.getTime() - 5 * 60 * 1000); // 5 mins ago + + return { now, stale, active, cutoff }; +}; + +const createCursor = (documents: WithId[]): FindCursor> => { + let index = 0; + return { + async *[Symbol.asyncIterator]() { + while (index < documents.length) { + yield documents[index++]; + } + }, + } as FindCursor>; }; describe('PresenceReaper', () => { let reaper: PresenceReaper; - let mockSessionCollection: Omit, 'col'> & { - col: jest.Mocked>; - }; - let mockOnUpdate: jest.Mock; + const bulkWriteMock = jest.fn(); + const findMock = jest.fn(); + const onUpdateMock = jest.fn(); + registerModel('IUsersSessionsModel', { + find: findMock, + col: { + bulkWrite: bulkWriteMock, + }, + } as any); beforeEach(() => { - // 1. Mock the Collections - mockSessionCollection = { - find: jest.fn(), - col: { - bulkWrite: jest.fn().mockResolvedValue({ modifiedCount: 1 }), - }, - } as any; - - // 2. Mock the onUpdate callback - mockOnUpdate = jest.fn(); - - // 3. Instantiate Reaper + bulkWriteMock.mockClear(); + findMock.mockClear(); + onUpdateMock.mockClear(); + reaper = new PresenceReaper({ - usersSessions: mockSessionCollection, - onUpdate: mockOnUpdate, + onUpdate: onUpdateMock, staleThresholdMs: 5 * 60 * 1000, // 5 minutes batchSize: 2, // small batch size for testing }); @@ -40,22 +65,16 @@ describe('PresenceReaper', () => { describe('processDocument (Business Logic)', () => { it('should identify stale connections by "id" and preserve valid ones', () => { - const now = new Date(); - const staleTime = new Date(now.getTime() - 10 * 60 * 1000); // 10 mins ago - const activeTime = new Date(now.getTime() - 1 * 60 * 1000); // 1 min ago - const cutoff = new Date(now.getTime() - 5 * 60 * 1000); // 5 mins ago - - const doc: MockSession = { + const { stale, active, cutoff } = createDates(); + const doc = createSession({ _id: 'user-123', connections: [ - { id: 'conn-stale', _updatedAt: staleTime }, // Should be removed - { id: 'conn-active', _updatedAt: activeTime }, // Should stay + createConnection({ id: 'conn-stale', _updatedAt: stale }), // Should be removed + createConnection({ id: 'conn-active', _updatedAt: active }), // Should stay ], - }; - - const changeMap = new Map(); + }); - // @ts-expect-error - testing private method + const changeMap = new Map(); reaper.processDocument(doc, cutoff, changeMap); const result = changeMap.get('user-123'); @@ -67,21 +86,38 @@ describe('PresenceReaper', () => { expect(result?.shouldMarkOffline).toBe(false); // User still has 1 active connection }); + it('should not mark user offline if there are still valid connections', () => { + const { stale, active, cutoff } = createDates(); + const doc = createSession({ + _id: 'user-456', + connections: [ + createConnection({ id: 'conn-stale', _updatedAt: stale, status: UserStatus.ONLINE }), + createConnection({ id: 'conn-active', _updatedAt: active, status: UserStatus.AWAY }), + ], + }); + + const changeMap = new Map(); + reaper.processDocument(doc, cutoff, changeMap); + + const result = changeMap.get('user-456'); + + expect(result).toBeDefined(); + expect(result?.removeIds).toHaveLength(1); + expect(result?.shouldMarkOffline).toBe(false); // Still has valid connection + }); + it('should mark user offline only if ALL connections are stale', () => { - const now = new Date(); - const staleTime = new Date(now.getTime() - 10000); - const cutoff = new Date(now); // Cutoff is now, so everything before is stale + const { stale, cutoff } = createDates(); - const doc: MockSession = { + const doc = createSession({ _id: 'user-456', connections: [ - { id: 'conn-1', _updatedAt: staleTime }, - { id: 'conn-2', _updatedAt: staleTime }, + createConnection({ id: 'conn-1', _updatedAt: stale, status: UserStatus.ONLINE }), + createConnection({ id: 'conn-2', _updatedAt: stale, status: UserStatus.AWAY }), ], - }; + }); - const changeMap = new Map(); - // @ts-expect-error - testing private method + const changeMap = new Map(); reaper.processDocument(doc, cutoff, changeMap); const result = changeMap.get('user-456'); @@ -95,77 +131,95 @@ describe('PresenceReaper', () => { describe('run (Integration Flow)', () => { it('should handle empty collections without errors', async () => { // Mock empty cursor - const mockCursor = { - async *[Symbol.asyncIterator]() { - // No documents - }, - } as FindCursor>; - mockSessionCollection.find.mockReturnValue(mockCursor); + findMock.mockReturnValue(createCursor([])); // Execute Run await reaper.run(); // Verify no updates were made - expect(mockOnUpdate).not.toHaveBeenCalled(); + expect(onUpdateMock).not.toHaveBeenCalled(); }); it('should generate correct bulkWrite operations', async () => { - const now = new Date(); - const staleTime = new Date(now.getTime() - 6 * 60 * 1000); // 6 mins ago (Stale) + const { stale } = createDates(); // Mock Data from DB Cursor - const mockCursor = { - async *[Symbol.asyncIterator]() { - yield { + + findMock.mockReturnValue( + createCursor([ + createSession({ _id: 'user-789', - connections: [{ id: 'zombie-conn', _updatedAt: staleTime }], - }; - }, - } as FindCursor>; - mockSessionCollection.find.mockReturnValue(mockCursor); + connections: [createConnection({ id: 'zombie-conn', _updatedAt: stale })], + }), + ]), + ); // Execute Run await reaper.run(); // Verify 'users' Update (Status Offline) - expect(mockOnUpdate).toHaveBeenCalledTimes(1); - expect(mockOnUpdate).toHaveBeenCalledWith(['user-789']); + expect(onUpdateMock).toHaveBeenCalledTimes(1); + expect(onUpdateMock).toHaveBeenCalledWith(['user-789']); }); }); describe('end-to-end Presence Reaping', () => { it('should process multiple users and batch updates correctly', async () => { - const now = new Date(); - const staleTime = new Date(now.getTime() - 10 * 60 * 1000); // 10 mins ago + const { stale } = createDates(); - // Mock Data from DB Cursor - const mockCursor = { - async *[Symbol.asyncIterator]() { - yield { + findMock.mockReturnValue( + createCursor([ + createSession({ _id: 'user-1', - - connections: [{ id: 'conn-1', _updatedAt: staleTime }], - }; - yield { + connections: [createConnection({ id: 'conn-1', _updatedAt: stale })], + }), + createSession({ _id: 'user-2', - connections: [{ id: 'conn-2', _updatedAt: staleTime }], - }; - yield { + connections: [createConnection({ id: 'conn-2', _updatedAt: stale })], + }), + createSession({ _id: 'user-3', - - connections: [{ id: 'conn-3', _updatedAt: staleTime }], - }; - }, - }; - mockSessionCollection.find.mockReturnValue(mockCursor as FindCursor>); + connections: [createConnection({ id: 'conn-3', _updatedAt: stale })], + }), + ]), + ); // Execute Run await reaper.run(); // Verify 'users' Update called twice due to batch size of 2 - expect(mockOnUpdate).toHaveBeenCalledTimes(2); - expect(mockOnUpdate).toHaveBeenNthCalledWith(1, ['user-1', 'user-2']); - expect(mockOnUpdate).toHaveBeenNthCalledWith(2, ['user-3']); + expect(onUpdateMock).toHaveBeenCalledTimes(2); + expect(onUpdateMock).toHaveBeenNthCalledWith(1, ['user-1', 'user-2']); + expect(onUpdateMock).toHaveBeenNthCalledWith(2, ['user-3']); + }); + + it('should process users with mixed connection states correctly', async () => { + const { stale, active } = createDates(); + + findMock.mockReturnValue( + createCursor([ + // User with all stale connections + createSession({ + _id: 'user-all-stale', + connections: [createConnection({ id: 'conn-stale-1', _updatedAt: stale })], + }), + // User with mixed connections + createSession({ + _id: 'user-mixed', + connections: [ + createConnection({ id: 'conn-stale-2', _updatedAt: stale }), + createConnection({ id: 'conn-active-1', _updatedAt: active }), + ], + }), + ]), + ); + + // Execute Run + await reaper.run(); + + // Verify 'users' Update called for both users + expect(onUpdateMock).toHaveBeenCalledTimes(1); + expect(onUpdateMock).toHaveBeenNthCalledWith(1, ['user-all-stale', 'user-mixed']); }); }); }); diff --git a/ee/packages/presence/src/lib/PresenceReaper.ts b/ee/packages/presence/src/lib/PresenceReaper.ts index 40c1779f0108b..c505ef2b44033 100644 --- a/ee/packages/presence/src/lib/PresenceReaper.ts +++ b/ee/packages/presence/src/lib/PresenceReaper.ts @@ -1,9 +1,10 @@ import { setInterval } from 'node:timers'; import type { IUserSession } from '@rocket.chat/core-typings'; -import type { IUsersSessionsModel } from '@rocket.chat/model-typings'; +import { UsersSessions } from '@rocket.chat/models'; +import type { AnyBulkWriteOperation } from 'mongodb'; -type ReaperPlan = { +export type ReaperPlan = { userId: string; removeIds: string[]; shouldMarkOffline: boolean; @@ -17,15 +18,12 @@ const isNonEmptyArray = (arr: T[]): arr is NonEmptyArray => arr.length > 0 type ReaperCallback = (userIds: NonEmptyArray) => void; type ReaperOptions = { - usersSessions: IUsersSessionsModel; onUpdate: ReaperCallback; staleThresholdMs: number; batchSize: number; }; export class PresenceReaper { - private usersSessions: IUsersSessionsModel; - private staleThresholdMs: number; private batchSize: number; @@ -37,7 +35,6 @@ export class PresenceReaper { private intervalId?: NodeJS.Timeout; constructor(options: ReaperOptions) { - this.usersSessions = options.usersSessions; this.onUpdate = options.onUpdate; this.staleThresholdMs = options.staleThresholdMs; this.batchSize = options.batchSize; @@ -69,11 +66,10 @@ export class PresenceReaper { } public async run(): Promise { - console.log('[PresenceReaper] Running presence reaper job...'); const cutoffDate = new Date(Date.now() - this.staleThresholdMs); // 1. Find users with potentially stale connections - const cursor = this.usersSessions.find( + const cursor = UsersSessions.find( { 'connections._updatedAt': { $lte: cutoffDate } }, { projection: { _id: 1, connections: 1 }, @@ -94,10 +90,9 @@ export class PresenceReaper { if (userChangeSet.size > 0) { await this.flushBatch(userChangeSet); } - console.log('[PresenceReaper] Presence reaper job completed.'); } - private processDocument(sessionDoc: IUserSession, cutoffDate: Date, changeMap: Map): void { + processDocument(sessionDoc: IUserSession, cutoffDate: Date, changeMap: Map): void { const userId = sessionDoc._id; const allConnections = sessionDoc.connections || []; @@ -116,7 +111,7 @@ export class PresenceReaper { } private async flushBatch(changeMap: Map): Promise { - const sessionOps = []; + const sessionOps: AnyBulkWriteOperation[] = []; const usersToUpdate: string[] = []; for (const plan of changeMap.values()) { @@ -137,15 +132,12 @@ export class PresenceReaper { }); } - // 2. Identify potential offline users - if (plan.shouldMarkOffline) { - usersToUpdate.push(plan.userId); - } + usersToUpdate.push(plan.userId); } // Step A: Clean the Database if (sessionOps.length > 0) { - await this.usersSessions.col.bulkWrite(sessionOps); + await UsersSessions.col.bulkWrite(sessionOps); } // Step B: Notify Presence Service From 94aef54fbb84b21a1fa9f4070a3fd7c260b8f16d Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Tue, 9 Dec 2025 16:10:48 -0300 Subject: [PATCH 22/23] chore: update changeset --- .changeset/spicy-nails-design.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.changeset/spicy-nails-design.md b/.changeset/spicy-nails-design.md index 0388b121763d1..23a5f82e7bb5e 100644 --- a/.changeset/spicy-nails-design.md +++ b/.changeset/spicy-nails-design.md @@ -1,10 +1,8 @@ --- "@rocket.chat/meteor": patch "@rocket.chat/core-services": patch -"@rocket.chat/model-typings": patch -"@rocket.chat/models": patch "@rocket.chat/ddp-streamer": patch "@rocket.chat/presence": patch --- -Fixes user status inaccuracy by refreshing active connections and filtering out the stale ones. +Ensures presence stays accurate by refreshing connections on heartbeats and removing stale sessions. \ No newline at end of file From dcb7fee8e57d7777f4042b4b269e3e7c82b254a2 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Wed, 10 Dec 2025 16:37:33 -0300 Subject: [PATCH 23/23] chore(presence): remove shouldMarkOffline --- ee/packages/presence/src/Presence.ts | 2 - .../presence/src/lib/PresenceReaper.spec.ts | 233 ++++++------------ .../presence/src/lib/PresenceReaper.ts | 72 +++--- .../src/lib/processConnectionStatus.ts | 3 +- 4 files changed, 111 insertions(+), 199 deletions(-) diff --git a/ee/packages/presence/src/Presence.ts b/ee/packages/presence/src/Presence.ts index 83fd494e83f21..0fe78b9e47dcd 100755 --- a/ee/packages/presence/src/Presence.ts +++ b/ee/packages/presence/src/Presence.ts @@ -166,8 +166,6 @@ export class Presence extends ServiceClass implements IPresence { } async updateConnection(uid: string, connectionId: string): Promise<{ uid: string; connectionId: string } | undefined> { - console.debug(`Updating connection for user ${uid} and connection ${connectionId}`); - const query = { '_id': uid, 'connections.id': connectionId, diff --git a/ee/packages/presence/src/lib/PresenceReaper.spec.ts b/ee/packages/presence/src/lib/PresenceReaper.spec.ts index f0bba7150c1ce..f8e6a83c498b2 100644 --- a/ee/packages/presence/src/lib/PresenceReaper.spec.ts +++ b/ee/packages/presence/src/lib/PresenceReaper.spec.ts @@ -2,17 +2,19 @@ import { UserStatus, type IUserSession, type IUserSessionConnection } from '@roc import { registerModel } from '@rocket.chat/models'; import type { FindCursor, WithId } from 'mongodb'; -import { PresenceReaper, type ReaperPlan } from './PresenceReaper'; +import { PresenceReaper } from './PresenceReaper'; +let sessions = 0; const createSession = (overrides: Partial = {}): IUserSession => ({ - _id: 'user-id', + _id: `user-${sessions++}`, connections: [], ...overrides, }); +let connections = 0; const createConnection = (overrides: Partial = {}): IUserSessionConnection => ({ - id: 'connection-id', - instanceId: 'instance-id', + id: `conn-${connections++}`, + instanceId: `instanceId`, status: UserStatus.ONLINE, _createdAt: new Date(), _updatedAt: new Date(), @@ -63,163 +65,90 @@ describe('PresenceReaper', () => { }); }); - describe('processDocument (Business Logic)', () => { - it('should identify stale connections by "id" and preserve valid ones', () => { - const { stale, active, cutoff } = createDates(); - const doc = createSession({ - _id: 'user-123', - connections: [ - createConnection({ id: 'conn-stale', _updatedAt: stale }), // Should be removed - createConnection({ id: 'conn-active', _updatedAt: active }), // Should stay - ], - }); - - const changeMap = new Map(); - reaper.processDocument(doc, cutoff, changeMap); - - const result = changeMap.get('user-123'); - - // Assertions - expect(result).toBeDefined(); - expect(result?.removeIds).toContain('conn-stale'); // Found the stale ID - expect(result?.removeIds).not.toContain('conn-active'); // Ignored the active ID - expect(result?.shouldMarkOffline).toBe(false); // User still has 1 active connection - }); - - it('should not mark user offline if there are still valid connections', () => { - const { stale, active, cutoff } = createDates(); - const doc = createSession({ - _id: 'user-456', - connections: [ - createConnection({ id: 'conn-stale', _updatedAt: stale, status: UserStatus.ONLINE }), - createConnection({ id: 'conn-active', _updatedAt: active, status: UserStatus.AWAY }), - ], - }); - - const changeMap = new Map(); - reaper.processDocument(doc, cutoff, changeMap); - - const result = changeMap.get('user-456'); - - expect(result).toBeDefined(); - expect(result?.removeIds).toHaveLength(1); - expect(result?.shouldMarkOffline).toBe(false); // Still has valid connection - }); - - it('should mark user offline only if ALL connections are stale', () => { - const { stale, cutoff } = createDates(); - - const doc = createSession({ - _id: 'user-456', - connections: [ - createConnection({ id: 'conn-1', _updatedAt: stale, status: UserStatus.ONLINE }), - createConnection({ id: 'conn-2', _updatedAt: stale, status: UserStatus.AWAY }), - ], - }); + it('should not call onUpdate when there no connections', async () => { + findMock.mockReturnValue(createCursor([])); - const changeMap = new Map(); - reaper.processDocument(doc, cutoff, changeMap); + await reaper.run(); - const result = changeMap.get('user-456'); - - expect(result).toBeDefined(); - expect(result?.removeIds).toHaveLength(2); - expect(result?.shouldMarkOffline).toBe(true); // No valid connections left - }); + expect(onUpdateMock).not.toHaveBeenCalled(); }); - describe('run (Integration Flow)', () => { - it('should handle empty collections without errors', async () => { - // Mock empty cursor - findMock.mockReturnValue(createCursor([])); + it('should process users with stale connections correctly', async () => { + const { stale } = createDates(); - // Execute Run - await reaper.run(); + findMock.mockReturnValue( + createCursor([ + createSession({ + _id: 'user-789', + connections: [createConnection({ _updatedAt: stale })], + }), + ]), + ); - // Verify no updates were made - expect(onUpdateMock).not.toHaveBeenCalled(); - }); - - it('should generate correct bulkWrite operations', async () => { - const { stale } = createDates(); - - // Mock Data from DB Cursor - - findMock.mockReturnValue( - createCursor([ - createSession({ - _id: 'user-789', - connections: [createConnection({ id: 'zombie-conn', _updatedAt: stale })], - }), - ]), - ); - - // Execute Run - await reaper.run(); + await reaper.run(); - // Verify 'users' Update (Status Offline) - expect(onUpdateMock).toHaveBeenCalledTimes(1); - expect(onUpdateMock).toHaveBeenCalledWith(['user-789']); - }); + expect(onUpdateMock).toHaveBeenCalledTimes(1); + expect(onUpdateMock).toHaveBeenCalledWith(['user-789']); }); - describe('end-to-end Presence Reaping', () => { - it('should process multiple users and batch updates correctly', async () => { - const { stale } = createDates(); - - findMock.mockReturnValue( - createCursor([ - createSession({ - _id: 'user-1', - connections: [createConnection({ id: 'conn-1', _updatedAt: stale })], - }), - createSession({ - _id: 'user-2', - connections: [createConnection({ id: 'conn-2', _updatedAt: stale })], - }), - createSession({ - _id: 'user-3', - connections: [createConnection({ id: 'conn-3', _updatedAt: stale })], - }), - ]), - ); - - // Execute Run - await reaper.run(); - - // Verify 'users' Update called twice due to batch size of 2 - expect(onUpdateMock).toHaveBeenCalledTimes(2); - expect(onUpdateMock).toHaveBeenNthCalledWith(1, ['user-1', 'user-2']); - expect(onUpdateMock).toHaveBeenNthCalledWith(2, ['user-3']); - }); + it('should process multiple users and batch updates correctly', async () => { + const { stale } = createDates(); + + findMock.mockReturnValue( + createCursor([ + createSession({ + _id: 'user-1', + connections: [createConnection({ _updatedAt: stale })], + }), + createSession({ + _id: 'user-2', + connections: [createConnection({ _updatedAt: stale })], + }), + createSession({ + _id: 'user-3', + connections: [createConnection({ _updatedAt: stale })], + }), + ]), + ); + + // Execute Run + await reaper.run(); + + // Verify 'users' Update called twice due to batch size of 2 + expect(onUpdateMock).toHaveBeenCalledTimes(2); + expect(onUpdateMock).toHaveBeenNthCalledWith(1, ['user-1', 'user-2']); + expect(onUpdateMock).toHaveBeenNthCalledWith(2, ['user-3']); + }); - it('should process users with mixed connection states correctly', async () => { - const { stale, active } = createDates(); - - findMock.mockReturnValue( - createCursor([ - // User with all stale connections - createSession({ - _id: 'user-all-stale', - connections: [createConnection({ id: 'conn-stale-1', _updatedAt: stale })], - }), - // User with mixed connections - createSession({ - _id: 'user-mixed', - connections: [ - createConnection({ id: 'conn-stale-2', _updatedAt: stale }), - createConnection({ id: 'conn-active-1', _updatedAt: active }), - ], - }), - ]), - ); - - // Execute Run - await reaper.run(); - - // Verify 'users' Update called for both users - expect(onUpdateMock).toHaveBeenCalledTimes(1); - expect(onUpdateMock).toHaveBeenNthCalledWith(1, ['user-all-stale', 'user-mixed']); - }); + it('should process users with mixed connection states correctly', async () => { + const { stale, active } = createDates(); + + findMock.mockReturnValue( + createCursor([ + createSession({ + _id: 'no-connections', + connections: [], + }), + createSession({ + _id: 'all-active', + connections: [createConnection({ _updatedAt: active })], + }), + createSession({ + _id: 'all-stale', + connections: [createConnection({ _updatedAt: stale })], + }), + createSession({ + _id: 'mixed', + connections: [createConnection({ _updatedAt: stale }), createConnection({ _updatedAt: active })], + }), + ]), + ); + + // Execute Run + await reaper.run(); + + // Verify 'users' Update called for both users + expect(onUpdateMock).toHaveBeenCalledTimes(1); + expect(onUpdateMock).toHaveBeenNthCalledWith(1, ['all-stale', 'mixed']); }); }); diff --git a/ee/packages/presence/src/lib/PresenceReaper.ts b/ee/packages/presence/src/lib/PresenceReaper.ts index c505ef2b44033..969465229cad1 100644 --- a/ee/packages/presence/src/lib/PresenceReaper.ts +++ b/ee/packages/presence/src/lib/PresenceReaper.ts @@ -4,18 +4,19 @@ import type { IUserSession } from '@rocket.chat/core-typings'; import { UsersSessions } from '@rocket.chat/models'; import type { AnyBulkWriteOperation } from 'mongodb'; -export type ReaperPlan = { +type ReaperPlan = { userId: string; - removeIds: string[]; - shouldMarkOffline: boolean; + removeIds: NonEmptyArray; cutoffDate: Date; }; -type NonEmptyArray = [T, ...T[]]; +type NonEmptyArray = Omit<[T, ...T[]], 'map'> & { + map(callbackfn: (value: T, index: number, array: T[]) => U): NonEmptyArray; +}; const isNonEmptyArray = (arr: T[]): arr is NonEmptyArray => arr.length > 0; -type ReaperCallback = (userIds: NonEmptyArray) => void; +type ReaperCallback = (userIds: NonEmptyArray) => Promise; type ReaperOptions = { onUpdate: ReaperCallback; @@ -49,8 +50,6 @@ export class PresenceReaper { this.intervalId = setInterval(() => { this.run().catch((err) => console.error('[PresenceReaper] Error:', err)); }, 60 * 1000); - - console.log('[PresenceReaper] Service started.'); } public stop() { @@ -61,8 +60,6 @@ export class PresenceReaper { clearInterval(this.intervalId); this.intervalId = undefined; } - - console.log('[PresenceReaper] Service stopped.'); } public async run(): Promise { @@ -92,57 +89,44 @@ export class PresenceReaper { } } - processDocument(sessionDoc: IUserSession, cutoffDate: Date, changeMap: Map): void { + private processDocument(sessionDoc: IUserSession, cutoffDate: Date, changeMap: Map): void { const userId = sessionDoc._id; const allConnections = sessionDoc.connections || []; // Filter connections based on the cutoff const staleConnections = allConnections.filter((c) => c._updatedAt <= cutoffDate); - const validConnections = allConnections.filter((c) => c._updatedAt > cutoffDate); - if (staleConnections.length === 0) return; - - changeMap.set(userId, { - userId, - removeIds: staleConnections.map((c) => c.id), - cutoffDate, // Keep reference for race-condition check - shouldMarkOffline: validConnections.length === 0, - }); + if (isNonEmptyArray(staleConnections)) { + changeMap.set(userId, { + userId, + removeIds: staleConnections.map((c) => c.id), + cutoffDate, // Keep reference for race-condition check + }); + } } private async flushBatch(changeMap: Map): Promise { - const sessionOps: AnyBulkWriteOperation[] = []; - const usersToUpdate: string[] = []; + const operations = []; for (const plan of changeMap.values()) { - // 1. Prepare DB Cleanup - if (plan.removeIds.length > 0) { - sessionOps.push({ - updateOne: { - filter: { _id: plan.userId }, - update: { - $pull: { - connections: { - id: { $in: plan.removeIds }, - _updatedAt: { $lte: plan.cutoffDate }, - }, + operations.push({ + updateOne: { + filter: { _id: plan.userId }, + update: { + $pull: { + connections: { + id: { $in: plan.removeIds }, + _updatedAt: { $lte: plan.cutoffDate }, }, }, }, - }); - } - - usersToUpdate.push(plan.userId); - } - - // Step A: Clean the Database - if (sessionOps.length > 0) { - await UsersSessions.col.bulkWrite(sessionOps); + }, + } satisfies AnyBulkWriteOperation); } - // Step B: Notify Presence Service - if (isNonEmptyArray(usersToUpdate)) { - this.onUpdate(usersToUpdate); + if (isNonEmptyArray(operations)) { + await UsersSessions.col.bulkWrite(operations); + await this.onUpdate(operations.map((op) => op.updateOne.filter._id)); } } } diff --git a/ee/packages/presence/src/lib/processConnectionStatus.ts b/ee/packages/presence/src/lib/processConnectionStatus.ts index 61790ce332927..a14aa45219e8a 100644 --- a/ee/packages/presence/src/lib/processConnectionStatus.ts +++ b/ee/packages/presence/src/lib/processConnectionStatus.ts @@ -1,4 +1,5 @@ -import { UserStatus, type IUserSessionConnection } from '@rocket.chat/core-typings'; +import type { IUserSessionConnection } from '@rocket.chat/core-typings'; +import { UserStatus } from '@rocket.chat/core-typings'; /** * Defines new connection status compared to a previous connection status