Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add heroes and room summary fields to Sliding Sync /sync #17419

Merged
merged 22 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
9deb387
Initial work of adding name/avatar to rooms response
MadLittleMods Jul 8, 2024
b0bb37f
Add tests
MadLittleMods Jul 8, 2024
b6e36ef
Add changelog
MadLittleMods Jul 8, 2024
6ef39dd
Actually test that invited should see current state
MadLittleMods Jul 8, 2024
32c5409
Start of heroes in Sliding Sync
MadLittleMods Jul 8, 2024
9641ca7
Merge branch 'develop' into madlittlemods/sliding-sync-room-name-avatar
MadLittleMods Jul 8, 2024
9692c76
Merge branch 'madlittlemods/sliding-sync-room-name-avatar' into madli…
MadLittleMods Jul 8, 2024
ee9114c
Merge branch 'develop' into madlittlemods/sliding-sync-heroes
MadLittleMods Jul 9, 2024
f58d6fc
`heroes` is not `required_state` for now
MadLittleMods Jul 9, 2024
82bf80c
Add changelog
MadLittleMods Jul 9, 2024
10f8540
Add tests
MadLittleMods Jul 9, 2024
62925b6
Remove unncessary check
MadLittleMods Jul 9, 2024
b9f1eb1
Test `joined_count`/`invited_count`
MadLittleMods Jul 9, 2024
e50bf86
Test invite before/after ban
MadLittleMods Jul 9, 2024
2fb77b3
Sort `leave` before `knock`
MadLittleMods Jul 9, 2024
275da50
Explain the order
MadLittleMods Jul 9, 2024
6060408
Add test to make sure we only return 5 heroes
MadLittleMods Jul 9, 2024
91cefaa
Fix lint
MadLittleMods Jul 9, 2024
868dcdc
Revert `heroes` order changes
MadLittleMods Jul 11, 2024
a4753bf
Better comment on what we expect
MadLittleMods Jul 11, 2024
5583ac1
Merge branch 'develop' into madlittlemods/sliding-sync-heroes
MadLittleMods Jul 11, 2024
e2138b7
Have to use basic assertion for now
MadLittleMods Jul 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17418.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Populate `name`/`avatar` fields in experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint.
261 changes: 199 additions & 62 deletions synapse/handlers/sliding_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
#
#
import logging
from typing import TYPE_CHECKING, Any, Dict, Final, List, Optional, Set, Tuple
from itertools import chain
from typing import TYPE_CHECKING, Any, Dict, Final, List, Mapping, Optional, Set, Tuple

import attr
from immutabledict import immutabledict
Expand All @@ -33,7 +34,9 @@
from synapse.events import EventBase
from synapse.events.utils import strip_event
from synapse.handlers.relations import BundledAggregations
from synapse.storage.databases.main.roommember import extract_heroes_from_room_summary
from synapse.storage.databases.main.stream import CurrentStateDeltaMembership
from synapse.storage.roommember import MemberSummary
from synapse.types import (
JsonDict,
PersistedEventPosition,
Expand Down Expand Up @@ -464,6 +467,7 @@ async def current_sync_for_user(
membership_state_keys = room_sync_config.required_state_map.get(
EventTypes.Member
)
# Also see `StateFilter.must_await_full_state(...)` for comparison
lazy_loading = (
membership_state_keys is not None
and len(membership_state_keys) == 1
Expand Down Expand Up @@ -1039,6 +1043,103 @@ async def sort_rooms(
reverse=True,
)

async def get_current_state_ids_at(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted this logic out into a couple helper functions (get_current_state_ids_at(...)/get_current_state_at(...)) since I now need to do fetch state twice. Once to check if the room name is populated and once for fetching the full state that we need.

self,
room_id: str,
room_membership_for_user_at_to_token: _RoomMembershipForUser,
state_filter: StateFilter,
to_token: StreamToken,
) -> StateMap[str]:
"""
Get current state IDs for the user in the room according to their membership. This
will be the current state at the time of their LEAVE/BAN, otherwise will be the
current state <= to_token.

Args:
room_id: The room ID to fetch data for
room_membership_for_user_at_token: Membership information for the user
in the room at the time of `to_token`.
to_token: The point in the stream to sync up to.
"""

room_state_ids: StateMap[str]
# People shouldn't see past their leave/ban event
if room_membership_for_user_at_to_token.membership in (
Membership.LEAVE,
Membership.BAN,
):
# TODO: `get_state_ids_at(...)` doesn't take into account the "current state"
room_state_ids = await self.storage_controllers.state.get_state_ids_at(
room_id,
stream_position=to_token.copy_and_replace(
StreamKeyType.ROOM,
room_membership_for_user_at_to_token.event_pos.to_room_stream_token(),
),
state_filter=state_filter,
# Partially-stated rooms should have all state events except for
# remote membership events. Since we've already excluded
# partially-stated rooms unless `required_state` only has
# `["m.room.member", "$LAZY"]` for membership, we should be able to
# retrieve everything requested. When we're lazy-loading, if there
# are some remote senders in the timeline, we should also have their
# membership event because we had to auth that timeline event. Plus
# we don't want to block the whole sync waiting for this one room.
await_full_state=False,
)
# Otherwise, we can get the latest current state in the room
else:
room_state_ids = await self.storage_controllers.state.get_current_state_ids(
room_id,
state_filter,
# Partially-stated rooms should have all state events except for
# remote membership events. Since we've already excluded
# partially-stated rooms unless `required_state` only has
# `["m.room.member", "$LAZY"]` for membership, we should be able to
# retrieve everything requested. When we're lazy-loading, if there
# are some remote senders in the timeline, we should also have their
# membership event because we had to auth that timeline event. Plus
# we don't want to block the whole sync waiting for this one room.
await_full_state=False,
)
# TODO: Query `current_state_delta_stream` and reverse/rewind back to the `to_token`

return room_state_ids

async def get_current_state_at(
self,
room_id: str,
room_membership_for_user_at_to_token: _RoomMembershipForUser,
state_filter: StateFilter,
to_token: StreamToken,
) -> StateMap[EventBase]:
"""
Get current state for the user in the room according to their membership. This
will be the current state at the time of their LEAVE/BAN, otherwise will be the
current state <= to_token.

Args:
room_id: The room ID to fetch data for
room_membership_for_user_at_token: Membership information for the user
in the room at the time of `to_token`.
to_token: The point in the stream to sync up to.
"""
room_state_ids = await self.get_current_state_ids_at(
room_id=room_id,
room_membership_for_user_at_to_token=room_membership_for_user_at_to_token,
state_filter=state_filter,
to_token=to_token,
)

event_map = await self.store.get_events(list(room_state_ids.values()))

state_map = {}
for key, event_id in room_state_ids.items():
event = event_map.get(event_id)
if event:
state_map[key] = event

return state_map

async def get_room_sync_data(
self,
user: UserID,
Expand Down Expand Up @@ -1202,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: List[JsonDict] = []
stripped_state: Optional[List[JsonDict]] = None
if room_membership_for_user_at_to_token.membership in (
Membership.INVITE,
Membership.KNOCK,
Expand Down Expand Up @@ -1239,21 +1340,57 @@ async def get_room_sync_data(
# updates.
initial = True

# Fetch the required state for the room
# Check whether the room has a name set
name_state_ids = await self.get_current_state_ids_at(
room_id=room_id,
room_membership_for_user_at_to_token=room_membership_for_user_at_to_token,
state_filter=StateFilter.from_types([(EventTypes.Name, "")]),
to_token=to_token,
)
name_event_id = name_state_ids.get((EventTypes.Name, ""))

room_membership_summary: Mapping[str, MemberSummary]
empty_membership_summary = MemberSummary([], 0)
if room_membership_for_user_at_to_token.membership in (
Membership.LEAVE,
Membership.BAN,
):
# TODO: Figure out how to get the membership summary for left/banned rooms
room_membership_summary = {}
Comment on lines +1360 to +1361
Copy link
Contributor Author

@MadLittleMods MadLittleMods Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to address this TODO here.

  1. It's an edge case for left/ban rooms so it doesn't matter that much
  2. Not sure of an efficient way to do this for a long-forgotten left/banned room

else:
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
if name_event_id is None:
hero_user_ids = extract_heroes_from_room_summary(
room_membership_summary, me=user.to_string()
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only do this calculation when initial true? And can we not do this after we've already fetched the room name later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only do this calculation when initial true?

We need to be careful about including these when membership changes as well. Since we can't really tell whether membership has changed without looking, it seems easiest just to fetch it out each time when the room name isn't set and we can have the future "whether this room has been sent down this connection before" tracking mechanism handle whether they need to be sent down.

And can we not do this after we've already fetched the room name later?

We could do either way (or even the third option):

  1. Right now, we first check whether the room name state is set without pulling out the full event for it. We bundle the hero member event fetching along with all of the other full state events we fetch (see meta_room_state)
  2. We could do it the opposite way and wait for the full state to come back (which will include the meta events like m.room.name) and then have another full event fetch for the heroes that we need.
  3. Or we could fetch the heroes in the room every time

I chose the current way (option 1) because we only need a single lookup for full events.

It does suck that we have this dependent waterfall though so option 3 is also appealing and simple.


# Fetch the `required_state` for the room
#
# No `required_state` for invite/knock rooms (just `stripped_state`)
#
# FIXME: It would be nice to make the `rooms` response more uniform regardless
# of membership. Currently, we have to make this optional because
# `invite`/`knock` rooms only have `stripped_state`. See
# 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()
)
if room_membership_for_user_at_to_token.membership not in (
Membership.INVITE,
Membership.KNOCK,
):
# Calculate the `StateFilter` based on the `required_state` for the room
state_filter: Optional[StateFilter] = StateFilter.none()
# If we have a double wildcard ("*", "*") in the `required_state`, we need
# to fetch all state for the room
#
Expand All @@ -1276,7 +1413,7 @@ async def get_room_sync_data(
if StateValues.WILDCARD in room_sync_config.required_state_map.get(
StateValues.WILDCARD, set()
):
state_filter = StateFilter.all()
required_state_filter = StateFilter.all()
# TODO: `StateFilter` currently doesn't support wildcard event types. We're
# currently working around this by returning all state to the client but it
# would be nice to fetch less from the database and return just what the
Expand All @@ -1285,7 +1422,7 @@ async def get_room_sync_data(
room_sync_config.required_state_map.get(StateValues.WILDCARD)
is not None
):
state_filter = StateFilter.all()
required_state_filter = StateFilter.all()
else:
required_state_types: List[Tuple[str, Optional[str]]] = []
for (
Expand Down Expand Up @@ -1317,51 +1454,49 @@ async def get_room_sync_data(
else:
required_state_types.append((state_type, state_key))

state_filter = StateFilter.from_types(required_state_types)

# We can skip fetching state if we don't need any
if state_filter != StateFilter.none():
# We can return all of the state that was requested if we're doing an
# initial sync
if initial:
# People shouldn't see past their leave/ban event
if room_membership_for_user_at_to_token.membership in (
Membership.LEAVE,
Membership.BAN,
):
room_state = await self.storage_controllers.state.get_state_at(
room_id,
stream_position=to_token.copy_and_replace(
StreamKeyType.ROOM,
room_membership_for_user_at_to_token.event_pos.to_room_stream_token(),
),
state_filter=state_filter,
# Partially-stated rooms should have all state events except for
# the membership events and since we've already excluded
# partially-stated rooms unless `required_state` only has
# `["m.room.member", "$LAZY"]` for membership, we should be able
# to retrieve everything requested. Plus we don't want to block
# the whole sync waiting for this one room.
await_full_state=False,
)
# Otherwise, we can get the latest current state in the room
else:
room_state = await self.storage_controllers.state.get_current_state(
room_id,
state_filter,
# Partially-stated rooms should have all state events except for
# the membership events and since we've already excluded
# partially-stated rooms unless `required_state` only has
# `["m.room.member", "$LAZY"]` for membership, we should be able
# to retrieve everything requested. Plus we don't want to block
# the whole sync waiting for this one room.
await_full_state=False,
)
# TODO: Query `current_state_delta_stream` and reverse/rewind back to the `to_token`
else:
# TODO: Once we can figure out if we've sent a room down this connection before,
# we can return updates instead of the full required state.
raise NotImplementedError()
required_state_filter = StateFilter.from_types(
chain(required_state_types, required_state_filter.to_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,
)

# We can return all of the state that was requested if this was the first
# time we've sent the room down this connection.
if initial:
room_state = await self.get_current_state_at(
room_id=room_id,
room_membership_for_user_at_to_token=room_membership_for_user_at_to_token,
state_filter=state_filter,
to_token=to_token,
)
else:
# TODO: Once we can figure out if we've sent a room down this connection before,
# we can return updates instead of the full required state.
raise NotImplementedError()

required_room_state: Optional[StateMap[EventBase]] = None
if required_state_filter != StateFilter.none():
required_room_state = required_state_filter.filter_state(room_state)

# Find the room name and avatar from the state
room_name: Optional[str] = None
room_avatar: Optional[str] = None
if room_state is not None:
name_event = room_state.get((EventTypes.Name, ""))
if name_event is not None:
room_name = name_event.content.get("name")

avatar_event = room_state.get((EventTypes.RoomAvatar, ""))
if avatar_event is not None:
room_avatar = avatar_event.content.get("url")

# Figure out the last bump event in the room
last_bump_event_result = (
Expand All @@ -1378,26 +1513,28 @@ async def get_room_sync_data(
bump_stamp = bump_event_pos.stream

return SlidingSyncResult.RoomResult(
# TODO: Dummy value
name=None,
# TODO: Dummy value
avatar=None,
# TODO: Dummy value
heroes=None,
name=room_name,
avatar=room_avatar,
heroes=hero_user_ids,
# TODO: Dummy value
is_dm=False,
initial=initial,
required_state=list(room_state.values()) if room_state else None,
required_state=(
list(required_room_state.values()) if required_room_state else None
),
timeline_events=timeline_events,
bundled_aggregations=bundled_aggregations,
stripped_state=stripped_state,
prev_batch=prev_batch_token,
limited=limited,
num_live=num_live,
bump_stamp=bump_stamp,
# TODO: Dummy values
joined_count=0,
invited_count=0,
joined_count=room_membership_summary.get(
Membership.JOIN, empty_membership_summary
).count,
invited_count=room_membership_summary.get(
Membership.INVITE, empty_membership_summary
).count,
# TODO: These are just dummy values. We could potentially just remove these
# since notifications can only really be done correctly on the client anyway
# (encrypted rooms).
Expand Down
Loading
Loading