From 34454ad88ced3fc594e321d74bc9dde59090b243 Mon Sep 17 00:00:00 2001 From: Alexander Fechler Date: Thu, 6 Jun 2024 16:18:20 +0200 Subject: [PATCH 1/5] Filter added to Admin-API GET /rooms --- docs/admin_api/rooms.md | 2 + synapse/rest/admin/rooms.py | 21 +++++++++-- synapse/storage/databases/main/room.py | 51 +++++++++++++++++++------- tests/rest/admin/test_room.py | 38 +++++++++++++++++++ 4 files changed, 94 insertions(+), 18 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 6935ec4a45..03f990bba8 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -36,6 +36,8 @@ The following query parameters are available: - 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). +* `filter_public_rooms` - Flag to filter public and non-public rooms. +* `filter_empty_rooms` - Flag to filter empty and non-empty rooms. A room is empty if joined_members is zero. Defaults to no filtering. diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 0d86a4e15f..bbbca32f7b 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -35,6 +35,7 @@ from synapse.http.servlet import ( ResolveRoomIdMixin, RestServlet, assert_params_in_dict, + parse_boolean, parse_enum, parse_integer, parse_json, @@ -242,13 +243,25 @@ class ListRoomRestServlet(RestServlet): errcode=Codes.INVALID_PARAM, ) + filter_public_rooms = parse_boolean(request, "filter_public_rooms") + filter_empty_rooms = parse_boolean(request, "filter_empty_rooms") + direction = parse_enum(request, "dir", Direction, default=Direction.FORWARDS) reverse_order = True if direction == Direction.BACKWARDS else False + try: + # Return list of rooms according to parameters + rooms, total_rooms = await self.store.get_rooms_paginate( + start, + limit, + order_by, + reverse_order, + search_term, + filter_public_rooms, + filter_empty_rooms, + ) + except Exception as e: + print(e) - # Return list of rooms according to parameters - rooms, total_rooms = await self.store.get_rooms_paginate( - start, limit, order_by, reverse_order, search_term - ) response = { # next_token should be opaque, so return a value the client can parse "offset": start, diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 616c941687..17060f05a3 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -606,6 +606,8 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): order_by: str, reverse_order: bool, search_term: Optional[str], + filter_public_rooms: Optional[bool], + filter_empty_rooms: Optional[bool], ) -> Tuple[List[Dict[str, Any]], int]: """Function to retrieve a paginated list of rooms as json. @@ -617,30 +619,49 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): search_term: a string to filter room names, canonical alias and room ids by. Room ID must match exactly. Canonical alias must match a substring of the local part. + filter_public_rooms: Optional flag to filter public and non-public rooms. If true, public rooms are queried. + if false, public rooms are excluded from the query. When it is + none (the default), both public rooms and none-public-rooms are queried. + filter_empty_rooms: Optional flag to filter empty and non-empty rooms. + A room is empty if joined_members is zero. + If true, empty rooms are queried. + if false, empty rooms are excluded from the query. When it is + none (the default), both empty rooms and none-empty rooms are queried. 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: List[object] = [] + filter_ = [] + where_args = [] if search_term: - where_statement = """ - WHERE LOWER(state.name) LIKE ? - OR LOWER(state.canonical_alias) LIKE ? - OR state.room_id = ? - """ + filter_ = [ + "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_pattern = [ - "%" + search_term.lower() + "%", - "#%" + search_term.lower() + "%:%", + where_args = [ + f"%{search_term.lower()}%", + f"#%{search_term.lower()}%:%", search_term, ] + if filter_public_rooms is not None: + filter_arg = "1" if filter_public_rooms else "0" + filter_.append(f"rooms.is_public = {filter_arg}") + + if filter_empty_rooms is not None: + if filter_empty_rooms: + filter_.append("curr.joined_members = 0") + else: + filter_.append("curr.joined_members <> 0") + + where_clause = "WHERE " + " AND ".join(filter_) if len(filter_) > 0 else "" # Set ordering if RoomSortOrder(order_by) == RoomSortOrder.SIZE: @@ -717,7 +738,7 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): LIMIT ? OFFSET ? """.format( - where=where_statement, + where=where_clause, order_by=order_by_column, direction="ASC" if order_by_asc else "DESC", ) @@ -726,10 +747,12 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): count_sql = """ SELECT count(*) FROM ( SELECT room_id FROM room_stats_state state + INNER JOIN room_stats_current curr USING (room_id) + INNER JOIN rooms USING (room_id) {where} ) AS get_room_ids """.format( - where=where_statement, + where=where_clause, ) def _get_rooms_paginate_txn( @@ -737,7 +760,7 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): ) -> Tuple[List[Dict[str, Any]], int]: # Add the search term into the WHERE clause # and execute the data query - txn.execute(info_sql, search_pattern + [limit, start]) + txn.execute(info_sql, where_args + [limit, start]) # Refactor room query data into a structured dictionary rooms = [] @@ -767,7 +790,7 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): # Execute the count query # Add the search term into the WHERE clause if present - txn.execute(count_sql, search_pattern) + txn.execute(count_sql, where_args) room_count = cast(Tuple[int], txn.fetchone()) return rooms, room_count[0] diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 7562747260..96b455ea83 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -1795,6 +1795,44 @@ class RoomTestCase(unittest.HomeserverTestCase): self.assertEqual(room_id, channel.json_body["rooms"][0].get("room_id")) self.assertEqual("ж", channel.json_body["rooms"][0].get("name")) + def test_filter_public_rooms(self) -> None: + self.helper.create_room_as( + self.admin_user, tok=self.admin_user_tok, is_public=True + ) + self.helper.create_room_as( + self.admin_user, tok=self.admin_user_tok, is_public=True + ) + self.helper.create_room_as( + self.admin_user, tok=self.admin_user_tok, is_public=False + ) + + response = self.make_request( + "GET", + "/_synapse/admin/v1/rooms", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, response.code, msg=response.json_body) + self.assertEqual(3, response.json_body["total_rooms"]) + self.assertEqual(3, len(response.json_body["rooms"])) + + response = self.make_request( + "GET", + "/_synapse/admin/v1/rooms?filter_public_rooms=true", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, response.code, msg=response.json_body) + self.assertEqual(2, response.json_body["total_rooms"]) + self.assertEqual(2, len(response.json_body["rooms"])) + + response = self.make_request( + "GET", + "/_synapse/admin/v1/rooms?filter_public_rooms=false", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, response.code, msg=response.json_body) + self.assertEqual(1, response.json_body["total_rooms"]) + self.assertEqual(1, len(response.json_body["rooms"])) + def test_single_room(self) -> None: """Test that a single room can be requested correctly""" # Create two test rooms From 712e3d17c22e625adfbbbd539dc18b2c03c13bc2 Mon Sep 17 00:00:00 2001 From: Alexander Fechler Date: Thu, 6 Jun 2024 16:42:24 +0200 Subject: [PATCH 2/5] changelog added. --- changelog.d/17276.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17276.feature diff --git a/changelog.d/17276.feature b/changelog.d/17276.feature new file mode 100644 index 0000000000..ba2611a70f --- /dev/null +++ b/changelog.d/17276.feature @@ -0,0 +1 @@ +Filter for public and empty rooms added to Admin-API [List Room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#list-room-api). From f09d00020a7143e0b5c8e82caff24a070ee60540 Mon Sep 17 00:00:00 2001 From: Alexander Fechler Date: Thu, 6 Jun 2024 17:21:44 +0200 Subject: [PATCH 3/5] - postgres incompatibility fixed. - Test for filter_empty_rooms added. --- synapse/storage/databases/main/room.py | 2 +- tests/rest/admin/test_room.py | 39 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 17060f05a3..a3f094a968 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -653,7 +653,7 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): ] if filter_public_rooms is not None: filter_arg = "1" if filter_public_rooms else "0" - filter_.append(f"rooms.is_public = {filter_arg}") + filter_.append(f"rooms.is_public = '{filter_arg}'") if filter_empty_rooms is not None: if filter_empty_rooms: diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 96b455ea83..5559ccccdc 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -1833,6 +1833,45 @@ class RoomTestCase(unittest.HomeserverTestCase): self.assertEqual(1, response.json_body["total_rooms"]) self.assertEqual(1, len(response.json_body["rooms"])) + def test_filter_empty_rooms(self) -> None: + self.helper.create_room_as( + self.admin_user, tok=self.admin_user_tok, is_public=True + ) + self.helper.create_room_as( + self.admin_user, tok=self.admin_user_tok, is_public=True + ) + room_id = self.helper.create_room_as( + self.admin_user, tok=self.admin_user_tok, is_public=False + ) + self.helper.leave(room_id, self.admin_user, tok=self.admin_user_tok) + + response = self.make_request( + "GET", + "/_synapse/admin/v1/rooms", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, response.code, msg=response.json_body) + self.assertEqual(3, response.json_body["total_rooms"]) + self.assertEqual(3, len(response.json_body["rooms"])) + + response = self.make_request( + "GET", + "/_synapse/admin/v1/rooms?filter_empty_rooms=false", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, response.code, msg=response.json_body) + self.assertEqual(2, response.json_body["total_rooms"]) + self.assertEqual(2, len(response.json_body["rooms"])) + + response = self.make_request( + "GET", + "/_synapse/admin/v1/rooms?filter_empty_rooms=true", + access_token=self.admin_user_tok, + ) + self.assertEqual(200, response.code, msg=response.json_body) + self.assertEqual(1, response.json_body["total_rooms"]) + self.assertEqual(1, len(response.json_body["rooms"])) + def test_single_room(self) -> None: """Test that a single room can be requested correctly""" # Create two test rooms From c55b6382597d55980bdeb6d962aa97978079a757 Mon Sep 17 00:00:00 2001 From: Alexander Fechler Date: Mon, 10 Jun 2024 17:44:13 +0200 Subject: [PATCH 4/5] - Query parameter renamed and description adjusted. - catch-block with print-statement removed. --- docs/admin_api/rooms.md | 6 ++++-- synapse/rest/admin/rooms.py | 28 ++++++++++++-------------- synapse/storage/databases/main/room.py | 16 +++++++-------- tests/rest/admin/test_room.py | 8 ++++---- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 03f990bba8..8e3a367e90 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -36,8 +36,10 @@ The following query parameters are available: - 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). -* `filter_public_rooms` - Flag to filter public and non-public rooms. -* `filter_empty_rooms` - Flag to filter empty and non-empty rooms. A room is empty if joined_members is zero. +* `public_rooms` - Optional flag to filter public rooms. If `true`, only public rooms are queried. If `false`, public rooms are excluded from + the query. When the flag is absent (the default), **both** public and non-public rooms are included in the search results. +* `empty_rooms` - Optional flag to filter empty rooms. A room is empty if joined_members is zero. If `true`, only empty rooms are queried. If `false`, empty rooms are excluded from + the query. When the flag is absent (the default), **both** empty and non-empty rooms are included in the search results. Defaults to no filtering. diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index bbbca32f7b..01f9de9ffa 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -243,24 +243,22 @@ class ListRoomRestServlet(RestServlet): errcode=Codes.INVALID_PARAM, ) - filter_public_rooms = parse_boolean(request, "filter_public_rooms") - filter_empty_rooms = parse_boolean(request, "filter_empty_rooms") + public_rooms = parse_boolean(request, "public_rooms") + empty_rooms = parse_boolean(request, "empty_rooms") direction = parse_enum(request, "dir", Direction, default=Direction.FORWARDS) reverse_order = True if direction == Direction.BACKWARDS else False - try: - # Return list of rooms according to parameters - rooms, total_rooms = await self.store.get_rooms_paginate( - start, - limit, - order_by, - reverse_order, - search_term, - filter_public_rooms, - filter_empty_rooms, - ) - except Exception as e: - print(e) + + # Return list of rooms according to parameters + rooms, total_rooms = await self.store.get_rooms_paginate( + start, + limit, + order_by, + reverse_order, + search_term, + public_rooms, + empty_rooms, + ) response = { # next_token should be opaque, so return a value the client can parse diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index a3f094a968..4972aef303 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -606,8 +606,8 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): order_by: str, reverse_order: bool, search_term: Optional[str], - filter_public_rooms: Optional[bool], - filter_empty_rooms: Optional[bool], + public_rooms: Optional[bool], + empty_rooms: Optional[bool], ) -> Tuple[List[Dict[str, Any]], int]: """Function to retrieve a paginated list of rooms as json. @@ -619,10 +619,10 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): search_term: a string to filter room names, canonical alias and room ids by. Room ID must match exactly. Canonical alias must match a substring of the local part. - filter_public_rooms: Optional flag to filter public and non-public rooms. If true, public rooms are queried. + public_rooms: Optional flag to filter public and non-public rooms. If true, public rooms are queried. if false, public rooms are excluded from the query. When it is none (the default), both public rooms and none-public-rooms are queried. - filter_empty_rooms: Optional flag to filter empty and non-empty rooms. + empty_rooms: Optional flag to filter empty and non-empty rooms. A room is empty if joined_members is zero. If true, empty rooms are queried. if false, empty rooms are excluded from the query. When it is @@ -651,12 +651,12 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): f"#%{search_term.lower()}%:%", search_term, ] - if filter_public_rooms is not None: - filter_arg = "1" if filter_public_rooms else "0" + if public_rooms is not None: + filter_arg = "1" if public_rooms else "0" filter_.append(f"rooms.is_public = '{filter_arg}'") - if filter_empty_rooms is not None: - if filter_empty_rooms: + if empty_rooms is not None: + if empty_rooms: filter_.append("curr.joined_members = 0") else: filter_.append("curr.joined_members <> 0") diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 5559ccccdc..95ed736451 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -1817,7 +1817,7 @@ class RoomTestCase(unittest.HomeserverTestCase): response = self.make_request( "GET", - "/_synapse/admin/v1/rooms?filter_public_rooms=true", + "/_synapse/admin/v1/rooms?public_rooms=true", access_token=self.admin_user_tok, ) self.assertEqual(200, response.code, msg=response.json_body) @@ -1826,7 +1826,7 @@ class RoomTestCase(unittest.HomeserverTestCase): response = self.make_request( "GET", - "/_synapse/admin/v1/rooms?filter_public_rooms=false", + "/_synapse/admin/v1/rooms?public_rooms=false", access_token=self.admin_user_tok, ) self.assertEqual(200, response.code, msg=response.json_body) @@ -1856,7 +1856,7 @@ class RoomTestCase(unittest.HomeserverTestCase): response = self.make_request( "GET", - "/_synapse/admin/v1/rooms?filter_empty_rooms=false", + "/_synapse/admin/v1/rooms?empty_rooms=false", access_token=self.admin_user_tok, ) self.assertEqual(200, response.code, msg=response.json_body) @@ -1865,7 +1865,7 @@ class RoomTestCase(unittest.HomeserverTestCase): response = self.make_request( "GET", - "/_synapse/admin/v1/rooms?filter_empty_rooms=true", + "/_synapse/admin/v1/rooms?empty_rooms=true", access_token=self.admin_user_tok, ) self.assertEqual(200, response.code, msg=response.json_body) From b886ca4a89b390e6b4c9c77f0b477f181ccec21b Mon Sep 17 00:00:00 2001 From: Alexander Fechler Date: Mon, 10 Jun 2024 18:31:06 +0200 Subject: [PATCH 5/5] Link to Admin-API adjusted. --- changelog.d/17276.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/17276.feature b/changelog.d/17276.feature index ba2611a70f..a1edfae0aa 100644 --- a/changelog.d/17276.feature +++ b/changelog.d/17276.feature @@ -1 +1 @@ -Filter for public and empty rooms added to Admin-API [List Room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#list-room-api). +Filter for public and empty rooms added to Admin-API [List Room API](https://element-hq.github.io/synapse/latest/admin_api/rooms.html#list-room-api).