Remove the is_user_mention support.

matrix-org · Feb 28, 2023 · b30a1a8 · b30a1a8
1 parent 7c4a3a5
commit b30a1a8
Show file tree

Hide file tree

Showing 6 changed files with 8 additions and 78 deletions.
diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs
@@ -44,7 +44,6 @@ fn bench_match_exact(b: &mut Bencher) {
     let eval = PushRuleEvaluator::py_new(
         flattened_keys,
         false,
-        BTreeSet::new(),
         10,
         Some(0),
         Default::default(),
@@ -92,7 +91,6 @@ fn bench_match_word(b: &mut Bencher) {
     let eval = PushRuleEvaluator::py_new(
         flattened_keys,
         false,
-        BTreeSet::new(),
         10,
         Some(0),
         Default::default(),
@@ -140,7 +138,6 @@ fn bench_match_word_miss(b: &mut Bencher) {
     let eval = PushRuleEvaluator::py_new(
         flattened_keys,
         false,
-        BTreeSet::new(),
         10,
         Some(0),
         Default::default(),
@@ -188,7 +185,6 @@ fn bench_eval_message(b: &mut Bencher) {
     let eval = PushRuleEvaluator::py_new(
         flattened_keys,
         false,
-        BTreeSet::new(),
         10,
         Some(0),
         Default::default(),

diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs
@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::borrow::Cow;
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::BTreeMap;
 
 use crate::push::{EventMatchPatternType, JsonValue};
 use anyhow::{Context, Error};
@@ -72,8 +72,6 @@ pub struct PushRuleEvaluator {
 
     /// True if the event has a mentions property and MSC3952 support is enabled.
     has_mentions: bool,
-    /// The user mentions that were part of the message.
-    user_mentions: BTreeSet<String>,
 
     /// The number of users in the room.
     room_member_count: u64,
@@ -114,7 +112,6 @@ impl PushRuleEvaluator {
     pub fn py_new(
         flattened_keys: BTreeMap<String, JsonValue>,
         has_mentions: bool,
-        user_mentions: BTreeSet<String>,
         room_member_count: u64,
         sender_power_level: Option<i64>,
         notification_power_levels: BTreeMap<String, i64>,
@@ -134,7 +131,6 @@ impl PushRuleEvaluator {
             flattened_keys,
             body,
             has_mentions,
-            user_mentions,
             room_member_count,
             notification_power_levels,
             sender_power_level,
@@ -335,13 +331,6 @@ impl PushRuleEvaluator {
                     Cow::Borrowed(&SimpleJsonValue::Str(pattern.to_string())),
                 )?
             }
-            KnownCondition::IsUserMention => {
-                if let Some(uid) = user_id {
-                    self.user_mentions.contains(uid)
-                } else {
-                    false
-                }
-            }
             KnownCondition::ContainsDisplayName => {
                 if let Some(dn) = display_name {
                     if !dn.is_empty() {
@@ -529,7 +518,6 @@ fn push_rule_evaluator() {
     let evaluator = PushRuleEvaluator::py_new(
         flattened_keys,
         false,
-        BTreeSet::new(),
         10,
         Some(0),
         BTreeMap::new(),
@@ -561,7 +549,6 @@ fn test_requires_room_version_supports_condition() {
     let evaluator = PushRuleEvaluator::py_new(
         flattened_keys,
         false,
-        BTreeSet::new(),
         10,
         Some(0),
         BTreeMap::new(),

diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs
@@ -346,8 +346,6 @@ pub enum KnownCondition {
         rename = "org.matrix.msc3966.exact_event_property_contains"
     )]
     ExactEventPropertyContainsType(ExactEventMatchTypeCondition),
-    #[serde(rename = "org.matrix.msc3952.is_user_mention")]
-    IsUserMention,
     ContainsDisplayName,
     RoomMemberCount {
         #[serde(skip_serializing_if = "Option::is_none")]
@@ -754,17 +752,6 @@ fn test_deserialize_unstable_msc3758_condition() {
     ));
 }
 
-#[test]
-fn test_deserialize_unstable_msc3952_user_condition() {
-    let json = r#"{"kind":"org.matrix.msc3952.is_user_mention"}"#;
-
-    let condition: Condition = serde_json::from_str(json).unwrap();
-    assert!(matches!(
-        condition,
-        Condition::Known(KnownCondition::IsUserMention)
-    ));
-}
-
 #[test]
 fn test_deserialize_custom_condition() {
     let json = r#"{"kind":"custom_tag"}"#;

diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Set, Tuple, Union
+from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Tuple, Union
 
 from synapse.types import JsonDict, JsonValue
 
@@ -58,7 +58,6 @@ class PushRuleEvaluator:
         self,
         flattened_keys: Mapping[str, JsonValue],
         has_mentions: bool,
-        user_mentions: Set[str],
         room_member_count: int,
         sender_power_level: Optional[int],
         notification_power_levels: Mapping[str, int],

diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py
@@ -23,7 +23,6 @@
     Mapping,
     Optional,
     Sequence,
-    Set,
     Tuple,
     Union,
 )
@@ -396,26 +395,17 @@ async def _action_for_event_by_user(
                         del notification_levels[key]
 
         # Pull out any user and room mentions.
-        mentions = event.content.get(EventContentFields.MSC3952_MENTIONS)
-        has_mentions = self._intentional_mentions_enabled and isinstance(mentions, dict)
-        user_mentions: Set[str] = set()
-        if has_mentions:
-            # mypy seems to have lost the type even though it must be a dict here.
-            assert isinstance(mentions, dict)
-            # Remove out any non-string items and convert to a set.
-            user_mentions_raw = mentions.get("user_ids")
-            if isinstance(user_mentions_raw, list):
-                user_mentions = set(
-                    filter(lambda item: isinstance(item, str), user_mentions_raw)
-                )
+        has_mentions = (
+            self._intentional_mentions_enabled
+            and EventContentFields.MSC3952_MENTIONS in event.content
+        )
 
         evaluator = PushRuleEvaluator(
             _flatten_dict(
                 event,
                 msc3873_escape_event_match_key=self.hs.config.experimental.msc3873_escape_event_match_key,
             ),
             has_mentions,
-            user_mentions,
             room_member_count,
             sender_power_level,
             notification_levels,

diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import Any, Dict, List, Optional, Set, Union, cast
+from typing import Any, Dict, List, Optional, Union, cast
 
 import frozendict
 
@@ -147,8 +147,6 @@ def _get_evaluator(
         self,
         content: JsonMapping,
         *,
-        has_mentions: bool = False,
-        user_mentions: Optional[Set[str]] = None,
         related_events: Optional[JsonDict] = None,
     ) -> PushRuleEvaluator:
         event = FrozenEvent(
@@ -167,8 +165,7 @@ def _get_evaluator(
         power_levels: Dict[str, Union[int, Dict[str, int]]] = {}
         return PushRuleEvaluator(
             _flatten_dict(event),
-            has_mentions,
-            user_mentions or set(),
+            False,
             room_member_count,
             sender_power_level,
             cast(Dict[str, int], power_levels.get("notifications", {})),
@@ -204,32 +201,6 @@ def test_display_name(self) -> None:
         # A display name with spaces should work fine.
         self.assertTrue(evaluator.matches(condition, "@user:test", "foo bar"))
 
-    def test_user_mentions(self) -> None:
-        """Check for user mentions."""
-        condition = {"kind": "org.matrix.msc3952.is_user_mention"}
-
-        # No mentions shouldn't match.
-        evaluator = self._get_evaluator({}, has_mentions=True)
-        self.assertFalse(evaluator.matches(condition, "@user:test", None))
-
-        # An empty set shouldn't match
-        evaluator = self._get_evaluator({}, has_mentions=True, user_mentions=set())
-        self.assertFalse(evaluator.matches(condition, "@user:test", None))
-
-        # The Matrix ID appearing anywhere in the mentions list should match
-        evaluator = self._get_evaluator(
-            {}, has_mentions=True, user_mentions={"@user:test"}
-        )
-        self.assertTrue(evaluator.matches(condition, "@user:test", None))
-
-        evaluator = self._get_evaluator(
-            {}, has_mentions=True, user_mentions={"@another:test", "@user:test"}
-        )
-        self.assertTrue(evaluator.matches(condition, "@user:test", None))
-
-        # Note that invalid data is tested at tests.push.test_bulk_push_rule_evaluator.TestBulkPushRuleEvaluator.test_mentions
-        # since the BulkPushRuleEvaluator is what handles data sanitisation.
-
     def _assert_matches(
         self, condition: JsonDict, content: JsonMapping, msg: Optional[str] = None
     ) -> None: