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

Add search by room ID and room alias to List Room admin API #11099

Merged
merged 6 commits into from
Nov 2, 2021
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/11099.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add search by room ID and room alias to List Room admin API.
11 changes: 8 additions & 3 deletions docs/admin_api/rooms.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,14 @@ The following query parameters are available:
- `history_visibility` - Rooms are ordered alphabetically by visibility of history of the room.
- `state_events` - Rooms are ordered by number of state events. Largest to smallest.
* `dir` - Direction of room order. Either `f` for forwards or `b` for backwards. Setting
this value to `b` will reverse the above sort order. Defaults to `f`.
* `search_term` - Filter rooms by their room name. Search term can be contained in any
part of the room name. Defaults to no filtering.
this value to `b` will reverse the above sort order. Defaults to `f`.
* `search_term` - Filter rooms by their room name, canonical alias and room id.
Specifically, rooms are selected if the search term is contained in
- the room's name,
- the local part of the room's canonical alias, or
- the complete (local and server part) room's id (case sensitive).

Defaults to no filtering.

**Response**

Expand Down
29 changes: 18 additions & 11 deletions synapse/storage/databases/main/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,22 +409,33 @@ async def get_rooms_paginate(
limit: maximum amount of rooms to retrieve
order_by: the sort order of the returned list
reverse_order: whether to reverse the room list
search_term: a string to filter room names by
search_term: a string to filter room names,
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
canonical alias and room ids by.
Room ID must match exactly. Canonical alias must match a substring of the local part.
Returns:
A list of room dicts and an integer representing the total number of
rooms that exist given this query
"""
# Filter room names by a string
where_statement = ""
search_pattern = []
if search_term:
where_statement = "WHERE LOWER(state.name) LIKE ?"
where_statement = """
WHERE LOWER(state.name) LIKE ?
OR LOWER(state.canonical_alias) LIKE ?
OR state.room_id = ?
"""

# Our postgres db driver converts ? -> %s in SQL strings as that's the
# placeholder for postgres.
# HOWEVER, if you put a % into your SQL then everything goes wibbly.
# To get around this, we're going to surround search_term with %'s
# before giving it to the database in python instead
search_term = "%" + search_term.lower() + "%"
search_pattern = [
"%" + search_term.lower() + "%",
"#%" + search_term.lower() + "%:%",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this feels a bit redundant. All canonical aliases should start with # and contain a domain part, so why not just:

Suggested change
"#%" + search_term.lower() + "%:%",
"%" + search_term.lower() + "%",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason/idea is to search only in local part and not in server part.
Similar to users list

# `name` is in database already in lower case
if name:
filters.append("(name LIKE ? OR LOWER(displayname) LIKE ?)")
args.extend(["@%" + name.lower() + "%:%", "%" + name.lower() + "%"])
elif user_id:
filters.append("name LIKE ?")
args.extend(["%" + user_id.lower() + "%"])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I'm not entirely sure we should only search the local part here, but ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change it, if you like.
A good example is: find rooms with "matrix" at matrix.org homeserver. You have no chance to filter and find the "matrix" rooms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair point!

search_term,
]

# Set ordering
if RoomSortOrder(order_by) == RoomSortOrder.SIZE:
Expand Down Expand Up @@ -516,12 +527,9 @@ async def get_rooms_paginate(
)

def _get_rooms_paginate_txn(txn):
# Execute the data query
sql_values = (limit, start)
if search_term:
# Add the search term into the WHERE clause
sql_values = (search_term,) + sql_values
txn.execute(info_sql, sql_values)
# Add the search term into the WHERE clause
# and execute the data query
txn.execute(info_sql, search_pattern + [limit, start])

# Refactor room query data into a structured dictionary
rooms = []
Expand All @@ -548,8 +556,7 @@ def _get_rooms_paginate_txn(txn):
# Execute the count query

# Add the search term into the WHERE clause if present
sql_values = (search_term,) if search_term else ()
txn.execute(count_sql, sql_values)
txn.execute(count_sql, search_pattern)

room_count = txn.fetchone()
return rooms, room_count[0]
Expand Down
88 changes: 49 additions & 39 deletions tests/rest/admin/test_room.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,36 +689,6 @@ def test_room_list_sort_order(self):
reversing the order, etc.
"""

def _set_canonical_alias(room_id: str, test_alias: str, admin_user_tok: str):
# Create a new alias to this room
url = "/_matrix/client/r0/directory/room/%s" % (
urllib.parse.quote(test_alias),
)
channel = self.make_request(
"PUT",
url.encode("ascii"),
{"room_id": room_id},
access_token=admin_user_tok,
)
self.assertEqual(
200, int(channel.result["code"]), msg=channel.result["body"]
)

# Set this new alias as the canonical alias for this room
self.helper.send_state(
room_id,
"m.room.aliases",
{"aliases": [test_alias]},
tok=admin_user_tok,
state_key="test",
)
self.helper.send_state(
room_id,
"m.room.canonical_alias",
{"alias": test_alias},
tok=admin_user_tok,
)

def _order_test(
order_type: str,
expected_room_list: List[str],
Expand Down Expand Up @@ -790,9 +760,9 @@ def _order_test(
)

# Set room canonical room aliases
_set_canonical_alias(room_id_1, "#A_alias:test", self.admin_user_tok)
_set_canonical_alias(room_id_2, "#B_alias:test", self.admin_user_tok)
_set_canonical_alias(room_id_3, "#C_alias:test", self.admin_user_tok)
self._set_canonical_alias(room_id_1, "#A_alias:test", self.admin_user_tok)
self._set_canonical_alias(room_id_2, "#B_alias:test", self.admin_user_tok)
self._set_canonical_alias(room_id_3, "#C_alias:test", self.admin_user_tok)

# Set room member size in the reverse order. room 1 -> 1 member, 2 -> 2, 3 -> 3
user_1 = self.register_user("bob1", "pass")
Expand Down Expand Up @@ -859,7 +829,7 @@ def test_search_term(self):
room_id_2 = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok)

room_name_1 = "something"
room_name_2 = "else"
room_name_2 = "LoremIpsum"

# Set the name for each room
self.helper.send_state(
Expand All @@ -875,6 +845,8 @@ def test_search_term(self):
tok=self.admin_user_tok,
)

self._set_canonical_alias(room_id_1, "#Room_Alias1:test", self.admin_user_tok)

def _search_test(
expected_room_id: Optional[str],
search_term: str,
Expand Down Expand Up @@ -923,24 +895,36 @@ def _search_test(
r = rooms[0]
self.assertEqual(expected_room_id, r["room_id"])

# Perform search tests
# Test searching by room name
_search_test(room_id_1, "something")
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
_search_test(room_id_1, "thing")

_search_test(room_id_2, "else")
_search_test(room_id_2, "se")
_search_test(room_id_2, "LoremIpsum")
_search_test(room_id_2, "lorem")

# Test case insensitive
_search_test(room_id_1, "SOMETHING")
_search_test(room_id_1, "THING")

_search_test(room_id_2, "ELSE")
_search_test(room_id_2, "SE")
_search_test(room_id_2, "LOREMIPSUM")
_search_test(room_id_2, "LOREM")

_search_test(None, "foo")
_search_test(None, "bar")
_search_test(None, "", expected_http_code=400)

# Test that the whole room id returns the room
_search_test(room_id_1, room_id_1)
# Test that the search by room_id is case sensitive
_search_test(None, room_id_1.lower())
# Test search part of local part of room id do not match
_search_test(None, room_id_1[1:10])

# Test that whole room alias return no result, because of domain
_search_test(None, "#Room_Alias1:test")
# Test search local part of alias
_search_test(room_id_1, "alias1")

def test_search_term_non_ascii(self):
"""Test that searching for a room with non-ASCII characters works correctly"""

Expand Down Expand Up @@ -1123,6 +1107,32 @@ def test_room_state(self):
# the create_room already does the right thing, so no need to verify that we got
# the state events it created.

def _set_canonical_alias(self, room_id: str, test_alias: str, admin_user_tok: str):
# Create a new alias to this room
url = "/_matrix/client/r0/directory/room/%s" % (urllib.parse.quote(test_alias),)
channel = self.make_request(
"PUT",
url.encode("ascii"),
{"room_id": room_id},
access_token=admin_user_tok,
)
self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])

# Set this new alias as the canonical alias for this room
self.helper.send_state(
room_id,
"m.room.aliases",
{"aliases": [test_alias]},
tok=admin_user_tok,
state_key="test",
)
self.helper.send_state(
room_id,
"m.room.canonical_alias",
{"alias": test_alias},
tok=admin_user_tok,
)


class JoinAliasRoomTestCase(unittest.HomeserverTestCase):

Expand Down