From 17c486f7bf2ba5aab3b0feb4b1fd8a3f321d05be Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Mon, 14 Jul 2025 17:36:34 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20extract=20notify?= =?UTF-8?q?=5Fparticipant=20to=20util=20function?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move from lobby service to utils for reuse across services. Method is generic enough for utility status. Future: create dedicated LiveKit service to encapsulate all LiveKit-related utilities. --- src/backend/core/services/lobby.py | 64 ++------- .../core/tests/rooms/test_api_rooms_lobby.py | 12 +- src/backend/core/tests/services/test_lobby.py | 124 +----------------- src/backend/core/tests/test_utils.py | 108 ++++++++++++++- src/backend/core/utils.py | 44 ++++++- 5 files changed, 171 insertions(+), 181 deletions(-) diff --git a/src/backend/core/services/lobby.py b/src/backend/core/services/lobby.py index 017aa509..4c51c67e 100644 --- a/src/backend/core/services/lobby.py +++ b/src/backend/core/services/lobby.py @@ -1,6 +1,5 @@ """Lobby Service""" -import json import logging import uuid from dataclasses import dataclass @@ -11,13 +10,6 @@ from uuid import UUID from django.conf import settings from django.core.cache import cache -from asgiref.sync import async_to_sync -from livekit.api import ( # pylint: disable=E0611 - ListRoomsRequest, - SendDataRequest, - TwirpError, -) - from core import models, utils logger = logging.getLogger(__name__) @@ -46,10 +38,6 @@ class LobbyParticipantNotFound(LobbyError): """Raised when participant is not found.""" -class LobbyNotificationError(LobbyError): - """Raised when LiveKit notification fails.""" - - @dataclass class LobbyParticipant: """Participant in a lobby system.""" @@ -211,9 +199,6 @@ class LobbyService: Create a new participant entry in waiting status and notify room participants of the new entry request. - - Raises: - LobbyNotificationError: If room notification fails """ color = utils.generate_color(participant_id) @@ -226,10 +211,15 @@ class LobbyService: ) try: - self.notify_participants(room_id=room_id) - except LobbyNotificationError: + utils.notify_participants( + room_name=room_id, + notification_data={ + "type": settings.LOBBY_NOTIFICATION_TYPE, + }, + ) + except utils.NotificationError: # If room not created yet, there is no participants to notify - pass + logger.exception("Failed to notify room participants") cache_key = self._get_cache_key(room_id, participant_id) cache.set( @@ -334,44 +324,6 @@ class LobbyService: participant.status = status cache.set(cache_key, participant.to_dict(), timeout=timeout) - @async_to_sync - async def notify_participants(self, room_id: UUID): - """Notify room participants about a new waiting participant using LiveKit. - - Raises: - LobbyNotificationError: If notification fails to send - """ - - notification_data = { - "type": settings.LOBBY_NOTIFICATION_TYPE, - } - - lkapi = utils.create_livekit_client() - - try: - room_response = await lkapi.room.list_rooms( - ListRoomsRequest( - names=[str(room_id)], - ) - ) - - # Check if the room exists - if not room_response.rooms: - return - - await lkapi.room.send_data( - SendDataRequest( - room=str(room_id), - data=json.dumps(notification_data).encode("utf-8"), - kind="RELIABLE", - ) - ) - except TwirpError as e: - logger.exception("Failed to notify room participants") - raise LobbyNotificationError("Failed to notify room participants") from e - finally: - await lkapi.aclose() - def clear_room_cache(self, room_id: UUID) -> None: """Clear all participant entries from the cache for a specific room.""" diff --git a/src/backend/core/tests/rooms/test_api_rooms_lobby.py b/src/backend/core/tests/rooms/test_api_rooms_lobby.py index 51ae97a8..22bc601f 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_lobby.py +++ b/src/backend/core/tests/rooms/test_api_rooms_lobby.py @@ -37,7 +37,7 @@ def test_request_entry_anonymous(settings): assert not lobby_keys with ( - mock.patch.object(LobbyService, "notify_participants", return_value=None), + mock.patch.object(utils, "notify_participants", return_value=None), mock.patch.object(utils, "generate_color", return_value="mocked-color"), ): response = client.post( @@ -86,7 +86,7 @@ def test_request_entry_authenticated_user(settings): assert not lobby_keys with ( - mock.patch.object(LobbyService, "notify_participants", return_value=None), + mock.patch.object(utils, "notify_participants", return_value=None), mock.patch.object(utils, "generate_color", return_value="mocked-color"), ): response = client.post( @@ -156,7 +156,7 @@ def test_request_entry_with_existing_participants(settings): # Mock external service calls to isolate the test with ( - mock.patch.object(LobbyService, "notify_participants", return_value=None), + mock.patch.object(utils, "notify_participants", return_value=None), mock.patch.object(utils, "generate_color", return_value="mocked-color"), ): # Make request as a new anonymous user @@ -205,7 +205,7 @@ def test_request_entry_public_room(settings): assert not lobby_keys with ( - mock.patch.object(LobbyService, "notify_participants", return_value=None), + mock.patch.object(utils, "notify_participants", return_value=None), mock.patch.object( LobbyService, "_get_or_create_participant_id", return_value="123" ), @@ -255,7 +255,7 @@ def test_request_entry_authenticated_user_public_room(settings): assert not lobby_keys with ( - mock.patch.object(LobbyService, "notify_participants", return_value=None), + mock.patch.object(utils, "notify_participants", return_value=None), mock.patch.object( LobbyService, "_get_or_create_participant_id", @@ -315,7 +315,7 @@ def test_request_entry_waiting_participant_public_room(settings): client.cookies.load({"mocked-cookie": "2f7f162fe7d1421b90e702bfbfbf8def"}) with ( - mock.patch.object(LobbyService, "notify_participants", return_value=None), + mock.patch.object(utils, "notify_participants", return_value=None), mock.patch.object( utils, "generate_livekit_config", return_value={"token": "test-token"} ), diff --git a/src/backend/core/tests/services/test_lobby.py b/src/backend/core/tests/services/test_lobby.py index aa139eff..8caf8026 100644 --- a/src/backend/core/tests/services/test_lobby.py +++ b/src/backend/core/tests/services/test_lobby.py @@ -5,7 +5,6 @@ Test lobby service. # pylint: disable=W0621,W0613, W0212, R0913 # ruff: noqa: PLR0913 -import json import uuid from unittest import mock @@ -14,18 +13,17 @@ from django.core.cache import cache from django.http import HttpResponse import pytest -from livekit.api import TwirpError from core.factories import RoomFactory from core.models import RoomAccessLevel from core.services.lobby import ( - LobbyNotificationError, LobbyParticipant, LobbyParticipantNotFound, LobbyParticipantParsingError, LobbyParticipantStatus, LobbyService, ) +from core.utils import NotificationError pytestmark = pytest.mark.django_db @@ -414,7 +412,7 @@ def test_refresh_waiting_status(mock_cache, lobby_service, participant_id): # pylint: disable=R0917 @mock.patch("core.services.lobby.cache") @mock.patch("core.utils.generate_color") -@mock.patch("core.services.lobby.LobbyService.notify_participants") +@mock.patch("core.utils.notify_participants") def test_enter_success( mock_notify, mock_generate_color, @@ -443,13 +441,15 @@ def test_enter_success( participant.to_dict(), timeout=settings.LOBBY_WAITING_TIMEOUT, ) - mock_notify.assert_called_once_with(room_id=room.id) + mock_notify.assert_called_once_with( + room_name=room.id, notification_data={"type": "participantWaiting"} + ) # pylint: disable=R0917 @mock.patch("core.services.lobby.cache") @mock.patch("core.utils.generate_color") -@mock.patch("core.services.lobby.LobbyService.notify_participants") +@mock.patch("core.utils.notify_participants") def test_enter_with_notification_error( mock_notify, mock_generate_color, @@ -460,7 +460,7 @@ def test_enter_with_notification_error( ): """Test participant entry with notification error.""" mock_generate_color.return_value = "#123456" - mock_notify.side_effect = LobbyNotificationError("Error notifying") + mock_notify.side_effect = NotificationError("Error notifying") lobby_service._get_cache_key = mock.Mock(return_value="mocked_cache_key") room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) @@ -776,116 +776,6 @@ def test_update_participant_status_success(mock_cache, lobby_service, participan lobby_service._get_cache_key.assert_called_once_with(room.id, participant_id) -@mock.patch("core.utils.create_livekit_client") -def test_notify_participants_success_no_room(mock_create_livekit_client, lobby_service): - """Test the notify_participants method when the LiveKit room doesn't exist yet.""" - - room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - - # Set up the mock LiveKitAPI and its behavior - mock_api_instance = mock.Mock() - mock_api_instance.room = mock.Mock() - mock_api_instance.room.send_data = mock.AsyncMock() - - # Create a proper response object with an empty rooms list - class MockResponse: - """LiveKit API response mock with empty rooms list.""" - - rooms = [] - - mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) - mock_api_instance.aclose = mock.AsyncMock() - mock_create_livekit_client.return_value = mock_api_instance - - # Act - lobby_service.notify_participants(room.id) - - # Verify that the service checked for existing rooms - mock_api_instance.room.list_rooms.assert_called_once() - - # Verify the send_data method was not called since no room exists - mock_api_instance.room.send_data.assert_not_called() - - # Verify the connection was properly closed - mock_api_instance.aclose.assert_called_once() - - -@mock.patch("core.utils.create_livekit_client") -def test_notify_participants_success(mock_create_livekit_client, lobby_service): - """Test successful participant notification.""" - room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - # Set up the mock LiveKitAPI and its behavior - mock_api_instance = mock.Mock() - mock_api_instance.room = mock.Mock() - mock_api_instance.room.send_data = mock.AsyncMock() - - class MockResponse: - """LiveKit API response mock with non-empty rooms list.""" - - rooms = ["room-1"] - - mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) - - mock_api_instance.aclose = mock.AsyncMock() - mock_create_livekit_client.return_value = mock_api_instance - - # Call the function - lobby_service.notify_participants(room.id) - - # Verify that the service checked for existing rooms - mock_api_instance.room.list_rooms.assert_called_once() - - # Verify the send_data method was called - mock_api_instance.room.send_data.assert_called_once() - send_data_request = mock_api_instance.room.send_data.call_args[0][0] - assert send_data_request.room == str(room.id) - assert ( - json.loads(send_data_request.data.decode("utf-8"))["type"] - == settings.LOBBY_NOTIFICATION_TYPE - ) - assert send_data_request.kind == 0 # RELIABLE mode in Livekit protocol - - # Verify aclose was called - mock_api_instance.aclose.assert_called_once() - - -@mock.patch("core.utils.create_livekit_client") -def test_notify_participants_error(mock_create_livekit_client, lobby_service): - """Test participant notification with API error.""" - room = RoomFactory(access_level=RoomAccessLevel.RESTRICTED) - # Set up the mock LiveKitAPI and its behavior - mock_api_instance = mock.Mock() - mock_api_instance.room = mock.Mock() - mock_api_instance.room.send_data = mock.AsyncMock( - side_effect=TwirpError(msg="test error", code=123, status=123) - ) - - class MockResponse: - """LiveKit API response mock with non-empty rooms list.""" - - rooms = ["room-1"] - - mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) - - mock_api_instance.aclose = mock.AsyncMock() - mock_create_livekit_client.return_value = mock_api_instance - - # Call the function and expect an exception - with pytest.raises( - LobbyNotificationError, match="Failed to notify room participants" - ): - lobby_service.notify_participants(room.id) - - # Verify that the service checked for existing rooms - mock_api_instance.room.list_rooms.assert_called_once() - - # Verify send_data was called - mock_api_instance.room.send_data.assert_called_once() - - # Verify aclose was still called after the exception - mock_api_instance.aclose.assert_called_once() - - def test_clear_room_cache(settings, lobby_service): """Test clearing room cache actually removes entries from cache.""" diff --git a/src/backend/core/tests/test_utils.py b/src/backend/core/tests/test_utils.py index 60e0e393..9f1f491f 100644 --- a/src/backend/core/tests/test_utils.py +++ b/src/backend/core/tests/test_utils.py @@ -2,9 +2,13 @@ Test utils functions """ +import json from unittest import mock -from core.utils import create_livekit_client +import pytest +from livekit.api import TwirpError + +from core.utils import NotificationError, create_livekit_client, notify_participants @mock.patch("asyncio.get_running_loop") @@ -60,3 +64,105 @@ def test_create_livekit_client_custom_configuration( create_livekit_client(custom_configuration) mock_livekit_api.assert_called_once_with(**custom_configuration, session=None) + + +@mock.patch("core.utils.create_livekit_client") +def test_notify_participants_error(mock_create_livekit_client): + """Test participant notification with API error.""" + + # Set up the mock LiveKitAPI and its behavior + mock_api_instance = mock.Mock() + mock_api_instance.room = mock.Mock() + mock_api_instance.room.send_data = mock.AsyncMock( + side_effect=TwirpError(msg="test error", code=123, status=123) + ) + + class MockResponse: + """LiveKit API response mock with non-empty rooms list.""" + + rooms = ["room-1"] + + mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) + + mock_api_instance.aclose = mock.AsyncMock() + mock_create_livekit_client.return_value = mock_api_instance + + # Call the function and expect an exception + with pytest.raises(NotificationError, match="Failed to notify room participants"): + notify_participants(room_name="room-number-1", notification_data={"foo": "foo"}) + + # Verify that the service checked for existing rooms + mock_api_instance.room.list_rooms.assert_called_once() + + # Verify send_data was called + mock_api_instance.room.send_data.assert_called_once() + + # Verify aclose was still called after the exception + mock_api_instance.aclose.assert_called_once() + + +@mock.patch("core.utils.create_livekit_client") +def test_notify_participants_success_no_room(mock_create_livekit_client): + """Test the notify_participants function when the LiveKit room doesn't exist.""" + + # Set up the mock LiveKitAPI and its behavior + mock_api_instance = mock.Mock() + mock_api_instance.room = mock.Mock() + mock_api_instance.room.send_data = mock.AsyncMock() + + # Create a proper response object with an empty rooms list + class MockResponse: + """LiveKit API response mock with empty rooms list.""" + + rooms = [] + + mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) + mock_api_instance.aclose = mock.AsyncMock() + mock_create_livekit_client.return_value = mock_api_instance + + notify_participants(room_name="room-number-1", notification_data={"foo": "foo"}) + + # Verify that the service checked for existing rooms + mock_api_instance.room.list_rooms.assert_called_once() + + # Verify the send_data method was not called since no room exists + mock_api_instance.room.send_data.assert_not_called() + + # Verify the connection was properly closed + mock_api_instance.aclose.assert_called_once() + + +@mock.patch("core.utils.create_livekit_client") +def test_notify_participants_success(mock_create_livekit_client): + """Test successful participant notification.""" + + # Set up the mock LiveKitAPI and its behavior + mock_api_instance = mock.Mock() + mock_api_instance.room = mock.Mock() + mock_api_instance.room.send_data = mock.AsyncMock() + + class MockResponse: + """LiveKit API response mock with non-empty rooms list.""" + + rooms = ["room-1"] + + mock_api_instance.room.list_rooms = mock.AsyncMock(return_value=MockResponse()) + + mock_api_instance.aclose = mock.AsyncMock() + mock_create_livekit_client.return_value = mock_api_instance + + # Call the function + notify_participants(room_name="room-number-1", notification_data={"foo": "foo"}) + + # Verify that the service checked for existing rooms + mock_api_instance.room.list_rooms.assert_called_once() + + # Verify the send_data method was called + mock_api_instance.room.send_data.assert_called_once() + send_data_request = mock_api_instance.room.send_data.call_args[0][0] + assert send_data_request.room == "room-number-1" + assert json.loads(send_data_request.data.decode("utf-8")) == {"foo": "foo"} + assert send_data_request.kind == 0 # RELIABLE mode in Livekit protocol + + # Verify aclose was called + mock_api_instance.aclose.assert_called_once() diff --git a/src/backend/core/utils.py b/src/backend/core/utils.py index fc51b63c..268a9be0 100644 --- a/src/backend/core/utils.py +++ b/src/backend/core/utils.py @@ -15,7 +15,15 @@ from django.core.files.storage import default_storage import aiohttp import botocore -from livekit.api import AccessToken, LiveKitAPI, VideoGrants +from asgiref.sync import async_to_sync +from livekit.api import ( # pylint: disable=E0611 + AccessToken, + ListRoomsRequest, + LiveKitAPI, + SendDataRequest, + TwirpError, + VideoGrants, +) def generate_color(identity: str) -> str: @@ -158,3 +166,37 @@ def create_livekit_client(custom_configuration=None): configuration = custom_configuration or settings.LIVEKIT_CONFIGURATION return LiveKitAPI(session=custom_session, **configuration) + + +class NotificationError(Exception): + """Notification delivery to room participants fails.""" + + +@async_to_sync +async def notify_participants(room_name: str, notification_data: dict): + """Send notification data to all participants in a LiveKit room.""" + + lkapi = create_livekit_client() + + try: + room_response = await lkapi.room.list_rooms( + ListRoomsRequest( + names=[room_name], + ) + ) + + # Check if the room exists + if not room_response.rooms: + return + + await lkapi.room.send_data( + SendDataRequest( + room=room_name, + data=json.dumps(notification_data).encode("utf-8"), + kind="RELIABLE", + ) + ) + except TwirpError as e: + raise NotificationError("Failed to notify room participants") from e + finally: + await lkapi.aclose()