MSC4171 Omit service members from room summary

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).

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"

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)

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

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(

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, ""))

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(

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 %s
                 ORDER BY
                     CASE membership WHEN ? THEN 1 WHEN ? THEN 2 WHEN ? THEN 3 ELSE 4 END ASC,
                     event_stream_ordering ASC
                 LIMIT ?
-            """
+            """ % (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.
 

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,
     )

tests/storage/test_roommember.py

@@ -628,14 +628,105 @@ 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
         self.assertListEqual(
             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