From 9627c99d676da597debf40f7fa6fa4ee18e7d6e6 Mon Sep 17 00:00:00 2001 From: matheusbsilva137 Date: Thu, 28 Sep 2023 13:06:42 -0300 Subject: [PATCH] Replace aggregation by 3 finds --- apps/meteor/app/api/server/v1/channels.ts | 11 +- apps/meteor/app/api/server/v1/groups.ts | 11 +- .../lib/findUsersOfRoomByHighestRole.ts | 102 ++++++++++++--- .../meteor/server/models/raw/Subscriptions.ts | 14 -- apps/meteor/server/models/raw/Users.js | 123 +++++++----------- .../src/models/ISubscriptionsModel.ts | 1 - .../model-typings/src/models/IUsersModel.ts | 28 ++-- 7 files changed, 144 insertions(+), 146 deletions(-) diff --git a/apps/meteor/app/api/server/v1/channels.ts b/apps/meteor/app/api/server/v1/channels.ts index 0c386b7c7ba2..0ab14b0e0a0d 100644 --- a/apps/meteor/app/api/server/v1/channels.ts +++ b/apps/meteor/app/api/server/v1/channels.ts @@ -1063,7 +1063,7 @@ API.v1.addRoute( const { sort = {} } = await this.parseJsonQuery(); const { status, filter } = this.queryParams; - const cursor = await findUsersOfRoomByHighestRole({ + const { members, total } = await findUsersOfRoomByHighestRole({ rid: findResult._id, ...(status && { status: { $in: status } }), limit, @@ -1071,15 +1071,6 @@ API.v1.addRoute( ...(sort?.username && { sort: { username: sort.username } }), }); - const result = await cursor.toArray(); - if (!result?.[0]) { - return API.v1.failure('No user info could be found for the channel provided'); - } - const { - members, - totalCount: [{ total } = { total: 0 }], - } = result[0]; - return API.v1.success({ members, count: members.length, diff --git a/apps/meteor/app/api/server/v1/groups.ts b/apps/meteor/app/api/server/v1/groups.ts index 5cbaec143ce2..403ceb62bd9d 100644 --- a/apps/meteor/app/api/server/v1/groups.ts +++ b/apps/meteor/app/api/server/v1/groups.ts @@ -757,7 +757,7 @@ API.v1.addRoute( const { sort = {} } = await this.parseJsonQuery(); const { status, filter } = this.queryParams; - const cursor = await findUsersOfRoomByHighestRole({ + const { members, total } = await findUsersOfRoomByHighestRole({ rid: findResult.rid, ...(status && { status: { $in: status } }), limit, @@ -765,15 +765,6 @@ API.v1.addRoute( ...(sort?.username && { sort: { username: sort.username } }), }); - const result = await cursor.toArray(); - if (!result?.[0]) { - return API.v1.failure('No user info could be found for the group provided'); - } - const { - members, - totalCount: [{ total } = { total: 0 }], - } = result[0]; - return API.v1.success({ members, count: members.length, diff --git a/apps/meteor/server/lib/findUsersOfRoomByHighestRole.ts b/apps/meteor/server/lib/findUsersOfRoomByHighestRole.ts index 7c1cd68eed9b..10eeb13c996f 100644 --- a/apps/meteor/server/lib/findUsersOfRoomByHighestRole.ts +++ b/apps/meteor/server/lib/findUsersOfRoomByHighestRole.ts @@ -1,6 +1,6 @@ -import type { IUserWithRoleInfo, ISubscription } from '@rocket.chat/core-typings'; +import type { IUserWithRoleInfo, ISubscription, IUser } from '@rocket.chat/core-typings'; import { Users, Subscriptions } from '@rocket.chat/models'; -import type { AggregationCursor, FilterOperators } from 'mongodb'; +import type { FilterOperators, FindOptions } from 'mongodb'; import { settings } from '../../app/settings/server'; @@ -13,14 +13,62 @@ type FindUsersParam = { sort?: Record; }; +async function findUsersWithRolesOfRoom( + { rid, status, limit = 0, filter = '' }: FindUsersParam, + options: FindOptions, +): Promise<{ members: IUserWithRoleInfo[]; totalCount: number; allMembersIds: string[] }> { + // Sort roles in descending order so that owners are listed before moderators + // This could possibly cause issues with custom roles, but that's intended to improve performance + const highestRolesMembersSubscriptions = await Subscriptions.findByRoomIdAndRoles(rid, ['owner', 'moderator'], { + projection: { 'u._id': 1, 'roles': 1 }, + sort: { roles: -1 }, + }).toArray(); + + const searchFields = settings.get('Accounts_SearchFields').trim().split(','); + + const totalCount = highestRolesMembersSubscriptions.length; + const allMembersIds = highestRolesMembersSubscriptions.map((s: ISubscription) => s.u?._id); + const highestRolesMembersIdsToFind = allMembersIds.slice(0, limit); + + const { cursor } = Users.findPaginatedActiveUsersByIds(filter, searchFields, highestRolesMembersIdsToFind, options, [ + { + __rooms: rid, + ...(status && { status }), + }, + ]); + const highestRolesMembers = await cursor.toArray(); + + // maps for efficient lookup of highest roles and sort order + const ordering: Record = {}; + const highestRoleById: Record = {}; + + const limitedHighestRolesMembersSubs = highestRolesMembersSubscriptions.slice(0, limit); + limitedHighestRolesMembersSubs.forEach((sub, index) => { + ordering[sub.u._id] = index; + highestRoleById[sub.u._id] = sub.roles?.includes('owner') ? 'owner' : 'moderator'; + }); + + highestRolesMembers.sort((a, b) => ordering[a._id] - ordering[b._id]); + const membersWithHighestRoles = highestRolesMembers.map( + (member): IUserWithRoleInfo => ({ + ...member, + highestRole: { + role: highestRoleById[member._id], + level: highestRoleById[member._id] === 'owner' ? 0 : 1, + }, + }), + ); + return { members: membersWithHighestRoles, totalCount, allMembersIds }; +} + export async function findUsersOfRoomByHighestRole({ rid, status, limit = 0, filter = '', sort, -}: FindUsersParam): Promise> { - const options = { +}: FindUsersParam): Promise<{ members: IUserWithRoleInfo[]; total: number }> { + const options: FindOptions = { projection: { name: 1, username: 1, @@ -37,18 +85,42 @@ export async function findUsersOfRoomByHighestRole({ }, limit, }; - + const extraQuery = { + __rooms: rid, + ...(status && { status }), + }; const searchFields = settings.get('Accounts_SearchFields').trim().split(','); - const ownersIds = (await Subscriptions.findByRoomIdAndHighestRole(rid, 'owner', { projection: { 'u._id': 1 } }).toArray()).map( - (s: ISubscription) => s.u?._id, - ); - const moderatorsIds = (await Subscriptions.findByRoomIdAndHighestRole(rid, 'moderator', { projection: { 'u._id': 1 } }).toArray()).map( - (s: ISubscription) => s.u?._id, - ); - return Users.findPaginatedActiveUsersByRoomIdWithHighestRole(filter, rid, searchFields, ownersIds, moderatorsIds, options, [ - { - ...(status && { status }), - }, + // Find highest roles members (owners and moderator) + const { + members: highestRolesMembers, + totalCount: totalMembersWithRoles, + allMembersIds: highestRolesMembersIds, + } = await findUsersWithRolesOfRoom({ rid, status, limit, filter }, options); + + if (limit <= highestRolesMembers.length) { + const totalMembersCount = await Users.countActiveUsersExcept(filter, highestRolesMembersIds, searchFields, [extraQuery]); + return { members: highestRolesMembers, total: totalMembersWithRoles + totalMembersCount }; + } + if (options.limit) { + options.limit -= highestRolesMembers.length; + } + + // Find average members + const { cursor, totalCount } = Users.findPaginatedByActiveUsersExcept(filter, highestRolesMembersIds, searchFields, options, [ + extraQuery, ]); + const [members, totalMembersCount] = await Promise.all([await cursor.toArray(), totalCount]); + const membersWithHighestRoles = members.map( + (member): IUserWithRoleInfo => ({ + ...member, + highestRole: { + role: 'member', + level: 2, + }, + }), + ); + + const allMembers = highestRolesMembers.concat(membersWithHighestRoles); + return { members: allMembers, total: totalMembersWithRoles + totalMembersCount }; } diff --git a/apps/meteor/server/models/raw/Subscriptions.ts b/apps/meteor/server/models/raw/Subscriptions.ts index fcffedf1d813..4b42367bad05 100644 --- a/apps/meteor/server/models/raw/Subscriptions.ts +++ b/apps/meteor/server/models/raw/Subscriptions.ts @@ -943,20 +943,6 @@ export class SubscriptionsRaw extends BaseRaw implements ISubscri return this.find(query, options); } - findByRoomIdAndHighestRole(roomId: string, role: string, options?: FindOptions): FindCursor { - // roles ordered from lowest to highest - const mainRoles = ['moderator', 'owner']; - - const roleIndex = mainRoles.indexOf(role || ''); - const rolesToExclude = mainRoles.slice(roleIndex + 1); - const query = { - rid: roomId, - roles: { $eq: role, $nin: rolesToExclude }, - }; - - return this.find(query, options); - } - countByRoomIdAndRoles(roomId: string, roles: string[]): Promise { roles = ([] as string[]).concat(roles); const query = { diff --git a/apps/meteor/server/models/raw/Users.js b/apps/meteor/server/models/raw/Users.js index 7bf9e24d8a43..773314bdac22 100644 --- a/apps/meteor/server/models/raw/Users.js +++ b/apps/meteor/server/models/raw/Users.js @@ -311,10 +311,8 @@ export class UsersRaw extends BaseRaw { $and: [ { active: true, - username: { - $exists: true, - ...(exceptions.length && { $nin: exceptions }), - }, + ...(exceptions.length && { _id: { $nin: exceptions } }), + username: { $exists: true }, // if the search term is empty, don't need to have the $or statement (because it would be an empty regex) ...(searchTerm && orStmt.length && { $or: orStmt }), }, @@ -325,12 +323,39 @@ export class UsersRaw extends BaseRaw { return this.findPaginated(query, options); } - findPaginatedActiveUsersByRoomIdWithHighestRole( + countActiveUsersExcept(searchTerm, exceptions, searchFields, extraQuery = [], { startsWith = false, endsWith = false } = {}) { + if (exceptions == null) { + exceptions = []; + } + if (!Array.isArray(exceptions)) { + exceptions = [exceptions]; + } + + const regexString = (startsWith ? '^' : '') + escapeRegExp(searchTerm) + (endsWith ? '$' : ''); + const termRegex = new RegExp(regexString, 'i'); + + const orStmt = (searchFields || []).map((el) => ({ [el.trim()]: termRegex })); + + const query = { + $and: [ + { + active: true, + ...(exceptions.length && { _id: { $nin: exceptions } }), + username: { $exists: true }, + // if the search term is empty, don't need to have the $or statement (because it would be an empty regex) + ...(searchTerm && orStmt.length && { $or: orStmt }), + }, + ...extraQuery, + ], + }; + + return this.col.countDocuments(query); + } + + findPaginatedActiveUsersByIds( searchTerm, - rid, searchFields, - ownersIds, - moderatorsIds, + ids = [], options = {}, extraQuery = [], { startsWith = false, endsWith = false } = {}, @@ -339,81 +364,21 @@ export class UsersRaw extends BaseRaw { const termRegex = new RegExp(regexString, 'i'); const orStmt = (searchFields || []).map((el) => ({ [el.trim()]: termRegex })); - const userSearchConditions = [ - { - active: true, - __rooms: rid, - username: { $exists: true }, - // if the search term is empty, don't need to have the $or statement (because it would be an empty regex) - ...(searchTerm && orStmt.length && { $or: orStmt }), - }, - ...extraQuery, - ]; - - const ownerQuery = { $and: [...userSearchConditions, { _id: { $in: ownersIds } }] }; - const moderatorQuery = { $and: [...userSearchConditions, { _id: { $in: moderatorsIds } }] }; - const memberQuery = { $and: [...userSearchConditions, { _id: { $nin: [...ownersIds, ...moderatorsIds] } }] }; - - const skip = - options.skip !== 0 - ? [ - { - $skip: options.skip, - }, - ] - : []; - - const limit = - options.limit !== 0 - ? [ - { - $limit: options.limit, - }, - ] - : []; - return this.col.aggregate( - [ - { - $facet: { - owners: [ - { $match: ownerQuery }, - { $project: options.projection }, - { $addFields: { highestRole: { role: 'owner', level: 0 } } }, - ], - moderators: [ - { $match: moderatorQuery }, - { $project: options.projection }, - { $addFields: { highestRole: { role: 'moderator', level: 1 } } }, - ], - members: [ - { $match: memberQuery }, - { $project: options.projection }, - { $addFields: { highestRole: { role: 'member', level: 2 } } }, - ], - }, - }, - { $project: { allMembers: { $concatArrays: ['$owners', '$moderators', '$members'] } } }, - { $unwind: '$allMembers' }, - { $replaceRoot: { newRoot: '$allMembers' } }, + const query = { + $and: [ { - $facet: { - members: [ - { - $sort: { - 'highestRole.level': 1, - ...options.sort, - }, - }, - ...skip, - ...limit, - ], - totalCount: [{ $group: { _id: null, total: { $sum: 1 } } }], - }, + active: true, + ...(ids.length && { _id: { $in: ids } }), + username: { $exists: true }, + // if the search term is empty, don't need to have the $or statement (because it would be an empty regex) + ...(searchTerm && orStmt.length && { $or: orStmt }), }, + ...extraQuery, ], - { allowDiskUse: true }, - ); + }; + + return this.findPaginated(query, options); } findPaginatedByActiveLocalUsersExcept(searchTerm, exceptions, options, forcedSearchFields, localDomain) { diff --git a/packages/model-typings/src/models/ISubscriptionsModel.ts b/packages/model-typings/src/models/ISubscriptionsModel.ts index 927420115e7b..aebda87c78cb 100644 --- a/packages/model-typings/src/models/ISubscriptionsModel.ts +++ b/packages/model-typings/src/models/ISubscriptionsModel.ts @@ -148,7 +148,6 @@ export interface ISubscriptionsModel extends IBaseModel { options?: FindOptions, ): FindCursor; findByRoomIdAndRoles(roomId: string, roles: string[], options?: FindOptions): FindCursor; - findByRoomIdAndHighestRole(roomId: string, role: string, options?: FindOptions): FindCursor; findByRoomIdAndUserIds(roomId: string, userIds: string[], options?: FindOptions): FindCursor; findByUserIdUpdatedAfter(userId: string, updatedAt: Date, options?: FindOptions): FindCursor; diff --git a/packages/model-typings/src/models/IUsersModel.ts b/packages/model-typings/src/models/IUsersModel.ts index 68f98e2b3416..03ee0aa93349 100644 --- a/packages/model-typings/src/models/IUsersModel.ts +++ b/packages/model-typings/src/models/IUsersModel.ts @@ -8,19 +8,8 @@ import type { IPersonalAccessToken, AtLeast, ILivechatAgentStatus, - IUserWithRoleInfo, } from '@rocket.chat/core-typings'; -import type { - Document, - UpdateResult, - FindCursor, - FindOptions, - Filter, - FilterOperators, - InsertOneResult, - DeleteResult, - AggregationCursor, -} from 'mongodb'; +import type { Document, UpdateResult, FindCursor, FindOptions, Filter, FilterOperators, InsertOneResult, DeleteResult } from 'mongodb'; import type { FindPaginated, IBaseModel } from './IBaseModel'; @@ -55,16 +44,21 @@ export interface IUsersModel extends IBaseModel { extraQuery?: any, params?: { startsWith?: boolean; endsWith?: boolean }, ): FindPaginated>; - findPaginatedActiveUsersByRoomIdWithHighestRole( + countActiveUsersExcept( + searchTerm: string, + exceptions: IUser['_id'][], + searchFields: string[], + extraQuery?: FilterOperators[], + params?: { startsWith?: boolean; endsWith?: boolean }, + ): Promise; + findPaginatedActiveUsersByIds( searchTerm: string, - rid: string, searchFields: string[], - ownersIds: IUser['_id'][], - moderatorsIds: IUser['_id'][], + ids: IUser['_id'][], options?: FindOptions, extraQuery?: FilterOperators[], params?: { startsWith?: boolean; endsWith?: boolean }, - ): Promise>; + ): FindPaginated>; findPaginatedByActiveLocalUsersExcept( searchTerm: any,