From a5444ffd51acd2c008a6b1054c43aaa156496fa7 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Thu, 3 Oct 2024 15:35:44 +0300 Subject: [PATCH 1/4] Implement MSC4210: Remove legacy mentions --- changelog.d/17783.feature | 1 + rust/benches/evaluator.rs | 4 ++++ rust/src/push/evaluator.rs | 11 ++++++++++- synapse/config/experimental.py | 3 +++ synapse/push/bulk_push_rule_evaluator.py | 1 + synapse/synapse_rust/push.pyi | 1 + tests/push/test_push_rule_evaluator.py | 2 ++ 7 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 changelog.d/17783.feature diff --git a/changelog.d/17783.feature b/changelog.d/17783.feature new file mode 100644 index 00000000000..ab9a1335d74 --- /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 4fea035b961..8f9101465a7 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(); diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 2f4b6d47bb2..333ccda5f7c 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, @@ -133,6 +137,7 @@ impl PushRuleEvaluator { related_event_match_enabled: bool, room_version_feature_flags: Vec, msc3931_enabled: bool, + msc4210_enabled: bool, ) -> Result { 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(); diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 99185db93d4..fd14db0d024 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -447,3 +447,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # 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 679cbe9afa0..9c0592a9026 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -436,6 +436,7 @@ async def _action_for_event_by_user( 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/synapse_rust/push.pyi b/synapse/synapse_rust/push.pyi index 27a974e1bbe..f205da54340 100644 --- a/synapse/synapse_rust/push.pyi +++ b/synapse/synapse_rust/push.pyi @@ -65,6 +65,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 420fbea9982..c1f8e18bd92 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -149,6 +149,7 @@ def _get_evaluator( content: JsonMapping, *, related_events: Optional[JsonDict] = None, + msc4210: bool = False, ) -> PushRuleEvaluator: event = FrozenEvent( { @@ -174,6 +175,7 @@ def _get_evaluator( 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: From 3e070694f926688681b3db14f679cc5ce86361d5 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Thu, 3 Oct 2024 15:37:49 +0300 Subject: [PATCH 2/4] . --- changelog.d/17783.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/17783.feature b/changelog.d/17783.feature index ab9a1335d74..ce8c2164185 100644 --- a/changelog.d/17783.feature +++ b/changelog.d/17783.feature @@ -1 +1 @@ -Implement [MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210): Remove legacy mentions. Contributed by @tulir @ Beeper +Implement [MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210): Remove legacy mentions. Contributed by @tulir @ Beeper. From 9c3cd362b1af9040f428d0ef50e4697f7ce594da Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 4 Oct 2024 12:41:53 +0300 Subject: [PATCH 3/4] Also filter push rules when serving to clients --- rust/benches/evaluator.rs | 1 + rust/src/push/evaluator.rs | 2 +- rust/src/push/mod.rs | 10 ++++++++++ synapse/storage/databases/main/push_rule.py | 1 + synapse/synapse_rust/push.pyi | 1 + 5 files changed, 14 insertions(+), 1 deletion(-) diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index 8f9101465a7..28537e187ec 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -209,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 333ccda5f7c..0d436a1d7b5 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -591,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 2a452b69a3c..c7af8acbedd 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,13 @@ 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/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py index bbdde177116..86c87f78bf1 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 f205da54340..3f317c32889 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]]: ... From aab2ae0638ad2cab6f4c46fb309a3b4e19c23664 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 4 Oct 2024 12:44:38 +0300 Subject: [PATCH 4/4] Fix rustfmt --- rust/src/push/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index c7af8acbedd..ef8ed150d42 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -602,7 +602,8 @@ impl FilteredPushRules { 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") { + || rule.rule_id == "global/override/.m.rule.roomnotif") + { return false; }