From 4c8e9b9ca8a6f05dbd249319c0bc3427fbe8ec4e Mon Sep 17 00:00:00 2001 From: Khaled Ferjani Date: Wed, 11 Dec 2024 14:12:15 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix=20last=5Fseen=20calculation?= =?UTF-8?q?=20is=20metrics=20API=20(#151)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ๐ŸŽจ feat: added user_ips table to collection types * ๐Ÿ› fix: user last_seen field calculation * ๐Ÿท๏ธ chore: added matrix user ip info type * ๐Ÿงช chore: fix tests --- .../src/matrixDb/index.ts | 1 + .../src/metrics-api/services/index.ts | 52 +++++- .../src/metrics-api/tests/controller.test.ts | 100 ++++++++++- .../src/metrics-api/tests/service.test.ts | 170 +++++++++++++++++- packages/tom-server/src/metrics-api/types.ts | 6 + 5 files changed, 314 insertions(+), 15 deletions(-) diff --git a/packages/matrix-identity-server/src/matrixDb/index.ts b/packages/matrix-identity-server/src/matrixDb/index.ts index b3c88fa8..43a24fc4 100644 --- a/packages/matrix-identity-server/src/matrixDb/index.ts +++ b/packages/matrix-identity-server/src/matrixDb/index.ts @@ -12,6 +12,7 @@ export type Collections = | 'room_stats_state' | 'event_json' | 'events' + | 'user_ips' type Get = ( table: Collections, diff --git a/packages/tom-server/src/metrics-api/services/index.ts b/packages/tom-server/src/metrics-api/services/index.ts index 0588fabb..3ef66d3c 100644 --- a/packages/tom-server/src/metrics-api/services/index.ts +++ b/packages/tom-server/src/metrics-api/services/index.ts @@ -2,6 +2,7 @@ import type { MatrixDBBackend } from '@twake/matrix-identity-server' import type { IMetricsService, MatrixUserInfo, + MatrixUserIpInfo, UserActivityStats, UserMessageCount, UserMessageEvent @@ -79,13 +80,24 @@ class MetricsService implements IMetricsService { */ private readonly _fetchUsersList = async (): Promise => { try { - const queryResult = (await this.matrixDb.getAll('users', [ + const matrixUsers: MatrixUserInfo[] = [] + const usersList = (await this.matrixDb.getAll('users', [ 'name', - 'creation_ts', - 'last_seen_ts' + 'creation_ts' ])) as unknown as MatrixUserInfo[] - return queryResult + await Promise.all( + usersList.map(async (user) => { + const lastSeenTs = await this._fetchUserLastSeenTime(user.name) + + matrixUsers.push({ + ...user, + last_seen_ts: lastSeenTs + }) + }) + ) + + return matrixUsers } catch (error) { this.logger.error(`Failed to fetch users list`, { error }) @@ -196,6 +208,38 @@ class MetricsService implements IMetricsService { throw Error('Failed to fetch user message count') } } + + /** + * Fetches the last time a user was seen. + * + * @param {string} userId - the user id. + * @returns {Promise} - the last time the user was seen. + */ + private readonly _fetchUserLastSeenTime = async ( + userId: string + ): Promise => { + try { + const queryResult = (await this.matrixDb.get( + 'user_ips', + ['user_id', 'last_seen'], + { + user_id: userId + } + )) as unknown as MatrixUserIpInfo[] + + const latestSeenTime = queryResult.reduce((latestTime, userIpInfo) => { + const parsedLastSeen = parseInt(userIpInfo.last_seen, 10) + + return parsedLastSeen > latestTime ? parsedLastSeen : latestTime + }, 0) + + return latestSeenTime + } catch (error) { + this.logger.warn(`Failed to fetch user access info`, { error }) + + return 0 + } + } } export default MetricsService diff --git a/packages/tom-server/src/metrics-api/tests/controller.test.ts b/packages/tom-server/src/metrics-api/tests/controller.test.ts index a7b3fe85..9facd992 100644 --- a/packages/tom-server/src/metrics-api/tests/controller.test.ts +++ b/packages/tom-server/src/metrics-api/tests/controller.test.ts @@ -116,6 +116,14 @@ describe('the mectrics API controller', () => { describe('the getActivityStats handler', () => { it('should try to fetch the user activity metrics', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER]) + dbMock.get.mockImplementation(async () => { + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + }) const response = await supertest(app).get(`${PATH}/activity`).send() @@ -131,7 +139,30 @@ describe('the mectrics API controller', () => { it('should calculate daily active users correctly', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER, PRE_TODAY_USER]) - + dbMock.get.mockImplementation( + async ( + _table: string, + _fields: string[], + filters: Record + ) => { + switch (filters.user_id) { + case TODAY_USER.name: + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + case PRE_TODAY_USER.name: + return await Promise.resolve([ + { + user_id: PRE_TODAY_USER.name, + last_seen: PRE_TODAY_USER.last_seen_ts + } + ]) + } + } + ) const response = await supertest(app).get(`${PATH}/activity`).send() expect(response.status).toBe(200) @@ -146,6 +177,30 @@ describe('the mectrics API controller', () => { it('should calculate weekly active users correctly', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER, PRE_WEEK_USER]) + dbMock.get.mockImplementation( + async ( + _table: string, + _fields: string[], + filters: Record + ) => { + switch (filters.user_id) { + case TODAY_USER.name: + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + case PRE_WEEK_USER.name: + return await Promise.resolve([ + { + user_id: PRE_WEEK_USER.name, + last_seen: PRE_WEEK_USER.last_seen_ts + } + ]) + } + } + ) const response = await supertest(app).get(`${PATH}/activity`).send() @@ -161,6 +216,30 @@ describe('the mectrics API controller', () => { it('should calculate monthly active users correctly', async () => { dbMock.getAll.mockResolvedValue([PRE_WEEK_USER, PRE_MONTH_USER]) + dbMock.get.mockImplementation( + async ( + _table: string, + _fields: string[], + filters: Record + ) => { + switch (filters.user_id) { + case PRE_WEEK_USER.name: + return await Promise.resolve([ + { + user_id: PRE_WEEK_USER.name, + last_seen: PRE_WEEK_USER.last_seen_ts + } + ]) + case PRE_MONTH_USER.name: + return await Promise.resolve([ + { + user_id: PRE_MONTH_USER.name, + last_seen: PRE_MONTH_USER.last_seen_ts + } + ]) + } + } + ) const response = await supertest(app).get(`${PATH}/activity`).send() @@ -187,7 +266,24 @@ describe('the mectrics API controller', () => { it('should try to fetch the message stats', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER]) - dbMock.get.mockResolvedValue(messages) + dbMock.get.mockImplementation( + async ( + table: string, + _fields: string[], + filters: Record + ) => { + if (table === 'events') { + return messages + } + + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + } + ) const response = await supertest(app).get(`${PATH}/messages`).send() diff --git a/packages/tom-server/src/metrics-api/tests/service.test.ts b/packages/tom-server/src/metrics-api/tests/service.test.ts index 9dab9acc..1edf4dbc 100644 --- a/packages/tom-server/src/metrics-api/tests/service.test.ts +++ b/packages/tom-server/src/metrics-api/tests/service.test.ts @@ -22,33 +22,25 @@ const ONE_WEEK_IN_MS = 7 * ONE_DAY_IN_MS const ONE_MONTH_IN_MS = 30 * ONE_DAY_IN_MS const TODAY_USER = { - avatar_url: '', creation_ts: 1, - displayname: 'user 1', last_seen_ts: new Date().getTime(), name: 'user1' } const PRE_TODAY_USER = { - avatar_url: '', creation_ts: 1, - displayname: 'user 1', last_seen_ts: new Date().getTime() - ONE_DAY_IN_MS - 1, name: 'user2' } const PRE_WEEK_USER = { - avatar_url: '', creation_ts: 1, - displayname: 'user 2', last_seen_ts: new Date().getTime() - ONE_WEEK_IN_MS - 1, name: 'user3' } const PRE_MONTH_USER = { - avatar_url: '', creation_ts: 1, - displayname: 'user 3', last_seen_ts: new Date().getTime() - ONE_MONTH_IN_MS - 1, name: 'user4' } @@ -93,6 +85,20 @@ describe('the Metrics API Service', () => { describe('the getUserActivityStats function', () => { it('should attempts to get user activity stats', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER]) + dbMock.get.mockImplementation( + async ( + _table: string, + _fields: string[], + filters: Record + ) => { + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + } + ) const result = await metricsService.getUserActivityStats() @@ -121,6 +127,30 @@ describe('the Metrics API Service', () => { it('should return only today active users', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER, PRE_TODAY_USER]) + dbMock.get.mockImplementation( + async ( + _table: string, + _fields: string[], + filters: Record + ) => { + switch (filters.user_id) { + case TODAY_USER.name: + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + case PRE_TODAY_USER.name: + return await Promise.resolve([ + { + user_id: PRE_TODAY_USER.name, + last_seen: PRE_TODAY_USER.last_seen_ts + } + ]) + } + } + ) const result = await metricsService.getUserActivityStats() @@ -140,6 +170,38 @@ describe('the Metrics API Service', () => { PRE_WEEK_USER ]) + dbMock.get.mockImplementation( + async ( + _table: string, + _fields: string[], + filters: Record + ) => { + switch (filters.user_id) { + case TODAY_USER.name: + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + case PRE_TODAY_USER.name: + return await Promise.resolve([ + { + user_id: PRE_TODAY_USER.name, + last_seen: PRE_TODAY_USER.last_seen_ts + } + ]) + case PRE_WEEK_USER.name: + return await Promise.resolve([ + { + user_id: PRE_WEEK_USER.name, + last_seen: PRE_WEEK_USER.last_seen_ts + } + ]) + } + } + ) + const result = await metricsService.getUserActivityStats() expect(result).toEqual({ @@ -159,6 +221,45 @@ describe('the Metrics API Service', () => { PRE_MONTH_USER ]) + dbMock.get.mockImplementation( + async ( + _table: string, + _fields: string[], + filters: Record + ) => { + switch (filters.user_id) { + case TODAY_USER.name: + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + case PRE_TODAY_USER.name: + return await Promise.resolve([ + { + user_id: PRE_TODAY_USER.name, + last_seen: PRE_TODAY_USER.last_seen_ts + } + ]) + case PRE_WEEK_USER.name: + return await Promise.resolve([ + { + user_id: PRE_WEEK_USER.name, + last_seen: PRE_WEEK_USER.last_seen_ts + } + ]) + case PRE_MONTH_USER.name: + return await Promise.resolve([ + { + user_id: PRE_MONTH_USER.name, + last_seen: PRE_MONTH_USER.last_seen_ts + } + ]) + } + } + ) + const result = await metricsService.getUserActivityStats() expect(result).toEqual({ @@ -174,6 +275,18 @@ describe('the Metrics API Service', () => { describe('the getUserMessageStats function', () => { it('should attempts to get user message stats', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER]) + dbMock.get.mockImplementation(async (table: string) => { + if (table === 'events') { + return messages + } + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + }) + dbMock.get.mockResolvedValue(messages) const result = await metricsService.getUserMessageStats() @@ -196,7 +309,17 @@ describe('the Metrics API Service', () => { it('should return zero message count for users with no messages', async () => { dbMock.getAll.mockResolvedValue([TODAY_USER]) - dbMock.get.mockResolvedValue([]) + dbMock.get.mockImplementation(async (table: string) => { + if (table === 'events') { + return [] + } + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + }) const result = await metricsService.getUserMessageStats() @@ -214,6 +337,35 @@ describe('the Metrics API Service', () => { .mockResolvedValueOnce(messages) // For first user .mockResolvedValueOnce([messages[0]]) // For second user + dbMock.get.mockImplementation( + async ( + table: string, + _fields: string[], + filters: Record + ) => { + if (table === 'events') { + return filters.sender === 'user1' ? messages : [messages[0]] + } + + switch (filters.user_id) { + case TODAY_USER.name: + return await Promise.resolve([ + { + user_id: TODAY_USER.name, + last_seen: TODAY_USER.last_seen_ts + } + ]) + case PRE_TODAY_USER.name: + return await Promise.resolve([ + { + user_id: PRE_TODAY_USER.name, + last_seen: PRE_TODAY_USER.last_seen_ts + } + ]) + } + } + ) + const result = await metricsService.getUserMessageStats() expect(result).toEqual([ diff --git a/packages/tom-server/src/metrics-api/types.ts b/packages/tom-server/src/metrics-api/types.ts index c5809835..4f9de3de 100644 --- a/packages/tom-server/src/metrics-api/types.ts +++ b/packages/tom-server/src/metrics-api/types.ts @@ -35,3 +35,9 @@ export interface UserMessageEvent { room_id: string content: string } + +export interface MatrixUserIpInfo { + user_id: string + device_id: string + last_seen: string +}