Rename previously_returned_user_state param

Author: erikjohnston

synapse/handlers/sliding_sync/__init__.py

@@ -1216,23 +1216,23 @@ async def get_room_sync_data(
                 # the case where the client added explicit user state requests
                 # for users they already had lazy loaded.
                 if all_required_user_state:
-                    previously_returned_user_state = (
+                    previously_returned_user_to_last_seen = (
                         await self.store.get_sliding_sync_connection_lazy_members(
                             connection_position=from_token.connection_position,
                             room_id=room_id,
                             user_ids=all_required_user_state,
                         )
                     )
                 else:
-                    previously_returned_user_state = {}
+                    previously_returned_user_to_last_seen = {}
 
                 # Check if there are any changes to the required state config
                 # that we need to handle.
                 changes_return = _required_state_changes(
                     user.to_string(),
                     prev_required_state_map=prev_room_sync_config.required_state_map,
                     request_required_state_map=room_sync_config.required_state_map,
-                    previously_returned_user_state=previously_returned_user_state,
+                    previously_returned_user_ids=previously_returned_user_to_last_seen.keys(),
                     lazy_load_user_ids=lazy_load_user_ids,
                     state_deltas=room_state_delta_id_map,
                 )
@@ -1570,7 +1570,7 @@ def _required_state_changes(
     *,
     prev_required_state_map: Mapping[str, AbstractSet[str]],
     request_required_state_map: Mapping[str, AbstractSet[str]],
-    previously_returned_user_state: Mapping[str, int | None],
+    previously_returned_user_ids: AbstractSet[str],
     lazy_load_user_ids: AbstractSet[str],
     state_deltas: StateMap[str],
 ) -> _RequiredStateChangesReturn:
@@ -1593,9 +1593,11 @@ def _required_state_changes(
             request.
         request_required_state_map: The required state map from the current
             request.
-        previously_returned_user_state: The set of user IDs whose lazy-loaded
-            membership we have previously returned to the client.
-        required_user_state: The set of user IDs whose lazy-loaded membership
+        previously_returned_user_state: The set of user IDs whose membership
+            we have previously returned to the client. This is filtered to
+            only include users who have either sent events in the timeline
+            or required state.
+        lazy_load_user_ids: The set of user IDs whose lazy-loaded membership
             is required for this request.
         state_deltas: The state deltas that have changed in the room since the
             previous request.
@@ -1613,7 +1615,7 @@ def _required_state_changes(
             # send this member change down.
             continue
 
-        if state_key not in previously_returned_user_state:
+        if state_key not in previously_returned_user_ids:
             # We've not previously returned this member so nothing to
             # invalidate.
             continue
@@ -1622,9 +1624,7 @@ def _required_state_changes(
 
     if prev_required_state_map == request_required_state_map:
         # There has been no change in state, just need to check lazy members.
-        newly_returned_lazy_members = (
-            lazy_load_user_ids - previously_returned_user_state.keys()
-        )
+        newly_returned_lazy_members = lazy_load_user_ids - previously_returned_user_ids
         if newly_returned_lazy_members:
             # There are some new lazy members we need to fetch.
             added_types: list[tuple[str, str | None]] = []
@@ -1812,7 +1812,7 @@ def _required_state_changes(
                     # not membership.
                     pass
                 elif event_type == EventTypes.Member:
-                    if state_key not in previously_returned_user_state:
+                    if state_key not in previously_returned_user_ids:
                         # Only add *explicit* members we haven't previously sent
                         # down.
                         added.append((event_type, state_key))
@@ -1823,7 +1823,7 @@ def _required_state_changes(
     # haven't previously been returned.
     for required_user_id in (
         lazy_load_user_ids
-        - previously_returned_user_state.keys()
+        - previously_returned_user_ids
         - lazy_members_previously_returned
     ):
         added.append((EventTypes.Member, required_user_id))

tests/handlers/test_sliding_sync.py

@@ -3834,7 +3834,7 @@ class RequiredStateChangesTestParameters:
     expected_with_state_deltas: _RequiredStateChangesReturn
     expected_without_state_deltas: _RequiredStateChangesReturn
 
-    previously_returned_user_state: AbstractSet[str] = frozenset()
+    previously_returned_user_ids: AbstractSet[str] = frozenset()
     lazy_load_user_ids: AbstractSet[str] = frozenset()
 
 
@@ -4370,7 +4370,7 @@ class RequiredStateChangesTestCase(unittest.TestCase):
                 RequiredStateChangesTestParameters(
                     previous_required_state_map={EventTypes.Member: {StateValues.LAZY}},
                     request_required_state_map={EventTypes.Member: {StateValues.LAZY}},
-                    previously_returned_user_state={"@user2:test", "@user3:test"},
+                    previously_returned_user_ids={"@user2:test", "@user3:test"},
                     lazy_load_user_ids=set(),
                     state_deltas={(EventTypes.Member, "@user2:test"): "$event_id"},
                     expected_with_state_deltas=_RequiredStateChangesReturn(
@@ -4415,7 +4415,7 @@ class RequiredStateChangesTestCase(unittest.TestCase):
                 RequiredStateChangesTestParameters(
                     previous_required_state_map={EventTypes.Member: {StateValues.LAZY}},
                     request_required_state_map={EventTypes.Member: {StateValues.LAZY}},
-                    previously_returned_user_state={"@user2:test", "@user3:test"},
+                    previously_returned_user_ids={"@user2:test", "@user3:test"},
                     lazy_load_user_ids={"@user4:test"},
                     state_deltas={(EventTypes.Member, "@user2:test"): "$event_id"},
                     expected_with_state_deltas=_RequiredStateChangesReturn(
@@ -4457,7 +4457,7 @@ class RequiredStateChangesTestCase(unittest.TestCase):
                         EventTypes.Member: {"@user2:test", "@user3:test"}
                     },
                     request_required_state_map={EventTypes.Member: {StateValues.LAZY}},
-                    previously_returned_user_state=frozenset(),
+                    previously_returned_user_ids=frozenset(),
                     lazy_load_user_ids={"@user3:test"},
                     state_deltas={(EventTypes.Member, "@user2:test"): "$event_id"},
                     expected_with_state_deltas=_RequiredStateChangesReturn(
@@ -4501,7 +4501,7 @@ class RequiredStateChangesTestCase(unittest.TestCase):
                 RequiredStateChangesTestParameters(
                     previous_required_state_map={EventTypes.Member: {StateValues.LAZY}},
                     request_required_state_map={},
-                    previously_returned_user_state={"@user2:test", "@user3:test"},
+                    previously_returned_user_ids={"@user2:test", "@user3:test"},
                     lazy_load_user_ids=set(),
                     state_deltas={(EventTypes.Member, "@user2:test"): "$event_id"},
                     expected_with_state_deltas=_RequiredStateChangesReturn(
@@ -4549,7 +4549,7 @@ class RequiredStateChangesTestCase(unittest.TestCase):
                 RequiredStateChangesTestParameters(
                     previous_required_state_map={EventTypes.Member: {StateValues.LAZY}},
                     request_required_state_map={EventTypes.Member: {"@user4:test"}},
-                    previously_returned_user_state={"@user2:test", "@user3:test"},
+                    previously_returned_user_ids={"@user2:test", "@user3:test"},
                     lazy_load_user_ids={"@user4:test"},
                     state_deltas={(EventTypes.Member, "@user2:test"): "$event_id"},
                     expected_with_state_deltas=_RequiredStateChangesReturn(
@@ -4720,19 +4720,12 @@ def test_xxx(
         _test_description: str,
         test_parameters: RequiredStateChangesTestParameters,
     ) -> None:
-        # `_required_state_changes` expects this to be a map from user ID to
-        # timestamp, so let's convert it into one by using a dummy timestamp
-        # value
-        previously_returned_user_state = dict.fromkeys(
-            test_parameters.previously_returned_user_state, 1234
-        )
-
         # Without `state_deltas`
         state_changes = _required_state_changes(
             user_id="@user:test",
             prev_required_state_map=test_parameters.previous_required_state_map,
             request_required_state_map=test_parameters.request_required_state_map,
-            previously_returned_user_state=previously_returned_user_state,
+            previously_returned_user_ids=test_parameters.previously_returned_user_ids,
             lazy_load_user_ids=test_parameters.lazy_load_user_ids,
             state_deltas={},
         )
@@ -4763,7 +4756,7 @@ def test_xxx(
             user_id="@user:test",
             prev_required_state_map=test_parameters.previous_required_state_map,
             request_required_state_map=test_parameters.request_required_state_map,
-            previously_returned_user_state=previously_returned_user_state,
+            previously_returned_user_ids=test_parameters.previously_returned_user_ids,
             lazy_load_user_ids=test_parameters.lazy_load_user_ids,
             state_deltas=test_parameters.state_deltas,
         )
@@ -4827,7 +4820,7 @@ def test_limit_retained_previous_state_keys(
             user_id="@user:test",
             prev_required_state_map=previous_required_state_map,
             request_required_state_map=request_required_state_map,
-            previously_returned_user_state={},
+            previously_returned_user_ids=frozenset(),
             lazy_load_user_ids=frozenset(),
             state_deltas={},
         )
@@ -4900,7 +4893,7 @@ def test_request_more_state_keys_than_remember_limit(self) -> None:
             user_id="@user:test",
             prev_required_state_map=previous_required_state_map,
             request_required_state_map=request_required_state_map,
-            previously_returned_user_state={},
+            previously_returned_user_ids=frozenset(),
             lazy_load_user_ids=frozenset(),
             state_deltas={},
         )