🐛(backend) lock rooms while updating them

Before this commit, a room update and a soft deletion racing each other
could leave the room active.

Both `perform_update`s now re-read the room with `SELECT ... FOR UPDATE`
inside a transaction to avoid this race condition.

src/backend/core/api/locks.py

@@ -0,0 +1,17 @@
+"""Row locks guarding the API write paths."""
+
+import contextlib
+
+from django.db import transaction
+
+from core import models
+from core.api.exceptions import ensure_room_not_deleted
+
+
+@contextlib.contextmanager
+def room_locked_for_update(room):
+    """Lock a room's row for the duration of a write."""
+    with transaction.atomic():
+        locked_room = models.Room.all_objects.select_for_update().get(pk=room.pk)
+        ensure_room_not_deleted(locked_room)
+        yield locked_room

src/backend/core/api/viewsets.py

@@ -44,6 +44,7 @@ from core import analytics, enums, models, utils
 from core.api import throttling
 from core.api.exceptions import ensure_room_not_deleted
 from core.api.filters import ListFileFilter
+from core.api.locks import room_locked_for_update
 from core.enums import MEDIA_STORAGE_URL_PATTERN
 from core.recording.enums import FileExtension
 from core.recording.event.authentication import RecordingProcessWebhookAuthentication
@@ -405,12 +406,20 @@ class RoomViewSet(  # pylint: disable=too-many-public-methods
         )
 
     def perform_update(self, serializer):
-        """Persist the room update, then sync metadata to LiveKit."""
+        """Persist the room update under a row lock, then sync metadata to LiveKit.
 
-        old_configuration = serializer.instance.configuration
-        old_access_level = serializer.instance.access_level
+        The lock is what keeps a concurrent soft delete from being undone: the
+        full-row write of `save()` would otherwise carry back the `deleted_at`
+        read before the deletion committed.
+        """
 
-        room = serializer.save()
+        with room_locked_for_update(serializer.instance) as locked_room:
+            serializer.instance = locked_room
+
+            old_configuration = locked_room.configuration
+            old_access_level = locked_room.access_level
+
+            room = serializer.save()
 
         if (
             room.configuration == old_configuration

src/backend/core/external_api/viewsets.py

@@ -26,6 +26,7 @@ from rest_framework.generics import get_object_or_404
 from core import analytics, api, models
 from core.api.exceptions import ensure_room_not_deleted
 from core.api.feature_flag import FeatureFlag
+from core.api.locks import room_locked_for_update
 from core.services.jwt_token import JwtTokenService
 from core.services.room_management import RoomManagement, RoomManagementException
 from core.services.transit_code import TransitCodeService
@@ -268,14 +269,17 @@ class RoomViewSet(
         self._track_room_event(instance, analytics.AnalyticsEvent.ROOM_DELETED)
 
     def perform_update(self, serializer: serializers.RoomSerializer):
-        """Persist the room update, sync it to LiveKit, then log and track it."""
+        """Persist the room update under a row lock, sync it to LiveKit, then track it."""
 
-        previous_values = {
-            "access_level": serializer.instance.access_level,
-            "configuration": copy.deepcopy(serializer.instance.configuration),
-        }
+        with room_locked_for_update(serializer.instance) as locked_room:
+            serializer.instance = locked_room
 
-        room = serializer.save()
+            previous_values = {
+                "access_level": locked_room.access_level,
+                "configuration": copy.deepcopy(locked_room.configuration),
+            }
+
+            room = serializer.save()
 
         # Report the fields that actually changed, not the ones that were submitted.
         updated_fields = sorted(

src/backend/core/tests/rooms/test_api_rooms_update.py

@@ -11,8 +11,9 @@ from django.utils import timezone
 import pytest
 from rest_framework.test import APIClient
 
+from ...api.serializers import RoomSerializer
 from ...factories import RoomFactory, UserFactory
-from ...models import RoomAccessLevel
+from ...models import Room, RoomAccessLevel
 from ...services.room_management import (
     RoomManagement,
     RoomManagementException,
@@ -468,3 +469,40 @@ def test_api_rooms_update_authenticated_with_user_access_token(privileged_role):
     assert response.status_code == 200
     room.refresh_from_db()
     assert room.name == "new name"
+
+
+@patch.object(RoomManagement, "sync_room_metadata")
+def test_api_rooms_update_soft_deleted_after_load(mock_sync_room_metadata):
+    """A room soft deleted while an update is in flight should not be restored."""
+    user = UserFactory()
+    room = RoomFactory(
+        name="Old name",
+        access_level=RoomAccessLevel.RESTRICTED,
+        users=[(user, "owner")],
+    )
+
+    original_is_valid = RoomSerializer.is_valid
+
+    def soft_delete_while_validating(self, **kwargs):
+        is_valid = original_is_valid(self, **kwargs)
+        Room.all_objects.get(pk=self.instance.pk).soft_delete()
+        return is_valid
+
+    client = APIClient()
+    client.force_login(user)
+
+    with patch.object(RoomSerializer, "is_valid", soft_delete_while_validating):
+        response = client.patch(
+            f"/api/v1.0/rooms/{room.id!s}/",
+            {"name": "New name", "access_level": RoomAccessLevel.PUBLIC},
+        )
+
+    assert response.status_code == 410
+    assert response.json() == {"detail": "This room has been deleted."}
+
+    refreshed_room = Room.all_objects.get(pk=room.pk)
+    assert refreshed_room.deleted_at is not None
+    assert refreshed_room.name == "Old name"
+    assert refreshed_room.access_level == RoomAccessLevel.RESTRICTED
+
+    mock_sync_room_metadata.assert_not_called()

src/backend/core/tests/test_external_api_rooms.py

@@ -17,6 +17,7 @@ from lasuite.oidc_resource_server.authentication import ResourceServerAuthentica
 from rest_framework.test import APIClient
 
 from core.analytics import AnalyticsEvent
+from core.external_api.serializers import RoomSerializer
 from core.factories import ApplicationFactory, RoomFactory, UserFactory
 from core.models import (
     Application,
@@ -1444,6 +1445,47 @@ def test_api_rooms_update_tracks_analytics(mock_update_metadata, mock_capture):
     mock_update_metadata.assert_called_once()
 
 
+@mock.patch("core.external_api.viewsets.analytics.capture")
+@mock.patch.object(RoomManagement, "sync_room_metadata")
+def test_api_rooms_update_soft_deleted_after_load(
+    mock_sync_room_metadata, mock_capture
+):
+    """A room soft deleted while an update is in flight should not be restored."""
+
+    user = UserFactory()
+    room = RoomFactory(
+        users=[(user, RoleChoices.OWNER)], access_level=RoomAccessLevel.TRUSTED
+    )
+
+    original_is_valid = RoomSerializer.is_valid
+
+    def soft_delete_while_validating(self, **kwargs):
+        is_valid = original_is_valid(self, **kwargs)
+        Room.all_objects.get(pk=self.instance.pk).soft_delete()
+        return is_valid
+
+    token = generate_test_token(user, [ApplicationScope.ROOMS_UPDATE])
+
+    client = APIClient()
+    client.credentials(HTTP_AUTHORIZATION=f"Bearer {token}")
+
+    with mock.patch.object(RoomSerializer, "is_valid", soft_delete_while_validating):
+        response = client.patch(
+            f"/external-api/v1.0/rooms/{room.id}/",
+            {"access_level": RoomAccessLevel.RESTRICTED},
+            format="json",
+        )
+
+    assert response.status_code == 410
+
+    refreshed_room = Room.all_objects.get(pk=room.pk)
+    assert refreshed_room.deleted_at is not None
+    assert refreshed_room.access_level == RoomAccessLevel.TRUSTED
+
+    mock_sync_room_metadata.assert_not_called()
+    mock_capture.assert_not_called()
+
+
 @mock.patch.object(RoomManagement, "delete_room")
 def test_api_rooms_delete_requires_authentication(mock_delete_room):
     """Deleting a room without authentication should return 401."""