-
Notifications
You must be signed in to change notification settings - Fork 349
Add heroes
and room summary fields to Sliding Sync /sync
#17419
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
Changes from 14 commits
9deb387
b0bb37f
b6e36ef
6ef39dd
32c5409
9641ca7
9692c76
ee9114c
f58d6fc
82bf80c
10f8540
62925b6
b9f1eb1
e50bf86
2fb77b3
275da50
6060408
91cefaa
868dcdc
a4753bf
5583ac1
e2138b7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Populate `heroes` and room summary fields (`joined_count`, `invited_count`) in experimental [MSC3575](https://github.com/matrix-org/matrix-spec-proposals/pull/3575) Sliding Sync `/sync` endpoint. |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -279,8 +279,19 @@ 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]: | ||
"""Get the details of a room roughly suitable for use by the room | ||
""" | ||
Get the details of a room roughly suitable for use by the room | ||
summary extension to /sync. Useful when lazy loading room members. | ||
|
||
Returns the total count of members in the room by membership type, and a | ||
truncated list of members (the heroes). This will be the first 6 members of the | ||
room: | ||
- We want 5 heroes plus 1, in case one of them is the | ||
calling user. | ||
- They are ordered by `stream_ordering`, which are joined or | ||
invited. When no joined or invited members are available, this also includes | ||
banned and left users. | ||
|
||
Args: | ||
room_id: The room ID to query | ||
Returns: | ||
|
@@ -308,23 +319,27 @@ def _get_room_summary_txn( | |
for count, membership in txn: | ||
res.setdefault(membership, MemberSummary([], count)) | ||
|
||
# we order by membership and then fairly arbitrarily by event_id so | ||
# heroes are consistent | ||
# Note, rejected events will have a null membership field, so | ||
# we we manually filter them out. | ||
# Order by membership (joins -> invites/knocks -> everything else), then by | ||
# `stream_ordering` so the first members in the room show up first and to | ||
# make the sort stable (consistent heroes). | ||
# | ||
# Note: rejected events will have a null membership field, so we we manually | ||
# filter them out. | ||
sql = """ | ||
SELECT state_key, membership, event_id | ||
FROM current_state_events | ||
WHERE type = 'm.room.member' AND room_id = ? | ||
AND membership IS NOT NULL | ||
ORDER BY | ||
CASE membership WHEN ? THEN 1 WHEN ? THEN 2 ELSE 3 END ASC, | ||
event_id ASC | ||
CASE membership WHEN ? THEN 1 WHEN ? THEN 2 WHEN ? THEN 2 ELSE 3 END ASC, | ||
event_stream_ordering ASC | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By changing this now, it will probably cause peoples rooms names to calculate to something different across the board (this function is also used by Sync v2). Are we ok with that? Potentially disruptive if people are really used to their old "flawed" name. It's already a Related to matrix-org/matrix-spec#1334 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to change the order now? I think we only need to have a "stable" ordering? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spec says this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, interesting. Can we keep as is for now and open a separate PR / issue to consider this? I'm failing to think through whether this is going to cause some UX issues or not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverted in this PR and split out to #17435 ⏩ |
||
LIMIT ? | ||
""" | ||
|
||
# 6 is 5 (number of heroes) plus 1, in case one of them is the calling user. | ||
txn.execute(sql, (room_id, Membership.JOIN, Membership.INVITE, 6)) | ||
txn.execute( | ||
sql, (room_id, Membership.JOIN, Membership.INVITE, Membership.KNOCK, 6) | ||
) | ||
for user_id, membership, event_id in txn: | ||
summary = res[membership] | ||
# we will always have a summary for this membership type at this | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -200,9 +200,6 @@ class SlidingSyncList(CommonRoomParameters): | |
} | ||
|
||
timeline_limit: The maximum number of timeline events to return per response. | ||
include_heroes: Return a stripped variant of membership events (containing | ||
`user_id` and optionally `avatar_url` and `displayname`) for the users used | ||
to calculate the room name. | ||
filters: Filters to apply to the list before sorting. | ||
""" | ||
|
||
|
@@ -270,7 +267,6 @@ class Filters(RequestBodyModel): | |
else: | ||
ranges: Optional[List[Tuple[conint(ge=0, strict=True), conint(ge=0, strict=True)]]] = None # type: ignore[valid-type] | ||
slow_get_all_rooms: Optional[StrictBool] = False | ||
include_heroes: Optional[StrictBool] = False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
filters: Optional[Filters] = None | ||
|
||
class RoomSubscription(CommonRoomParameters): | ||
|
Uh oh!
There was an error while loading. Please reload this page.