diff --git a/changelog.d/17866.feature b/changelog.d/17866.feature new file mode 100644 index 00000000000..b7ca02978b9 --- /dev/null +++ b/changelog.d/17866.feature @@ -0,0 +1 @@ +Add experimental support for filtering out "service members" from room summary responses, as described in [MSC4171](https://github.com/matrix-org/matrix-spec-proposals/pull/4171). \ No newline at end of file diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 8db302b3d8b..73ee9eea6ed 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -135,6 +135,8 @@ class EventTypes: PollStart: Final = "m.poll.start" + MSC4171FunctionalMembers: Final = "io.element.functional_members" + class ToDeviceEventTypes: RoomKeyRequest: Final = "m.room_key_request" diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index fd14db0d024..4edbe4de2a9 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -448,5 +448,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # MSC4151: Report room API (Client-Server API) self.msc4151_enabled: bool = experimental.get("msc4151_enabled", False) + # MSC4171: Service members + self.msc4171_enabled: bool = experimental.get("msc4171_enabled", False) + # MSC4210: Remove legacy mentions self.msc4210_enabled: bool = experimental.get("msc4210_enabled", False) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 1932fa82a4a..aba01f29a34 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -732,6 +732,8 @@ async def on_send_join_request( state_event_ids: Collection[str] servers_in_room: Optional[Collection[str]] if caller_supports_partial_state: + # NOTE: We do not exclude service members from the federated + # room summary. summary = await self.store.get_room_summary(room_id) state_event_ids = _get_event_ids_for_partial_state_join( event, prev_state_ids, summary diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index a1a6728fb93..21d7feee7bb 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -90,6 +90,8 @@ def __init__(self, hs: "HomeServer"): self.event_sources = hs.get_event_sources() self.relations_handler = hs.get_relations_handler() self.rooms_to_exclude_globally = hs.config.server.rooms_to_exclude_from_sync + self.should_exclude_service_members = hs.config.experimental.msc4171_enabled + self.is_mine_id = hs.is_mine_id self.connection_store = SlidingSyncConnectionStore(self.store) @@ -829,7 +831,11 @@ async def get_room_sync_data( # For invite/knock rooms we don't include the information. room_membership_summary = {} else: - room_membership_summary = await self.store.get_room_summary(room_id) + room_membership_summary = await self.store.get_room_summary( + room_id, + self.should_exclude_service_members + and sync_config.exclude_service_members_from_heroes, + ) # TODO: Reverse/rewind back to the `to_token` hero_user_ids = extract_heroes_from_room_summary( diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index f4ea90fbd78..5f6c2e79b3f 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -143,6 +143,7 @@ class SyncConfig: filter_collection: FilterCollection is_guest: bool device_id: Optional[str] + exclude_service_members_from_heroes: bool @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -343,6 +344,7 @@ def __init__(self, hs: "HomeServer"): self._task_scheduler = hs.get_task_scheduler() self.should_calculate_push_rules = hs.config.push.enable_push + self.should_exclude_service_members = hs.config.experimental.msc4171_enabled # TODO: flush cache entries on subsequent sync request. # Once we get the next /sync request (ie, one with the same access token @@ -1040,7 +1042,11 @@ async def compute_summary( ) # this is heavily cached, thus: fast. - details = await self.store.get_room_summary(room_id) + details = await self.store.get_room_summary( + room_id, + self.should_exclude_service_members + and sync_config.exclude_service_members_from_heroes, + ) name_id = state_ids.get((EventTypes.Name, "")) canonical_alias_id = state_ids.get((EventTypes.CanonicalAlias, "")) diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 122708e933c..e4c53722606 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -151,16 +151,20 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: ) filter_id = parse_string(request, "filter") full_state = parse_boolean(request, "full_state", default=False) + exclude_service_members_from_heroes = parse_boolean( + request, "msc4171_exclude_service_members", default=False + ) logger.debug( "/sync: user=%r, timeout=%r, since=%r, " - "set_presence=%r, filter_id=%r, device_id=%r", + "set_presence=%r, filter_id=%r, device_id=%r, exclude_service_members=%r", user, timeout, since, set_presence, filter_id, device_id, + exclude_service_members_from_heroes, ) # Stream position of the last ignored users account data event for this user, @@ -220,6 +224,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: filter_collection=filter_collection, is_guest=requester.is_guest, device_id=device_id, + exclude_service_members_from_heroes=exclude_service_members_from_heroes, ) since_token = None @@ -682,12 +687,16 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: timeout = parse_integer(request, "timeout", default=0) since = parse_string(request, "since") + exclude_service_members_from_heroes = parse_boolean( + request, "msc4171_exclude_service_members", default=False + ) sync_config = SyncConfig( user=user, filter_collection=self.only_member_events_filter_collection, is_guest=requester.is_guest, device_id=device_id, + exclude_service_members_from_heroes=exclude_service_members_from_heroes, ) since_token = None @@ -886,6 +895,10 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: # Position in the stream from_token_string = parse_string(request, "pos") + exclude_service_members_from_heroes = parse_boolean( + request, "msc4171_exclude_service_members", default=False + ) + from_token = None if from_token_string is not None: from_token = await SlidingSyncStreamToken.from_string( @@ -935,6 +948,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: lists=body.lists, room_subscriptions=body.room_subscriptions, extensions=body.extensions, + exclude_service_members_from_heroes=exclude_service_members_from_heroes, ) sliding_sync_results = await self.sliding_sync_handler.wait_for_sync_for_user( diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 4249cf77e55..f8c112b825f 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -285,7 +285,9 @@ def _get_users_in_room_with_profiles( ) @cached(max_entries=100000) # type: ignore[synapse-@cached-mutable] - async def get_room_summary(self, room_id: str) -> Mapping[str, MemberSummary]: + async def get_room_summary( + self, room_id: str, exclude_service_users: bool = False + ) -> Mapping[str, MemberSummary]: """ Get the details of a room roughly suitable for use by the room summary extension to /sync. Useful when lazy loading room members. @@ -301,12 +303,13 @@ async def get_room_summary(self, room_id: str) -> Mapping[str, MemberSummary]: Args: room_id: The room ID to query + exclude_service_users: Should MSC4171 be used to exclude service members Returns: dict of membership states, pointing to a MemberSummary named tuple. """ def _get_room_summary_txn( - txn: LoggingTransaction, + txn: LoggingTransaction, exclude_members: List[str] ) -> Dict[str, MemberSummary]: # first get counts. # We do this all in one transaction to keep the cache small. @@ -318,6 +321,10 @@ def _get_room_summary_txn( for membership, count in counts.items(): res.setdefault(membership, MemberSummary([], count)) + exclude_users_clause, args = make_in_list_sql_clause( + self.database_engine, "state_key", exclude_members, negative=True + ) + # Order by membership (joins -> invites -> leave (former insiders) -> # everything else (outsiders like bans/knocks), then by `stream_ordering` so # the first members in the room show up first and to make the sort stable @@ -330,16 +337,18 @@ def _get_room_summary_txn( FROM current_state_events WHERE type = 'm.room.member' AND room_id = ? AND membership IS NOT NULL + AND {} ORDER BY CASE membership WHEN ? THEN 1 WHEN ? THEN 2 WHEN ? THEN 3 ELSE 4 END ASC, event_stream_ordering ASC LIMIT ? - """ + """.format(exclude_users_clause) txn.execute( sql, ( room_id, + *args, # Sort order Membership.JOIN, Membership.INVITE, @@ -357,8 +366,33 @@ def _get_room_summary_txn( return res + exclude_members = [] + if exclude_service_users: + functional_members_event_id = await self.db_pool.simple_select_one_onecol( + table="current_state_events", + keyvalues={ + "room_id": room_id, + "type": EventTypes.MSC4171FunctionalMembers, + "state_key": "", + }, + retcol="event_id", + allow_none=True, + ) + if functional_members_event_id: + functional_members_event = await self.get_event( + functional_members_event_id + ) + functional_members_data = functional_members_event.content.get( + "service_members" + ) + # ONLY use this value if this looks like a valid list of strings. Otherwise, ignore. + if isinstance(functional_members_data, list) and all( + isinstance(item, str) for item in functional_members_data + ): + exclude_members = functional_members_data + return await self.db_pool.runInteraction( - "get_room_summary", _get_room_summary_txn + "get_room_summary", _get_room_summary_txn, exclude_members ) @cached() @@ -1754,7 +1788,8 @@ def __init__( def extract_heroes_from_room_summary( - details: Mapping[str, MemberSummary], me: str + details: Mapping[str, MemberSummary], + me: str, ) -> List[str]: """Determine the users that represent a room, from the perspective of the `me` user. diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index aae60fddeab..7d27e647156 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -68,6 +68,7 @@ class SlidingSyncConfig(SlidingSyncBody): user: UserID requester: Requester + exclude_service_members_from_heroes: bool # Pydantic config class Config: diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py index d7bbc680373..38b89e92459 100644 --- a/tests/handlers/test_sync.py +++ b/tests/handlers/test_sync.py @@ -1079,4 +1079,5 @@ def generate_sync_config( filter_collection=filter_collection, is_guest=False, device_id=device_id, + exclude_service_members_from_heroes=False, ) diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 330fea0e624..c8af9038b36 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -628,7 +628,7 @@ def test_extract_heroes_from_room_summary_first_five_joins(self) -> None: room_membership_summary = self.get_success(self.store.get_room_summary(room_id)) hero_user_ids = extract_heroes_from_room_summary( - room_membership_summary, me="@fakuser" + room_membership_summary, me="@fakeuser" ) # First 5 users to join the room @@ -636,6 +636,97 @@ def test_extract_heroes_from_room_summary_first_five_joins(self) -> None: hero_user_ids, [user1_id, user2_id, user3_id, user4_id, user5_id] ) + def test_extract_heroes_from_room_summary_exclude_service_members(self) -> None: + """ + Test that `extract_heroes_from_room_summary(...)` returns the first 5 joins who are + not mentioned in the functional members state event. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + user3_id = self.register_user("user3", "pass") + user3_tok = self.login(user3_id, "pass") + user4_id = self.register_user("user4", "pass") + user4_tok = self.login(user4_id, "pass") + user5_id = self.register_user("user5", "pass") + user5_tok = self.login(user5_id, "pass") + user6_id = self.register_user("user6", "pass") + user6_tok = self.login(user6_id, "pass") + user7_id = self.register_user("user7", "pass") + user7_tok = self.login(user7_id, "pass") + + # Setup the room (user1 is the creator and is joined to the room) + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Exclude some users + self.helper.send_state( + room_id, + event_type=EventTypes.MSC4171FunctionalMembers, + body={"service_members": [user2_id, user3_id]}, + tok=user1_tok, + ) + + # User2 -> User7 joins + self.helper.join(room_id, user2_id, tok=user2_tok) + self.helper.join(room_id, user3_id, tok=user3_tok) + self.helper.join(room_id, user4_id, tok=user4_tok) + self.helper.join(room_id, user5_id, tok=user5_tok) + self.helper.join(room_id, user6_id, tok=user6_tok) + self.helper.join(room_id, user7_id, tok=user7_tok) + + room_membership_summary = self.get_success( + self.store.get_room_summary(room_id, True) + ) + + hero_user_ids = extract_heroes_from_room_summary( + room_membership_summary, me="@fakeuser" + ) + + # First 5 users to join the room, excluding service members. + self.assertListEqual( + hero_user_ids, [user1_id, user4_id, user5_id, user6_id, user7_id] + ) + + def test_extract_heroes_from_room_summary_exclude_service_members_with_empty_heroes( + self, + ) -> None: + """ + Test that `extract_heroes_from_room_summary(...)` will return an + empty set of heroes if all users have been excluded. + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + user3_id = self.register_user("user3", "pass") + user3_tok = self.login(user3_id, "pass") + + # Setup the room (user1 is the creator and is joined to the room) + room_id = self.helper.create_room_as(user1_id, tok=user1_tok) + + # Exclude all users (except the creator, who is excluded from the results anyway) + self.helper.send_state( + room_id, + event_type=EventTypes.MSC4171FunctionalMembers, + body={"service_members": [user2_id, user3_id]}, + tok=user1_tok, + ) + + self.helper.join(room_id, user2_id, tok=user2_tok) + self.helper.join(room_id, user3_id, tok=user3_tok) + + room_membership_summary = self.get_success( + self.store.get_room_summary(room_id, True) + ) + + hero_user_ids = extract_heroes_from_room_summary( + room_membership_summary, me=user1_id + ) + + # First 5 users to join the room, excluding service members. + self.assertListEqual(hero_user_ids, []) + def test_extract_heroes_from_room_summary_membership_order(self) -> None: """ Test that `extract_heroes_from_room_summary(...)` prefers joins/invites over