From fd952407121ad4d73f2669f6f1eba86eee934b08 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 10 Feb 2023 10:28:45 -0500 Subject: [PATCH] temp temp temp This won't work since it will failed if mentions: {} --- rust/benches/evaluator.rs | 12 +++----- rust/src/push/evaluator.rs | 37 +++++++++--------------- rust/src/push/mod.rs | 13 --------- stubs/synapse/synapse_rust/push.pyi | 5 ++-- synapse/push/bulk_push_rule_evaluator.py | 26 ++--------------- tests/push/test_push_rule_evaluator.py | 29 +------------------ 6 files changed, 22 insertions(+), 100 deletions(-) diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index efd19a216541..33d6c6a95f0a 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -43,8 +43,6 @@ fn bench_match_exact(b: &mut Bencher) { let eval = PushRuleEvaluator::py_new( flattened_keys, - false, - BTreeSet::new(), 10, Some(0), Default::default(), @@ -54,6 +52,7 @@ fn bench_match_exact(b: &mut Bencher) { false, false, false, + false, ) .unwrap(); @@ -92,8 +91,6 @@ fn bench_match_word(b: &mut Bencher) { let eval = PushRuleEvaluator::py_new( flattened_keys, - false, - BTreeSet::new(), 10, Some(0), Default::default(), @@ -103,6 +100,7 @@ fn bench_match_word(b: &mut Bencher) { false, false, false, + false, ) .unwrap(); @@ -141,8 +139,6 @@ fn bench_match_word_miss(b: &mut Bencher) { let eval = PushRuleEvaluator::py_new( flattened_keys, - false, - BTreeSet::new(), 10, Some(0), Default::default(), @@ -152,6 +148,7 @@ fn bench_match_word_miss(b: &mut Bencher) { false, false, false, + false, ) .unwrap(); @@ -190,8 +187,6 @@ fn bench_eval_message(b: &mut Bencher) { let eval = PushRuleEvaluator::py_new( flattened_keys, - false, - BTreeSet::new(), 10, Some(0), Default::default(), @@ -201,6 +196,7 @@ fn bench_eval_message(b: &mut Bencher) { false, false, false, + false, ) .unwrap(); diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 9530192db20a..0c1fc79bf276 100644 --- 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::JsonValue; use anyhow::{Context, Error}; @@ -70,11 +70,6 @@ pub struct PushRuleEvaluator { /// The "content.body", if any. body: String, - /// 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, - /// The number of users in the room. room_member_count: u64, @@ -104,6 +99,9 @@ pub struct PushRuleEvaluator { /// If MSC3966 (exact_event_property_contains push rule condition) is enabled. msc3966_exact_event_property_contains: bool, + + /// True if the event has a mentions property and MSC3952 support is enabled. + msc3952_intentional_mentions_enabled: bool, } #[pymethods] @@ -113,8 +111,6 @@ impl PushRuleEvaluator { #[new] pub fn py_new( flattened_keys: BTreeMap, - has_mentions: bool, - user_mentions: BTreeSet, room_member_count: u64, sender_power_level: Option, notification_power_levels: BTreeMap, @@ -124,6 +120,7 @@ impl PushRuleEvaluator { msc3931_enabled: bool, msc3758_exact_event_match: bool, msc3966_exact_event_property_contains: bool, + msc3952_intentional_mentions_enabled: bool, ) -> Result { let body = match flattened_keys.get("content.body") { Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone(), @@ -133,8 +130,6 @@ impl PushRuleEvaluator { Ok(PushRuleEvaluator { flattened_keys, body, - has_mentions, - user_mentions, room_member_count, notification_power_levels, sender_power_level, @@ -144,6 +139,7 @@ impl PushRuleEvaluator { msc3931_enabled, msc3758_exact_event_match, msc3966_exact_event_property_contains, + msc3952_intentional_mentions_enabled, }) } @@ -170,9 +166,11 @@ impl PushRuleEvaluator { // 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 self.has_mentions + // experimental feature is enabled). + if self.msc3952_intentional_mentions_enabled + && self + .flattened_keys + .contains_key("contents.org.matrix.msc3952.mentions") && (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") @@ -269,13 +267,6 @@ impl PushRuleEvaluator { KnownCondition::ExactEventPropertyContains(exact_event_match) => { self.match_exact_event_property_contains(exact_event_match, user_id)? } - 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() { @@ -546,8 +537,6 @@ fn push_rule_evaluator() { ); let evaluator = PushRuleEvaluator::py_new( flattened_keys, - false, - BTreeSet::new(), 10, Some(0), BTreeMap::new(), @@ -557,6 +546,7 @@ fn push_rule_evaluator() { true, true, true, + true, ) .unwrap(); @@ -578,8 +568,6 @@ fn test_requires_room_version_supports_condition() { let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()]; let evaluator = PushRuleEvaluator::py_new( flattened_keys, - false, - BTreeSet::new(), 10, Some(0), BTreeMap::new(), @@ -589,6 +577,7 @@ fn test_requires_room_version_supports_condition() { true, true, true, + true, ) .unwrap(); diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 3ae2912ce5d2..189cc096b2bf 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -334,8 +334,6 @@ pub enum KnownCondition { RelatedEventMatch(RelatedEventMatchCondition), #[serde(rename = "org.matrix.msc3966.exact_event_property_contains")] ExactEventPropertyContains(ExactEventMatchCondition), - #[serde(rename = "org.matrix.msc3952.is_user_mention")] - IsUserMention, ContainsDisplayName, RoomMemberCount { #[serde(skip_serializing_if = "Option::is_none")] @@ -657,17 +655,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 index a8f0ed2435c5..7930af399bcf 100644 --- 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 @@ -57,8 +57,6 @@ class PushRuleEvaluator: def __init__( 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], @@ -68,6 +66,7 @@ class PushRuleEvaluator: msc3931_enabled: bool, msc3758_exact_event_match: bool, msc3966_exact_event_property_contains: bool, + msc3952_intentional_mentions_enabled: bool, ): ... def run( self, diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 5fc38431babe..a3ca6fce1fe3 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -23,20 +23,13 @@ Mapping, Optional, Sequence, - Set, Tuple, Union, ) from prometheus_client import Counter -from synapse.api.constants import ( - MAIN_TIMELINE, - EventContentFields, - EventTypes, - Membership, - RelationTypes, -) +from synapse.api.constants import MAIN_TIMELINE, EventTypes, Membership, RelationTypes from synapse.api.room_versions import PushRuleRoomFlag from synapse.event_auth import auth_types_for_event, get_user_power_level from synapse.events import EventBase, relation_from_event @@ -396,27 +389,11 @@ async def _action_for_event_by_user( except (TypeError, ValueError): 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) - ) - evaluator = PushRuleEvaluator( _flatten_dict( event, msc3783_escape_event_match_key=self.hs.config.experimental.msc3783_escape_event_match_key, ), - has_mentions, - user_mentions, room_member_count, sender_power_level, notification_levels, @@ -426,6 +403,7 @@ async def _action_for_event_by_user( self.hs.config.experimental.msc1767_enabled, # MSC3931 flag self.hs.config.experimental.msc3758_exact_event_match, self.hs.config.experimental.msc3966_exact_event_property_contains, + self.hs.config.experimental.msc3952_intentional_mentions, ) users = rules_by_user.keys() diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index d320a12f968d..c883e3efc7c8 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -167,8 +167,6 @@ def _get_evaluator( power_levels: Dict[str, Union[int, Dict[str, int]]] = {} return PushRuleEvaluator( _flatten_dict(event), - has_mentions, - user_mentions or set(), room_member_count, sender_power_level, cast(Dict[str, int], power_levels.get("notifications", {})), @@ -178,6 +176,7 @@ def _get_evaluator( msc3931_enabled=True, msc3758_exact_event_match=True, msc3966_exact_event_property_contains=True, + msc3952_intention_mentions=True, ) def test_display_name(self) -> None: @@ -204,32 +203,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: