From 49dddb99b3a3e65a478c89bbe573b753fa4e5588 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 9 Dec 2021 20:20:16 +0000 Subject: [PATCH 1/3] Adjust _get_rooms_changed comments C.f. https://github.com/matrix-org/synapse/pull/11494#pullrequestreview-827780886 --- synapse/handlers/sync.py | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index f3039c3c3fb7..0e1a42e9b620 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1662,20 +1662,20 @@ async def _get_rooms_changed( ) -> _RoomChanges: """Determine the changes in rooms to report to the user. - Ideally, we want to report all events whose stream ordering `s` lies in the - range `since_token < s <= now_token`, where the two tokens are read from the - sync_result_builder. + This function is a first pass at generating the rooms part of the sync response. + It determines which rooms have changed during the sync period, and categorises + them into four buckets: "knock", "invite", "join" and "leave". - If there are too many events in that range to report, things get complicated. - In this situation we return a truncated list of the most recent events, and - indicate in the response that there is a "gap" of omitted events. Additionally: + 1. Finds all membership changes for the user in the sync period (from + `since_token` up to `now_token`). + 2. Uses those to place the room in one of the four categories above. + 3. Builds a `_RoomChanges` struct to record this, and return that struct. - - we include a "state_delta", to describe the changes in state over the gap, - - we include all membership events applying to the user making the request, - even those in the gap. - - See the spec for the rationale: - https://spec.matrix.org/v1.1/client-server-api/#syncing + For rooms classified as "knock", "invite" or "leave", we just need to report + a single membership event in the eventual /sync response. For "join" we need + to fetch additional non-membership events, e.g. messages in the room. That is + more complicated, so instead we report an intermediary `RoomSyncResultBuilder` + struct, and leave the additional work to `_generate_room_entry`. The sync_result_builder is not modified by this function. """ @@ -1686,16 +1686,6 @@ async def _get_rooms_changed( assert since_token - # The spec - # https://spec.matrix.org/v1.1/client-server-api/#get_matrixclientv3sync - # notes that membership events need special consideration: - # - # > When a sync is limited, the server MUST return membership events for events - # > in the gap (between since and the start of the returned timeline), regardless - # > as to whether or not they are redundant. - # - # We fetch such events here, but we only seem to use them for categorising rooms - # as newly joined, newly left, invited or knocked. # TODO: we've already called this function and ran this query in # _have_rooms_changed. We could keep the results in memory to avoid a # second query, at the cost of more complicated source code. From b428167ed5eea352f62e714cbefd175e0d6c5314 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 9 Dec 2021 20:30:11 +0000 Subject: [PATCH 2/3] Changelog --- changelog.d/11550.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11550.misc diff --git a/changelog.d/11550.misc b/changelog.d/11550.misc new file mode 100644 index 000000000000..d5577e0b6300 --- /dev/null +++ b/changelog.d/11550.misc @@ -0,0 +1 @@ +Fix an inaccurate and misleading comment in the `/sync` code. \ No newline at end of file From d88469265a8d74a5b75f5f09dc842dde3d794ee6 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 10 Dec 2021 14:24:50 +0000 Subject: [PATCH 3/3] Reintroduce comment to a better place --- synapse/handlers/sync.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 0e1a42e9b620..ebc5b5bce6f0 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1999,6 +1999,23 @@ async def _generate_room_entry( """Populates the `joined` and `archived` section of `sync_result_builder` based on the `room_builder`. + Ideally, we want to report all events whose stream ordering `s` lies in the + range `since_token < s <= now_token`, where the two tokens are read from the + sync_result_builder. + + If there are too many events in that range to report, things get complicated. + In this situation we return a truncated list of the most recent events, and + indicate in the response that there is a "gap" of omitted events. Lots of this + is handled in `_load_filtered_recents`, but some of is handled in this method. + + Additionally: + - we include a "state_delta", to describe the changes in state over the gap, + - we include all membership events applying to the user making the request, + even those in the gap. + + See the spec for the rationale: + https://spec.matrix.org/v1.1/client-server-api/#syncing + Args: sync_result_builder ignored_users: Set of users ignored by user.