diff --git a/synapse/handlers/sliding_sync.py b/synapse/handlers/sliding_sync.py index b854d41f681..2429264b9b1 100644 --- a/synapse/handlers/sliding_sync.py +++ b/synapse/handlers/sliding_sync.py @@ -1171,7 +1171,7 @@ async def get_room_sync_data( # membership. Currently, we have to make all of these optional because # `invite`/`knock` rooms only have `stripped_state`. See # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1653045932 - timeline_events: Optional[List[EventBase]] = None + timeline_events: List[EventBase] = [] bundled_aggregations: Optional[Dict[str, BundledAggregations]] = None limited: Optional[bool] = None prev_batch_token: Optional[StreamToken] = None @@ -1303,7 +1303,7 @@ async def get_room_sync_data( # Figure out any stripped state events for invite/knocks. This allows the # potential joiner to identify the room. - stripped_state: Optional[List[JsonDict]] = None + stripped_state: List[JsonDict] = [] if room_membership_for_user_at_to_token.membership in ( Membership.INVITE, Membership.KNOCK, @@ -1361,8 +1361,18 @@ async def get_room_sync_data( room_membership_summary = await self.store.get_room_summary(room_id) # TODO: Reverse/rewind back to the `to_token` - # `heroes` are required if the room name is not set - hero_user_ids: Optional[List[str]] = None + # `heroes` are required if the room name is not set. + # + # Note: When you're the first one on your server to be invited to a new room + # over federation, we only have access to some stripped state in + # `event.unsigned.invite_room_state` which currently doesn't include `heroes`, + # see https://github.com/matrix-org/matrix-spec/issues/380. This means that + # clients won't be able to calculate the room name when necessary and just a + # pitfall we have to deal with until that spec issue is resolved. + hero_user_ids: List[str] = [] + # TODO: Should we also check for `EventTypes.CanonicalAlias` + # (`m.room.canonical_alias`) as a fallback for the room name? see + # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1671260153 if name_event_id is None: hero_user_ids = extract_heroes_from_room_summary( room_membership_summary, me=user.to_string() @@ -1378,15 +1388,7 @@ async def get_room_sync_data( # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1653045932 # # Calculate the `StateFilter` based on the `required_state` for the room - room_state: Optional[StateMap[EventBase]] = None - # Start off with the heroes of the room - required_state_filter = ( - StateFilter.from_types( - [(EventTypes.Member, hero_user_id) for hero_user_id in hero_user_ids] - ) - if hero_user_ids - else StateFilter.none() - ) + required_state_filter = StateFilter.none() if room_membership_for_user_at_to_token.membership not in ( Membership.INVITE, Membership.KNOCK, @@ -1454,22 +1456,25 @@ async def get_room_sync_data( else: required_state_types.append((state_type, state_key)) - required_state_filter = StateFilter.from_types( - chain(required_state_types, required_state_filter.to_types()) - ) + required_state_filter = StateFilter.from_types(required_state_types) # We need this base set of info for the response so let's just fetch it along # with the `required_state` for the room - meta_room_state = [(EventTypes.Name, ""), (EventTypes.RoomAvatar, "")] - state_filter = StateFilter( - types=StateFilter.from_types( - chain(meta_room_state, required_state_filter.to_types()) - ).types, - include_others=required_state_filter.include_others, - ) + meta_room_state = [(EventTypes.Name, ""), (EventTypes.RoomAvatar, "")] + [ + (EventTypes.Member, hero_user_id) for hero_user_id in hero_user_ids + ] + state_filter = StateFilter.all() + if required_state_filter != StateFilter.all(): + state_filter = StateFilter( + types=StateFilter.from_types( + chain(meta_room_state, required_state_filter.to_types()) + ).types, + include_others=required_state_filter.include_others, + ) # We can return all of the state that was requested if this was the first # time we've sent the room down this connection. + room_state: StateMap[EventBase] = {} if initial: room_state = await self.get_current_state_at( room_id=room_id, @@ -1482,7 +1487,7 @@ async def get_room_sync_data( # we can return updates instead of the full required state. raise NotImplementedError() - required_room_state: Optional[StateMap[EventBase]] = None + required_room_state: StateMap[EventBase] = {} if required_state_filter != StateFilter.none(): required_room_state = required_state_filter.filter_state(room_state) @@ -1490,6 +1495,9 @@ async def get_room_sync_data( room_name: Optional[str] = None room_avatar: Optional[str] = None if room_state is not None: + # TODO: Should we also check for `EventTypes.CanonicalAlias` + # (`m.room.canonical_alias`) as a fallback for the room name? see + # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1671260153 name_event = room_state.get((EventTypes.Name, "")) if name_event is not None: room_name = name_event.content.get("name") @@ -1498,6 +1506,19 @@ async def get_room_sync_data( if avatar_event is not None: room_avatar = avatar_event.content.get("url") + # Assemble heroes: extract the info from the state we just fetched + heroes: List[SlidingSyncResult.RoomResult.StrippedHero] = [] + for hero_user_id in hero_user_ids: + member_event = room_state.get((EventTypes.Member, hero_user_id)) + if member_event is not None: + heroes.append( + SlidingSyncResult.RoomResult.StrippedHero( + user_id=hero_user_id, + display_name=member_event.content.get("displayname"), + avatar_url=member_event.content.get("avatar_url"), + ) + ) + # Figure out the last bump event in the room last_bump_event_result = ( await self.store.get_last_event_pos_in_room_before_stream_ordering( @@ -1515,13 +1536,11 @@ async def get_room_sync_data( return SlidingSyncResult.RoomResult( name=room_name, avatar=room_avatar, - heroes=hero_user_ids, + heroes=heroes, # TODO: Dummy value is_dm=False, initial=initial, - required_state=( - list(required_room_state.values()) if required_room_state else None - ), + required_state=list(required_room_state.values()), timeline_events=timeline_events, bundled_aggregations=bundled_aggregations, stripped_state=stripped_state, diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index 13aed1dc85c..966b42ca1f4 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -995,8 +995,17 @@ async def encode_rooms( if room_result.avatar: serialized_rooms[room_id]["avatar"] = room_result.avatar - if room_result.heroes: - serialized_rooms[room_id]["heroes"] = room_result.heroes + if room_result.heroes is not None and len(room_result.heroes) > 0: + serialized_heroes = [] + for hero in room_result.heroes: + serialized_heroes.append( + { + "user_id": hero.user_id, + "displayname": hero.display_name, + "avatar_url": hero.avatar_url, + } + ) + serialized_rooms[room_id]["heroes"] = serialized_heroes # We should only include the `initial` key if it's `True` to save bandwidth. # The absense of this flag means `False`. @@ -1004,7 +1013,10 @@ async def encode_rooms( serialized_rooms[room_id]["initial"] = room_result.initial # This will be omitted for invite/knock rooms with `stripped_state` - if room_result.required_state is not None: + if ( + room_result.required_state is not None + and len(room_result.required_state) > 0 + ): serialized_required_state = ( await self.event_serializer.serialize_events( room_result.required_state, @@ -1015,7 +1027,10 @@ async def encode_rooms( serialized_rooms[room_id]["required_state"] = serialized_required_state # This will be omitted for invite/knock rooms with `stripped_state` - if room_result.timeline_events is not None: + if ( + room_result.timeline_events is not None + and len(room_result.timeline_events) > 0 + ): serialized_timeline = await self.event_serializer.serialize_events( room_result.timeline_events, time_now, @@ -1043,7 +1058,10 @@ async def encode_rooms( serialized_rooms[room_id]["is_dm"] = room_result.is_dm # Stripped state only applies to invite/knock rooms - if room_result.stripped_state is not None: + if ( + room_result.stripped_state is not None + and len(room_result.stripped_state) > 0 + ): # TODO: `knocked_state` but that isn't specced yet. # # TODO: Instead of adding `knocked_state`, it would be good to rename diff --git a/synapse/types/handlers/__init__.py b/synapse/types/handlers/__init__.py index 9da3b88cc2d..bee992fd2f9 100644 --- a/synapse/types/handlers/__init__.py +++ b/synapse/types/handlers/__init__.py @@ -154,9 +154,8 @@ class RoomResult: Attributes: name: Room name or calculated room name. avatar: Room avatar - heroes: List of user_ids which can be used to generate a room name if the - room does not have one. The `required_state` will include the membership - events for these users. + heroes: List of stripped membership events (containing `user_id` and optionally + `avatar_url` and `displayname`) for the users used to calculate the room name. is_dm: Flag to specify whether the room is a direct-message room (most likely between two people). initial: Flag which is set when this is the first time the server is sending this @@ -201,18 +200,24 @@ class RoomResult: flag set. (same as sync v2) """ + @attr.s(slots=True, frozen=True, auto_attribs=True) + class StrippedHero: + user_id: str + display_name: Optional[str] + avatar_url: Optional[str] + name: Optional[str] avatar: Optional[str] - heroes: Optional[List[str]] + heroes: Optional[List[StrippedHero]] is_dm: bool initial: bool - # Only optional because it won't be included for invite/knock rooms with `stripped_state` - required_state: Optional[List[EventBase]] - # Only optional because it won't be included for invite/knock rooms with `stripped_state` - timeline_events: Optional[List[EventBase]] + # Should be empty for invite/knock rooms with `stripped_state` + required_state: List[EventBase] + # Should be empty for invite/knock rooms with `stripped_state` + timeline_events: List[EventBase] bundled_aggregations: Optional[Dict[str, "BundledAggregations"]] # Optional because it's only relevant to invite/knock rooms - stripped_state: Optional[List[JsonDict]] + stripped_state: List[JsonDict] # Only optional because it won't be included for invite/knock rooms with `stripped_state` prev_batch: Optional[StreamToken] # Only optional because it won't be included for invite/knock rooms with `stripped_state`