From 481cb3cecc9d36ea45b51a2097c11345ee0ed08a Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 15 Oct 2021 21:44:17 +0200 Subject: [PATCH 1/6] Add search by room ID and room alias to List Room admin API --- docs/admin_api/rooms.md | 5 +- synapse/storage/databases/main/room.py | 21 +++++-- tests/rest/admin/test_room.py | 84 ++++++++++++++------------ 3 files changed, 64 insertions(+), 46 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 8e524e65090e..e98fd6b1b57d 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -39,8 +39,9 @@ The following query parameters are available: - `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. +* `search_term` - Filter rooms by their room name, canonical alias and room id. + Search term can be contained in any part of the room name and + local part of canonical alias or room id. Defaults to no filtering. **Response** diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index d69eaf80cefe..ae73cbb82654 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -409,7 +409,8 @@ 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, + canonical alias and room ids by Returns: A list of room dicts and an integer representing the total number of rooms that exist given this query @@ -417,14 +418,22 @@ async def get_rooms_paginate( # Filter room names by a string where_statement = "" if search_term: - where_statement = "WHERE LOWER(state.name) LIKE ?" + where_statement = """ + WHERE LOWER(state.name) LIKE ? + OR LOWER(state.canonical_alias) LIKE ? + OR LOWER(state.room_id) LIKE ? + """ # 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_term = [ + "%" + search_term.lower() + "%", + "#%" + search_term.lower() + "%:%", + "!%" + search_term.lower() + "%:%", + ] # Set ordering if RoomSortOrder(order_by) == RoomSortOrder.SIZE: @@ -517,10 +526,10 @@ async def get_rooms_paginate( def _get_rooms_paginate_txn(txn): # Execute the data query - sql_values = (limit, start) + sql_values = [limit, start] if search_term: # Add the search term into the WHERE clause - sql_values = (search_term,) + sql_values + sql_values = search_term + sql_values txn.execute(info_sql, sql_values) # Refactor room query data into a structured dictionary @@ -548,7 +557,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 () + sql_values = search_term if search_term else () txn.execute(count_sql, sql_values) room_count = txn.fetchone() diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 0fa55e03b45b..a84c2c37318f 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -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], @@ -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") @@ -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( @@ -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, @@ -927,20 +899,30 @@ def _search_test( _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") # 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 whole room name return no result, because of domain + _search_test(None, room_id_1) + # Test search local part of room id + _search_test(room_id_1, 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""" @@ -1123,6 +1105,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): From 3970b7c206b8b95ad172aabc2c3a9577dc36d925 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 15 Oct 2021 21:48:19 +0200 Subject: [PATCH 2/6] newsfile --- changelog.d/11099.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11099.feature diff --git a/changelog.d/11099.feature b/changelog.d/11099.feature new file mode 100644 index 000000000000..c9126d4a9d8f --- /dev/null +++ b/changelog.d/11099.feature @@ -0,0 +1 @@ +Add search by room ID and room alias to List Room admin API. \ No newline at end of file From f1e92e659de8f7239e874fb9e6417d6537032ef2 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 20 Oct 2021 09:25:37 +0200 Subject: [PATCH 3/6] rename var, update doc --- docs/admin_api/rooms.md | 10 +++++++--- synapse/storage/databases/main/room.py | 9 +++++---- tests/rest/admin/test_room.py | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index e98fd6b1b57d..328853a9e1fa 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -38,10 +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`. + 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. - Search term can be contained in any part of the room name and - local part of canonical alias or room id. Defaults to no filtering. + 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 local part of the room's id. + + Defaults to no filtering. **Response** diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index ae73cbb82654..b56c3d88ec83 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -417,6 +417,7 @@ async def get_rooms_paginate( """ # Filter room names by a string where_statement = "" + search_pattern = "" if search_term: where_statement = """ WHERE LOWER(state.name) LIKE ? @@ -429,7 +430,7 @@ async def get_rooms_paginate( # 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_pattern = [ "%" + search_term.lower() + "%", "#%" + search_term.lower() + "%:%", "!%" + search_term.lower() + "%:%", @@ -527,9 +528,9 @@ async def get_rooms_paginate( def _get_rooms_paginate_txn(txn): # Execute the data query sql_values = [limit, start] - if search_term: + if search_pattern: # Add the search term into the WHERE clause - sql_values = search_term + sql_values + sql_values = search_pattern + sql_values txn.execute(info_sql, sql_values) # Refactor room query data into a structured dictionary @@ -557,7 +558,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 () + sql_values = search_pattern if search_pattern else () txn.execute(count_sql, sql_values) room_count = txn.fetchone() diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index a84c2c37318f..1ddad0d249c8 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -895,7 +895,7 @@ 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") _search_test(room_id_1, "thing") From 2ab73a007b03bec6412b0d42efc040c069c2b0ab Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 1 Nov 2021 18:01:25 +0100 Subject: [PATCH 4/6] update search by `room_id` --- docs/admin_api/rooms.md | 2 +- synapse/storage/databases/main/room.py | 7 ++++--- tests/rest/admin/test_room.py | 10 ++++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 328853a9e1fa..b5de12829296 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -43,7 +43,7 @@ The following query parameters are available: 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 local part of the room's id. + - the complete (local and server part) room's id (case sensitive). Defaults to no filtering. diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index b56c3d88ec83..63172fdd196a 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -411,18 +411,19 @@ async def get_rooms_paginate( reverse_order: whether to reverse the room list search_term: a string to filter room names, canonical alias and room ids by + room ids should only match case sensitive and the complete ID 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 = "" + search_pattern = [] if search_term: where_statement = """ WHERE LOWER(state.name) LIKE ? OR LOWER(state.canonical_alias) LIKE ? - OR LOWER(state.room_id) LIKE ? + OR state.room_id = ? """ # Our postgres db driver converts ? -> %s in SQL strings as that's the @@ -433,7 +434,7 @@ async def get_rooms_paginate( search_pattern = [ "%" + search_term.lower() + "%", "#%" + search_term.lower() + "%:%", - "!%" + search_term.lower() + "%:%", + search_term, ] # Set ordering diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 1ddad0d249c8..4158615cb777 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -913,10 +913,12 @@ def _search_test( _search_test(None, "bar") _search_test(None, "", expected_http_code=400) - # Test that whole room name return no result, because of domain - _search_test(None, room_id_1) - # Test search local part of room id - _search_test(room_id_1, room_id_1[1:10]) + # 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") From 2451fc4ca1207681329a2ec90b5fb67bd2112ade Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 2 Nov 2021 07:51:01 +0100 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/databases/main/room.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 63172fdd196a..6333ec2d7ce4 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -410,8 +410,8 @@ async def get_rooms_paginate( 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, - canonical alias and room ids by - room ids should only match case sensitive and the complete ID + 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 @@ -559,8 +559,7 @@ def _get_rooms_paginate_txn(txn): # Execute the count query # Add the search term into the WHERE clause if present - sql_values = search_pattern if search_pattern else () - txn.execute(count_sql, sql_values) + txn.execute(count_sql, search_pattern) room_count = txn.fetchone() return rooms, room_count[0] From 831e2ce9105d67542917f225da1fa22d1d843953 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 2 Nov 2021 07:54:53 +0100 Subject: [PATCH 6/6] update `txn.execute` --- synapse/storage/databases/main/room.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 6333ec2d7ce4..9b90fb50468c 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -527,12 +527,9 @@ async def get_rooms_paginate( ) def _get_rooms_paginate_txn(txn): - # Execute the data query - sql_values = [limit, start] - if search_pattern: - # Add the search term into the WHERE clause - sql_values = search_pattern + 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 = []