From fd7260863f48c6badb6f036f8c57586ce2dbcf6f Mon Sep 17 00:00:00 2001 From: juancarmore Date: Fri, 20 Jun 2025 15:00:05 +0200 Subject: [PATCH] backend: enhance bulk room deletion logic to ensure proper handling of active meetings and update tests for consistency --- backend/src/services/room.service.ts | 42 +++++++++++++++---- .../api/rooms/bulk-delete-rooms.test.ts | 10 +++-- .../integration/api/rooms/delete-room.test.ts | 1 + 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/backend/src/services/room.service.ts b/backend/src/services/room.service.ts index d8977df..5466c83 100644 --- a/backend/src/services/room.service.ts +++ b/backend/src/services/room.service.ts @@ -193,7 +193,8 @@ export class RoomService { /** * Deletes multiple rooms in bulk, with the option to force delete or gracefully handle rooms with active participants. * For rooms with participants, when `forceDelete` is false, the method performs a "graceful deletion" - * by marking the room as deleted without disrupting active sessions. + * by marking the room for deletion without disrupting active sessions. + * However, if `forceDelete` is true, it will also end the meetings by removing the rooms from LiveKit. * * @param roomIds - Array of room identifiers to be deleted * @param forceDelete - If true, deletes rooms even if they have active participants. @@ -210,7 +211,6 @@ export class RoomService { const { toDelete, toMark } = await this.classifyRoomsForDeletion(roomIds, forceDelete); // Process each group in parallel - const [deletedRooms, markedRooms] = await Promise.all([ this.batchDeleteRooms(toDelete), this.batchMarkRoomsForDeletion(toMark) @@ -368,8 +368,8 @@ export class RoomService { const classificationResults = await Promise.allSettled( roomIds.map(async (roomId) => { try { - const hasParticipants = await this.livekitService.roomHasParticipants(roomId); - const shouldDelete = forceDelete || !hasParticipants; + const activeMeeting = await this.livekitService.roomExists(roomId); + const shouldDelete = forceDelete || !activeMeeting; return { roomId, @@ -419,11 +419,37 @@ export class RoomService { this.logger.info(`Batch deleting ${roomIds.length} rooms`); try { - await Promise.all([ - this.storageService.deleteMeetRooms(roomIds), - this.livekitService.batchDeleteRooms(roomIds) - ]); + // Check which rooms have an active LiveKit room (active meeting) + const activeRoomChecks = await Promise.all( + roomIds.map(async (roomId) => ({ + roomId, + activeMeeting: await this.livekitService.roomExists(roomId) + })) + ); + const withActiveMeeting = activeRoomChecks.filter((r) => r.activeMeeting).map((r) => r.roomId); + const withoutActiveMeeting = activeRoomChecks.filter((r) => !r.activeMeeting).map((r) => r.roomId); + + // Mark all rooms with active meetings for deletion (in batch) + // This must be done before deleting the LiveKit rooms to ensure + // the rooms are marked when 'room_finished' webhook is sent + if (withActiveMeeting.length > 0) { + await this.batchMarkRoomsForDeletion(withActiveMeeting); + } + + // Delete all LiveKit rooms for rooms with active meetings (in batch) + const livekitDeletePromise = + withActiveMeeting.length > 0 + ? this.livekitService.batchDeleteRooms(withActiveMeeting) + : Promise.resolve(); + + // Delete Meet rooms that do not have an active meeting (in batch) + const meetRoomsDeletePromise = + withoutActiveMeeting.length > 0 + ? this.storageService.deleteMeetRooms(withoutActiveMeeting) + : Promise.resolve(); + + await Promise.all([livekitDeletePromise, meetRoomsDeletePromise]); return roomIds; } catch (error) { this.logger.error(`Batch deletion failed for rooms: ${roomIds.join(', ')}`, error); diff --git a/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts b/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts index 7054530..77d7fe9 100644 --- a/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts +++ b/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts @@ -6,6 +6,7 @@ import { disconnectFakeParticipants, getRoom, joinFakeParticipant, + sleep, startTestServer } from '../../../helpers/request-helpers.js'; @@ -157,25 +158,26 @@ describe('Room API Tests', () => { }); it('should delete rooms when force=true and participants exist', async () => { - // Create a test room + // Create test rooms const [room1, room2] = await Promise.all([ createRoom({ roomIdPrefix: 'test-bulk-1' }), createRoom({ roomIdPrefix: 'test-bulk-2' }) ]); - // Join a participant to the room + // Join a participant to the rooms await Promise.all([ joinFakeParticipant(room1.roomId, 'test-participant-1'), joinFakeParticipant(room2.roomId, 'test-participant-2') ]); - // Attempt to delete the room with force=false + // Attempt to delete the rooms with force=false const response = await bulkDeleteRooms([room1.roomId, room2.roomId], true); expect(response.status).toBe(204); expect(response.body).toStrictEqual({}); - // Verify that the room is deleted + // Verify that the rooms are deleted + await sleep('1s'); // Wait a bit for the meetings to be closed and the rooms deleted const deletedRoom1 = await getRoom(room1.roomId); const deletedRoom2 = await getRoom(room2.roomId); expect(deletedRoom1.status).toBe(404); diff --git a/backend/tests/integration/api/rooms/delete-room.test.ts b/backend/tests/integration/api/rooms/delete-room.test.ts index d8b0388..6e7955f 100644 --- a/backend/tests/integration/api/rooms/delete-room.test.ts +++ b/backend/tests/integration/api/rooms/delete-room.test.ts @@ -176,6 +176,7 @@ describe('Room API Tests', () => { expect(response.status).toBe(204); // Try to retrieve the room again + await sleep('1s'); // Wait a bit for the meeting to be closed and the room deleted const responseAfterDelete = await getRoom(roomId); expect(responseAfterDelete.status).toBe(404); });