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(
[
|
