From 11bc9a1b3ae21bd2041bf7cc4b93dd70a8c3b93e Mon Sep 17 00:00:00 2001
From: Tulir Asokan <tulir@maunium.net>
Date: Mon, 14 Oct 2024 15:24:28 +0200
Subject: [PATCH] Implement MSC4210: Remove legacy mentions (#17783)

---
 changelog.d/17783.feature                   |  1 +
 rust/benches/evaluator.rs                   |  5 +++++
 rust/src/push/evaluator.rs                  | 13 +++++++++++--
 rust/src/push/mod.rs                        | 11 +++++++++++
 synapse/config/experimental.py              |  3 +++
 synapse/push/bulk_push_rule_evaluator.py    |  1 +
 synapse/storage/databases/main/push_rule.py |  1 +
 synapse/synapse_rust/push.pyi               |  2 ++
 tests/push/test_push_rule_evaluator.py      |  2 ++
 9 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/17783.feature

diff --git a/changelog.d/17783.feature b/changelog.d/17783.feature
new file mode 100644
index 0000000000..ce8c216418
--- /dev/null
+++ b/changelog.d/17783.feature
@@ -0,0 +1 @@
+Implement [MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210): Remove legacy mentions. Contributed by @tulir @ Beeper.
diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs
index 4fea035b96..28537e187e 100644
--- a/rust/benches/evaluator.rs
+++ b/rust/benches/evaluator.rs
@@ -60,6 +60,7 @@ fn bench_match_exact(b: &mut Bencher) {
         true,
         vec![],
         false,
+        false,
     )
     .unwrap();
 
@@ -105,6 +106,7 @@ fn bench_match_word(b: &mut Bencher) {
         true,
         vec![],
         false,
+        false,
     )
     .unwrap();
 
@@ -150,6 +152,7 @@ fn bench_match_word_miss(b: &mut Bencher) {
         true,
         vec![],
         false,
+        false,
     )
     .unwrap();
 
@@ -195,6 +198,7 @@ fn bench_eval_message(b: &mut Bencher) {
         true,
         vec![],
         false,
+        false,
     )
     .unwrap();
 
@@ -205,6 +209,7 @@ fn bench_eval_message(b: &mut Bencher) {
         false,
         false,
         false,
+        false,
     );
 
     b.iter(|| eval.run(&rules, Some("bob"), Some("person")));
diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs
index 2f4b6d47bb..0d436a1d7b 100644
--- a/rust/src/push/evaluator.rs
+++ b/rust/src/push/evaluator.rs
@@ -105,6 +105,9 @@ pub struct PushRuleEvaluator {
     /// If MSC3931 (room version feature flags) is enabled. Usually controlled by the same
     /// flag as MSC1767 (extensible events core).
     msc3931_enabled: bool,
+
+    // If MSC4210 (remove legacy mentions) is enabled.
+    msc4210_enabled: bool,
 }
 
 #[pymethods]
@@ -122,6 +125,7 @@ impl PushRuleEvaluator {
         related_event_match_enabled,
         room_version_feature_flags,
         msc3931_enabled,
+        msc4210_enabled,
     ))]
     pub fn py_new(
         flattened_keys: BTreeMap<String, JsonValue>,
@@ -133,6 +137,7 @@ impl PushRuleEvaluator {
         related_event_match_enabled: bool,
         room_version_feature_flags: Vec<String>,
         msc3931_enabled: bool,
+        msc4210_enabled: bool,
     ) -> Result<Self, Error> {
         let body = match flattened_keys.get("content.body") {
             Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone().into_owned(),
@@ -150,6 +155,7 @@ impl PushRuleEvaluator {
             related_event_match_enabled,
             room_version_feature_flags,
             msc3931_enabled,
+            msc4210_enabled,
         })
     }
 
@@ -176,7 +182,8 @@ impl PushRuleEvaluator {
 
             // For backwards-compatibility the legacy mention rules are disabled
             // if the event contains the 'm.mentions' property.
-            if self.has_mentions
+            // Additionally, MSC4210 always disables the legacy rules.
+            if (self.has_mentions || self.msc4210_enabled)
                 && (rule_id == "global/override/.m.rule.contains_display_name"
                     || rule_id == "global/content/.m.rule.contains_user_name"
                     || rule_id == "global/override/.m.rule.roomnotif")
@@ -526,6 +533,7 @@ fn push_rule_evaluator() {
         true,
         vec![],
         true,
+        false,
     )
     .unwrap();
 
@@ -555,6 +563,7 @@ fn test_requires_room_version_supports_condition() {
         false,
         flags,
         true,
+        false,
     )
     .unwrap();
 
@@ -582,7 +591,7 @@ fn test_requires_room_version_supports_condition() {
     };
     let rules = PushRules::new(vec![custom_rule]);
     result = evaluator.run(
-        &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false),
+        &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false, false),
         None,
         None,
     );
diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs
index 2a452b69a3..ef8ed150d4 100644
--- a/rust/src/push/mod.rs
+++ b/rust/src/push/mod.rs
@@ -534,6 +534,7 @@ pub struct FilteredPushRules {
     msc3381_polls_enabled: bool,
     msc3664_enabled: bool,
     msc4028_push_encrypted_events: bool,
+    msc4210_enabled: bool,
 }
 
 #[pymethods]
@@ -546,6 +547,7 @@ impl FilteredPushRules {
         msc3381_polls_enabled: bool,
         msc3664_enabled: bool,
         msc4028_push_encrypted_events: bool,
+        msc4210_enabled: bool,
     ) -> Self {
         Self {
             push_rules,
@@ -554,6 +556,7 @@ impl FilteredPushRules {
             msc3381_polls_enabled,
             msc3664_enabled,
             msc4028_push_encrypted_events,
+            msc4210_enabled,
         }
     }
 
@@ -596,6 +599,14 @@ impl FilteredPushRules {
                     return false;
                 }
 
+                if self.msc4210_enabled
+                    && (rule.rule_id == "global/override/.m.rule.contains_display_name"
+                        || rule.rule_id == "global/content/.m.rule.contains_user_name"
+                        || rule.rule_id == "global/override/.m.rule.roomnotif")
+                {
+                    return false;
+                }
+
                 true
             })
             .map(|r| {
diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py
index 99185db93d..fd14db0d02 100644
--- a/synapse/config/experimental.py
+++ b/synapse/config/experimental.py
@@ -447,3 +447,6 @@ class ExperimentalConfig(Config):
 
         # MSC4151: Report room API (Client-Server API)
         self.msc4151_enabled: bool = experimental.get("msc4151_enabled", False)
+
+        # MSC4210: Remove legacy mentions
+        self.msc4210_enabled: bool = experimental.get("msc4210_enabled", False)
diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py
index 679cbe9afa..9c0592a902 100644
--- a/synapse/push/bulk_push_rule_evaluator.py
+++ b/synapse/push/bulk_push_rule_evaluator.py
@@ -436,6 +436,7 @@ class BulkPushRuleEvaluator:
             self._related_event_match_enabled,
             event.room_version.msc3931_push_features,
             self.hs.config.experimental.msc1767_enabled,  # MSC3931 flag
+            self.hs.config.experimental.msc4210_enabled,
         )
 
         for uid, rules in rules_by_user.items():
diff --git a/synapse/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py
index bbdde17711..86c87f78bf 100644
--- a/synapse/storage/databases/main/push_rule.py
+++ b/synapse/storage/databases/main/push_rule.py
@@ -109,6 +109,7 @@ def _load_rules(
         msc3664_enabled=experimental_config.msc3664_enabled,
         msc3381_polls_enabled=experimental_config.msc3381_polls_enabled,
         msc4028_push_encrypted_events=experimental_config.msc4028_push_encrypted_events,
+        msc4210_enabled=experimental_config.msc4210_enabled,
     )
 
     return filtered_rules
diff --git a/synapse/synapse_rust/push.pyi b/synapse/synapse_rust/push.pyi
index 27a974e1bb..3f317c3288 100644
--- a/synapse/synapse_rust/push.pyi
+++ b/synapse/synapse_rust/push.pyi
@@ -48,6 +48,7 @@ class FilteredPushRules:
         msc3381_polls_enabled: bool,
         msc3664_enabled: bool,
         msc4028_push_encrypted_events: bool,
+        msc4210_enabled: bool,
     ): ...
     def rules(self) -> Collection[Tuple[PushRule, bool]]: ...
 
@@ -65,6 +66,7 @@ class PushRuleEvaluator:
         related_event_match_enabled: bool,
         room_version_feature_flags: Tuple[str, ...],
         msc3931_enabled: bool,
+        msc4210_enabled: bool,
     ): ...
     def run(
         self,
diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py
index 420fbea998..c1f8e18bd9 100644
--- a/tests/push/test_push_rule_evaluator.py
+++ b/tests/push/test_push_rule_evaluator.py
@@ -149,6 +149,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
         content: JsonMapping,
         *,
         related_events: Optional[JsonDict] = None,
+        msc4210: bool = False,
     ) -> PushRuleEvaluator:
         event = FrozenEvent(
             {
@@ -174,6 +175,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
             related_event_match_enabled=True,
             room_version_feature_flags=event.room_version.msc3931_push_features,
             msc3931_enabled=True,
+            msc4210_enabled=msc4210,
         )
 
     def test_display_name(self) -> None:
-- 
GitLab