Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix 'delete room' admin api to work on incomplete rooms (#11523)
Browse files Browse the repository at this point in the history
If, for some reason, we don't have the create event, we should still be able to
purge a room.
  • Loading branch information
richvdh authored Dec 7, 2021
1 parent 9c55ded commit b1ecd19
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 37 deletions.
1 change: 1 addition & 0 deletions changelog.d/11523.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Extend the "delete room" admin api to work correctly on rooms which have previously been partially deleted.
3 changes: 0 additions & 3 deletions synapse/handlers/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,6 @@ async def purge_room(self, room_id: str, force: bool = False) -> None:
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)
Expand Down
21 changes: 7 additions & 14 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -1535,20 +1535,13 @@ async def shutdown_room(
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):
Expand Down
3 changes: 0 additions & 3 deletions synapse/rest/admin/rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,6 @@ async def on_DELETE(
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"),
Expand Down
42 changes: 25 additions & 17 deletions tests/rest/admin/test_room.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_requester_is_no_admin(self):

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"

Expand All @@ -94,8 +94,7 @@ def test_room_does_not_exist(self):
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):
"""
Expand Down Expand Up @@ -508,27 +507,36 @@ def test_requester_is_no_admin(self, method: str, url: str):
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(
[
Expand Down

0 comments on commit b1ecd19

Please sign in to comment.