Fix 'delete room' admin api to work on incomplete rooms (#11523)

If, for some reason, we don't have the create event, we should still be able to
purge a room.

5 files changed, 33 insertions, 37 deletions

diff --git a/changelog.d/11523.feature b/changelog.d/11523.feature
new file mode 100644
index 0000000000..ecac7f9db9
--- /dev/null
+++ b/changelog.d/11523.feature
@@ -0,0 +1 @@
+Extend the "delete room" admin api to work correctly on rooms which have previously been partially deleted.
diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py
index cd64142735..4f42438053 100644
--- a/synapse/handlers/pagination.py
+++ b/synapse/handlers/pagination.py
@@ -406,9 +406,6 @@ class PaginationHandler:
             force: set true to skip checking for joined users.
         """
         with await self.pagination_lock.write(room_id):
-            # check we know about the room
-            await self.store.get_room_version_id(room_id)
-
             # first check that we have no users in this room
             if not force:
                 joined = await self.store.is_host_joined(room_id, self._server_name)
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 2bcdf32dcc..ead2198e14 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -1535,20 +1535,13 @@ class RoomShutdownHandler:
             await self.store.block_room(room_id, requester_user_id)
 
         if not await self.store.get_room(room_id):
-            if block:
-                # We allow you to block an unknown room.
-                return {
-                    "kicked_users": [],
-                    "failed_to_kick_users": [],
-                    "local_aliases": [],
-                    "new_room_id": None,
-                }
-            else:
-                # But if you don't want to preventatively block another room,
-                # this function can't do anything useful.
-                raise NotFoundError(
-                    "Cannot shut down room: unknown room id %s" % (room_id,)
-                )
+            # if we don't know about the room, there is nothing left to do.
+            return {
+                "kicked_users": [],
+                "failed_to_kick_users": [],
+                "local_aliases": [],
+                "new_room_id": None,
+            }
 
         if new_room_user_id is not None:
             if not self.hs.is_mine_id(new_room_user_id):
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 669ab44a45..829e86675a 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -106,9 +106,6 @@ class RoomRestV2Servlet(RestServlet):
                 HTTPStatus.BAD_REQUEST, "%s is not a legal room ID" % (room_id,)
             )
 
-        if not await self._store.get_room(room_id):
-            raise NotFoundError("Unknown room id %s" % (room_id,))
-
         delete_id = self._pagination_handler.start_shutdown_and_purge_room(
             room_id=room_id,
             new_room_user_id=content.get("new_room_user_id"),
diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py
index d3858e460d..22f9aa6234 100644
--- a/tests/rest/admin/test_room.py
+++ b/tests/rest/admin/test_room.py
@@ -83,7 +83,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase):
 
     def test_room_does_not_exist(self):
         """
-        Check that unknown rooms/server return error HTTPStatus.NOT_FOUND.
+        Check that unknown rooms/server return 200
         """
         url = "/_synapse/admin/v1/rooms/%s" % "!unknown:test"
 
@@ -94,8 +94,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase):
             access_token=self.admin_user_tok,
         )
 
-        self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body)
-        self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"])
+        self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
 
     def test_room_is_not_valid(self):
         """
@@ -508,27 +507,36 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase):
         self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body)
         self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"])
 
-    @parameterized.expand(
-        [
-            ("DELETE", "/_synapse/admin/v2/rooms/%s"),
-            ("GET", "/_synapse/admin/v2/rooms/%s/delete_status"),
-            ("GET", "/_synapse/admin/v2/rooms/delete_status/%s"),
-        ]
-    )
-    def test_room_does_not_exist(self, method: str, url: str):
-        """
-        Check that unknown rooms/server return error HTTPStatus.NOT_FOUND.
+    def test_room_does_not_exist(self):
         """
+        Check that unknown rooms/server return 200
 
+        This is important, as it allows incomplete vestiges of rooms to be cleared up
+        even if the create event/etc is missing.
+        """
+        room_id = "!unknown:test"
         channel = self.make_request(
-            method,
-            url % "!unknown:test",
+            "DELETE",
+            f"/_synapse/admin/v2/rooms/{room_id}",
             content={},
             access_token=self.admin_user_tok,
         )
 
-        self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body)
-        self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"])
+        self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
+        self.assertIn("delete_id", channel.json_body)
+        delete_id = channel.json_body["delete_id"]
+
+        # get status
+        channel = self.make_request(
+            "GET",
+            f"/_synapse/admin/v2/rooms/{room_id}/delete_status",
+            access_token=self.admin_user_tok,
+        )
+
+        self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
+        self.assertEqual(1, len(channel.json_body["results"]))
+        self.assertEqual("complete", channel.json_body["results"][0]["status"])
+        self.assertEqual(delete_id, channel.json_body["results"][0]["delete_id"])
 
     @parameterized.expand(
         [