From 491c3392ce0cfc0acf4d815349dcee34fcf07f14 Mon Sep 17 00:00:00 2001 From: juancarmore Date: Thu, 19 Feb 2026 14:03:55 +0100 Subject: [PATCH] backend: update field selection to use arrays instead of comma-separated strings across repositories and services --- .../src/repositories/base.repository.ts | 66 +++++++------------ .../src/repositories/recording.repository.ts | 16 ++--- .../repositories/room-member.repository.ts | 14 ++-- .../src/repositories/room.repository.ts | 19 +++--- .../src/repositories/user.repository.ts | 4 +- .../src/services/room-member.service.ts | 9 ++- meet-ce/backend/src/services/room.service.ts | 2 +- 7 files changed, 54 insertions(+), 76 deletions(-) diff --git a/meet-ce/backend/src/repositories/base.repository.ts b/meet-ce/backend/src/repositories/base.repository.ts index 33ea6e18..eca59f88 100644 --- a/meet-ce/backend/src/repositories/base.repository.ts +++ b/meet-ce/backend/src/repositories/base.repository.ts @@ -1,4 +1,4 @@ -import { SortAndPagination } from '@openvidu-meet/typings'; +import { SortAndPagination, SortOrder } from '@openvidu-meet/typings'; import { inject, injectable, unmanaged } from 'inversify'; import { Document, FilterQuery, Model, UpdateQuery } from 'mongoose'; import { PaginatedResult, PaginationCursor } from '../models/db-pagination.model.js'; @@ -30,22 +30,17 @@ export abstract class BaseRepository { /** * Finds a single document matching the given filter. * @param filter - MongoDB query filter - * @param fields - Optional comma-separated list of fields to select from database + * @param fields - Optional array of field names to select from database * @returns The document or null if not found */ - protected async findOne(filter: FilterQuery, fields?: string): Promise { + protected async findOne(filter: FilterQuery, fields?: string[]): Promise { try { let query = this.model.findOne(filter); - if (fields) { - //!FIXME: This transform should be optimized to avoid unnecessary string manipulation - - const fieldSelection = fields - .split(',') - .map((field) => field.trim()) - .filter((field) => field !== '') - .join(' '); - query = query.select(fieldSelection); + // Apply field selection if specified + if (fields && fields.length > 0) { + // Convert array of fields to space-separated string for Mongoose select() + query = query.select(fields.join(' ')); } return await query.exec(); @@ -62,20 +57,17 @@ export abstract class BaseRepository { * WARNING: Use with caution on large collections. Consider using findMany() with pagination instead. * * @param filter - Base MongoDB query filter - * @param fields - Optional comma-separated list of fields to select from database + * @param fields - Optional array of field names to select from database * @returns Array of domain objects matching the filter */ - protected async findAll(filter: FilterQuery = {}, fields?: string): Promise { + protected async findAll(filter: FilterQuery = {}, fields?: string[]): Promise { try { let query = this.model.find(filter); - if (fields) { - const fieldSelection = fields - .split(',') - .map((field) => field.trim()) - .filter((field) => field !== '') - .join(' '); - query = query.select(fieldSelection); + // Apply field selection if specified + if (fields && fields.length > 0) { + // Convert array of fields to space-separated string for Mongoose select() + query = query.select(fields.join(' ')); } // Transform documents to domain objects @@ -96,15 +88,15 @@ export abstract class BaseRepository { * @param options.nextPageToken - Token for pagination (encoded cursor) * @param options.sortField - Field to sort by (default: 'createdAt') * @param options.sortOrder - Sort order: 'asc' or 'desc' (default: 'desc') - * @param fields - Optional comma-separated list of fields to select from database + * @param fields - Optional array of field names to select from database * @returns Paginated result with items, truncation flag, and optional next token */ protected async findMany( filter: FilterQuery = {}, options: SortAndPagination = {}, - fields?: string + fields?: string[] ): Promise> { - const { maxItems = 100, nextPageToken, sortField = '_id', sortOrder = 'desc' } = options; + const { maxItems = 100, nextPageToken, sortField = '_id', sortOrder = SortOrder.DESC } = options; // Parse and apply pagination cursor if provided if (nextPageToken) { @@ -113,7 +105,7 @@ export abstract class BaseRepository { } // Convert sort order to MongoDB format - const mongoSortOrder: 1 | -1 = sortOrder === 'asc' ? 1 : -1; + const mongoSortOrder: 1 | -1 = sortOrder === SortOrder.ASC ? 1 : -1; // Build compound sort: primary field + _id const sort: Record = { @@ -128,17 +120,9 @@ export abstract class BaseRepository { let query = this.model.find(filter).sort(sort).limit(limit); // Apply field selection if specified - if (fields) { - // !FIXME: This transform should be optimized to avoid unnecessary string manipulation. - // !The argument method should ideally accept an array of fields instead of a comma-separated string to avoid this overhead. - // Convert comma-separated string to space-separated format for MongoDB select() - const fieldSelection = fields - .split(',') - .map((field) => field.trim()) - .filter((field) => field !== '') - .join(' '); - - query = query.select(fieldSelection); + if (fields && fields.length > 0) { + // Convert array of fields to space-separated string for Mongoose select() + query = query.select(fields.join(' ')); } const documents = await query.exec(); @@ -350,10 +334,10 @@ export abstract class BaseRepository { filter: FilterQuery, cursor: PaginationCursor, sortField: string, - sortOrder: 'asc' | 'desc' + sortOrder: SortOrder ): void { - const comparison = sortOrder === 'asc' ? '$gt' : '$lt'; - const equalComparison = sortOrder === 'asc' ? '$gt' : '$lt'; + const comparison = sortOrder === SortOrder.ASC ? '$gt' : '$lt'; + const equalComparison = sortOrder === SortOrder.ASC ? '$gt' : '$lt'; // Build compound filter for pagination // This ensures correct ordering even when sortField values are not unique @@ -368,7 +352,7 @@ export abstract class BaseRepository { } as FilterQuery); // In ascending order, also include documents where the field exists (they come after missing fields) - if (sortOrder === 'asc') { + if (sortOrder === SortOrder.ASC) { orConditions.push({ [sortField]: { $exists: true } } as FilterQuery); @@ -386,7 +370,7 @@ export abstract class BaseRepository { ); // In descending order, also include documents where the field doesn't exist (they come after all values) - if (sortOrder === 'desc') { + if (sortOrder === SortOrder.DESC) { orConditions.push({ [sortField]: { $exists: false } } as FilterQuery); diff --git a/meet-ce/backend/src/repositories/recording.repository.ts b/meet-ce/backend/src/repositories/recording.repository.ts index 956c6320..1764543e 100644 --- a/meet-ce/backend/src/repositories/recording.repository.ts +++ b/meet-ce/backend/src/repositories/recording.repository.ts @@ -2,7 +2,8 @@ import { MeetRecordingField, MeetRecordingFilters, MeetRecordingInfo, - MeetRecordingStatus + MeetRecordingStatus, + SortOrder } from '@openvidu-meet/typings'; import { inject, injectable } from 'inversify'; import { uid as secureUid } from 'uid/secure'; @@ -78,13 +79,11 @@ export class RecordingRepository { - //!FIXME: This transform should be removed because the findOne method should accept an array of fields instead of a comma-separated string, to avoid unnecessary string manipulation - const fieldsString = fields ? fields.join(',') : undefined; - const document = await this.findOne({ recordingId }, fieldsString); + const document = await this.findOne({ recordingId }, fields); return document ? this.toDomain(document) : null; } @@ -100,7 +99,7 @@ export class RecordingRepository { + async findByRoomAndMemberIds(roomId: string, memberIds: string[], fields?: string[]): Promise { return await this.findAll({ roomId, memberId: { $in: memberIds } }, fields); } @@ -81,7 +81,7 @@ export class RoomMemberRepository { - const members = await this.findAll({ memberId }, 'roomId'); + const members = await this.findAll({ memberId }, ['roomId']); return members.map((m) => m.roomId); } @@ -101,7 +101,7 @@ export class RoomMemberRepository member.roomId); } @@ -112,7 +112,7 @@ export class RoomMemberRepository extends BaseRepos * Returns the room with enriched URLs (including base URL). * * @param roomId - The unique room identifier - * @param fields - Comma-separated list of fields to include in the result + * @param fields - Array of field names to include in the result * @returns The room or null if not found */ async findByRoomId(roomId: string, fields?: MeetRoomField[]): Promise { - //!FIXME: This transform should be removed once the controller is updated to pass the fields as an array of MeetRoomField instead of a comma-separated string. - const fieldsString = fields ? fields.join(',') : undefined; - const document = await this.findOne({ roomId }, fieldsString); + const document = await this.findOne({ roomId }, fields); return document ? this.enrichRoomWithBaseUrls(document) : null; } @@ -88,10 +86,10 @@ export class RoomRepository extends BaseRepos * Returns rooms with enriched URLs (including base URL). * * @param owner - The userId of the room owner - * @param fields - Comma-separated list of fields to include in the result + * @param fields - Array of field names to include in the result * @returns Array of rooms owned by the user */ - async findByOwner(owner: string, fields?: string): Promise { + async findByOwner(owner: string, fields?: MeetRoomField[]): Promise { return await this.findAll({ owner }, fields); } @@ -107,7 +105,7 @@ export class RoomRepository extends BaseRepos * @param options.status - Optional room status to filter by * @param options.owner - Optional owner userId to filter by * @param options.roomIds - Optional array of room IDs to filter by, representing rooms the user is a member of - * @param options.fields - Comma-separated list of fields to include in the result + * @param options.fields - Array of field names to include in the result * @param options.maxItems - Maximum number of results to return (default: 100) * @param options.nextPageToken - Token for pagination (encoded cursor with last sortField value and _id) * @param options.sortField - Field to sort by (default: 'creationDate') @@ -128,7 +126,7 @@ export class RoomRepository extends BaseRepos maxItems = 100, nextPageToken, sortField = 'creationDate', - sortOrder = 'desc' + sortOrder = SortOrder.DESC } = options; // Build base filter @@ -160,8 +158,7 @@ export class RoomRepository extends BaseRepos sortField, sortOrder }, - //! FIXME: This transform should be removed because the findMany method should accept an array of fields instead of a comma-separated string, to avoid unnecessary string manipulation - fields?.join(',') + fields ); return { diff --git a/meet-ce/backend/src/repositories/user.repository.ts b/meet-ce/backend/src/repositories/user.repository.ts index 9bb84fbe..8f0cee90 100644 --- a/meet-ce/backend/src/repositories/user.repository.ts +++ b/meet-ce/backend/src/repositories/user.repository.ts @@ -1,4 +1,4 @@ -import { MeetUser, MeetUserFilters } from '@openvidu-meet/typings'; +import { MeetUser, MeetUserFilters, SortOrder } from '@openvidu-meet/typings'; import { inject, injectable } from 'inversify'; import { MeetUserDocument, MeetUserModel } from '../models/mongoose-schemas/user.schema.js'; import { LoggerService } from '../services/logger.service.js'; @@ -95,7 +95,7 @@ export class UserRepository extends BaseRepos maxItems = 100, nextPageToken, sortField = 'registrationDate', - sortOrder = 'desc' + sortOrder = SortOrder.DESC } = options; // Build base filter diff --git a/meet-ce/backend/src/services/room-member.service.ts b/meet-ce/backend/src/services/room-member.service.ts index 784ed649..ebfa6485 100644 --- a/meet-ce/backend/src/services/room-member.service.ts +++ b/meet-ce/backend/src/services/room-member.service.ts @@ -361,11 +361,10 @@ export class RoomMemberService { deleted: string[]; failed: { memberId: string; error: string }[]; }> { - const membersToDelete = await this.roomMemberRepository.findByRoomAndMemberIds( - roomId, - memberIds, - 'memberId,currentParticipantIdentity' - ); + const membersToDelete = await this.roomMemberRepository.findByRoomAndMemberIds(roomId, memberIds, [ + 'memberId', + 'currentParticipantIdentity' + ]); const foundMemberIds = membersToDelete.map((m) => m.memberId); const failed = memberIds diff --git a/meet-ce/backend/src/services/room.service.ts b/meet-ce/backend/src/services/room.service.ts index 20d7ce16..455737b2 100644 --- a/meet-ce/backend/src/services/room.service.ts +++ b/meet-ce/backend/src/services/room.service.ts @@ -396,7 +396,7 @@ export class RoomService { // If USER role, also get owned room IDs if (user.role === MeetUserRole.USER) { - const ownedRooms = await this.roomRepository.findByOwner(user.userId, 'roomId'); + const ownedRooms = await this.roomRepository.findByOwner(user.userId, ['roomId']); ownedRoomIds = ownedRooms.map((r) => r.roomId); }