Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

A third batch of refactors to the sync handler #11532

Closed
wants to merge 9 commits into from
Closed

A third batch of refactors to the sync handler #11532

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7aee344
Pull out `_classify_rooms_by_membership_changes`
Dec 6, 2021
bca64f9
Remove unused branch
Dec 7, 2021
7662ba4
Signposting in `_classify_rooms_by_membership`
Dec 7, 2021
6ff95ee
Pull out `_fetch_membership_event_at`
Dec 7, 2021
0d0783c
Always call `old_mem_ev`---exactly once.
Dec 7, 2021
a5deabb
Remove check on `events[-1].membership`
Dec 7, 2021
50fee89
Small style rewrite to emphasise exclusive cases
Dec 7, 2021
8bd3565
Changelog
Dec 7, 2021
824ae4e
docstring formatting
Dec 8, 2021
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
58 changes: 32 additions & 26 deletions synapse/handlers/sync.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1845,7 +1845,6 @@ async def _classify_rooms_by_membership_changes(
# We want to figure out if we joined the room at some point since
# the last sync (even if we have since left). This is to make sure
# we do send down the room, and with full state, where necessary.
old_state_ids = None
if room_id in sync_result_builder.joined_room_ids and non_joins:
# Always include if the user (re)joined the room, especially
# important so that device list changes are calculated correctly.
Expand All @@ -1857,27 +1856,20 @@ async def _classify_rooms_by_membership_changes(
continue

if room_id in sync_result_builder.joined_room_ids or has_join:
old_state_ids = await self.get_state_at(room_id, since_token)
old_mem_ev_id = old_state_ids.get((EventTypes.Member, user_id), None)
old_mem_ev = None
if old_mem_ev_id:
old_mem_ev = await self.store.get_event(
old_mem_ev_id, allow_none=True
)
old_mem_ev = await self._fetch_membership_event_at(
room_id, user_id, since_token
)

# debug for #4422
if has_join:
prev_membership = None
if old_mem_ev:
prev_membership = old_mem_ev.membership
if has_join and old_mem_ev is not None:
Copy link
Member

Choose a reason for hiding this comment

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

this is a change in behaviour, where we will no longer log for joins where there was no previous membership event (ie, first-time joins). But #4422 got fixed a long time ago - I suggest you rip this whole thing out rather than trying to maintain it.

issue4422_logger.debug(
"Previous membership for room %s with join: %s (event %s)",
room_id,
prev_membership,
old_mem_ev_id,
old_mem_ev.membership,
old_mem_ev.event_id,
)

if not old_mem_ev or old_mem_ev.membership != Membership.JOIN:
if old_mem_ev is None or old_mem_ev.membership != Membership.JOIN:
newly_joined_rooms.append(room_id)

# If user is in the room then we don't need to do the invite/leave checks
Expand All @@ -1891,17 +1883,13 @@ async def _classify_rooms_by_membership_changes(
if has_join:
newly_left_rooms.append(room_id)
else:
if not old_state_ids:
old_state_ids = await self.get_state_at(room_id, since_token)
old_mem_ev_id = old_state_ids.get(
(EventTypes.Member, user_id), None
)
old_mem_ev = None
if old_mem_ev_id:
old_mem_ev = await self.store.get_event(
old_mem_ev_id, allow_none=True
)
if old_mem_ev and old_mem_ev.membership == Membership.JOIN:
old_mem_ev = await self._fetch_membership_event_at(
room_id, user_id, since_token
)
if (
old_mem_ev is not None
and old_mem_ev.membership == Membership.JOIN
):
newly_left_rooms.append(room_id)

# 3. Should we add this room to `invited`?
Expand Down Expand Up @@ -1984,6 +1972,24 @@ async def _classify_rooms_by_membership_changes(
newly_left_rooms,
)

async def _fetch_membership_event_at(
self, room_id: str, user_id: str, since_token: StreamToken
) -> Optional[EventBase]:
"""What was the user's membership in this room at the given stream_token?

Returns None if
- there was no membership for the user at the given time
- the user had a membership event, but we couldn't find it.

Otherwise, returns the membership event itself.
"""

old_state_ids = await self.get_state_at(room_id, since_token)
old_mem_ev_id = old_state_ids.get((EventTypes.Member, user_id), None)
if old_mem_ev_id is not None:
return await self.store.get_event(old_mem_ev_id, allow_none=True)
return None

async def _get_all_rooms(
self, sync_result_builder: "SyncResultBuilder", ignored_users: FrozenSet[str]
) -> _RoomChanges:
Expand Down