Add failing same pos test

rust/benches/evaluator.rs

@@ -62,7 +62,6 @@ fn bench_match_exact(b: &mut Bencher) {
         false,
         false,
         false,
-        false,
     )
     .unwrap();
 
@@ -110,7 +109,6 @@ fn bench_match_word(b: &mut Bencher) {
         false,
         false,
         false,
-        false,
     )
     .unwrap();
 
@@ -158,7 +156,6 @@ fn bench_match_word_miss(b: &mut Bencher) {
         false,
         false,
         false,
-        false,
     )
     .unwrap();
 
@@ -206,7 +203,6 @@ fn bench_eval_message(b: &mut Bencher) {
         false,
         false,
         false,
-        false,
     )
     .unwrap();
 
@@ -219,7 +215,6 @@ fn bench_eval_message(b: &mut Bencher) {
         false,
         false,
         false,
-        false,
     );
 
     b.iter(|| eval.run(&rules, Some("bob"), Some("person"), None));

rust/src/push/evaluator.rs

@@ -111,8 +111,6 @@ pub struct PushRuleEvaluator {
 
     /// If MSC4306 (thread subscriptions) is enabled.
     msc4306_enabled: bool,
-
-    xxx_enabled: bool,
 }
 
 #[pymethods]
@@ -132,7 +130,6 @@ impl PushRuleEvaluator {
         msc3931_enabled,
         msc4210_enabled,
         msc4306_enabled,
-        xxx_enabled,
     ))]
     pub fn py_new(
         flattened_keys: BTreeMap<String, JsonValue>,
@@ -146,7 +143,6 @@ impl PushRuleEvaluator {
         msc3931_enabled: bool,
         msc4210_enabled: bool,
         msc4306_enabled: bool,
-        xxx_enabled: bool,
     ) -> Result<Self, Error> {
         let body = match flattened_keys.get("content.body") {
             Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone().into_owned(),
@@ -166,7 +162,6 @@ impl PushRuleEvaluator {
             msc3931_enabled,
             msc4210_enabled,
             msc4306_enabled,
-            xxx_enabled,
         })
     }
 
@@ -574,7 +569,6 @@ fn push_rule_evaluator() {
         true,
         false,
         false,
-        false,
     )
     .unwrap();
 
@@ -606,7 +600,6 @@ fn test_requires_room_version_supports_condition() {
         true,
         false,
         false,
-        false,
     )
     .unwrap();
 
@@ -644,7 +637,6 @@ fn test_requires_room_version_supports_condition() {
             false,
             false,
             false,
-            false,
         ),
         None,
         None,

rust/src/push/mod.rs

@@ -552,7 +552,6 @@ pub struct FilteredPushRules {
     msc4028_push_encrypted_events: bool,
     msc4210_enabled: bool,
     msc4306_enabled: bool,
-    xxx_enabled: bool,
 }
 
 #[pymethods]
@@ -568,7 +567,6 @@ impl FilteredPushRules {
         msc4028_push_encrypted_events: bool,
         msc4210_enabled: bool,
         msc4306_enabled: bool,
-        xxx_enabled: bool,
     ) -> Self {
         Self {
             push_rules,
@@ -579,7 +577,6 @@ impl FilteredPushRules {
             msc4028_push_encrypted_events,
             msc4210_enabled,
             msc4306_enabled,
-            xxx_enabled,
         }
     }
 
@@ -634,10 +631,6 @@ impl FilteredPushRules {
                     return false;
                 }
 
-                if self.xxx_enabled {
-                    return true;
-                }
-
                 true
             })
             .map(|r| {

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

synapse/push/bulk_push_rule_evaluator.py

@@ -479,7 +479,6 @@ class BulkPushRuleEvaluator:
             self.hs.config.experimental.msc1767_enabled,  # MSC3931 flag
             self.hs.config.experimental.msc4210_enabled,
             self.hs.config.experimental.msc4306_enabled,
-            False,
         )
 
         msc4306_thread_subscribers: Optional[FrozenSet[str]] = None

tests/rest/client/sliding_sync/test_rooms_required_state.py

@@ -1597,6 +1597,107 @@ class SlidingSyncRoomsRequiredStateTestCase(SlidingSyncBase):
             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"""
@@ -1686,3 +1787,77 @@ class SlidingSyncRoomsRequiredStateTestCase(SlidingSyncBase):
         # 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,
+        )