From 990c7d2c2a2bec999c91db9eb2187a0f9bcb7c7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 18 Jun 2022 10:41:32 +0200 Subject: [PATCH 01/35] Add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- changelog.d/13031.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13031.feature diff --git a/changelog.d/13031.feature b/changelog.d/13031.feature new file mode 100644 index 000000000000..fee8e9d1ffc3 --- /dev/null +++ b/changelog.d/13031.feature @@ -0,0 +1 @@ +Implement [MSC3827](https://github.com/matrix-org/matrix-spec-proposals/pull/3827): Filtering of /publicRooms by room type. From 50d0c2afe7cae89b0248d8af301909b22d184e88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sun, 12 Jun 2022 19:43:20 +0200 Subject: [PATCH 02/35] Handle `room_type` in `room_stats_state` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/handlers/stats.py | 3 +++ synapse/storage/databases/main/stats.py | 6 ++++++ .../delta/72/01add_room_type_to_state_stats.sql | 16 ++++++++++++++++ 3 files changed, 25 insertions(+) create mode 100644 synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql diff --git a/synapse/handlers/stats.py b/synapse/handlers/stats.py index f45e06eb0e08..2c84761e9770 100644 --- a/synapse/handlers/stats.py +++ b/synapse/handlers/stats.py @@ -271,6 +271,9 @@ async def _handle_deltas( room_state["is_federatable"] = ( event_content.get(EventContentFields.FEDERATE, True) is True ) + room_state["room_type"] = event_content.get( + EventContentFields.ROOM_TYPE + ) elif typ == EventTypes.JoinRules: room_state["join_rules"] = event_content.get("join_rule") elif typ == EventTypes.RoomHistoryVisibility: diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index 82851ffa95a0..e1a1602196a2 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -238,6 +238,7 @@ async def update_room_state(self, room_id: str, fields: Dict[str, Any]) -> None: * avatar * canonical_alias * guest_access + * room_type A is_federatable key can also be included with a boolean value. @@ -263,6 +264,7 @@ async def update_room_state(self, room_id: str, fields: Dict[str, Any]) -> None: "avatar", "canonical_alias", "guest_access", + "room_type", ): field = fields.get(col, sentinel) if field is not sentinel and (not isinstance(field, str) or "\0" in field): @@ -604,6 +606,10 @@ def _fetch_current_state_stats( room_state["is_federatable"] = ( event.content.get(EventContentFields.FEDERATE, True) is True ) + elif event.state["room_type"]: + room_state["room_type"] = event.content.get( + EventContentFields.ROOM_TYPE + ) await self.update_room_state(room_id, room_state) diff --git a/synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql b/synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql new file mode 100644 index 000000000000..f966f36e557f --- /dev/null +++ b/synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql @@ -0,0 +1,16 @@ +/* Copyright 2022 The Matrix.org Foundation C.I.C + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +ALTER TABLE room_stats_state ADD room_type TEXT; From 2eb6714a35607ede4867ab49a35b3e72f11b508c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sun, 12 Jun 2022 19:45:30 +0200 Subject: [PATCH 03/35] Add basic `room_type` filtering to `/public_rooms` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/handlers/room_list.py | 5 ++++- synapse/storage/databases/main/room.py | 26 ++++++++++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py index 183d4ae3c4fe..95f8c187f0b6 100644 --- a/synapse/handlers/room_list.py +++ b/synapse/handlers/room_list.py @@ -181,6 +181,7 @@ def build_room_entry(room: JsonDict) -> JsonDict: == HistoryVisibility.WORLD_READABLE, "guest_can_join": room["guest_access"] == "can_join", "join_rule": room["join_rules"], + "room_type": room["room_type"], } # Filter out Nones – rather omit the field altogether @@ -239,7 +240,9 @@ def build_room_entry(room: JsonDict) -> JsonDict: response["chunk"] = results response["total_room_count_estimate"] = await self.store.count_public_rooms( - network_tuple, ignore_non_federatable=from_federation + network_tuple, + ignore_non_federatable=from_federation, + search_filter=search_filter, ) return response diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 5760d3428e66..74262b20df47 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -37,7 +37,7 @@ from synapse.api.room_versions import RoomVersion, RoomVersions from synapse.config.homeserver import HomeServerConfig from synapse.events import EventBase -from synapse.storage._base import SQLBaseStore, db_to_json +from synapse.storage._base import SQLBaseStore, db_to_json, make_in_list_sql_clause from synapse.storage.database import ( DatabasePool, LoggingDatabaseConnection, @@ -203,6 +203,7 @@ async def count_public_rooms( self, network_tuple: Optional[ThirdPartyInstanceID], ignore_non_federatable: bool, + search_filter: Optional[dict], ) -> int: """Counts the number of public rooms as tracked in the room_stats_current and room_stats_state table. @@ -210,11 +211,22 @@ async def count_public_rooms( Args: network_tuple ignore_non_federatable: If true filters out non-federatable rooms + search_filter """ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: query_args = [] + where_clause = "" + if search_filter and search_filter.get("room_type", None): + room_type = search_filter["room_type"] + clause, args = make_in_list_sql_clause( + self.database_engine, "room_type", room_type + ) + + where_clause = f" AND {clause}" + query_args += args + if network_tuple: if network_tuple.appservice_id: published_sql = """ @@ -249,6 +261,7 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: OR join_rules = '{JoinRules.KNOCK_RESTRICTED}' OR history_visibility = 'world_readable' ) + {where_clause} AND joined_members > 0 """ @@ -365,6 +378,15 @@ async def get_largest_public_rooms( search_term.lower(), ] + if search_filter and search_filter.get("room_type", None): + room_type = search_filter["room_type"] + clause, args = make_in_list_sql_clause( + self.database_engine, "room_type", room_type + ) + + where_clauses.append(clause) + query_args += args + where_clause = "" if where_clauses: where_clause = " AND " + " AND ".join(where_clauses) @@ -373,7 +395,7 @@ async def get_largest_public_rooms( sql = f""" SELECT room_id, name, topic, canonical_alias, joined_members, - avatar, history_visibility, guest_access, join_rules + avatar, history_visibility, guest_access, join_rules, room_type FROM ( {published_sql} ) published From 1aa4dbed52ca213b7295532fd1e152eb70fb645d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 18 Jun 2022 09:20:06 +0200 Subject: [PATCH 04/35] Add `_background_add_room_type_column()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 69 +++++++++++++++++++ .../72/01add_room_type_to_state_stats.sql | 3 + tests/storage/databases/main/test_room.py | 69 +++++++++++++++++++ 3 files changed, 141 insertions(+) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 74262b20df47..6e75a0db7a90 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1188,6 +1188,7 @@ class _BackgroundUpdates: POPULATE_ROOM_DEPTH_MIN_DEPTH2 = "populate_room_depth_min_depth2" REPLACE_ROOM_DEPTH_MIN_DEPTH = "replace_room_depth_min_depth" POPULATE_ROOMS_CREATOR_COLUMN = "populate_rooms_creator_column" + ADD_ROOM_TYPE_COLUMN = "add_room_type_column" _REPLACE_ROOM_DEPTH_SQL_COMMANDS = ( @@ -1222,6 +1223,11 @@ def __init__( self._background_add_rooms_room_version_column, ) + self.db_pool.updates.register_background_update_handler( + _BackgroundUpdates.ADD_ROOM_TYPE_COLUMN, + self._background_add_room_type_column, + ) + # BG updates to change the type of room_depth.min_depth self.db_pool.updates.register_background_update_handler( _BackgroundUpdates.POPULATE_ROOM_DEPTH_MIN_DEPTH2, @@ -1591,6 +1597,69 @@ def _background_populate_rooms_creator_column_txn( return batch_size + async def _background_add_room_type_column( + self, progress: JsonDict, batch_size: int + ) -> int: + """Background update to go and add room_type information to `room_stats_state` + table from `event_json` table. + """ + + last_room_id = progress.get("room_id", "") + + def _background_add_room_type_column_txn( + txn: LoggingTransaction, + ) -> bool: + sql = """ + SELECT state.room_id, json FROM event_json + INNER JOIN current_state_events AS state USING (event_id) + WHERE state.room_id > ? AND type = 'm.room.create' + ORDER BY state.room_id + LIMIT ? + """ + + txn.execute(sql, (last_room_id, batch_size)) + room_id_to_create_event_results = txn.fetchall() + + new_last_room_id = "" + for room_id, event_json in room_id_to_create_event_results: + event_dict = db_to_json(event_json) + + room_type = event_dict.get("content", {}).get( + EventContentFields.ROOM_TYPE, None + ) + if room_type: + self.db_pool.simple_update_txn( + txn, + table="room_stats_state", + keyvalues={"room_id": room_id}, + updatevalues={"room_type": room_type}, + ) + + new_last_room_id = room_id + + if new_last_room_id == "": + return True + + self.db_pool.updates._background_update_progress_txn( + txn, + _BackgroundUpdates.ADD_ROOM_TYPE_COLUMN, + {"room_id": new_last_room_id}, + ) + + return False + + end = await self.db_pool.runInteraction( + "_background_add_room_type_column", + _background_add_room_type_column_txn, + ) + + if end: + await self.db_pool.updates._end_background_update( + _BackgroundUpdates.ADD_ROOM_TYPE_COLUMN + ) + + return batch_size + class RoomStore(RoomBackgroundUpdateStore, RoomWorkerStore): def __init__( diff --git a/synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql b/synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql index f966f36e557f..d5e0765471d3 100644 --- a/synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql +++ b/synapse/storage/schema/main/delta/72/01add_room_type_to_state_stats.sql @@ -14,3 +14,6 @@ */ ALTER TABLE room_stats_state ADD room_type TEXT; + +INSERT INTO background_updates (update_name, progress_json) + VALUES ('add_room_type_column', '{}'); diff --git a/tests/storage/databases/main/test_room.py b/tests/storage/databases/main/test_room.py index 9abd0cb4468d..1e75df92c71a 100644 --- a/tests/storage/databases/main/test_room.py +++ b/tests/storage/databases/main/test_room.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json + +from synapse.api.constants import RoomTypes from synapse.rest import admin from synapse.rest.client import login, room from synapse.storage.databases.main.room import _BackgroundUpdates @@ -91,3 +94,69 @@ def test_background_populate_rooms_creator_column(self): ) ) self.assertEqual(room_creator_after, self.user_id) + + def test_background_add_room_type_column(self): + """Test that the background update to populate the rooms creator column + works properly. + """ + + # Create a room without a type + room_id = self._generate_room() + + # Get event_id of the m.room.create event + event_id = self.get_success( + self.store.db_pool.simple_select_one_onecol( + table="current_state_events", + keyvalues={ + "room_id": room_id, + "type": "m.room.create", + }, + retcol="event_id", + ) + ) + + # Fake a room creation event with a room type + event = { + "content": { + "creator": "@user:server.org", + "room_version": "9", + "type": RoomTypes.SPACE, + }, + "type": "m.room.create", + } + self.get_success( + self.store.db_pool.simple_update( + table="event_json", + keyvalues={"event_id": event_id}, + updatevalues={"json": json.dumps(event)}, + desc="test", + ) + ) + + # Insert and run the background update + self.get_success( + self.store.db_pool.simple_insert( + "background_updates", + { + "update_name": _BackgroundUpdates.ADD_ROOM_TYPE_COLUMN, + "progress_json": "{}", + }, + ) + ) + + # ... and tell the DataStore that it hasn't finished all updates yet + self.store.db_pool.updates._all_done = False + + # Now let's actually drive the updates to completion + self.wait_for_background_updates() + + # Make sure the background update filled in the room type + room_type_after = self.get_success( + self.store.db_pool.simple_select_one_onecol( + table="room_stats_state", + keyvalues={"room_id": room_id}, + retcol="room_type", + allow_none=True, + ) + ) + self.assertEqual(room_type_after, RoomTypes.SPACE) From 726acb23ae37ac150766bbd5629992d2587b1700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 18 Jun 2022 09:51:14 +0200 Subject: [PATCH 05/35] Handle rooms of default type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 29 +++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 6e75a0db7a90..d8465bfc07cc 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -199,6 +199,23 @@ async def get_public_room_ids(self) -> List[str]: desc="get_public_room_ids", ) + def _construct_room_type_where_clause( + self, room_types: List[Union[str, None]] + ) -> Tuple[str, List[str]]: + room_types_copy = room_types.copy() + + # We use None when we want get rooms without a type + isNullClause = "" + if (None in room_types_copy): + isNullClause = "OR room_type IS NULL" + room_types_copy.remove(None) + + listClause, args = make_in_list_sql_clause( + self.database_engine, "room_type", room_types_copy + ) + + return f"({listClause} {isNullClause})", args + async def count_public_rooms( self, network_tuple: Optional[ThirdPartyInstanceID], @@ -219,11 +236,7 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: where_clause = "" if search_filter and search_filter.get("room_type", None): - room_type = search_filter["room_type"] - clause, args = make_in_list_sql_clause( - self.database_engine, "room_type", room_type - ) - + clause, args = self._construct_room_type_where_clause(search_filter["room_type"]) where_clause = f" AND {clause}" query_args += args @@ -379,11 +392,7 @@ async def get_largest_public_rooms( ] if search_filter and search_filter.get("room_type", None): - room_type = search_filter["room_type"] - clause, args = make_in_list_sql_clause( - self.database_engine, "room_type", room_type - ) - + clause, args = self._construct_room_type_where_clause(search_filter["room_type"]) where_clauses.append(clause) query_args += args From 7225c5807b909d07f8c1655ea9087b56b4ad474c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 18 Jun 2022 10:39:50 +0200 Subject: [PATCH 06/35] Hide MSC3827 behind a config flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/config/experimental.py | 3 +++ synapse/rest/client/versions.py | 2 ++ synapse/storage/databases/main/room.py | 34 ++++++++++++++++++++++---- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 0a285dba3115..ee443cea0054 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -87,3 +87,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # MSC3715: dir param on /relations. self.msc3715_enabled: bool = experimental.get("msc3715_enabled", False) + + # MSC3827: Filtering of /publicRooms by room type + self.msc3827_enabled: bool = experimental.get("msc3827_enabled", False) diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index c1bd775fece4..f4f06563dddf 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -95,6 +95,8 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]: "org.matrix.msc3026.busy_presence": self.config.experimental.msc3026_enabled, # Supports receiving private read receipts as per MSC2285 "org.matrix.msc2285": self.config.experimental.msc2285_enabled, + # Supports filtering of /publicRooms by room type MSC3827 + "org.matrix.msc3827": self.config.experimental.msc3827_enabled, # Adds support for importing historical messages as per MSC2716 "org.matrix.msc2716": self.config.experimental.msc2716_enabled, # Adds support for jump to date endpoints (/timestamp_to_event) as per MSC3030 diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index d8465bfc07cc..f887dbc48138 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -206,7 +206,7 @@ def _construct_room_type_where_clause( # We use None when we want get rooms without a type isNullClause = "" - if (None in room_types_copy): + if None in room_types_copy: isNullClause = "OR room_type IS NULL" room_types_copy.remove(None) @@ -235,8 +235,20 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: query_args = [] where_clause = "" - if search_filter and search_filter.get("room_type", None): - clause, args = self._construct_room_type_where_clause(search_filter["room_type"]) + if ( + not self.config.experimental.msc3827_enabled + or not search_filter + or search_filter.get("room_type", None) is None + ): + where_clause = "AND room_type IS NULL" + elif ( + self.config.experimental.msc3827_enabled + and search_filter + and search_filter.get("room_type", None) + ): + clause, args = self._construct_room_type_where_clause( + search_filter["room_type"] + ) where_clause = f" AND {clause}" query_args += args @@ -391,8 +403,20 @@ async def get_largest_public_rooms( search_term.lower(), ] - if search_filter and search_filter.get("room_type", None): - clause, args = self._construct_room_type_where_clause(search_filter["room_type"]) + if ( + not self.config.experimental.msc3827_enabled + or not search_filter + or search_filter.get("room_type", None) is None + ): + where_clauses.append("room_type IS NULL") + elif ( + self.config.experimental.msc3827_enabled + and search_filter + and search_filter.get("room_type", None) + ): + clause, args = self._construct_room_type_where_clause( + search_filter["room_type"] + ) where_clauses.append(clause) query_args += args From f7d6402053ea8000679ab627f9c2e0013fcd1295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 18 Jun 2022 11:48:54 +0200 Subject: [PATCH 07/35] Use plural MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index f887dbc48138..88bc526f27e5 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -238,16 +238,16 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: if ( not self.config.experimental.msc3827_enabled or not search_filter - or search_filter.get("room_type", None) is None + or search_filter.get("room_types", None) is None ): where_clause = "AND room_type IS NULL" elif ( self.config.experimental.msc3827_enabled and search_filter - and search_filter.get("room_type", None) + and search_filter.get("room_types", None) ): clause, args = self._construct_room_type_where_clause( - search_filter["room_type"] + search_filter["room_types"] ) where_clause = f" AND {clause}" query_args += args @@ -406,16 +406,16 @@ async def get_largest_public_rooms( if ( not self.config.experimental.msc3827_enabled or not search_filter - or search_filter.get("room_type", None) is None + or search_filter.get("room_types", None) is None ): where_clauses.append("room_type IS NULL") elif ( self.config.experimental.msc3827_enabled and search_filter - and search_filter.get("room_type", None) + and search_filter.get("room_types", None) ): clause, args = self._construct_room_type_where_clause( - search_filter["room_type"] + search_filter["room_types"] ) where_clauses.append(clause) query_args += args From 8199e873dfd8cd36472885ac9078d6a9db8946af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 18 Jun 2022 12:07:23 +0200 Subject: [PATCH 08/35] Add `PublicRoomsFilterFields` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/api/constants.py | 5 +++++ synapse/handlers/room_list.py | 9 +++++++-- synapse/storage/databases/main/room.py | 27 +++++++++++++++++--------- tests/rest/client/test_rooms.py | 5 +++-- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index e1d31cabed23..4405ef9e6a93 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -259,3 +259,8 @@ class ReceiptTypes: READ: Final = "m.read" READ_PRIVATE: Final = "org.matrix.msc2285.read.private" FULLY_READ: Final = "m.fully_read" + + +class PublicRoomsFilterFields: + GENERIC_SEARCH_TERM: Final = "generic_search_term" + ROOM_TYPES: Final = "org.matrix.msc3827.room_types" diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py index 95f8c187f0b6..a11c869b69bf 100644 --- a/synapse/handlers/room_list.py +++ b/synapse/handlers/room_list.py @@ -25,6 +25,7 @@ GuestAccess, HistoryVisibility, JoinRules, + PublicRoomsFilterFields, ) from synapse.api.errors import ( Codes, @@ -511,8 +512,12 @@ def copy_and_replace(self, **kwds: Any) -> "RoomListNextBatch": def _matches_room_entry(room_entry: JsonDict, search_filter: dict) -> bool: - if search_filter and search_filter.get("generic_search_term", None): - generic_search_term = search_filter["generic_search_term"].upper() + if search_filter and search_filter.get( + PublicRoomsFilterFields.GENERIC_SEARCH_TERM, None + ): + generic_search_term = search_filter[ + PublicRoomsFilterFields.GENERIC_SEARCH_TERM + ].upper() if generic_search_term in room_entry.get("name", "").upper(): return True elif generic_search_term in room_entry.get("topic", "").upper(): diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 88bc526f27e5..8f769eb5d915 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -32,7 +32,12 @@ import attr -from synapse.api.constants import EventContentFields, EventTypes, JoinRules +from synapse.api.constants import ( + EventContentFields, + EventTypes, + JoinRules, + PublicRoomsFilterFields, +) from synapse.api.errors import StoreError from synapse.api.room_versions import RoomVersion, RoomVersions from synapse.config.homeserver import HomeServerConfig @@ -238,16 +243,16 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: if ( not self.config.experimental.msc3827_enabled or not search_filter - or search_filter.get("room_types", None) is None + or search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) is None ): where_clause = "AND room_type IS NULL" elif ( self.config.experimental.msc3827_enabled and search_filter - and search_filter.get("room_types", None) + and search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) ): clause, args = self._construct_room_type_where_clause( - search_filter["room_types"] + search_filter[PublicRoomsFilterFields.ROOM_TYPES] ) where_clause = f" AND {clause}" query_args += args @@ -385,8 +390,12 @@ async def get_largest_public_rooms( if ignore_non_federatable: where_clauses.append("is_federatable") - if search_filter and search_filter.get("generic_search_term", None): - search_term = "%" + search_filter["generic_search_term"] + "%" + if search_filter and search_filter.get( + PublicRoomsFilterFields.GENERIC_SEARCH_TERM, None + ): + search_term = ( + "%" + search_filter[PublicRoomsFilterFields.GENERIC_SEARCH_TERM] + "%" + ) where_clauses.append( """ @@ -406,16 +415,16 @@ async def get_largest_public_rooms( if ( not self.config.experimental.msc3827_enabled or not search_filter - or search_filter.get("room_types", None) is None + or search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) is None ): where_clauses.append("room_type IS NULL") elif ( self.config.experimental.msc3827_enabled and search_filter - and search_filter.get("room_types", None) + and search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) ): clause, args = self._construct_room_type_where_clause( - search_filter["room_types"] + search_filter[PublicRoomsFilterFields.ROOM_TYPES] ) where_clauses.append(clause) query_args += args diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 35c59ee9e034..829f1e36515b 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -33,6 +33,7 @@ EventContentFields, EventTypes, Membership, + PublicRoomsFilterFields, RelationTypes, ) from synapse.api.errors import Codes, HttpResponseException @@ -1882,7 +1883,7 @@ def test_simple(self) -> None: "Simple test for searching rooms over federation" self.federation_client.get_public_rooms.return_value = make_awaitable({}) # type: ignore[attr-defined] - search_filter = {"generic_search_term": "foobar"} + search_filter = {PublicRoomsFilterFields.GENERIC_SEARCH_TERM: "foobar"} channel = self.make_request( "POST", @@ -1911,7 +1912,7 @@ def test_fallback(self) -> None: make_awaitable({}), ) - search_filter = {"generic_search_term": "foobar"} + search_filter = {PublicRoomsFilterFields.GENERIC_SEARCH_TERM: "foobar"} channel = self.make_request( "POST", From cb3bf7e7e43840c315a9ce3a97e248b417c05fa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sun, 19 Jun 2022 15:36:12 +0200 Subject: [PATCH 09/35] Add `PublicRoomsRoomTypeFilterTestCase` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- tests/rest/client/test_rooms.py | 95 ++++++++++++++++++++++++++++++++- 1 file changed, 94 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 829f1e36515b..815a6a54d8b7 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -18,7 +18,7 @@ """Tests REST events for /rooms paths.""" import json -from typing import Any, Dict, Iterable, List, Optional, Union +from typing import Any, Dict, Iterable, List, Optional, Tuple, Union from unittest.mock import Mock, call from urllib import parse as urlparse @@ -35,6 +35,7 @@ Membership, PublicRoomsFilterFields, RelationTypes, + RoomTypes, ) from synapse.api.errors import Codes, HttpResponseException from synapse.handlers.pagination import PurgeStatus @@ -48,6 +49,7 @@ from tests import unittest from tests.http.server._base import make_request_with_cancellation_test from tests.test_utils import make_awaitable +from tests.unittest import override_config PATH_PREFIX = b"/_matrix/client/api/v1" @@ -1859,6 +1861,97 @@ def test_restricted_auth(self) -> None: self.assertEqual(channel.code, 200, channel.result) +class PublicRoomsRoomTypeFilterTestCase(unittest.HomeserverTestCase): + + servlets = [ + synapse.rest.admin.register_servlets_for_client_rest_resource, + room.register_servlets, + login.register_servlets, + ] + + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: + + config = self.default_config() + config["allow_public_rooms_without_auth"] = True + self.hs = self.setup_test_homeserver(config=config) + self.url = b"/_matrix/client/r0/publicRooms" + + return self.hs + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + user = self.register_user("alice", "pass") + self.token = self.login(user, "pass") + + # Create a room + self.helper.create_room_as( + user, + is_public=True, + extra_content={"visibility": "public"}, + tok=self.token, + ) + # Create a space + self.helper.create_room_as( + user, + is_public=True, + extra_content={ + "visibility": "public", + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE}, + }, + tok=self.token, + ) + + def make_public_rooms_request( + self, room_types: Union[List[Union[str, None]], None] + ) -> Tuple[List[Dict[str, Any]], int]: + channel = self.make_request( + "POST", + self.url, + {"filter": {PublicRoomsFilterFields.ROOM_TYPES: room_types}}, + self.token, + ) + body = json.loads(channel.result["body"].decode("utf8")) + chunk = body["chunk"] + count = body["total_room_count_estimate"] + + self.assertEqual(len(chunk), count) + + return chunk, count + + @override_config({"experimental_features": {"msc3827_enabled": False}}) + def test_returns_only_rooms_if_feature_disabled(self) -> None: + chunk, count = self.make_public_rooms_request(["m.space"]) + + self.assertEqual(count, 1) + self.assertEqual(chunk[0].get("room_type", None), None) + + @override_config({"experimental_features": {"msc3827_enabled": True}}) + def test_returns_only_rooms_if_no_filter(self) -> None: + chunk, count = self.make_public_rooms_request(None) + + self.assertEqual(count, 1) + self.assertEqual(chunk[0].get("room_type", None), None) + + @override_config({"experimental_features": {"msc3827_enabled": True}}) + def test_returns_only_rooms_based_on_filter(self) -> None: + chunk, count = self.make_public_rooms_request([None]) + + self.assertEqual(count, 1) + self.assertEqual(chunk[0].get("room_type", None), None) + + @override_config({"experimental_features": {"msc3827_enabled": True}}) + def test_returns_only_space_based_on_filter(self) -> None: + chunk, count = self.make_public_rooms_request(["m.space"]) + + self.assertEqual(count, 1) + self.assertEqual(chunk[0].get("room_type", None), "m.space") + + @override_config({"experimental_features": {"msc3827_enabled": True}}) + def test_returns_both_rooms_and_space_based_on_filter(self) -> None: + chunk, count = self.make_public_rooms_request(["m.space", None]) + + self.assertEqual(count, 2) + + class PublicRoomsTestRemoteSearchFallbackTestCase(unittest.HomeserverTestCase): """Test that we correctly fallback to local filtering if a remote server doesn't support search. From 7b5776608cc245f43f3b9a8bf6e9f97e31e6283f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sun, 19 Jun 2022 15:42:10 +0200 Subject: [PATCH 10/35] Use a prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/handlers/room_list.py | 2 +- tests/rest/client/test_rooms.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py index a11c869b69bf..aa79f56bd1de 100644 --- a/synapse/handlers/room_list.py +++ b/synapse/handlers/room_list.py @@ -182,7 +182,7 @@ def build_room_entry(room: JsonDict) -> JsonDict: == HistoryVisibility.WORLD_READABLE, "guest_can_join": room["guest_access"] == "can_join", "join_rule": room["join_rules"], - "room_type": room["room_type"], + "org.matrix.msc3827.room_type": room["room_type"], } # Filter out Nones – rather omit the field altogether diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 815a6a54d8b7..7b60f7c45730 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1922,28 +1922,28 @@ def test_returns_only_rooms_if_feature_disabled(self) -> None: chunk, count = self.make_public_rooms_request(["m.space"]) self.assertEqual(count, 1) - self.assertEqual(chunk[0].get("room_type", None), None) + self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), None) @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_only_rooms_if_no_filter(self) -> None: chunk, count = self.make_public_rooms_request(None) self.assertEqual(count, 1) - self.assertEqual(chunk[0].get("room_type", None), None) + self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), None) @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_only_rooms_based_on_filter(self) -> None: chunk, count = self.make_public_rooms_request([None]) self.assertEqual(count, 1) - self.assertEqual(chunk[0].get("room_type", None), None) + self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), None) @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_only_space_based_on_filter(self) -> None: chunk, count = self.make_public_rooms_request(["m.space"]) self.assertEqual(count, 1) - self.assertEqual(chunk[0].get("room_type", None), "m.space") + self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), "m.space") @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_both_rooms_and_space_based_on_filter(self) -> None: From 73cf558fdd40e549a5104e059dfd40222dccd545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:04:09 +0200 Subject: [PATCH 11/35] Link to the spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/api/constants.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 4405ef9e6a93..42ad4bec4f70 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -261,6 +261,7 @@ class ReceiptTypes: FULLY_READ: Final = "m.fully_read" +# As defined in https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3publicrooms class PublicRoomsFilterFields: GENERIC_SEARCH_TERM: Final = "generic_search_term" ROOM_TYPES: Final = "org.matrix.msc3827.room_types" From 04c390356696bb0e296b8d77bc56ddabc8f0a1ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:17:27 +0200 Subject: [PATCH 12/35] Filter out all `None`s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 8f769eb5d915..5293d0146428 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -213,7 +213,7 @@ def _construct_room_type_where_clause( isNullClause = "" if None in room_types_copy: isNullClause = "OR room_type IS NULL" - room_types_copy.remove(None) + room_types_copy = [value for value in room_types_copy if value is not None] listClause, args = make_in_list_sql_clause( self.database_engine, "room_type", room_types_copy From 825eb7752d5a7833370b03b9c01046a01ae8d23c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:24:38 +0200 Subject: [PATCH 13/35] Rename variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 5293d0146428..2d63f724e645 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -239,13 +239,13 @@ async def count_public_rooms( def _count_public_rooms_txn(txn: LoggingTransaction) -> int: query_args = [] - where_clause = "" + room_type_clause = "" if ( not self.config.experimental.msc3827_enabled or not search_filter or search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) is None ): - where_clause = "AND room_type IS NULL" + room_type_clause = "AND room_type IS NULL" elif ( self.config.experimental.msc3827_enabled and search_filter @@ -254,7 +254,7 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: clause, args = self._construct_room_type_where_clause( search_filter[PublicRoomsFilterFields.ROOM_TYPES] ) - where_clause = f" AND {clause}" + room_type_clause = f" AND {clause}" query_args += args if network_tuple: @@ -291,7 +291,7 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: OR join_rules = '{JoinRules.KNOCK_RESTRICTED}' OR history_visibility = 'world_readable' ) - {where_clause} + {room_type_clause} AND joined_members > 0 """ From ff9f188f0d7333eeaf4c164484503bb3ef45817c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:24:59 +0200 Subject: [PATCH 14/35] Use `None` instead of `""` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 2d63f724e645..0b19767ea450 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1662,7 +1662,7 @@ def _background_add_room_type_column_txn( txn.execute(sql, (last_room_id, batch_size)) room_id_to_create_event_results = txn.fetchall() - new_last_room_id = "" + new_last_room_id = None for room_id, event_json in room_id_to_create_event_results: event_dict = db_to_json(event_json) @@ -1679,7 +1679,7 @@ def _background_add_room_type_column_txn( new_last_room_id = room_id - if new_last_room_id == "": + if new_last_room_id is None: return True self.db_pool.updates._background_update_progress_txn( From bf39cb67cc3577fa459d16ea4d511e9ca544ddfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:32:06 +0200 Subject: [PATCH 15/35] Add `test_returns_both_rooms_and_spaces_if_array_is_empty` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- tests/rest/client/test_rooms.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 7b60f7c45730..9d08fde18e73 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1951,6 +1951,12 @@ def test_returns_both_rooms_and_space_based_on_filter(self) -> None: self.assertEqual(count, 2) + @override_config({"experimental_features": {"msc3827_enabled": True}}) + def test_returns_both_rooms_and_spaces_if_array_is_empty(self) -> None: + chunk, count = self.make_public_rooms_request([]) + + self.assertEqual(count, 2) + class PublicRoomsTestRemoteSearchFallbackTestCase(unittest.HomeserverTestCase): """Test that we correctly fallback to local filtering if a remote server From 0e8cd97abf8af3494d9a6f65dff042ffc2619a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:33:50 +0200 Subject: [PATCH 16/35] Fix typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/stats.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index e1a1602196a2..70cc2c1d8aa1 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -606,7 +606,6 @@ def _fetch_current_state_stats( room_state["is_federatable"] = ( event.content.get(EventContentFields.FEDERATE, True) is True ) - elif event.state["room_type"]: room_state["room_type"] = event.content.get( EventContentFields.ROOM_TYPE ) From 037fe5fd26c53122f36211e1d0dbe061cd37aa59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:35:19 +0200 Subject: [PATCH 17/35] Simplify code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- tests/rest/client/test_rooms.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 9d08fde18e73..2f54844293c2 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1909,9 +1909,8 @@ def make_public_rooms_request( {"filter": {PublicRoomsFilterFields.ROOM_TYPES: room_types}}, self.token, ) - body = json.loads(channel.result["body"].decode("utf8")) - chunk = body["chunk"] - count = body["total_room_count_estimate"] + chunk = channel.json_body["chunk"] + count = channel.json_body["total_room_count_estimate"] self.assertEqual(len(chunk), count) From 0b3e71ce835e676a7a5057152794352278631fc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:40:45 +0200 Subject: [PATCH 18/35] Check type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/handlers/stats.py | 6 +++--- synapse/storage/databases/main/stats.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/stats.py b/synapse/handlers/stats.py index 2c84761e9770..5c01482acfd7 100644 --- a/synapse/handlers/stats.py +++ b/synapse/handlers/stats.py @@ -271,9 +271,9 @@ async def _handle_deltas( room_state["is_federatable"] = ( event_content.get(EventContentFields.FEDERATE, True) is True ) - room_state["room_type"] = event_content.get( - EventContentFields.ROOM_TYPE - ) + room_type = event_content.get(EventContentFields.ROOM_TYPE) + if isinstance(room_type, str): + room_state["room_type"] = room_type elif typ == EventTypes.JoinRules: room_state["join_rules"] = event_content.get("join_rule") elif typ == EventTypes.RoomHistoryVisibility: diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index 70cc2c1d8aa1..e0ddd8c02ac3 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -606,9 +606,9 @@ def _fetch_current_state_stats( room_state["is_federatable"] = ( event.content.get(EventContentFields.FEDERATE, True) is True ) - room_state["room_type"] = event.content.get( - EventContentFields.ROOM_TYPE - ) + room_type = event_content.get(EventContentFields.ROOM_TYPE) + if isinstance(room_type, str): + room_state["room_type"] = room_type await self.update_room_state(room_id, room_state) From 7e50b785b6923782ad3b50e4e524d209cdef07ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 16:53:43 +0200 Subject: [PATCH 19/35] Don't change default behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 12 ++++-------- tests/rest/client/test_rooms.py | 5 ++--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 0b19767ea450..6bd8aaddabe0 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -241,14 +241,12 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: room_type_clause = "" if ( - not self.config.experimental.msc3827_enabled - or not search_filter + not search_filter or search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) is None ): room_type_clause = "AND room_type IS NULL" elif ( - self.config.experimental.msc3827_enabled - and search_filter + search_filter and search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) ): clause, args = self._construct_room_type_where_clause( @@ -413,14 +411,12 @@ async def get_largest_public_rooms( ] if ( - not self.config.experimental.msc3827_enabled - or not search_filter + not search_filter or search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) is None ): where_clauses.append("room_type IS NULL") elif ( - self.config.experimental.msc3827_enabled - and search_filter + search_filter and search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) ): clause, args = self._construct_room_type_where_clause( diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 2f54844293c2..4c4807ab0cdd 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1917,11 +1917,10 @@ def make_public_rooms_request( return chunk, count @override_config({"experimental_features": {"msc3827_enabled": False}}) - def test_returns_only_rooms_if_feature_disabled(self) -> None: + def test_returns_both_rooms_and_spaces_if_feature_disabled(self) -> None: chunk, count = self.make_public_rooms_request(["m.space"]) - self.assertEqual(count, 1) - self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), None) + self.assertEqual(count, 2) @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_only_rooms_if_no_filter(self) -> None: From da7ed94fdebf975cc8cfeedd36bdf9c2ba2769ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 17:32:45 +0200 Subject: [PATCH 20/35] Simplify code and revert las commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 65 +++++++++++--------------- tests/rest/client/test_rooms.py | 4 +- 2 files changed, 28 insertions(+), 41 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 6bd8aaddabe0..8969998ace6c 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -207,19 +207,24 @@ async def get_public_room_ids(self) -> List[str]: def _construct_room_type_where_clause( self, room_types: List[Union[str, None]] ) -> Tuple[str, List[str]]: - room_types_copy = room_types.copy() + if room_types is None or not self.config.experimental.msc3827_enabled: + return "room_type IS NULL", [] + elif room_types == []: + return None, [] + else: + room_types_copy = room_types.copy() - # We use None when we want get rooms without a type - isNullClause = "" - if None in room_types_copy: - isNullClause = "OR room_type IS NULL" - room_types_copy = [value for value in room_types_copy if value is not None] + # We use None when we want get rooms without a type + isNullClause = "" + if None in room_types_copy: + isNullClause = "OR room_type IS NULL" + room_types_copy = [value for value in room_types_copy if value is not None] - listClause, args = make_in_list_sql_clause( - self.database_engine, "room_type", room_types_copy - ) + listClause, args = make_in_list_sql_clause( + self.database_engine, "room_type", room_types_copy + ) - return f"({listClause} {isNullClause})", args + return f"({listClause} {isNullClause})", args async def count_public_rooms( self, @@ -239,21 +244,11 @@ async def count_public_rooms( def _count_public_rooms_txn(txn: LoggingTransaction) -> int: query_args = [] - room_type_clause = "" - if ( - not search_filter - or search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) is None - ): - room_type_clause = "AND room_type IS NULL" - elif ( - search_filter - and search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) - ): - clause, args = self._construct_room_type_where_clause( - search_filter[PublicRoomsFilterFields.ROOM_TYPES] - ) - room_type_clause = f" AND {clause}" - query_args += args + room_type_clause, args = self._construct_room_type_where_clause( + search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) if search_filter else None + ) + room_type_clause = f" AND {room_type_clause}" if room_type_clause else "" + query_args += args if network_tuple: if network_tuple.appservice_id: @@ -410,20 +405,12 @@ async def get_largest_public_rooms( search_term.lower(), ] - if ( - not search_filter - or search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) is None - ): - where_clauses.append("room_type IS NULL") - elif ( - search_filter - and search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) - ): - clause, args = self._construct_room_type_where_clause( - search_filter[PublicRoomsFilterFields.ROOM_TYPES] - ) - where_clauses.append(clause) - query_args += args + room_type_clause, args = self._construct_room_type_where_clause( + search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) if search_filter else None + ) + if room_type_clause: + where_clauses.append(room_type_clause) + query_args += args where_clause = "" if where_clauses: diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 4c4807ab0cdd..dbf94c3e980c 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1917,10 +1917,10 @@ def make_public_rooms_request( return chunk, count @override_config({"experimental_features": {"msc3827_enabled": False}}) - def test_returns_both_rooms_and_spaces_if_feature_disabled(self) -> None: + def test_returns_only_rooms_if_feature_disabled(self) -> None: chunk, count = self.make_public_rooms_request(["m.space"]) - self.assertEqual(count, 2) + self.assertEqual(count, 1) @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_only_rooms_if_no_filter(self) -> None: From 84af15651bb0be134063f6d887bd4da4ba7b17d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 17:35:41 +0200 Subject: [PATCH 21/35] Delint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 8969998ace6c..0805c82a1503 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -218,7 +218,9 @@ def _construct_room_type_where_clause( isNullClause = "" if None in room_types_copy: isNullClause = "OR room_type IS NULL" - room_types_copy = [value for value in room_types_copy if value is not None] + room_types_copy = [ + value for value in room_types_copy if value is not None + ] listClause, args = make_in_list_sql_clause( self.database_engine, "room_type", room_types_copy @@ -245,7 +247,9 @@ def _count_public_rooms_txn(txn: LoggingTransaction) -> int: query_args = [] room_type_clause, args = self._construct_room_type_where_clause( - search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) if search_filter else None + search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) + if search_filter + else None ) room_type_clause = f" AND {room_type_clause}" if room_type_clause else "" query_args += args @@ -406,7 +410,9 @@ async def get_largest_public_rooms( ] room_type_clause, args = self._construct_room_type_where_clause( - search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) if search_filter else None + search_filter.get(PublicRoomsFilterFields.ROOM_TYPES, None) + if search_filter + else None ) if room_type_clause: where_clauses.append(room_type_clause) From 6a0f0fa33dd06d6bb9cf4fe45d82c29bdf7807a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 17:43:47 +0200 Subject: [PATCH 22/35] More delint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 4 ++-- synapse/storage/databases/main/stats.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 0805c82a1503..0948276f2eb7 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -205,8 +205,8 @@ async def get_public_room_ids(self) -> List[str]: ) def _construct_room_type_where_clause( - self, room_types: List[Union[str, None]] - ) -> Tuple[str, List[str]]: + self, room_types: Union[List[Union[str, None]], None] + ) -> Tuple[Union[str, None], List[str]]: if room_types is None or not self.config.experimental.msc3827_enabled: return "room_type IS NULL", [] elif room_types == []: diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index e0ddd8c02ac3..f49ff9814971 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -606,7 +606,7 @@ def _fetch_current_state_stats( room_state["is_federatable"] = ( event.content.get(EventContentFields.FEDERATE, True) is True ) - room_type = event_content.get(EventContentFields.ROOM_TYPE) + room_type = event.content.get(EventContentFields.ROOM_TYPE) if isinstance(room_type, str): room_state["room_type"] = room_type From 217f63a706593f7e1db7d3582f143a90aa90704b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 17:51:51 +0200 Subject: [PATCH 23/35] Another delint attempt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/stats.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index f49ff9814971..abc377b58136 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -583,6 +583,7 @@ def _fetch_current_state_stats( "avatar": None, "canonical_alias": None, "is_federatable": True, + "room_type": None, } for event in state_event_map.values(): @@ -606,9 +607,7 @@ def _fetch_current_state_stats( room_state["is_federatable"] = ( event.content.get(EventContentFields.FEDERATE, True) is True ) - room_type = event.content.get(EventContentFields.ROOM_TYPE) - if isinstance(room_type, str): - room_state["room_type"] = room_type + room_state["room_type"] = event.content.get(EventContentFields.ROOM_TYPE) await self.update_room_state(room_id, room_state) From d440f0ab395611157b35268413619c95e881c398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 28 Jun 2022 17:54:29 +0200 Subject: [PATCH 24/35] Getting a little repetetive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/stats.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index abc377b58136..5619d1223066 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -607,7 +607,9 @@ def _fetch_current_state_stats( room_state["is_federatable"] = ( event.content.get(EventContentFields.FEDERATE, True) is True ) - room_state["room_type"] = event.content.get(EventContentFields.ROOM_TYPE) + room_state["room_type"] = event.content.get( + EventContentFields.ROOM_TYPE + ) await self.update_room_state(room_id, room_state) From 484024438c6fbb59ac5bb0e9f625e7e6df69ea55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 06:11:24 +0200 Subject: [PATCH 25/35] Improve comment Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- synapse/api/constants.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 42ad4bec4f70..a5c53fc9467f 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -261,7 +261,10 @@ class ReceiptTypes: FULLY_READ: Final = "m.fully_read" -# As defined in https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3publicrooms class PublicRoomsFilterFields: + """Fields in the search filter for `/publicRooms` that we understand. + + As defined in https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3publicrooms + """ GENERIC_SEARCH_TERM: Final = "generic_search_term" ROOM_TYPES: Final = "org.matrix.msc3827.room_types" From 41623037e1d783b477b2b6e88c564502fdb7911b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 06:18:48 +0200 Subject: [PATCH 26/35] Add docstring Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- synapse/handlers/room_list.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py index aa79f56bd1de..7730856e2d54 100644 --- a/synapse/handlers/room_list.py +++ b/synapse/handlers/room_list.py @@ -512,6 +512,15 @@ def copy_and_replace(self, **kwds: Any) -> "RoomListNextBatch": def _matches_room_entry(room_entry: JsonDict, search_filter: dict) -> bool: + """Determines whether the given search filter matches a room entry returned over + federation. + + Only used if the remote server does not support MSC2197 remote-filtered search, and + hence does not support MSC3827 filtering of `/publicRooms` by room type either. + + In this case, we cannot apply the `room_type` filter since no `room_type` field is + returned. + """ if search_filter and search_filter.get( PublicRoomsFilterFields.GENERIC_SEARCH_TERM, None ): From 4fd3e51718e7157254393abefcf1f678c77b46f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 06:20:26 +0200 Subject: [PATCH 27/35] Don't copy and follow conventions Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- synapse/storage/databases/main/room.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 0948276f2eb7..7748158c70c0 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -212,21 +212,17 @@ def _construct_room_type_where_clause( elif room_types == []: return None, [] else: - room_types_copy = room_types.copy() - # We use None when we want get rooms without a type - isNullClause = "" - if None in room_types_copy: - isNullClause = "OR room_type IS NULL" - room_types_copy = [ - value for value in room_types_copy if value is not None - ] - - listClause, args = make_in_list_sql_clause( - self.database_engine, "room_type", room_types_copy + is_null_clause = "" + if None in room_types: + is_null_clause = "OR room_type IS NULL" + room_types = [value for value in room_types if value is not None] + + list_clause, args = make_in_list_sql_clause( + self.database_engine, "room_type", room_types ) - return f"({listClause} {isNullClause})", args + return f"({list_clause} {is_null_clause})", args async def count_public_rooms( self, From a269756baca7b3e89bf2e181d41baf78e904023a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 06:21:03 +0200 Subject: [PATCH 28/35] Check for type Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- synapse/storage/databases/main/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 7748158c70c0..d7760e0945e7 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1654,7 +1654,7 @@ def _background_add_room_type_column_txn( room_type = event_dict.get("content", {}).get( EventContentFields.ROOM_TYPE, None ) - if room_type: + if isinstance(room_type, str): self.db_pool.simple_update_txn( txn, table="room_stats_state", From 45e9909ec28713a434c1c53c85072776c871d130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 06:21:52 +0200 Subject: [PATCH 29/35] Check for type Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- synapse/storage/databases/main/stats.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index 5619d1223066..a58a0b5cae8d 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -607,9 +607,9 @@ def _fetch_current_state_stats( room_state["is_federatable"] = ( event.content.get(EventContentFields.FEDERATE, True) is True ) - room_state["room_type"] = event.content.get( - EventContentFields.ROOM_TYPE - ) + room_type = event.content.get(EventContentFields.ROOM_TYPE) + if isinstance(room_type, str): + room_state["room_type"] = room_type await self.update_room_state(room_id, room_state) From 281ab62a65282bb50079d43c6a56f0a6deb9bc96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 06:23:31 +0200 Subject: [PATCH 30/35] Delint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/api/constants.py | 3 ++- synapse/handlers/room_list.py | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index a5c53fc9467f..265376411907 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -263,8 +263,9 @@ class ReceiptTypes: class PublicRoomsFilterFields: """Fields in the search filter for `/publicRooms` that we understand. - + As defined in https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3publicrooms """ + GENERIC_SEARCH_TERM: Final = "generic_search_term" ROOM_TYPES: Final = "org.matrix.msc3827.room_types" diff --git a/synapse/handlers/room_list.py b/synapse/handlers/room_list.py index 7730856e2d54..29868eb74311 100644 --- a/synapse/handlers/room_list.py +++ b/synapse/handlers/room_list.py @@ -514,10 +514,10 @@ def copy_and_replace(self, **kwds: Any) -> "RoomListNextBatch": def _matches_room_entry(room_entry: JsonDict, search_filter: dict) -> bool: """Determines whether the given search filter matches a room entry returned over federation. - + Only used if the remote server does not support MSC2197 remote-filtered search, and hence does not support MSC3827 filtering of `/publicRooms` by room type either. - + In this case, we cannot apply the `room_type` filter since no `room_type` field is returned. """ From 0d0dae322ab88cef25f0c2ff08c1b0358a90d2d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 16:52:50 +0200 Subject: [PATCH 31/35] Fix types (hopefully) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/stats.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index a58a0b5cae8d..ce97bc86ecb2 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -574,7 +574,7 @@ def _fetch_current_state_stats( state_event_map = await self.get_events(event_ids, get_prev_content=False) # type: ignore[attr-defined] - room_state = { + room_state: Dict[str, Union[None, bool, str]] = { "join_rules": None, "history_visibility": None, "encryption": None, From 7c342f5d0864a70fd4f64e01010b75a5137e8aa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 17:21:59 +0200 Subject: [PATCH 32/35] Don't change the defautl behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 4 +--- tests/rest/client/test_rooms.py | 17 +++-------------- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index d7760e0945e7..c7728f200370 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -207,9 +207,7 @@ async def get_public_room_ids(self) -> List[str]: def _construct_room_type_where_clause( self, room_types: Union[List[Union[str, None]], None] ) -> Tuple[Union[str, None], List[str]]: - if room_types is None or not self.config.experimental.msc3827_enabled: - return "room_type IS NULL", [] - elif room_types == []: + if not room_types: return None, [] else: # We use None when we want get rooms without a type diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index dbf94c3e980c..cb54fbbb903d 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1873,6 +1873,7 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() config["allow_public_rooms_without_auth"] = True + config["experimental_features"] = {"msc3827_enabled": True} self.hs = self.setup_test_homeserver(config=config) self.url = b"/_matrix/client/r0/publicRooms" @@ -1916,40 +1917,28 @@ def make_public_rooms_request( return chunk, count - @override_config({"experimental_features": {"msc3827_enabled": False}}) - def test_returns_only_rooms_if_feature_disabled(self) -> None: - chunk, count = self.make_public_rooms_request(["m.space"]) - - self.assertEqual(count, 1) - - @override_config({"experimental_features": {"msc3827_enabled": True}}) - def test_returns_only_rooms_if_no_filter(self) -> None: + def test_returns_both_rooms_and_spaces_if_no_filter(self) -> None: chunk, count = self.make_public_rooms_request(None) - self.assertEqual(count, 1) - self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), None) + self.assertEqual(count, 2) - @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_only_rooms_based_on_filter(self) -> None: chunk, count = self.make_public_rooms_request([None]) self.assertEqual(count, 1) self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), None) - @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_only_space_based_on_filter(self) -> None: chunk, count = self.make_public_rooms_request(["m.space"]) self.assertEqual(count, 1) self.assertEqual(chunk[0].get("org.matrix.msc3827.room_type", None), "m.space") - @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_both_rooms_and_space_based_on_filter(self) -> None: chunk, count = self.make_public_rooms_request(["m.space", None]) self.assertEqual(count, 2) - @override_config({"experimental_features": {"msc3827_enabled": True}}) def test_returns_both_rooms_and_spaces_if_array_is_empty(self) -> None: chunk, count = self.make_public_rooms_request([]) From 8329f407ec5b63d60a485dbbc4b8799d6e0914f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 17:23:35 +0200 Subject: [PATCH 33/35] Delint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/stats.py | 2 +- tests/rest/client/test_rooms.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index ce97bc86ecb2..b4c652acf34e 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -16,7 +16,7 @@ import logging from enum import Enum from itertools import chain -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, cast +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union, cast from typing_extensions import Counter diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index cb54fbbb903d..1ccd96a207ea 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -49,7 +49,6 @@ from tests import unittest from tests.http.server._base import make_request_with_cancellation_test from tests.test_utils import make_awaitable -from tests.unittest import override_config PATH_PREFIX = b"/_matrix/client/api/v1" From 0ad8a400b9c49afb75d22774d96a1741852121e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 18:03:27 +0200 Subject: [PATCH 34/35] Check if MSC3827 is enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- synapse/storage/databases/main/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index c7728f200370..d8026e3fac7f 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -207,7 +207,7 @@ async def get_public_room_ids(self) -> List[str]: def _construct_room_type_where_clause( self, room_types: Union[List[Union[str, None]], None] ) -> Tuple[Union[str, None], List[str]]: - if not room_types: + if not room_types or not self.config.experimental.msc3827_enabled: return None, [] else: # We use None when we want get rooms without a type From b5fec2fafa064dc71f09c4cf790cd897cdb4a038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Wed, 29 Jun 2022 18:26:20 +0200 Subject: [PATCH 35/35] Fix comment Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- tests/storage/databases/main/test_room.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/storage/databases/main/test_room.py b/tests/storage/databases/main/test_room.py index 1e75df92c71a..1edb61963026 100644 --- a/tests/storage/databases/main/test_room.py +++ b/tests/storage/databases/main/test_room.py @@ -96,8 +96,8 @@ def test_background_populate_rooms_creator_column(self): self.assertEqual(room_creator_after, self.user_id) def test_background_add_room_type_column(self): - """Test that the background update to populate the rooms creator column - works properly. + """Test that the background update to populate the `room_type` column in + `room_stats_state` works properly. """ # Create a room without a type