Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix filtering of rooms when supplying the destination query parameter to /_synapse/admin/v1/federation/destinations/<destination>/rooms #17077

Merged
merged 4 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17077.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes a bug introduced in v1.52.0 where the `destination` query parameter for the [Destination Rooms Admin API](https://element-hq.github.io/synapse/v1.105/usage/administration/admin_api/federation.html#destination-rooms) failed to actually filter returned rooms.
1 change: 1 addition & 0 deletions synapse/storage/databases/main/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,7 @@ def get_destination_rooms_paginate_txn(
limit=limit,
retcols=("room_id", "stream_ordering"),
order_direction=order,
keyvalues={"destination": destination},
),
)
return rooms, count
Expand Down
67 changes: 64 additions & 3 deletions tests/rest/admin/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -778,20 +778,81 @@ def test_destination_rooms(self) -> None:
self.assertEqual(number_rooms, len(channel.json_body["rooms"]))
self._check_fields(channel.json_body["rooms"])

def _create_destination_rooms(self, number_rooms: int) -> None:
"""Create a number rooms for destination
def test_room_filtering(self) -> None:
"""Tests that rooms are correctly filtered"""

# Create two rooms on the homeserver. Each has a different remote homeserver
# participating in it.
other_destination = "other.destination.org"
room_ids_self_dest = self._create_destination_rooms(2, destination=self.dest)
room_ids_other_dest = self._create_destination_rooms(
1, destination=other_destination
)

# Ask for the rooms that `self.dest` is participating in.
channel = self.make_request("GET", self.url, access_token=self.admin_user_tok)
self.assertEqual(200, channel.code, msg=channel.json_body)

# Verify that we received only the rooms that `self.dest` is participating in.
# This assertion method name is a bit misleading. It does check that both lists
# contain the same items, and the same counts.
self.assertCountEqual(
[r["room_id"] for r in channel.json_body["rooms"]], room_ids_self_dest
)
self.assertEqual(channel.json_body["total"], len(room_ids_self_dest))

# Ask for the rooms that `other_destination` is participating in.
channel = self.make_request(
"GET",
self.url.replace(self.dest, other_destination),
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)

# Verify that we received only the rooms that `other_destination` is
# participating in.
self.assertCountEqual(
[r["room_id"] for r in channel.json_body["rooms"]], room_ids_other_dest
)
self.assertEqual(channel.json_body["total"], len(room_ids_other_dest))

def _create_destination_rooms(
self,
number_rooms: int,
destination: Optional[str] = None,
) -> List[str]:
"""
Create the given number of rooms. The given `destination` homeserver will
be recorded as a participant.

Args:
number_rooms: Number of rooms to be created
destination: The domain of the homeserver that will be considered
as a participant in the rooms.

Returns:
The IDs of the rooms that have been created.
"""
room_ids = []

# If no destination was provided, default to `self.dest`.
if destination is None:
destination = self.dest

for _ in range(number_rooms):
room_id = self.helper.create_room_as(
self.admin_user, tok=self.admin_user_tok
)
room_ids.append(room_id)

self.get_success(
self.store.store_destination_rooms_entries((self.dest,), room_id, 1234)
self.store.store_destination_rooms_entries(
(destination,), room_id, 1234
)
)

return room_ids

def _check_fields(self, content: List[JsonDict]) -> None:
"""Checks that the expected room attributes are present in content

Expand Down
Loading