From f80bcfc3b515533302fa3c8df2c39001f5972e4c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 25 Jan 2023 10:31:06 -0500 Subject: [PATCH 1/6] Copy suppress_edits push rule from Beeper: https://github.com/beeper/synapse/blame/beeper/rust/src/push/base_rules.rs#L98-L114 --- rust/src/push/base_rules.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index 880eed0ef405..6552de95ad84 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -63,6 +63,23 @@ pub const BASE_PREPEND_OVERRIDE_RULES: &[PushRule] = &[PushRule { }]; pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ + // We don't want to notify on edits. Not only can this be confusing in real + // time (2 notifications, one message) but it's especially confusing + // if a bridge needs to edit a previously backfilled message. + PushRule { + rule_id: Cow::Borrowed("global/override/.com.beeper.suppress_edits"), + priority_class: 5, + conditions: Cow::Borrowed(&[ + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("content.m.relates_to.rel_type"), + pattern: Some(Cow::Borrowed("m.replace")), + pattern_type: None, + })), + ]), + actions: Cow::Borrowed(&[Action::DontNotify]), + default: true, + default_enabled: true, + }, PushRule { rule_id: Cow::Borrowed("global/override/.m.rule.suppress_notices"), priority_class: 5, From 88f791e10973b085c2861edc828eb2b197de7f5d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 25 Jan 2023 10:49:38 -0500 Subject: [PATCH 2/6] Experimental config flag & lint. --- rust/benches/evaluator.rs | 1 + rust/src/push/base_rules.rs | 8 ++++---- rust/src/push/evaluator.rs | 2 +- rust/src/push/mod.rs | 8 ++++++++ stubs/synapse/synapse_rust/push.pyi | 1 + synapse/config/experimental.py | 5 +++++ synapse/storage/databases/main/push_rule.py | 1 + 7 files changed, 21 insertions(+), 5 deletions(-) diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index 6b16a3f75b75..4311e8c2d54c 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -166,6 +166,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/base_rules.rs b/rust/src/push/base_rules.rs index 6552de95ad84..7e2a994abb37 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -69,13 +69,13 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/override/.com.beeper.suppress_edits"), priority_class: 5, - conditions: Cow::Borrowed(&[ - Condition::Known(KnownCondition::EventMatch(EventMatchCondition { + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { key: Cow::Borrowed("content.m.relates_to.rel_type"), pattern: Some(Cow::Borrowed("m.replace")), pattern_type: None, - })), - ]), + }, + ))]), actions: Cow::Borrowed(&[Action::DontNotify]), default: true, default_enabled: true, diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index aa71202e4379..628fcf8685e0 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -504,7 +504,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 7e449f243371..3c4f876cabf5 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -419,6 +419,7 @@ pub struct FilteredPushRules { msc3381_polls_enabled: bool, msc3664_enabled: bool, msc3952_intentional_mentions: bool, + msc3958_suppress_edits_enabled: bool, } #[pymethods] @@ -431,6 +432,7 @@ impl FilteredPushRules { msc3381_polls_enabled: bool, msc3664_enabled: bool, msc3952_intentional_mentions: bool, + msc3958_suppress_edits_enabled: bool, ) -> Self { Self { push_rules, @@ -439,6 +441,7 @@ impl FilteredPushRules { msc3381_polls_enabled, msc3664_enabled, msc3952_intentional_mentions, + msc3958_suppress_edits_enabled, } } @@ -476,6 +479,11 @@ impl FilteredPushRules { { return false; } + if !self.msc3958_suppress_edits_enabled + && rule.rule_id == "global/override/.com.beeper.suppress_edits" + { + return false; + } true }) diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi index 588d90c25a82..2a464b1fd3f3 100644 --- a/stubs/synapse/synapse_rust/push.pyi +++ b/stubs/synapse/synapse_rust/push.pyi @@ -47,6 +47,7 @@ class FilteredPushRules: msc3381_polls_enabled: bool, msc3664_enabled: bool, msc3952_intentional_mentions: bool, + msc3958_suppress_edits_enabled: bool, ): ... def rules(self) -> Collection[Tuple[PushRule, bool]]: ... diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d2d0270dddb1..53c0682dfdb6 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -173,3 +173,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.msc3952_intentional_mentions = experimental.get( "msc3952_intentional_mentions", False ) + + # MSC3959: Do not generate notifications for edits. + self.msc3958_supress_edit_notifs = experimental.get( + "msc3958_supress_edit_notifs", False + ) diff --git a/synapse/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py index 466a1145b78c..9b2bbe060ddc 100644 --- a/synapse/storage/databases/main/push_rule.py +++ b/synapse/storage/databases/main/push_rule.py @@ -90,6 +90,7 @@ def _load_rules( msc3664_enabled=experimental_config.msc3664_enabled, msc3381_polls_enabled=experimental_config.msc3381_polls_enabled, msc3952_intentional_mentions=experimental_config.msc3952_intentional_mentions, + msc3958_suppress_edits_enabled=experimental_config.msc3958_supress_edit_notifs, ) return filtered_rules From 9e89df46e27a33b688607b21c888284abd669671 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 31 Jan 2023 14:41:44 -0500 Subject: [PATCH 3/6] Newsfragment --- changelog.d/14960.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14960.feature diff --git a/changelog.d/14960.feature b/changelog.d/14960.feature new file mode 100644 index 000000000000..b9bb331273a7 --- /dev/null +++ b/changelog.d/14960.feature @@ -0,0 +1 @@ +Experimental support to suppress notifications from message edits ([MSC3958](https://github.com/matrix-org/matrix-spec-proposals/pull/3958)). From c5dd292511e5ddacf11cd5241020d1a617408868 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Feb 2023 12:29:03 -0500 Subject: [PATCH 4/6] Add a test. --- tests/push/test_bulk_push_rule_evaluator.py | 42 ++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py index 3b2d082dcb2b..10ccc073cf1e 100644 --- a/tests/push/test_bulk_push_rule_evaluator.py +++ b/tests/push/test_bulk_push_rule_evaluator.py @@ -19,7 +19,7 @@ from twisted.test.proto_helpers import MemoryReactor -from synapse.api.constants import EventContentFields +from synapse.api.constants import EventContentFields, RelationTypes from synapse.api.room_versions import RoomVersions from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator from synapse.rest import admin @@ -370,3 +370,43 @@ def test_room_mentions(self) -> None: }, ) ) + + @override_config({"experimental_features": {"msc3958_supress_edit_notifs": True}}) + def test_suppress_edits(self) -> None: + """Event edits should not generate notifications.""" + bulk_evaluator = BulkPushRuleEvaluator(self.hs) + + # Create & persist an event to use as the parent of the relation. + event, context = self.get_success( + self.event_creation_handler.create_event( + self.requester, + { + "type": "m.room.message", + "room_id": self.room_id, + "content": { + "msgtype": "m.text", + "body": "helo", + }, + "sender": self.alice, + }, + ) + ) + self.get_success( + self.event_creation_handler.handle_new_client_event( + self.requester, events_and_context=[(event, context)] + ) + ) + + # Room mentions from those without power should not notify. + self.assertFalse( + self._create_and_process( + bulk_evaluator, + { + "body": self.alice, + "m.relates_to": { + "rel_type": RelationTypes.REPLACE, + "event_id": event.event_id, + }, + }, + ) + ) From 4051d63f1afc92972a878ea932ee172d0aeae5b4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Feb 2023 12:37:06 -0500 Subject: [PATCH 5/6] Clarify comment. Co-authored-by: David Robertson --- tests/push/test_bulk_push_rule_evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py index 10ccc073cf1e..2c77e5267250 100644 --- a/tests/push/test_bulk_push_rule_evaluator.py +++ b/tests/push/test_bulk_push_rule_evaluator.py @@ -373,7 +373,7 @@ def test_room_mentions(self) -> None: @override_config({"experimental_features": {"msc3958_supress_edit_notifs": True}}) def test_suppress_edits(self) -> None: - """Event edits should not generate notifications.""" +"""Under the default push rules, event edits should not generate notifications.""" bulk_evaluator = BulkPushRuleEvaluator(self.hs) # Create & persist an event to use as the parent of the relation. From 9b2088700b0d5686f2ed43dcce47f604aff7d605 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Feb 2023 13:09:33 -0500 Subject: [PATCH 6/6] Fix formatting. --- tests/push/test_bulk_push_rule_evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py index 2c77e5267250..7567756135b7 100644 --- a/tests/push/test_bulk_push_rule_evaluator.py +++ b/tests/push/test_bulk_push_rule_evaluator.py @@ -373,7 +373,7 @@ def test_room_mentions(self) -> None: @override_config({"experimental_features": {"msc3958_supress_edit_notifs": True}}) def test_suppress_edits(self) -> None: -"""Under the default push rules, event edits should not generate notifications.""" + """Under the default push rules, event edits should not generate notifications.""" bulk_evaluator = BulkPushRuleEvaluator(self.hs) # Create & persist an event to use as the parent of the relation.