From f39bdc2a0b990a79cc41ff91f6227ed3bda0217d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 8 Oct 2024 18:02:25 -0500 Subject: [PATCH] Add wildcard to explicit state_key tests See https://github.com/element-hq/synapse/pull/17785#discussion_r1790933329 --- synapse/handlers/sliding_sync/__init__.py | 24 +++- tests/handlers/test_sliding_sync.py | 142 +++++++++++++++++++--- 2 files changed, 145 insertions(+), 21 deletions(-) diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index 4915682f6e8..9afa7a43d17 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -1313,6 +1313,13 @@ def _required_state_changes( prev_wildcard = prev_required_state_map.get(StateValues.WILDCARD, set()) request_wildcard = request_required_state_map.get(StateValues.WILDCARD, set()) + # If we were previously fetching everything ("*", "*"), always update the effective + # room required state config to match the request. And since we we're previously + # already fetching everything, we don't have to fetch anything now that they've + # narrowed. + if StateValues.WILDCARD in prev_wildcard: + return request_required_state_map, StateFilter.none() + # If a event type wildcard has been added or removed we don't try and do # anything fancy, and instead always update the effective room required # state config to match the request. @@ -1349,6 +1356,11 @@ def _required_state_changes( # Always update changes to include the newly added keys changes[event_type] = request_state_keys + if StateValues.WILDCARD in old_state_keys: + # We were previously fetching everything for this type, so we don't need to + # fetch anything new. + continue + # Record the new state keys to fetch for this type. if StateValues.WILDCARD in request_state_keys: # If we have added a wildcard then we always just fetch everything. @@ -1358,8 +1370,8 @@ def _required_state_changes( if state_key == StateValues.ME: added.append((event_type, user_id)) elif state_key == StateValues.LAZY: - # We handle lazy loading separately, so don't need to - # explicitly add anything here. + # We handle lazy loading separately (outside this function), so + # don't need to explicitly add anything here. pass else: added.append((event_type, state_key)) @@ -1397,8 +1409,8 @@ def _required_state_changes( request_state_key_wildcard = StateValues.WILDCARD in request_state_keys if old_state_key_wildcard != request_state_key_wildcard: - # If a wildcard has been added or removed we always update the - # required state when any state with the same type has changed. + # If a state_key wildcard has been added or removed, we always update the + # effective room required state config to match the request. changes[event_type] = request_state_keys continue @@ -1406,8 +1418,8 @@ def _required_state_changes( request_state_key_lazy = StateValues.LAZY in request_state_keys if old_state_key_lazy != request_state_key_lazy: - # If a "$LAZY" has been added or removed we always update the - # required state for simplicity. + # If a "$LAZY" has been added or removed we always update the effective room + # required state config to match the request. changes[event_type] = request_state_keys continue diff --git a/tests/handlers/test_sliding_sync.py b/tests/handlers/test_sliding_sync.py index 2c9b113d80d..659968fce85 100644 --- a/tests/handlers/test_sliding_sync.py +++ b/tests/handlers/test_sliding_sync.py @@ -3628,17 +3628,14 @@ class RequiredStateChangesTestCase(unittest.TestCase): "state_key_lazy_add", """ Test adding state keys work when using "$LAZY" - - If a "$LAZY" has been added or removed we always update the - required state to what was requested for simplicity. """, RequiredStateChangesTestParameters( previous_required_state_map={}, request_required_state_map={EventTypes.Member: {StateValues.LAZY}}, state_deltas={(EventTypes.Member, "@user:test"): "$event_id"}, - # We've added the $LAZY state key, but that gets handled separately so - # it should not appear in `added` expected_with_state_deltas=( + # If a "$LAZY" has been added or removed we always update the + # required state to what was requested for simplicity. {EventTypes.Member: {StateValues.LAZY}}, StateFilter.none(), ), @@ -3652,21 +3649,14 @@ class RequiredStateChangesTestCase(unittest.TestCase): "state_key_lazy_remove", """ Test removing state keys work when using "$LAZY" - - If a "$LAZY" has been added or removed we always update the - required state to what was requested for simplicity. """, RequiredStateChangesTestParameters( previous_required_state_map={EventTypes.Member: {StateValues.LAZY}}, request_required_state_map={}, state_deltas={(EventTypes.Member, "@user:test"): "$event_id"}, expected_with_state_deltas=( - # Remove `EventTypes.Member` since there's been a change to that - # state, (persist the change to required state). That way next - # time, they request `EventTypes.Member`, we see that we haven't - # sent it before and send the new state. (if we were tracking - # that we sent any other state, we should still keep track - # that). + # If a "$LAZY" has been added or removed we always update the + # required state to what was requested for simplicity. {}, # We don't need to request anything more if they are requesting # less state now @@ -3683,9 +3673,131 @@ class RequiredStateChangesTestCase(unittest.TestCase): ), ), ), + ( + "type_wildcard_with_state_key_wildcard_to_explicit_state_keys", + """ + Test switching from a wildcard ("*", "*") to explicit state keys + """, + RequiredStateChangesTestParameters( + previous_required_state_map={ + StateValues.WILDCARD: {StateValues.WILDCARD} + }, + request_required_state_map={ + StateValues.WILDCARD: {"state_key1", "state_key2", "state_key3"} + }, + state_deltas={("type1", "state_key1"): "$event_id"}, + # If we were previously fetching everything ("*", "*"), always update the effective + # room required state config to match the request. And since we we're previously + # already fetching everything, we don't have to fetch anything now that they've + # narrowed. + expected_with_state_deltas=( + { + StateValues.WILDCARD: { + "state_key1", + "state_key2", + "state_key3", + } + }, + StateFilter.none(), + ), + expected_without_state_deltas=( + { + StateValues.WILDCARD: { + "state_key1", + "state_key2", + "state_key3", + } + }, + StateFilter.none(), + ), + ), + ), + ( + "type_wildcard_with_explicit_state_keys_to_wildcard_state_key", + """ + Test switching from explicit to wildcard state keys ("*", "*") + """, + RequiredStateChangesTestParameters( + previous_required_state_map={ + StateValues.WILDCARD: {"state_key1", "state_key2", "state_key3"} + }, + request_required_state_map={ + StateValues.WILDCARD: {StateValues.WILDCARD} + }, + state_deltas={("type1", "state_key1"): "$event_id"}, + # We've added a wildcard, so we persist the change and request everything + expected_with_state_deltas=( + {StateValues.WILDCARD: {StateValues.WILDCARD}}, + StateFilter.all(), + ), + expected_without_state_deltas=( + {StateValues.WILDCARD: {StateValues.WILDCARD}}, + StateFilter.all(), + ), + ), + ), + ( + "state_key_wildcard_to_explicit_state_keys", + """Test switching from a wildcard to explicit state keys with a concrete type""", + RequiredStateChangesTestParameters( + previous_required_state_map={"type1": {StateValues.WILDCARD}}, + request_required_state_map={ + "type1": {"state_key1", "state_key2", "state_key3"} + }, + state_deltas={("type1", "state_key1"): "$event_id"}, + # If a state_key wildcard has been added or removed, we always + # update the effective room required state config to match the + # request. And since we we're previously already fetching + # everything, we don't have to fetch anything now that they've + # narrowed. + expected_with_state_deltas=( + { + "type1": { + "state_key1", + "state_key2", + "state_key3", + } + }, + StateFilter.none(), + ), + expected_without_state_deltas=( + { + "type1": { + "state_key1", + "state_key2", + "state_key3", + } + }, + StateFilter.none(), + ), + ), + ), + ( + "state_key_wildcard_to_explicit_state_keys", + """Test switching from a wildcard to explicit state keys with a concrete type""", + RequiredStateChangesTestParameters( + previous_required_state_map={ + "type1": {"state_key1", "state_key2", "state_key3"} + }, + request_required_state_map={"type1": {StateValues.WILDCARD}}, + state_deltas={("type1", "state_key1"): "$event_id"}, + # If a state_key wildcard has been added or removed, we always + # update the effective room required state config to match the + # request. And we need to request all of the state for that type + # because we previously, only sent down a few keys. + expected_with_state_deltas=( + {"type1": {StateValues.WILDCARD}}, + StateFilter.from_types([("type1", None)]), + ), + expected_without_state_deltas=( + {"type1": {StateValues.WILDCARD}}, + StateFilter.from_types([("type1", None)]), + ), + ), + ), ] ) - def test_required_state_changes( + def test_xxx( self, _test_label: str, _test_description: str,