From f60859ca722f81d118e2d858eab19748d2ee40dd Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Aug 2025 10:41:17 -0500 Subject: [PATCH 1/3] Clarify `_required_state_changes` docstring --- synapse/handlers/sliding_sync/__init__.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 071a271ab7b..f8476f91897 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -1493,6 +1493,18 @@ def _required_state_changes( added, removed and then added again to the required state. In that case we only want to re-send that entry down sync if it has changed. + Args: + prev_required_state_map: Map from state event type to state_keys requested for + the room. The values are close to `StateKey` but actually use a syntax where you + can provide `*` wildcard. `$ME and `$LAZY` for lazy-loading room members should + already be expanded into their explicit forms by this point. + request_required_state_map: Map from state event type to state_keys requested for + the room. The values are close to `StateKey` but actually use a syntax where you + can provide `*` wildcard. `$ME and `$LAZY` for lazy-loading room members should + already be expanded into their explicit forms by this point. + state_deltas: Relevant changes to the current state. "Relevant" for sync means + in the token range. + Returns: A 2-tuple of updated required state config (or None if there is no update) and the state filter to use to fetch extra current state that we need to From dff8c6934d5838052b704493ab3891be2c61ade0 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Aug 2025 10:42:46 -0500 Subject: [PATCH 2/3] Add failing test --- .../sliding_sync/test_rooms_required_state.py | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index cfff167c6ec..dcf738de464 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -1597,6 +1597,107 @@ def test_rooms_required_state_expand_retract_expand(self) -> None: exact=True, ) + def test_rooms_required_state_expand_retract_expand_without_new_activity( + self, + ) -> None: + """ + Test that when expanding, retracting and then expanding the required state, we + get the changes that happened; even without new activity in the room that would + send the room down the connection otherwise. + """ + + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + # Create a room with a room name. + room_id1 = self.helper.create_room_as( + user1_id, tok=user1_tok, extra_content={"name": "Foo"} + ) + + # Only request the state event to begin with + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 1]], + "required_state": [ + [EventTypes.Create, ""], + ], + "timeline_limit": 1, + } + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Create, "")], + }, + exact=True, + ) + + # Update the sliding sync requests to include the room name + sync_body["lists"]["foo-list"]["required_state"] = [ + [EventTypes.Create, ""], + [EventTypes.Name, ""], + ] + response_body, from_token = self.do_sync( + sync_body, since=from_token, tok=user1_tok + ) + + # We should see the room name, even though there haven't been any + # changes. + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Name, "")], + }, + exact=True, + ) + + # Update the room name + self.helper.send_state( + room_id1, EventTypes.Name, {"name": "Bar"}, state_key="", tok=user1_tok + ) + + # Update the sliding sync requests to exclude the room name again + sync_body["lists"]["foo-list"]["required_state"] = [ + [EventTypes.Create, ""], + ] + response_body, from_token = self.do_sync( + sync_body, since=from_token, tok=user1_tok + ) + + # We should not see the updated room name in state (though it will be in + # the timeline). + self.assertIsNone(response_body["rooms"][room_id1].get("required_state")) + + # Update the sliding sync requests to include the room name again + sync_body["lists"]["foo-list"]["required_state"] = [ + [EventTypes.Create, ""], + [EventTypes.Name, ""], + ] + response_body, from_token = self.do_sync( + sync_body, since=from_token, tok=user1_tok + ) + + # We should see the *new* room name, even though there haven't been any + # changes. + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Name, "")], + }, + exact=True, + ) + def test_rooms_required_state_expand_deduplicate(self) -> None: """Test that when expanding, retracting and then expanding the required state, we don't get the state down again if it hasn't changed""" From e5f500140c32447b324c9870cb6b0dc45caebb33 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 19 Aug 2025 10:48:37 -0500 Subject: [PATCH 3/3] Add failing same `pos` test --- .../sliding_sync/test_rooms_required_state.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/rest/client/sliding_sync/test_rooms_required_state.py b/tests/rest/client/sliding_sync/test_rooms_required_state.py index dcf738de464..568529d8255 100644 --- a/tests/rest/client/sliding_sync/test_rooms_required_state.py +++ b/tests/rest/client/sliding_sync/test_rooms_required_state.py @@ -1787,3 +1787,77 @@ def test_rooms_required_state_expand_deduplicate(self) -> None: # We should not see the room name again, as we have already sent that # down. self.assertIsNone(response_body["rooms"][room_id1].get("required_state")) + + def test_rooms_required_state_expand_with_same_pos( + self, + ) -> None: + """ + Test that when expanding the required state, we get the changes that happened + even if we're using the same `pos`. + """ + + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + + # Create a room with a room name. + room_id1 = self.helper.create_room_as( + user1_id, tok=user1_tok, extra_content={"name": "Foo"} + ) + + # Only request the state event to begin with (initial sync) + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 1]], + "required_state": [ + [EventTypes.Create, ""], + ], + "timeline_limit": 1, + } + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + + state_map = self.get_success( + self.storage_controllers.state.get_current_state(room_id1) + ) + + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Create, "")], + }, + exact=True, + ) + + # Do an incremental sync using the `pos` token from the initial sync + response_body, from_token = self.do_sync( + sync_body, since=from_token, tok=user1_tok + ) + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Create, "")], + }, + exact=True, + ) + + # Update the sliding sync requests to include the room name + sync_body["lists"]["foo-list"]["required_state"] = [ + [EventTypes.Create, ""], + [EventTypes.Name, ""], + ] + # Make a request using the same `pos` token from the initial sync + response_body, from_token = self.do_sync( + sync_body, since=from_token, tok=user1_tok + ) + + # We should see the room name, even though there haven't been any + # changes. + self._assertRequiredStateIncludes( + response_body["rooms"][room_id1]["required_state"], + { + state_map[(EventTypes.Name, "")], + }, + exact=True, + )