From d4532eeb6487e38ae68181344714aa4e6094a438 Mon Sep 17 00:00:00 2001 From: lebaudantoine Date: Tue, 12 Nov 2024 18:30:51 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20remove=20unnecess?= =?UTF-8?q?ary=20manipulation=20of=20the=20room=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoided unnecessary manipulation of the room name to prevent issues when starting an egress worker. Previously, hyphens were stripped from the room name, likely inherited from the legacy setup with Jitsi in Magnify, though the purpose of this change is unclear and might be an undesired legacy feature. To ensure accurate room matching during egress worker requests, this update removes any manipulation of the room name. This approach minimizes the risk of errors when initiating recordings and maintains the integrity of the original room name throughout the process. --- src/backend/core/api/serializers.py | 3 +-- src/backend/core/recording/worker/mediator.py | 4 +--- .../core/tests/recording/worker/test_mediator.py | 2 +- .../core/tests/rooms/test_api_rooms_retrieve.py | 10 +++++----- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 37933be6..ff3b268f 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -130,8 +130,7 @@ class RoomSerializer(serializers.ModelSerializer): del output["configuration"] if role is not None or instance.is_public: - slug = f"{instance.id!s}".replace("-", "") - + slug = f"{instance.id!s}" username = request.query_params.get("username", None) output["livekit"] = { diff --git a/src/backend/core/recording/worker/mediator.py b/src/backend/core/recording/worker/mediator.py index e2829699..12311c2b 100644 --- a/src/backend/core/recording/worker/mediator.py +++ b/src/backend/core/recording/worker/mediator.py @@ -41,13 +41,11 @@ class WorkerServiceMediator: RecordingStartError: If there is an error starting the recording. """ - # FIXME - no manipulations of room_name should be required - room_name = f"{recording.room.id!s}".replace("-", "") - if recording.status != RecordingStatusChoices.INITIATED: logger.error("Cannot start recording in %s status.", recording.status) raise RecordingStartError() + room_name = str(recording.room.id) try: worker_id = self._worker_service.start(room_name, recording.id) except (WorkerRequestError, WorkerConnectionError, WorkerResponseError) as e: diff --git a/src/backend/core/tests/recording/worker/test_mediator.py b/src/backend/core/tests/recording/worker/test_mediator.py index f84dc260..81fd7904 100644 --- a/src/backend/core/tests/recording/worker/test_mediator.py +++ b/src/backend/core/tests/recording/worker/test_mediator.py @@ -45,7 +45,7 @@ def test_start_recording_success(mediator, mock_worker_service): mediator.start(mock_recording) # Verify worker service call - expected_room_name = str(mock_recording.room.id).replace("-", "") + expected_room_name = str(mock_recording.room.id) mock_worker_service.start.assert_called_once_with( expected_room_name, mock_recording.id ) diff --git a/src/backend/core/tests/rooms/test_api_rooms_retrieve.py b/src/backend/core/tests/rooms/test_api_rooms_retrieve.py index ffa17eaa..4762a4d6 100644 --- a/src/backend/core/tests/rooms/test_api_rooms_retrieve.py +++ b/src/backend/core/tests/rooms/test_api_rooms_retrieve.py @@ -38,7 +38,7 @@ def test_api_rooms_retrieve_anonymous_private_pk(): def test_api_rooms_retrieve_anonymous_private_pk_no_dashes(): """It should be possible to get a room by its id stripped of its dashes.""" room = RoomFactory(is_public=False) - id_no_dashes = str(room.id).replace("-", "") + id_no_dashes = str(room.id) client = APIClient() response = client.get(f"/api/v1.0/rooms/{id_no_dashes:s}/") @@ -178,7 +178,7 @@ def test_api_rooms_retrieve_anonymous_public(mock_token): response = client.get(f"/api/v1.0/rooms/{room.id!s}/") assert response.status_code == 200 - expected_name = f"{room.id!s}".replace("-", "") + expected_name = f"{room.id!s}" assert response.json() == { "id": str(room.id), "is_administrable": False, @@ -220,7 +220,7 @@ def test_api_rooms_retrieve_authenticated_public(mock_token): ) assert response.status_code == 200 - expected_name = f"{room.id!s}".replace("-", "") + expected_name = f"{room.id!s}" assert response.json() == { "id": str(room.id), "is_administrable": False, @@ -318,7 +318,7 @@ def test_api_rooms_retrieve_members(mock_token, django_assert_num_queries): key=lambda x: x["id"], ) - expected_name = str(room.id).replace("-", "") + expected_name = str(room.id) assert content_dict == { "id": str(room.id), "is_administrable": False, @@ -390,7 +390,7 @@ def test_api_rooms_retrieve_administrators(mock_token, django_assert_num_queries ], key=lambda x: x["id"], ) - expected_name = str(room.id).replace("-", "") + expected_name = str(room.id) assert content_dict == { "id": str(room.id), "is_administrable": True,