✨(backend) purge soft-deleted rooms after a retention period

Add a ROOM_DELETED_RETENTION_DAYS setting, unset by default. The
purge_inactive_rooms command now also deletes rooms soft deleted more
than that many days ago, whether they are inactive or not. Inactive
soft-deleted rooms are now purged under ROOM_INACTIVITY_DELETION_DAYS
too. As for inactive rooms, a room holding a saved recording that has
not expired yet is kept.

src/backend/core/management/commands/purge_inactive_rooms.py

@@ -1,6 +1,8 @@
-"""Purge inactive rooms."""
+"""Purge inactive and soft-deleted rooms."""
 
+import operator
 from datetime import timedelta
+from functools import reduce
 from itertools import batched
 from logging import getLogger
 
@@ -22,10 +24,12 @@ class Command(BaseCommand):
     - rooms which were last started before that period
     - rooms never started and created before that period
 
+    Also delete rooms soft deleted more than ROOM_DELETED_RETENTION_DAYS days ago.
+
     Rooms holding a saved recording that has not expired are kept.
     """
 
-    help = "Purge inactive rooms"
+    help = "Purge inactive and soft-deleted rooms"
 
     def add_arguments(self, parser):
         parser.add_argument(
@@ -35,53 +39,68 @@ class Command(BaseCommand):
         )
 
     def handle(self, *args, **options):
-        """Browse inactive rooms and delete them chunk by chunk."""
+        """Browse purgeable rooms and delete them chunk by chunk."""
 
-        if not settings.ROOM_INACTIVITY_DELETION_DAYS:
+        if not (
+            settings.ROOM_INACTIVITY_DELETION_DAYS
+            or settings.ROOM_DELETED_RETENTION_DAYS
+        ):
             self.stdout.write(
-                "Purging inactive rooms is disabled "
-                "(ROOM_INACTIVITY_DELETION_DAYS is not set)."
+                "Purging rooms is disabled (neither ROOM_INACTIVITY_DELETION_DAYS "
+                "nor ROOM_DELETED_RETENTION_DAYS is set)."
             )
             return
 
         now = timezone.now()
-        inactive_rooms = self.get_inactive_rooms(now)
+        purgeable_rooms = self.get_purgeable_rooms(now)
 
-        inactive_count = inactive_rooms.count()
-        if not inactive_count:
-            self.stdout.write("No inactive room to purge.")
+        purgeable_count = purgeable_rooms.count()
+        if not purgeable_count:
+            self.stdout.write("No room to purge.")
             return
 
         if options["dry_run"]:
-            self.stdout.write(
-                f"[dry-run] {inactive_count} inactive room(s) would be purged:"
-            )
-            names = inactive_rooms.values_list("name", flat=True)
-            for name in names.iterator(chunk_size=CHUNK_SIZE):
-                self.stdout.write(f"- {name}")
+            self.stdout.write(f"[dry-run] {purgeable_count} room(s) would be purged:")
+            rooms = purgeable_rooms.values_list("name", "deleted_at")
+            for name, deleted_at in rooms.iterator(chunk_size=CHUNK_SIZE):
+                self.stdout.write(f"- {name}{' (deleted)' if deleted_at else ''}")
             return
 
         purged_count = 0
-        rooms = inactive_rooms.values_list("pk", "slug").iterator(chunk_size=CHUNK_SIZE)
+        rooms = purgeable_rooms.values_list("pk", "slug", "deleted_at").iterator(
+            chunk_size=CHUNK_SIZE
+        )
         for chunk in batched(rooms, CHUNK_SIZE, strict=False):
-            for room_id, slug in chunk:
-                logger.info("Purging inactive room %s (%s)", room_id, slug)
+            for room_id, slug, deleted_at in chunk:
+                state = "deleted" if deleted_at else "inactive"
+                logger.info("Purging %s room %s (%s)", state, room_id, slug)
 
-            _, deleted_by_model = inactive_rooms.filter(
-                pk__in=[room_id for room_id, _ in chunk]
+            _, deleted_by_model = purgeable_rooms.filter(
+                pk__in=[room_id for room_id, _, _ in chunk]
             ).delete()
             purged_count += deleted_by_model.get("core.Room", 0)
 
-        self.stdout.write(f"Purged {purged_count} inactive room(s).")
+        self.stdout.write(f"Purged {purged_count} room(s).")
 
     @staticmethod
-    def get_inactive_rooms(now):
-        """Return the rooms inactive for too long that no recording protects."""
+    def get_purgeable_rooms(now):
+        """Return the inactive or long-deleted rooms that no recording protects.
 
-        threshold = now - timedelta(days=settings.ROOM_INACTIVITY_DELETION_DAYS)
-        is_inactive = Q(last_started_at__lt=threshold) | Q(
-            last_started_at__isnull=True, created_at__lt=threshold
-        )
+        Soft-deleted rooms are hidden by `Room.objects`, hence `Room.all_objects`.
+        """
+
+        conditions = []
+        if settings.ROOM_INACTIVITY_DELETION_DAYS:
+            threshold = now - timedelta(days=settings.ROOM_INACTIVITY_DELETION_DAYS)
+            conditions.append(
+                Q(last_started_at__lt=threshold)
+                | Q(last_started_at__isnull=True, created_at__lt=threshold)
+            )
+        if settings.ROOM_DELETED_RETENTION_DAYS:
+            deleted_before = now - timedelta(days=settings.ROOM_DELETED_RETENTION_DAYS)
+            conditions.append(Q(deleted_at__lt=deleted_before))
+        if not conditions:
+            return Room.all_objects.none()
 
         protected_recordings = Recording.objects.filter(
             room=OuterRef("pk"), status__in=RecordingStatusChoices.saved_statuses()
@@ -91,4 +110,6 @@ class Command(BaseCommand):
                 created_at__gte=now - timedelta(days=settings.RECORDING_EXPIRATION_DAYS)
             )
 
-        return Room.objects.filter(is_inactive, ~Exists(protected_recordings))
+        return Room.all_objects.filter(
+            reduce(operator.or_, conditions), ~Exists(protected_recordings)
+        )

src/backend/core/tests/commands/test_purge_inactive_rooms.py

@@ -9,6 +9,7 @@ from django.core.management import call_command
 from django.utils import timezone
 
 import pytest
+from rest_framework.test import APIClient
 
 from core import factories, models
 
@@ -19,14 +20,25 @@ COMMAND_MODULE = "core.management.commands.purge_inactive_rooms"
 BEFORE_PERIOD = timedelta(days=366)
 WITHIN_PERIOD = timedelta(days=364)
 
+BEFORE_RETENTION = timedelta(days=31)
+WITHIN_RETENTION = timedelta(days=29)
+
 
 @pytest.fixture(name="purge_enabled", autouse=True)
 def fixture_purge_enabled(settings):
     """Enable the purge of the rooms inactive for a year."""
     settings.ROOM_INACTIVITY_DELETION_DAYS = 365
+    settings.ROOM_DELETED_RETENTION_DAYS = None
     settings.RECORDING_EXPIRATION_DAYS = 30
 
 
+@pytest.fixture(name="retention_only")
+def fixture_retention_only(settings):
+    """Only purge the rooms soft deleted more than 30 days ago."""
+    settings.ROOM_INACTIVITY_DELETION_DAYS = None
+    settings.ROOM_DELETED_RETENTION_DAYS = 30
+
+
 def create_at(date, factory, **kwargs):
     """Build an object with the factory as if it was created at the given date."""
     with mock.patch("django.utils.timezone.now", return_value=date):
@@ -41,12 +53,12 @@ def call_purge(*args):
 
 
 def room_exists(room):
-    """Tell whether the room is still in database."""
-    return models.Room.objects.filter(pk=room.pk).exists()
+    """Tell whether the room is still in database, soft deleted or not."""
+    return models.Room.all_objects.filter(pk=room.pk).exists()
 
 
 def test_purge_inactive_rooms_disabled(settings):
-    """Should delete nothing when no inactivity period is configured."""
+    """Should delete nothing when neither period is configured."""
     settings.ROOM_INACTIVITY_DELETION_DAYS = None
     room = create_at(timezone.now() - BEFORE_PERIOD, factories.RoomFactory)
 
@@ -68,7 +80,7 @@ def test_purge_inactive_rooms_without_recording_expiration(settings):
         status=models.RecordingStatusChoices.SAVED,
     )
 
-    assert call_purge() == "Purged 1 inactive room(s).\n"
+    assert call_purge() == "Purged 1 room(s).\n"
 
     assert not room_exists(room)
     assert room_exists(room_with_recording)
@@ -97,7 +109,7 @@ def test_purge_inactive_rooms_started_before_period(caplog):
     with caplog.at_level(logging.INFO, logger=COMMAND_MODULE):
         output = call_purge()
 
-    assert output == "Purged 1 inactive room(s).\n"
+    assert output == "Purged 1 room(s).\n"
     assert not room_exists(room)
     assert f"Purging inactive room {room.pk} ({room.slug})" in caplog.text
 
@@ -120,7 +132,7 @@ def test_purge_inactive_rooms_started_within_period():
         last_started_at=now - WITHIN_PERIOD,
     )
 
-    assert call_purge() == "No inactive room to purge.\n"
+    assert call_purge() == "No room to purge.\n"
 
     assert room_exists(room)
 
@@ -129,7 +141,7 @@ def test_purge_inactive_rooms_never_started_created_within_period():
     """Should keep a room that was never started but created within the period."""
     room = create_at(timezone.now() - WITHIN_PERIOD, factories.RoomFactory)
 
-    assert call_purge() == "No inactive room to purge.\n"
+    assert call_purge() == "No room to purge.\n"
 
     assert room_exists(room)
 
@@ -221,7 +233,7 @@ def test_purge_inactive_rooms_dry_run():
     factories.RoomFactory(name="Recent room")
 
     assert call_purge("--dry-run") == (
-        "[dry-run] 2 inactive room(s) would be purged:\n- Alpha room\n- Beta room\n"
+        "[dry-run] 2 room(s) would be purged:\n- Alpha room\n- Beta room\n"
     )
 
     assert all(room_exists(room) for room in rooms)
@@ -235,5 +247,134 @@ def test_purge_inactive_rooms_several_chunks():
     with mock.patch(f"{COMMAND_MODULE}.CHUNK_SIZE", 2):
         output = call_purge()
 
-    assert output == "Purged 5 inactive room(s).\n"
+    assert output == "Purged 5 room(s).\n"
     assert not any(room_exists(room) for room in rooms)
+
+
+def test_purge_inactive_rooms_soft_deleted(caplog):
+    """Should delete an inactive room even if it was soft deleted recently."""
+    now = timezone.now()
+    room = create_at(
+        now - BEFORE_PERIOD,
+        factories.RoomFactory,
+        deleted_at=now - timedelta(days=1),
+    )
+
+    with caplog.at_level(logging.INFO, logger=COMMAND_MODULE):
+        output = call_purge()
+
+    assert output == "Purged 1 room(s).\n"
+    assert not room_exists(room)
+    assert f"Purging deleted room {room.pk} ({room.slug})" in caplog.text
+
+
+@pytest.mark.usefixtures("retention_only")
+def test_purge_deleted_rooms_before_retention(caplog):
+    """Should delete a room soft deleted before the retention period."""
+    room = factories.RoomFactory(deleted_at=timezone.now() - BEFORE_RETENTION)
+
+    with caplog.at_level(logging.INFO, logger=COMMAND_MODULE):
+        output = call_purge()
+
+    assert output == "Purged 1 room(s).\n"
+    assert not room_exists(room)
+    assert f"Purging deleted room {room.pk} ({room.slug})" in caplog.text
+
+
+@pytest.mark.usefixtures("retention_only")
+def test_purge_deleted_rooms_within_retention():
+    """Should keep a room soft deleted within the retention period."""
+    room = factories.RoomFactory(deleted_at=timezone.now() - WITHIN_RETENTION)
+
+    assert call_purge() == "No room to purge.\n"
+
+    assert room_exists(room)
+
+
+@pytest.mark.usefixtures("retention_only")
+def test_purge_deleted_rooms_retention_only_keeps_inactive_rooms():
+    """Should keep inactive rooms that are not deleted when only retention is set."""
+    room = create_at(timezone.now() - BEFORE_PERIOD, factories.RoomFactory)
+
+    assert call_purge() == "No room to purge.\n"
+
+    assert room_exists(room)
+
+
+def test_purge_deleted_rooms_within_retention_but_inactive(settings):
+    """Should delete an inactive room even if it was deleted within the retention."""
+    settings.ROOM_DELETED_RETENTION_DAYS = 30
+    now = timezone.now()
+    room = create_at(
+        now - BEFORE_PERIOD,
+        factories.RoomFactory,
+        deleted_at=now - WITHIN_RETENTION,
+    )
+
+    call_purge()
+
+    assert not room_exists(room)
+
+
+@pytest.mark.usefixtures("retention_only")
+def test_purge_deleted_rooms_recording_not_expired():
+    """Should keep a deleted room holding a saved recording that has not expired."""
+    room = factories.RoomFactory(deleted_at=timezone.now() - BEFORE_RETENTION)
+    factories.RecordingFactory(room=room, status=models.RecordingStatusChoices.SAVED)
+
+    assert call_purge() == "No room to purge.\n"
+
+    assert room_exists(room)
+
+
+@pytest.mark.usefixtures("retention_only")
+def test_purge_deleted_rooms_recording_expired():
+    """Should delete a deleted room along with its expired recordings."""
+    now = timezone.now()
+    room = create_at(
+        now - timedelta(days=40),
+        factories.RoomFactory,
+        deleted_at=now - BEFORE_RETENTION,
+    )
+    recording = create_at(
+        now - timedelta(days=40),
+        factories.RecordingFactory,
+        room=room,
+        status=models.RecordingStatusChoices.SAVED,
+    )
+
+    call_purge()
+
+    assert not room_exists(room)
+    assert not models.Recording.objects.filter(pk=recording.pk).exists()
+
+
+def test_purge_rooms_dry_run_flags_deleted_rooms(settings):
+    """Should flag the soft-deleted rooms listed on a dry run."""
+    settings.ROOM_DELETED_RETENTION_DAYS = 30
+    now = timezone.now()
+    rooms = [
+        create_at(now - BEFORE_PERIOD, factories.RoomFactory, name="Alpha room"),
+        factories.RoomFactory(name="Beta room", deleted_at=now - BEFORE_RETENTION),
+    ]
+    factories.RoomFactory(name="Gamma room", deleted_at=now - WITHIN_RETENTION)
+
+    assert call_purge("--dry-run") == (
+        "[dry-run] 2 room(s) would be purged:\n- Alpha room\n- Beta room (deleted)\n"
+    )
+
+    assert all(room_exists(room) for room in rooms)
+
+
+@pytest.mark.usefixtures("retention_only")
+def test_purge_deleted_rooms_frees_slug():
+    """Should let a new room take the slug of a purged deleted room."""
+    factories.RoomFactory(name="my room", deleted_at=timezone.now() - BEFORE_RETENTION)
+    client = APIClient()
+    client.force_login(factories.UserFactory())
+
+    call_purge()
+    response = client.post("/api/v1.0/rooms/", {"name": "My Room!"})
+
+    assert response.status_code == 201
+    assert models.Room.objects.get().slug == "my-room"

src/backend/meet/settings.py

@@ -778,6 +778,9 @@ class Base(Configuration):
     ROOM_INACTIVITY_DELETION_DAYS = values.PositiveIntegerValue(
         None, environ_name="ROOM_INACTIVITY_DELETION_DAYS", environ_prefix=None
     )
+    ROOM_DELETED_RETENTION_DAYS = values.PositiveIntegerValue(
+        None, environ_name="ROOM_DELETED_RETENTION_DAYS", environ_prefix=None
+    )
     # if provided, treat as suspicious (possible privilege escalation attempt).
     PARTICIPANT_FORBIDDEN_PERMISSION_FIELDS = values.ListValue(
         ["hidden", "recorder", "agent"],
@@ -1475,21 +1478,26 @@ class Base(Configuration):
                 stacklevel=2,
             )
 
-        if cls.ROOM_INACTIVITY_DELETION_DAYS:
+        for setting_name, rooms, period in (
+            ("ROOM_INACTIVITY_DELETION_DAYS", "Inactive rooms", "inactivity period"),
+            ("ROOM_DELETED_RETENTION_DAYS", "Deleted rooms", "retention period"),
+        ):
+            purge_days = getattr(cls, setting_name)
+            if not purge_days:
+                continue
             if not cls.RECORDING_EXPIRATION_DAYS:
                 warnings.warn(
-                    "ROOM_INACTIVITY_DELETION_DAYS is set but "
-                    "RECORDING_EXPIRATION_DAYS is not. Recordings never expire, so "
-                    "inactive rooms holding a saved recording will never be purged.",
+                    f"{setting_name} is set but RECORDING_EXPIRATION_DAYS is not. "
+                    f"Recordings never expire, so {rooms.lower()} holding a saved "
+                    "recording will never be purged.",
                     UserWarning,
                     stacklevel=2,
                 )
-            elif cls.RECORDING_EXPIRATION_DAYS >= cls.ROOM_INACTIVITY_DELETION_DAYS:
+            elif cls.RECORDING_EXPIRATION_DAYS >= purge_days:
                 warnings.warn(
                     "RECORDING_EXPIRATION_DAYS is greater than or equal to "
-                    "ROOM_INACTIVITY_DELETION_DAYS. Inactive rooms holding a saved "
-                    "recording will be kept past the inactivity period, until their "
-                    "recordings expire.",
+                    f"{setting_name}. {rooms} holding a saved recording will be "
+                    f"kept past the {period}, until their recordings expire.",
                     UserWarning,
                     stacklevel=2,
                 )