From 979f237b282cbdaab8d74cc4c7473117093d63d9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 09:51:22 -0500 Subject: Update intentional mentions (MSC3952) to depend on `exact_event_match` (MSC3758). (#15037) This replaces the specific `is_room_mention` push rule condition used in MSC3952 with the generic `exact_event_match` push rule condition from MSC3758. No functionality changes due to this. --- stubs/synapse/synapse_rust/push.pyi | 1 - 1 file changed, 1 deletion(-) (limited to 'stubs/synapse/synapse_rust/push.pyi') diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi index 7b33c30cc9..a8f0ed2435 100644 --- a/stubs/synapse/synapse_rust/push.pyi +++ b/stubs/synapse/synapse_rust/push.pyi @@ -59,7 +59,6 @@ class PushRuleEvaluator: flattened_keys: Mapping[str, JsonValue], has_mentions: bool, user_mentions: Set[str], - room_mention: bool, room_member_count: int, sender_power_level: Optional[int], notification_power_levels: Mapping[str, int], -- cgit 1.5.1 From 8ef324ea6f1390876940989eacc8734fe0d15582 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Mar 2023 08:30:51 -0500 Subject: Update intentional mentions (MSC3952) to depend on `exact_event_property_contains` (MSC3966). (#15051) This replaces the specific `is_user_mention` push rule condition used in MSC3952 with the generic `exact_event_property_contains` push rule condition from MSC3966. --- changelog.d/15051.misc | 1 + rust/benches/evaluator.rs | 4 --- rust/src/push/base_rules.rs | 9 ++++-- rust/src/push/evaluator.rs | 50 ++++++++++++++++------------- rust/src/push/mod.rs | 28 ++++++++-------- stubs/synapse/synapse_rust/push.pyi | 3 +- synapse/config/experimental.py | 8 ++++- synapse/push/bulk_push_rule_evaluator.py | 18 +++-------- synapse/push/clientformat.py | 11 ++++--- tests/push/test_bulk_push_rule_evaluator.py | 2 ++ tests/push/test_push_rule_evaluator.py | 33 ++----------------- 11 files changed, 73 insertions(+), 94 deletions(-) create mode 100644 changelog.d/15051.misc (limited to 'stubs/synapse/synapse_rust/push.pyi') diff --git a/changelog.d/15051.misc b/changelog.d/15051.misc new file mode 100644 index 0000000000..fabfe77d35 --- /dev/null +++ b/changelog.d/15051.misc @@ -0,0 +1 @@ +Update [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952) support based on changes to the MSC. diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index 9a871f5693..7c987d4948 100644 --- 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/base_rules.rs b/rust/src/push/base_rules.rs index 62de51d915..3d72a4a4c3 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -21,13 +21,13 @@ use lazy_static::lazy_static; use serde_json::Value; use super::KnownCondition; -use crate::push::PushRule; use crate::push::RelatedEventMatchTypeCondition; use crate::push::SetTweak; use crate::push::TweakValue; use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue}; use crate::push::{Condition, EventMatchTypeCondition}; use crate::push::{EventMatchCondition, EventMatchPatternType}; +use crate::push::{ExactEventMatchTypeCondition, PushRule}; const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak { set_tweak: Cow::Borrowed("highlight"), @@ -144,7 +144,12 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"), priority_class: 5, - conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::IsUserMention)]), + conditions: Cow::Borrowed(&[Condition::Known( + KnownCondition::ExactEventPropertyContainsType(ExactEventMatchTypeCondition { + key: Cow::Borrowed("content.org.matrix.msc3952.mentions.user_ids"), + value_type: Cow::Borrowed(&EventMatchPatternType::UserId), + }), + )]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), default: true, default_enabled: true, diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index a65c645caf..55846627cc 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::{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, /// The number of users in the room. room_member_count: u64, @@ -114,7 +112,6 @@ impl PushRuleEvaluator { 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, @@ -134,7 +131,6 @@ impl PushRuleEvaluator { flattened_keys, body, has_mentions, - user_mentions, room_member_count, notification_power_levels, sender_power_level, @@ -310,15 +306,30 @@ impl PushRuleEvaluator { Some(Cow::Borrowed(pattern)), )? } - KnownCondition::ExactEventPropertyContains(exact_event_match) => { - self.match_exact_event_property_contains(exact_event_match)? - } - KnownCondition::IsUserMention => { - if let Some(uid) = user_id { - self.user_mentions.contains(uid) + KnownCondition::ExactEventPropertyContains(exact_event_match) => self + .match_exact_event_property_contains( + exact_event_match.key.clone(), + exact_event_match.value.clone(), + )?, + KnownCondition::ExactEventPropertyContainsType(exact_event_match) => { + // The `pattern_type` can either be "user_id" or "user_localpart", + // either way if we don't have a `user_id` then the condition can't + // match. + let user_id = if let Some(user_id) = user_id { + user_id } else { - false - } + return Ok(false); + }; + + let pattern = match &*exact_event_match.value_type { + EventMatchPatternType::UserId => user_id, + EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?, + }; + + self.match_exact_event_property_contains( + exact_event_match.key.clone(), + Cow::Borrowed(&SimpleJsonValue::Str(pattern.to_string())), + )? } KnownCondition::ContainsDisplayName => { if let Some(dn) = display_name { @@ -456,24 +467,21 @@ impl PushRuleEvaluator { /// Evaluates a `exact_event_property_contains` condition. (MSC3758) fn match_exact_event_property_contains( &self, - exact_event_match: &ExactEventMatchCondition, + key: Cow, + value: Cow, ) -> Result { // First check if the feature is enabled. if !self.msc3966_exact_event_property_contains { return Ok(false); } - let value = &exact_event_match.value; - - let haystack = if let Some(JsonValue::Array(haystack)) = - self.flattened_keys.get(&*exact_event_match.key) - { + let haystack = if let Some(JsonValue::Array(haystack)) = self.flattened_keys.get(&*key) { haystack } else { return Ok(false); }; - Ok(haystack.contains(&**value)) + Ok(haystack.contains(&value)) } /// Match the member count against an 'is' condition @@ -510,7 +518,6 @@ fn push_rule_evaluator() { let evaluator = PushRuleEvaluator::py_new( flattened_keys, false, - BTreeSet::new(), 10, Some(0), BTreeMap::new(), @@ -542,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 index 97feb6efc9..6391d2ed47 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -340,8 +340,12 @@ pub enum KnownCondition { RelatedEventMatchType(RelatedEventMatchTypeCondition), #[serde(rename = "org.matrix.msc3966.exact_event_property_contains")] ExactEventPropertyContains(ExactEventMatchCondition), - #[serde(rename = "org.matrix.msc3952.is_user_mention")] - IsUserMention, + // Identical to exact_event_property_contains but gives predefined patterns. Cannot be added by users. + #[serde( + skip_deserializing, + rename = "org.matrix.msc3966.exact_event_property_contains" + )] + ExactEventPropertyContainsType(ExactEventMatchTypeCondition), ContainsDisplayName, RoomMemberCount { #[serde(skip_serializing_if = "Option::is_none")] @@ -398,6 +402,15 @@ pub struct ExactEventMatchCondition { pub value: Cow<'static, SimpleJsonValue>, } +/// The body of a [`Condition::ExactEventMatch`] that uses user_id or user_localpart as a pattern. +#[derive(Serialize, Debug, Clone)] +pub struct ExactEventMatchTypeCondition { + pub key: Cow<'static, str>, + // During serialization, the pattern_type property gets replaced with a + // pattern property of the correct value in synapse.push.clientformat.format_push_rules_for_user. + pub value_type: Cow<'static, EventMatchPatternType>, +} + /// The body of a [`Condition::RelatedEventMatch`] #[derive(Serialize, Deserialize, Debug, Clone)] pub struct RelatedEventMatchCondition { @@ -739,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 index a8f0ed2435..c17796ffbd 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 @@ -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/config/experimental.py b/synapse/config/experimental.py index 7c81f055b6..fc64f2bda1 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -179,10 +179,16 @@ class ExperimentalConfig(Config): "msc3873_escape_event_match_key", False ) - # MSC3952: Intentional mentions, this depends on MSC3758. + # MSC3966: exact_event_property_contains push rule condition. + self.msc3966_exact_event_property_contains = experimental.get( + "msc3966_exact_event_property_contains", False + ) + + # MSC3952: Intentional mentions, this depends on MSC3758 and MSC3966. self.msc3952_intentional_mentions = ( experimental.get("msc3952_intentional_mentions", False) and self.msc3758_exact_event_match + and self.msc3966_exact_event_property_contains ) # MSC3959: Do not generate notifications for edits. diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 3c4a152d6b..abcf687f05 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -23,7 +23,6 @@ from typing import ( Mapping, Optional, Sequence, - Set, Tuple, Union, ) @@ -396,18 +395,10 @@ class BulkPushRuleEvaluator: 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( @@ -415,7 +406,6 @@ class BulkPushRuleEvaluator: 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/synapse/push/clientformat.py b/synapse/push/clientformat.py index bb76c169c6..222afbdcc8 100644 --- a/synapse/push/clientformat.py +++ b/synapse/push/clientformat.py @@ -41,11 +41,12 @@ def format_push_rules_for_user( rulearray.append(template_rule) - pattern_type = template_rule.pop("pattern_type", None) - if pattern_type == "user_id": - template_rule["pattern"] = user.to_string() - elif pattern_type == "user_localpart": - template_rule["pattern"] = user.localpart + for type_key in ("pattern", "value"): + type_value = template_rule.pop(f"{type_key}_type", None) + if type_value == "user_id": + template_rule[type_key] = user.to_string() + elif type_value == "user_localpart": + template_rule[type_key] = user.localpart template_rule["enabled"] = enabled diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py index 1458076a90..73fecfd4ad 100644 --- a/tests/push/test_bulk_push_rule_evaluator.py +++ b/tests/push/test_bulk_push_rule_evaluator.py @@ -233,6 +233,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase): "experimental_features": { "msc3758_exact_event_match": True, "msc3952_intentional_mentions": True, + "msc3966_exact_event_property_contains": True, } } ) @@ -336,6 +337,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase): "experimental_features": { "msc3758_exact_event_match": True, "msc3952_intentional_mentions": True, + "msc3966_exact_event_property_contains": True, } } ) diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index 1d30e3c3e4..d4a4bc4d93 100644 --- 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 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): 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 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): 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 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): # 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: -- cgit 1.5.1 From fd9cadcf532ce0dbd005541fe635b214aa6d2438 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 6 Mar 2023 08:38:01 -0500 Subject: Stabilize support for MSC3758: event_property_is push condition (#15185) This removes the configuration flag & updates the identifiers to use the stable version. --- changelog.d/15185.feature | 1 + rust/benches/evaluator.rs | 4 ---- rust/src/push/base_rules.rs | 8 +++---- rust/src/push/evaluator.rs | 36 ++++++++++------------------- rust/src/push/mod.rs | 36 +++++++++++++---------------- stubs/synapse/synapse_rust/push.pyi | 1 - synapse/config/experimental.py | 8 +------ synapse/push/bulk_push_rule_evaluator.py | 1 - tests/push/test_bulk_push_rule_evaluator.py | 2 -- tests/push/test_push_rule_evaluator.py | 23 ++++-------------- 10 files changed, 39 insertions(+), 81 deletions(-) create mode 100644 changelog.d/15185.feature (limited to 'stubs/synapse/synapse_rust/push.pyi') diff --git a/changelog.d/15185.feature b/changelog.d/15185.feature new file mode 100644 index 0000000000..901900bdec --- /dev/null +++ b/changelog.d/15185.feature @@ -0,0 +1 @@ +Stabilise support for [MSC3758](https://github.com/matrix-org/matrix-spec-proposals/pull/3758): `event_property_is` push condition. diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index 44477e63f7..79b553dbb0 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -53,7 +53,6 @@ fn bench_match_exact(b: &mut Bencher) { vec![], false, false, - false, ) .unwrap(); @@ -100,7 +99,6 @@ fn bench_match_word(b: &mut Bencher) { vec![], false, false, - false, ) .unwrap(); @@ -147,7 +145,6 @@ fn bench_match_word_miss(b: &mut Bencher) { vec![], false, false, - false, ) .unwrap(); @@ -194,7 +191,6 @@ fn bench_eval_message(b: &mut Bencher) { vec![], false, false, - false, ) .unwrap(); diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index 3d72a4a4c3..ec8d96656a 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -24,10 +24,10 @@ use super::KnownCondition; use crate::push::RelatedEventMatchTypeCondition; use crate::push::SetTweak; use crate::push::TweakValue; -use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue}; +use crate::push::{Action, EventPropertyIsCondition, SimpleJsonValue}; use crate::push::{Condition, EventMatchTypeCondition}; use crate::push::{EventMatchCondition, EventMatchPatternType}; -use crate::push::{ExactEventMatchTypeCondition, PushRule}; +use crate::push::{EventPropertyIsTypeCondition, PushRule}; const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak { set_tweak: Cow::Borrowed("highlight"), @@ -145,7 +145,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"), priority_class: 5, conditions: Cow::Borrowed(&[Condition::Known( - KnownCondition::ExactEventPropertyContainsType(ExactEventMatchTypeCondition { + KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition { key: Cow::Borrowed("content.org.matrix.msc3952.mentions.user_ids"), value_type: Cow::Borrowed(&EventMatchPatternType::UserId), }), @@ -166,7 +166,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mention"), priority_class: 5, conditions: Cow::Borrowed(&[ - Condition::Known(KnownCondition::ExactEventMatch(ExactEventMatchCondition { + Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition { key: Cow::Borrowed("content.org.matrix.msc3952.mentions.room"), value: Cow::Borrowed(&SimpleJsonValue::Bool(true)), })), diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 1c2a05ad9a..67fe6a4823 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -23,7 +23,7 @@ use regex::Regex; use super::{ utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, - Action, Condition, ExactEventMatchCondition, FilteredPushRules, KnownCondition, + Action, Condition, EventPropertyIsCondition, FilteredPushRules, KnownCondition, SimpleJsonValue, }; use crate::push::{EventMatchPatternType, JsonValue}; @@ -97,9 +97,6 @@ pub struct PushRuleEvaluator { /// flag as MSC1767 (extensible events core). msc3931_enabled: bool, - /// If MSC3758 (exact_event_match push rule condition) is enabled. - msc3758_exact_event_match: bool, - /// If MSC3966 (exact_event_property_contains push rule condition) is enabled. msc3966_exact_event_property_contains: bool, } @@ -119,7 +116,6 @@ impl PushRuleEvaluator { related_event_match_enabled: bool, room_version_feature_flags: Vec, msc3931_enabled: bool, - msc3758_exact_event_match: bool, msc3966_exact_event_property_contains: bool, ) -> Result { let body = match flattened_keys.get("content.body") { @@ -138,7 +134,6 @@ impl PushRuleEvaluator { related_event_match_enabled, room_version_feature_flags, msc3931_enabled, - msc3758_exact_event_match, msc3966_exact_event_property_contains, }) } @@ -275,8 +270,8 @@ impl PushRuleEvaluator { self.match_event_match(&self.flattened_keys, &event_match.key, pattern)? } - KnownCondition::ExactEventMatch(exact_event_match) => { - self.match_exact_event_match(exact_event_match)? + KnownCondition::EventPropertyIs(event_property_is) => { + self.match_event_property_is(event_property_is)? } KnownCondition::RelatedEventMatch(event_match) => self.match_related_event_match( &event_match.rel_type.clone(), @@ -306,10 +301,10 @@ impl PushRuleEvaluator { Some(Cow::Borrowed(pattern)), )? } - KnownCondition::ExactEventPropertyContains(exact_event_match) => self + KnownCondition::ExactEventPropertyContains(event_property_is) => self .match_exact_event_property_contains( - exact_event_match.key.clone(), - exact_event_match.value.clone(), + event_property_is.key.clone(), + event_property_is.value.clone(), )?, KnownCondition::ExactEventPropertyContainsType(exact_event_match) => { // The `pattern_type` can either be "user_id" or "user_localpart", @@ -405,20 +400,15 @@ impl PushRuleEvaluator { compiled_pattern.is_match(haystack) } - /// Evaluates a `exact_event_match` condition. (MSC3758) - fn match_exact_event_match( + /// Evaluates a `event_property_is` condition. + fn match_event_property_is( &self, - exact_event_match: &ExactEventMatchCondition, + event_property_is: &EventPropertyIsCondition, ) -> Result { - // First check if the feature is enabled. - if !self.msc3758_exact_event_match { - return Ok(false); - } - - let value = &exact_event_match.value; + let value = &event_property_is.value; let haystack = if let Some(JsonValue::Value(haystack)) = - self.flattened_keys.get(&*exact_event_match.key) + self.flattened_keys.get(&*event_property_is.key) { haystack } else { @@ -464,7 +454,7 @@ impl PushRuleEvaluator { } } - /// Evaluates a `exact_event_property_contains` condition. (MSC3758) + /// Evaluates a `exact_event_property_contains` condition. (MSC3966) fn match_exact_event_property_contains( &self, key: Cow, @@ -526,7 +516,6 @@ fn push_rule_evaluator() { vec![], true, true, - true, ) .unwrap(); @@ -557,7 +546,6 @@ fn test_requires_room_version_supports_condition() { flags, true, true, - true, ) .unwrap(); diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 6391d2ed47..7fde88e825 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -331,21 +331,20 @@ pub enum KnownCondition { // Identical to event_match but gives predefined patterns. Cannot be added by users. #[serde(skip_deserializing, rename = "event_match")] EventMatchType(EventMatchTypeCondition), - #[serde(rename = "com.beeper.msc3758.exact_event_match")] - ExactEventMatch(ExactEventMatchCondition), + EventPropertyIs(EventPropertyIsCondition), #[serde(rename = "im.nheko.msc3664.related_event_match")] RelatedEventMatch(RelatedEventMatchCondition), // Identical to related_event_match but gives predefined patterns. Cannot be added by users. #[serde(skip_deserializing, rename = "im.nheko.msc3664.related_event_match")] RelatedEventMatchType(RelatedEventMatchTypeCondition), #[serde(rename = "org.matrix.msc3966.exact_event_property_contains")] - ExactEventPropertyContains(ExactEventMatchCondition), + ExactEventPropertyContains(EventPropertyIsCondition), // Identical to exact_event_property_contains but gives predefined patterns. Cannot be added by users. #[serde( skip_deserializing, rename = "org.matrix.msc3966.exact_event_property_contains" )] - ExactEventPropertyContainsType(ExactEventMatchTypeCondition), + ExactEventPropertyContainsType(EventPropertyIsTypeCondition), ContainsDisplayName, RoomMemberCount { #[serde(skip_serializing_if = "Option::is_none")] @@ -395,16 +394,16 @@ pub struct EventMatchTypeCondition { pub pattern_type: Cow<'static, EventMatchPatternType>, } -/// The body of a [`Condition::ExactEventMatch`] +/// The body of a [`Condition::EventPropertyIs`] #[derive(Serialize, Deserialize, Debug, Clone)] -pub struct ExactEventMatchCondition { +pub struct EventPropertyIsCondition { pub key: Cow<'static, str>, pub value: Cow<'static, SimpleJsonValue>, } -/// The body of a [`Condition::ExactEventMatch`] that uses user_id or user_localpart as a pattern. +/// The body of a [`Condition::EventPropertyIs`] that uses user_id or user_localpart as a pattern. #[derive(Serialize, Debug, Clone)] -pub struct ExactEventMatchTypeCondition { +pub struct EventPropertyIsTypeCondition { pub key: Cow<'static, str>, // During serialization, the pattern_type property gets replaced with a // pattern property of the correct value in synapse.push.clientformat.format_push_rules_for_user. @@ -711,44 +710,41 @@ fn test_deserialize_unstable_msc3931_condition() { } #[test] -fn test_deserialize_unstable_msc3758_condition() { +fn test_deserialize_event_property_is_condition() { // A string condition should work. - let json = - r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":"foo"}"#; + let json = r#"{"kind":"event_property_is","key":"content.value","value":"foo"}"#; let condition: Condition = serde_json::from_str(json).unwrap(); assert!(matches!( condition, - Condition::Known(KnownCondition::ExactEventMatch(_)) + Condition::Known(KnownCondition::EventPropertyIs(_)) )); // A boolean condition should work. - let json = - r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":true}"#; + let json = r#"{"kind":"event_property_is","key":"content.value","value":true}"#; let condition: Condition = serde_json::from_str(json).unwrap(); assert!(matches!( condition, - Condition::Known(KnownCondition::ExactEventMatch(_)) + Condition::Known(KnownCondition::EventPropertyIs(_)) )); // An integer condition should work. - let json = r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":1}"#; + let json = r#"{"kind":"event_property_is","key":"content.value","value":1}"#; let condition: Condition = serde_json::from_str(json).unwrap(); assert!(matches!( condition, - Condition::Known(KnownCondition::ExactEventMatch(_)) + Condition::Known(KnownCondition::EventPropertyIs(_)) )); // A null condition should work - let json = - r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":null}"#; + let json = r#"{"kind":"event_property_is","key":"content.value","value":null}"#; let condition: Condition = serde_json::from_str(json).unwrap(); assert!(matches!( condition, - Condition::Known(KnownCondition::ExactEventMatch(_)) + Condition::Known(KnownCondition::EventPropertyIs(_)) )); } diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi index c17796ffbd..c040944aac 100644 --- a/stubs/synapse/synapse_rust/push.pyi +++ b/stubs/synapse/synapse_rust/push.pyi @@ -65,7 +65,6 @@ class PushRuleEvaluator: related_event_match_enabled: bool, room_version_feature_flags: Tuple[str, ...], msc3931_enabled: bool, - msc3758_exact_event_match: bool, msc3966_exact_event_property_contains: bool, ): ... def run( diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index fc64f2bda1..9c58cee2c8 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -169,11 +169,6 @@ class ExperimentalConfig(Config): # MSC3925: do not replace events with their edits self.msc3925_inhibit_edit = experimental.get("msc3925_inhibit_edit", False) - # MSC3758: exact_event_match push rule condition - self.msc3758_exact_event_match = experimental.get( - "msc3758_exact_event_match", False - ) - # MSC3873: Disambiguate event_match keys. self.msc3873_escape_event_match_key = experimental.get( "msc3873_escape_event_match_key", False @@ -184,10 +179,9 @@ class ExperimentalConfig(Config): "msc3966_exact_event_property_contains", False ) - # MSC3952: Intentional mentions, this depends on MSC3758 and MSC3966. + # MSC3952: Intentional mentions, this depends on MSC3966. self.msc3952_intentional_mentions = ( experimental.get("msc3952_intentional_mentions", False) - and self.msc3758_exact_event_match and self.msc3966_exact_event_property_contains ) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index abcf687f05..ba12b6d79a 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -413,7 +413,6 @@ class BulkPushRuleEvaluator: self._related_event_match_enabled, event.room_version.msc3931_push_features, 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, ) diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py index 73fecfd4ad..c6591c50de 100644 --- a/tests/push/test_bulk_push_rule_evaluator.py +++ b/tests/push/test_bulk_push_rule_evaluator.py @@ -231,7 +231,6 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase): @override_config( { "experimental_features": { - "msc3758_exact_event_match": True, "msc3952_intentional_mentions": True, "msc3966_exact_event_property_contains": True, } @@ -335,7 +334,6 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase): @override_config( { "experimental_features": { - "msc3758_exact_event_match": True, "msc3952_intentional_mentions": True, "msc3966_exact_event_property_contains": True, } diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index d4a4bc4d93..ff5a9a66f5 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -173,7 +173,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): related_event_match_enabled=True, room_version_feature_flags=event.room_version.msc3931_push_features, msc3931_enabled=True, - msc3758_exact_event_match=True, msc3966_exact_event_property_contains=True, ) @@ -404,7 +403,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): # Test against a string value. condition = { - "kind": "com.beeper.msc3758.exact_event_match", + "kind": "event_property_is", "key": "content.value", "value": "foobaz", } @@ -442,11 +441,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): """Check that exact_event_match conditions work as expected for booleans.""" # Test against a True boolean value. - condition = { - "kind": "com.beeper.msc3758.exact_event_match", - "key": "content.value", - "value": True, - } + condition = {"kind": "event_property_is", "key": "content.value", "value": True} self._assert_matches( condition, {"value": True}, @@ -466,7 +461,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): # Test against a False boolean value. condition = { - "kind": "com.beeper.msc3758.exact_event_match", + "kind": "event_property_is", "key": "content.value", "value": False, } @@ -491,11 +486,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): def test_exact_event_match_null(self) -> None: """Check that exact_event_match conditions work as expected for null.""" - condition = { - "kind": "com.beeper.msc3758.exact_event_match", - "key": "content.value", - "value": None, - } + condition = {"kind": "event_property_is", "key": "content.value", "value": None} self._assert_matches( condition, {"value": None}, @@ -511,11 +502,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): def test_exact_event_match_integer(self) -> None: """Check that exact_event_match conditions work as expected for integers.""" - condition = { - "kind": "com.beeper.msc3758.exact_event_match", - "key": "content.value", - "value": 1, - } + condition = {"kind": "event_property_is", "key": "content.value", "value": 1} self._assert_matches( condition, {"value": 1}, -- cgit 1.5.1