Fix lints

See https://github.com/element-hq/synapse/pull/18794#discussion_r2263543402

changelog.d/18794.misc

@@ -0,0 +1 @@
+Update tests to ensure all database tables are emptied when purging a room.

synapse/storage/databases/main/purge_events.py

@@ -33,6 +33,73 @@ from synapse.types import RoomStreamToken
 logger = logging.getLogger(__name__)
 
 
+purge_room_tables_with_event_id_index = (
+    "event_auth",
+    "event_edges",
+    "event_json",
+    "event_push_actions_staging",
+    "event_relations",
+    "event_to_state_groups",
+    "event_auth_chains",
+    "event_auth_chain_to_calculate",
+    "redactions",
+    "rejections",
+    "state_events",
+)
+"""
+Tables which lack an index on `room_id` but have one on `event_id`
+"""
+
+purge_room_tables_with_room_id_column = (
+    "current_state_events",
+    "destination_rooms",
+    "event_backward_extremities",
+    "event_forward_extremities",
+    "event_push_actions",
+    "event_search",
+    "event_failed_pull_attempts",
+    # Note: the partial state tables have foreign keys between each other, and to
+    # `events` and `rooms`. We need to delete from them in the right order.
+    "partial_state_events",
+    "partial_state_rooms_servers",
+    "partial_state_rooms",
+    # Note: the _membership(s) tables have foreign keys to the `events` table
+    # so must be deleted first.
+    "local_current_membership",
+    "room_memberships",
+    # Note: the sliding_sync_ tables have foreign keys to the `events` table
+    # so must be deleted first.
+    "sliding_sync_joined_rooms",
+    "sliding_sync_membership_snapshots",
+    "events",
+    "federation_inbound_events_staging",
+    "receipts_graph",
+    "receipts_linearized",
+    "room_aliases",
+    "room_depth",
+    "room_stats_state",
+    "room_stats_current",
+    "room_stats_earliest_token",
+    "stream_ordering_to_exterm",
+    "users_in_public_rooms",
+    "users_who_share_private_rooms",
+    # no useful index, but let's clear them anyway
+    "appservice_room_list",
+    "e2e_room_keys",
+    "event_push_summary",
+    "pusher_throttle",
+    "room_account_data",
+    "room_tags",
+    # "rooms" happens last, to keep the foreign keys in the other tables
+    # happy
+    "rooms",
+)
+"""
+The tables with a `room_id` column regardless of whether they have a useful index on
+`room_id`.
+"""
+
+
 class PurgeEventsStore(StateGroupWorkerStore, CacheInvalidationWorkerStore):
     async def purge_history(
         self, room_id: str, token: str, delete_local_events: bool
@@ -398,20 +465,8 @@ class PurgeEventsStore(StateGroupWorkerStore, CacheInvalidationWorkerStore):
             referenced_chain_id_tuples,
         )
 
-        # Now we delete tables which lack an index on room_id but have one on event_id
-        for table in (
-            "event_auth",
-            "event_edges",
-            "event_json",
-            "event_push_actions_staging",
-            "event_relations",
-            "event_to_state_groups",
-            "event_auth_chains",
-            "event_auth_chain_to_calculate",
-            "redactions",
-            "rejections",
-            "state_events",
-        ):
+        # Now we delete tables which lack an index on `room_id` but have one on `event_id`
+        for table in purge_room_tables_with_event_id_index:
             logger.info("[purge] removing from %s", table)
 
             txn.execute(
@@ -424,51 +479,9 @@ class PurgeEventsStore(StateGroupWorkerStore, CacheInvalidationWorkerStore):
                 (room_id,),
             )
 
-        # next, the tables with an index on room_id (or no useful index)
-        for table in (
-            "current_state_events",
-            "destination_rooms",
-            "event_backward_extremities",
-            "event_forward_extremities",
-            "event_push_actions",
-            "event_search",
-            "event_failed_pull_attempts",
-            # Note: the partial state tables have foreign keys between each other, and to
-            # `events` and `rooms`. We need to delete from them in the right order.
-            "partial_state_events",
-            "partial_state_rooms_servers",
-            "partial_state_rooms",
-            # Note: the _membership(s) tables have foreign keys to the `events` table
-            # so must be deleted first.
-            "local_current_membership",
-            "room_memberships",
-            # Note: the sliding_sync_ tables have foreign keys to the `events` table
-            # so must be deleted first.
-            "sliding_sync_joined_rooms",
-            "sliding_sync_membership_snapshots",
-            "events",
-            "federation_inbound_events_staging",
-            "receipts_graph",
-            "receipts_linearized",
-            "room_aliases",
-            "room_depth",
-            "room_stats_state",
-            "room_stats_current",
-            "room_stats_earliest_token",
-            "stream_ordering_to_exterm",
-            "users_in_public_rooms",
-            "users_who_share_private_rooms",
-            # no useful index, but let's clear them anyway
-            "appservice_room_list",
-            "e2e_room_keys",
-            "event_push_summary",
-            "pusher_throttle",
-            "room_account_data",
-            "room_tags",
-            # "rooms" happens last, to keep the foreign keys in the other tables
-            # happy
-            "rooms",
-        ):
+        # next, the tables with a `room_id` column regardless of whether they have a
+        # useful index on `room_id`
+        for table in purge_room_tables_with_room_id_column:
             logger.info("[purge] removing from %s", table)
             txn.execute("DELETE FROM %s WHERE room_id=?" % (table,), (room_id,))
 

tests/rest/admin/test_room.py

@@ -39,6 +39,10 @@ from synapse.handlers.pagination import (
 )
 from synapse.rest.client import directory, events, knock, login, room, sync
 from synapse.server import HomeServer
+from synapse.storage.databases.main.purge_events import (
+    purge_room_tables_with_event_id_index,
+    purge_room_tables_with_room_id_column,
+)
 from synapse.types import UserID
 from synapse.util import Clock
 from synapse.util.task_scheduler import TaskScheduler
@@ -546,7 +550,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase):
 
     def _is_purged(self, room_id: str) -> None:
         """Test that the following tables have been purged of all rows related to the room."""
-        for table in PURGE_TABLES:
+        for table in purge_room_tables_with_room_id_column:
             count = self.get_success(
                 self.store.db_pool.simple_select_one_onecol(
                     table=table,
@@ -555,7 +559,21 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase):
                     desc="test_purge_room",
                 )
             )
+            self.assertEqual(count, 0, msg=f"Rows not purged in {table}")
 
+        for table in purge_room_tables_with_event_id_index:
+            rows = self.get_success(
+                self.store.db_pool.execute(
+                    "find_event_count_for_table",
+                    f"""
+                    SELECT COUNT(*) FROM {table} WHERE event_id IN (
+                        SELECT event_id FROM events WHERE room_id=?
+                    )
+                    """,
+                    room_id,
+                )
+            )
+            count = rows[0][0]
             self.assertEqual(count, 0, msg=f"Rows not purged in {table}")
 
     def _assert_peek(self, room_id: str, expect_code: int) -> None:
@@ -1228,7 +1246,7 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase):
 
     def _is_purged(self, room_id: str) -> None:
         """Test that the following tables have been purged of all rows related to the room."""
-        for table in PURGE_TABLES:
+        for table in purge_room_tables_with_room_id_column:
             count = self.get_success(
                 self.store.db_pool.simple_select_one_onecol(
                     table=table,
@@ -1237,7 +1255,21 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase):
                     desc="test_purge_room",
                 )
             )
+            self.assertEqual(count, 0, msg=f"Rows not purged in {table}")
 
+        for table in purge_room_tables_with_event_id_index:
+            rows = self.get_success(
+                self.store.db_pool.execute(
+                    "find_event_count_for_table",
+                    f"""
+                    SELECT COUNT(*) FROM {table} WHERE event_id IN (
+                        SELECT event_id FROM events WHERE room_id=?
+                    )
+                    """,
+                    room_id,
+                )
+            )
+            count = rows[0][0]
             self.assertEqual(count, 0, msg=f"Rows not purged in {table}")
 
     def _assert_peek(self, room_id: str, expect_code: int) -> None:
@@ -3119,35 +3151,3 @@ class BlockRoomTestCase(unittest.HomeserverTestCase):
         """Block a room in database"""
         self.get_success(self._store.block_room(room_id, self.other_user))
         self._is_blocked(room_id, expect=True)
-
-
-PURGE_TABLES = [
-    "current_state_events",
-    "event_backward_extremities",
-    "event_forward_extremities",
-    "event_json",
-    "event_push_actions",
-    "event_search",
-    "events",
-    "receipts_graph",
-    "receipts_linearized",
-    "room_aliases",
-    "room_depth",
-    "room_memberships",
-    "room_stats_state",
-    "room_stats_current",
-    "room_stats_earliest_token",
-    "rooms",
-    "stream_ordering_to_exterm",
-    "users_in_public_rooms",
-    "users_who_share_private_rooms",
-    "appservice_room_list",
-    "e2e_room_keys",
-    "event_push_summary",
-    "pusher_throttle",
-    "room_account_data",
-    "room_tags",
-    "state_groups",
-    "state_groups_state",
-    "federation_inbound_events_staging",
-]