Stabilize support for MSC3952: Intentional mentions. (#15520)

13 files changed, 27 insertions, 64 deletions

diff --git a/changelog.d/15520.feature b/changelog.d/15520.feature
new file mode 100644
index 0000000000..f4fd40ab94
--- /dev/null
+++ b/changelog.d/15520.feature
@@ -0,0 +1 @@
+Enable support for [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952): intentional mentions.
diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs
index 64e13f6486..c2f33258a4 100644
--- a/rust/benches/evaluator.rs
+++ b/rust/benches/evaluator.rs
@@ -13,8 +13,6 @@
 // limitations under the License.
 
 #![feature(test)]
-use std::collections::BTreeSet;
-
 use synapse::push::{
     evaluator::PushRuleEvaluator, Condition, EventMatchCondition, FilteredPushRules, JsonValue,
     PushRules, SimpleJsonValue,
@@ -197,7 +195,6 @@ 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 51372e1553..9d6c304d92 100644
--- a/rust/src/push/base_rules.rs
+++ b/rust/src/push/base_rules.rs
@@ -142,11 +142,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
         default_enabled: true,
     },
     PushRule {
-        rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"),
+        rule_id: Cow::Borrowed("global/override/.m.is_user_mention"),
         priority_class: 5,
         conditions: Cow::Borrowed(&[Condition::Known(
             KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition {
-                key: Cow::Borrowed("content.org\\.matrix\\.msc3952\\.mentions.user_ids"),
+                key: Cow::Borrowed("content.m\\.mentions.user_ids"),
                 value_type: Cow::Borrowed(&EventMatchPatternType::UserId),
             }),
         )]),
@@ -163,11 +163,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
         default_enabled: true,
     },
     PushRule {
-        rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mention"),
+        rule_id: Cow::Borrowed("global/override/.m.is_room_mention"),
         priority_class: 5,
         conditions: Cow::Borrowed(&[
             Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition {
-                key: Cow::Borrowed("content.org\\.matrix\\.msc3952\\.mentions.room"),
+                key: Cow::Borrowed("content.m\\.mentions.room"),
                 value: Cow::Borrowed(&SimpleJsonValue::Bool(true)),
             })),
             Condition::Known(KnownCondition::SenderNotificationPermission {
diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs
index 2d7c4c06be..59c53b1776 100644
--- a/rust/src/push/evaluator.rs
+++ b/rust/src/push/evaluator.rs
@@ -70,7 +70,9 @@ pub struct PushRuleEvaluator {
     /// The "content.body", if any.
     body: String,
 
-    /// True if the event has a mentions property and MSC3952 support is enabled.
+    /// True if the event has a m.mentions property. (Note that this is a separate
+    /// flag instead of checking flattened_keys since the m.mentions property
+    /// might be an empty map and not appear in flattened_keys.
     has_mentions: bool,
 
     /// The number of users in the room.
@@ -155,9 +157,7 @@ impl PushRuleEvaluator {
             let rule_id = &push_rule.rule_id().to_string();
 
             // For backwards-compatibility the legacy mention rules are disabled
-            // if the event contains the 'm.mentions' property (and if the
-            // experimental feature is enabled, both of these are represented
-            // by the has_mentions flag).
+            // if the event contains the 'm.mentions' property.
             if self.has_mentions
                 && (rule_id == "global/override/.m.rule.contains_display_name"
                     || rule_id == "global/content/.m.rule.contains_user_name"
@@ -562,7 +562,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, false),
+        &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false),
         None,
         None,
     );
diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs
index f19d3c739f..514980579b 100644
--- a/rust/src/push/mod.rs
+++ b/rust/src/push/mod.rs
@@ -527,7 +527,6 @@ pub struct FilteredPushRules {
     msc1767_enabled: bool,
     msc3381_polls_enabled: bool,
     msc3664_enabled: bool,
-    msc3952_intentional_mentions: bool,
     msc3958_suppress_edits_enabled: bool,
 }
 
@@ -540,7 +539,6 @@ impl FilteredPushRules {
         msc1767_enabled: bool,
         msc3381_polls_enabled: bool,
         msc3664_enabled: bool,
-        msc3952_intentional_mentions: bool,
         msc3958_suppress_edits_enabled: bool,
     ) -> Self {
         Self {
@@ -549,7 +547,6 @@ impl FilteredPushRules {
             msc1767_enabled,
             msc3381_polls_enabled,
             msc3664_enabled,
-            msc3952_intentional_mentions,
             msc3958_suppress_edits_enabled,
         }
     }
@@ -587,10 +584,6 @@ impl FilteredPushRules {
                     return false;
                 }
 
-                if !self.msc3952_intentional_mentions && rule.rule_id.contains("org.matrix.msc3952")
-                {
-                    return false;
-                }
                 if !self.msc3958_suppress_edits_enabled
                     && rule.rule_id == "global/override/.com.beeper.suppress_edits"
                 {
diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi
index 5d0ce4b1a4..d573a37b9a 100644
--- a/stubs/synapse/synapse_rust/push.pyi
+++ b/stubs/synapse/synapse_rust/push.pyi
@@ -46,7 +46,6 @@ class FilteredPushRules:
         msc1767_enabled: bool,
         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/api/constants.py b/synapse/api/constants.py
index cde9a2ecef..faf0770c66 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -236,7 +236,7 @@ class EventContentFields:
     AUTHORISING_USER: Final = "join_authorised_via_users_server"
 
     # Use for mentioning users.
-    MSC3952_MENTIONS: Final = "org.matrix.msc3952.mentions"
+    MENTIONS: Final = "m.mentions"
 
     # an unspecced field added to to-device messages to identify them uniquely-ish
     TO_DEVICE_MSGID: Final = "org.matrix.msgid"
diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py
index a9e002cf08..1d5b5ded45 100644
--- a/synapse/config/experimental.py
+++ b/synapse/config/experimental.py
@@ -358,11 +358,6 @@ class ExperimentalConfig(Config):
         # MSC3391: Removing account data.
         self.msc3391_enabled = experimental.get("msc3391_enabled", False)
 
-        # MSC3952: Intentional mentions, this depends on MSC3966.
-        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/events/validator.py b/synapse/events/validator.py
index 47203209db..9278f1a1aa 100644
--- a/synapse/events/validator.py
+++ b/synapse/events/validator.py
@@ -134,13 +134,8 @@ class EventValidator:
                 )
 
         # If the event contains a mentions key, validate it.
-        if (
-            EventContentFields.MSC3952_MENTIONS in event.content
-            and config.experimental.msc3952_intentional_mentions
-        ):
-            validate_json_object(
-                event.content[EventContentFields.MSC3952_MENTIONS], Mentions
-            )
+        if EventContentFields.MENTIONS in event.content:
+            validate_json_object(event.content[EventContentFields.MENTIONS], Mentions)
 
     def _validate_retention(self, event: EventBase) -> None:
         """Checks that an event that defines the retention policy for a room respects the
diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py
index 320084f5f5..33002cc0f2 100644
--- a/synapse/push/bulk_push_rule_evaluator.py
+++ b/synapse/push/bulk_push_rule_evaluator.py
@@ -120,9 +120,6 @@ class BulkPushRuleEvaluator:
         self.should_calculate_push_rules = self.hs.config.push.enable_push
 
         self._related_event_match_enabled = self.hs.config.experimental.msc3664_enabled
-        self._intentional_mentions_enabled = (
-            self.hs.config.experimental.msc3952_intentional_mentions
-        )
 
         self.room_push_rule_cache_metrics = register_cache(
             "cache",
@@ -390,10 +387,7 @@ class BulkPushRuleEvaluator:
                         del notification_levels[key]
 
         # Pull out any user and room mentions.
-        has_mentions = (
-            self._intentional_mentions_enabled
-            and EventContentFields.MSC3952_MENTIONS in event.content
-        )
+        has_mentions = EventContentFields.MENTIONS in event.content
 
         evaluator = PushRuleEvaluator(
             _flatten_dict(event),
diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py
index 547bf34df1..1910648755 100644
--- a/synapse/rest/client/versions.py
+++ b/synapse/rest/client/versions.py
@@ -124,8 +124,6 @@ class VersionsRestServlet(RestServlet):
                     is not None,
                     # Adds support for relation-based redactions as per MSC3912.
                     "org.matrix.msc3912": self.config.experimental.msc3912_enabled,
-                    # Adds support for unstable "intentional mentions" behaviour.
-                    "org.matrix.msc3952_intentional_mentions": self.config.experimental.msc3952_intentional_mentions,
                     # Whether recursively provide relations is supported.
                     "org.matrix.msc3981": self.config.experimental.msc3981_recurse_relations,
                     # Adds support for deleting account data.
diff --git a/synapse/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py
index 9f862f00c1..e098ceea3c 100644
--- a/synapse/storage/databases/main/push_rule.py
+++ b/synapse/storage/databases/main/push_rule.py
@@ -88,7 +88,6 @@ def _load_rules(
         msc1767_enabled=experimental_config.msc1767_enabled,
         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,
     )
 
diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py
index 9501096a77..1e06f86071 100644
--- a/tests/push/test_bulk_push_rule_evaluator.py
+++ b/tests/push/test_bulk_push_rule_evaluator.py
@@ -228,7 +228,6 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
         )
         return len(result) > 0
 
-    @override_config({"experimental_features": {"msc3952_intentional_mentions": True}})
     def test_user_mentions(self) -> None:
         """Test the behavior of an event which includes invalid user mentions."""
         bulk_evaluator = BulkPushRuleEvaluator(self.hs)
@@ -237,9 +236,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
         self.assertFalse(self._create_and_process(bulk_evaluator))
         # An empty mentions field should not notify.
         self.assertFalse(
-            self._create_and_process(
-                bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {}}
-            )
+            self._create_and_process(bulk_evaluator, {EventContentFields.MENTIONS: {}})
         )
 
         # Non-dict mentions should be ignored.
@@ -253,7 +250,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
             for mentions in (None, True, False, 1, "foo", []):
                 self.assertFalse(
                     self._create_and_process(
-                        bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: mentions}
+                        bulk_evaluator, {EventContentFields.MENTIONS: mentions}
                     )
                 )
 
@@ -262,7 +259,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                 self.assertFalse(
                     self._create_and_process(
                         bulk_evaluator,
-                        {EventContentFields.MSC3952_MENTIONS: {"user_ids": mentions}},
+                        {EventContentFields.MENTIONS: {"user_ids": mentions}},
                     )
                 )
 
@@ -270,14 +267,14 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
         self.assertTrue(
             self._create_and_process(
                 bulk_evaluator,
-                {EventContentFields.MSC3952_MENTIONS: {"user_ids": [self.alice]}},
+                {EventContentFields.MENTIONS: {"user_ids": [self.alice]}},
             )
         )
         self.assertTrue(
             self._create_and_process(
                 bulk_evaluator,
                 {
-                    EventContentFields.MSC3952_MENTIONS: {
+                    EventContentFields.MENTIONS: {
                         "user_ids": ["@another:test", self.alice]
                     }
                 },
@@ -288,11 +285,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
         self.assertTrue(
             self._create_and_process(
                 bulk_evaluator,
-                {
-                    EventContentFields.MSC3952_MENTIONS: {
-                        "user_ids": [self.alice, self.alice]
-                    }
-                },
+                {EventContentFields.MENTIONS: {"user_ids": [self.alice, self.alice]}},
             )
         )
 
@@ -307,7 +300,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                 self._create_and_process(
                     bulk_evaluator,
                     {
-                        EventContentFields.MSC3952_MENTIONS: {
+                        EventContentFields.MENTIONS: {
                             "user_ids": [None, True, False, {}, []]
                         }
                     },
@@ -317,7 +310,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                 self._create_and_process(
                     bulk_evaluator,
                     {
-                        EventContentFields.MSC3952_MENTIONS: {
+                        EventContentFields.MENTIONS: {
                             "user_ids": [None, True, False, {}, [], self.alice]
                         }
                     },
@@ -331,12 +324,11 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                 {
                     "body": self.alice,
                     "msgtype": "m.text",
-                    EventContentFields.MSC3952_MENTIONS: {},
+                    EventContentFields.MENTIONS: {},
                 },
             )
         )
 
-    @override_config({"experimental_features": {"msc3952_intentional_mentions": True}})
     def test_room_mentions(self) -> None:
         """Test the behavior of an event which includes invalid room mentions."""
         bulk_evaluator = BulkPushRuleEvaluator(self.hs)
@@ -344,7 +336,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
         # Room mentions from those without power should not notify.
         self.assertFalse(
             self._create_and_process(
-                bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}}
+                bulk_evaluator, {EventContentFields.MENTIONS: {"room": True}}
             )
         )
 
@@ -358,7 +350,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
         )
         self.assertTrue(
             self._create_and_process(
-                bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}}
+                bulk_evaluator, {EventContentFields.MENTIONS: {"room": True}}
             )
         )
 
@@ -374,7 +366,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                 self.assertFalse(
                     self._create_and_process(
                         bulk_evaluator,
-                        {EventContentFields.MSC3952_MENTIONS: {"room": mentions}},
+                        {EventContentFields.MENTIONS: {"room": mentions}},
                     )
                 )
 
@@ -385,7 +377,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
                 {
                     "body": "@room",
                     "msgtype": "m.text",
-                    EventContentFields.MSC3952_MENTIONS: {},
+                    EventContentFields.MENTIONS: {},
                 },
             )
         )