From da018859716a26c1bb7ba448b7750727111caa31 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 15:23:48 -0500 Subject: [PATCH 01/10] Add a (failing) test. --- tests/push/test_push_rule_evaluator.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index d320a12f968d..12dca41801fe 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -401,6 +401,21 @@ def test_event_match_non_body(self) -> None: "pattern should not match before a newline", ) + def test_event_match_pattern(self) -> None: + """Check that event_match conditions do not use a "pattern_type" from user data.""" + + # The pattern_type should not be deserialized into anything valid. + condition = { + "kind": "event_match", + "key": "content.value", + "pattern_type": "user_id", + } + self._assert_not_matches( + condition, + {"value": "@user:test"}, + "should not be possible to pass a pattern_type in", + ) + def test_exact_event_match_string(self) -> None: """Check that exact_event_match conditions work as expected for strings.""" From 4ce3786a7e62edc25dee8f787872c9cc159ad26d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 15:18:58 -0500 Subject: [PATCH 02/10] Separate the pattern vs. pattern_type for event_match. --- rust/benches/evaluator.rs | 9 +- rust/src/push/base_rules.rs | 122 +++++++++---------------- rust/src/push/evaluator.rs | 55 +++++------ rust/src/push/mod.rs | 22 +++-- tests/push/test_push_rule_evaluator.py | 12 +++ 5 files changed, 96 insertions(+), 124 deletions(-) diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index efd19a216541..9a871f569334 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -60,8 +60,7 @@ fn bench_match_exact(b: &mut Bencher) { let condition = Condition::Known(synapse::push::KnownCondition::EventMatch( EventMatchCondition { key: "room_id".into(), - pattern: Some("!room:server".into()), - pattern_type: None, + pattern: "!room:server".into(), }, )); @@ -109,8 +108,7 @@ fn bench_match_word(b: &mut Bencher) { let condition = Condition::Known(synapse::push::KnownCondition::EventMatch( EventMatchCondition { key: "content.body".into(), - pattern: Some("test".into()), - pattern_type: None, + pattern: "test".into(), }, )); @@ -158,8 +156,7 @@ fn bench_match_word_miss(b: &mut Bencher) { let condition = Condition::Known(synapse::push::KnownCondition::EventMatch( EventMatchCondition { key: "content.body".into(), - pattern: Some("foobar".into()), - pattern_type: None, + pattern: "foobar".into(), }, )); diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index 4a62b9696f9d..91fddf91551c 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::Condition; use crate::push::EventMatchCondition; use crate::push::PushRule; use crate::push::RelatedEventMatchCondition; use crate::push::SetTweak; use crate::push::TweakValue; use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue}; +use crate::push::{Condition, EventMatchTypeCondition}; const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak { set_tweak: Cow::Borrowed("highlight"), @@ -72,8 +72,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ 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, + pattern: Cow::Borrowed("m.replace"), }, ))]), actions: Cow::Borrowed(&[]), @@ -86,8 +85,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("content.msgtype"), - pattern: Some(Cow::Borrowed("m.notice")), - pattern_type: None, + pattern: Cow::Borrowed("m.notice"), }, ))]), actions: Cow::Borrowed(&[Action::DontNotify]), @@ -100,18 +98,15 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.member")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.member"), })), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("content.membership"), - pattern: Some(Cow::Borrowed("invite")), - pattern_type: None, + pattern: Cow::Borrowed("invite"), })), - Condition::Known(KnownCondition::EventMatch(EventMatchCondition { + Condition::Known(KnownCondition::EventMatchType(EventMatchTypeCondition { key: Cow::Borrowed("state_key"), - pattern: None, - pattern_type: Some(Cow::Borrowed("user_id")), + pattern_type: Cow::Borrowed("user_id"), })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION, SOUND_ACTION]), @@ -124,8 +119,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.member")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.member"), }, ))]), actions: Cow::Borrowed(&[Action::DontNotify]), @@ -189,8 +183,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ }), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("content.body"), - pattern: Some(Cow::Borrowed("@room")), - pattern_type: None, + pattern: Cow::Borrowed("@room"), })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION]), @@ -203,13 +196,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.tombstone")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.tombstone"), })), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("state_key"), - pattern: Some(Cow::Borrowed("")), - pattern_type: None, + pattern: Cow::Borrowed(""), })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION]), @@ -222,8 +213,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.reaction")), - pattern_type: None, + pattern: Cow::Borrowed("m.reaction"), }, ))]), actions: Cow::Borrowed(&[]), @@ -236,13 +226,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.server_acl")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.server_acl"), })), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("state_key"), - pattern: Some(Cow::Borrowed("")), - pattern_type: None, + pattern: Cow::Borrowed(""), })), ]), actions: Cow::Borrowed(&[]), @@ -255,8 +243,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("org.matrix.msc3381.poll.response")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc3381.poll.response"), }, ))]), actions: Cow::Borrowed(&[]), @@ -268,11 +255,10 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ pub const BASE_APPEND_CONTENT_RULES: &[PushRule] = &[PushRule { rule_id: Cow::Borrowed("global/content/.m.rule.contains_user_name"), priority_class: 4, - conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( - EventMatchCondition { + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatchType( + EventMatchTypeCondition { key: Cow::Borrowed("content.body"), - pattern: None, - pattern_type: Some(Cow::Borrowed("user_localpart")), + pattern_type: Cow::Borrowed("user_localpart"), }, ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), @@ -287,8 +273,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.call.invite")), - pattern_type: None, + pattern: Cow::Borrowed("m.call.invite"), }, ))]), actions: Cow::Borrowed(&[Action::Notify, RING_ACTION, HIGHLIGHT_FALSE_ACTION]), @@ -301,8 +286,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.message")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.message"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -318,8 +302,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.encrypted")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.encrypted"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -338,8 +321,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("org.matrix.msc1767.encrypted")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc1767.encrypted"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -363,8 +345,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("org.matrix.msc1767.message")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc1767.message"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -388,8 +369,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("org.matrix.msc1767.file")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc1767.file"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -413,8 +393,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("org.matrix.msc1767.image")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc1767.image"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -438,8 +417,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("org.matrix.msc1767.video")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc1767.video"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -463,8 +441,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("org.matrix.msc1767.audio")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc1767.audio"), })), Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), @@ -485,8 +462,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.message")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.message"), }, ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), @@ -499,8 +475,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.encrypted")), - pattern_type: None, + pattern: Cow::Borrowed("m.room.encrypted"), }, ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), @@ -514,8 +489,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("m.encrypted")), - pattern_type: None, + pattern: Cow::Borrowed("m.encrypted"), })), // MSC3933: Add condition on top of template rule - see MSC. Condition::Known(KnownCondition::RoomVersionSupports { @@ -534,8 +508,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("m.message")), - pattern_type: None, + pattern: Cow::Borrowed("m.message"), })), // MSC3933: Add condition on top of template rule - see MSC. Condition::Known(KnownCondition::RoomVersionSupports { @@ -554,8 +527,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("m.file")), - pattern_type: None, + pattern: Cow::Borrowed("m.file"), })), // MSC3933: Add condition on top of template rule - see MSC. Condition::Known(KnownCondition::RoomVersionSupports { @@ -574,8 +546,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("m.image")), - pattern_type: None, + pattern: Cow::Borrowed("m.image"), })), // MSC3933: Add condition on top of template rule - see MSC. Condition::Known(KnownCondition::RoomVersionSupports { @@ -594,8 +565,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("m.video")), - pattern_type: None, + pattern: Cow::Borrowed("m.video"), })), // MSC3933: Add condition on top of template rule - see MSC. Condition::Known(KnownCondition::RoomVersionSupports { @@ -614,8 +584,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), // MSC3933: Type changed from template rule - see MSC. - pattern: Some(Cow::Borrowed("m.audio")), - pattern_type: None, + pattern: Cow::Borrowed("m.audio"), })), // MSC3933: Add condition on top of template rule - see MSC. Condition::Known(KnownCondition::RoomVersionSupports { @@ -633,18 +602,15 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("im.vector.modular.widgets")), - pattern_type: None, + pattern: Cow::Borrowed("im.vector.modular.widgets"), })), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("content.type"), - pattern: Some(Cow::Borrowed("jitsi")), - pattern_type: None, + pattern: Cow::Borrowed("jitsi"), })), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("state_key"), - pattern: Some(Cow::Borrowed("*")), - pattern_type: None, + pattern: Cow::Borrowed("*"), })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), @@ -660,8 +626,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ }), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("org.matrix.msc3381.poll.start")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc3381.poll.start"), })), ]), actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION]), @@ -674,8 +639,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("org.matrix.msc3381.poll.start")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc3381.poll.start"), }, ))]), actions: Cow::Borrowed(&[Action::Notify]), @@ -691,8 +655,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ }), Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("org.matrix.msc3381.poll.end")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc3381.poll.end"), })), ]), actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION]), @@ -705,8 +668,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("org.matrix.msc3381.poll.end")), - pattern_type: None, + pattern: Cow::Borrowed("org.matrix.msc3381.poll.end"), }, ))]), actions: Cow::Borrowed(&[Action::Notify]), diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 55551ecb56a7..b8ffc50ae14a 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -23,8 +23,8 @@ use regex::Regex; use super::{ utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, - Action, Condition, EventMatchCondition, ExactEventMatchCondition, FilteredPushRules, - KnownCondition, RelatedEventMatchCondition, SimpleJsonValue, + Action, Condition, ExactEventMatchCondition, FilteredPushRules, KnownCondition, + RelatedEventMatchCondition, SimpleJsonValue, }; lazy_static! { @@ -257,7 +257,25 @@ impl PushRuleEvaluator { let result = match known_condition { KnownCondition::EventMatch(event_match) => { - self.match_event_match(event_match, user_id)? + self.match_event_match(&event_match.key.clone(), &event_match.pattern.clone())? + } + KnownCondition::EventMatchType(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 { + return Ok(false); + }; + + let pattern = match &*event_match.pattern_type { + "user_id" => user_id, + "user_localpart" => get_localpart_from_id(user_id)?, + _ => return Ok(false), + }; + + self.match_event_match(&event_match.key.clone(), pattern)? } KnownCondition::ExactEventMatch(exact_event_match) => { self.match_exact_event_match(exact_event_match)? @@ -323,34 +341,9 @@ impl PushRuleEvaluator { } /// Evaluates a `event_match` condition. - fn match_event_match( - &self, - event_match: &EventMatchCondition, - user_id: Option<&str>, - ) -> Result { - let pattern = if let Some(pattern) = &event_match.pattern { - pattern - } else if let Some(pattern_type) = &event_match.pattern_type { - // 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 { - return Ok(false); - }; - - match &**pattern_type { - "user_id" => user_id, - "user_localpart" => get_localpart_from_id(user_id)?, - _ => return Ok(false), - } - } else { - return Ok(false); - }; - + fn match_event_match(&self, key: &str, pattern: &str) -> Result { let haystack = if let Some(JsonValue::Value(SimpleJsonValue::Str(haystack))) = - self.flattened_keys.get(&*event_match.key) + self.flattened_keys.get(&*key) { haystack } else { @@ -359,7 +352,7 @@ impl PushRuleEvaluator { // For the content.body we match against "words", but for everything // else we match against the entire value. - let match_type = if event_match.key == "content.body" { + let match_type = if key == "content.body" { GlobMatchType::Word } else { GlobMatchType::Whole diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index fdd2b2c1439b..de2cc9ff552f 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -328,6 +328,9 @@ pub enum Condition { #[serde(tag = "kind")] pub enum KnownCondition { EventMatch(EventMatchCondition), + // 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), #[serde(rename = "im.nheko.msc3664.related_event_match")] @@ -362,14 +365,20 @@ impl<'source> FromPyObject<'source> for Condition { } } -/// The body of a [`Condition::EventMatch`] +/// The body of a [`Condition::EventMatch`] with a pattern. #[derive(Serialize, Deserialize, Debug, Clone)] pub struct EventMatchCondition { pub key: Cow<'static, str>, - #[serde(skip_serializing_if = "Option::is_none")] - pub pattern: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - pub pattern_type: Option>, + pub pattern: Cow<'static, str>, +} + +/// The body of a [`Condition::EventMatch`] that uses user_id or user_localpart as a pattern. +#[derive(Serialize, Debug, Clone)] +pub struct EventMatchTypeCondition { + 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 pattern_type: Cow<'static, str>, } /// The body of a [`Condition::ExactEventMatch`] @@ -571,8 +580,7 @@ impl FilteredPushRules { fn test_serialize_condition() { let condition = Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: "content.body".into(), - pattern: Some("coffee".into()), - pattern_type: None, + pattern: "coffee".into(), })); let json = serde_json::to_string(&condition).unwrap(); diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index 12dca41801fe..3f645f0d8661 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -416,6 +416,18 @@ def test_event_match_pattern(self) -> None: "should not be possible to pass a pattern_type in", ) + # This is an internal-only condition which shouldn't get deserialized. + condition = { + "kind": "event_match_type", + "key": "content.value", + "pattern_type": "user_id", + } + self._assert_not_matches( + condition, + {"value": "@user:test"}, + "should not be possible to pass a pattern_type in", + ) + def test_exact_event_match_string(self) -> None: """Check that exact_event_match conditions work as expected for strings.""" From e8b970980d7f65ac90564564107ace9098f18221 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 15:47:47 -0500 Subject: [PATCH 03/10] Share some duplicated code. --- rust/src/push/evaluator.rs | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index b8ffc50ae14a..c130c2659929 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -256,9 +256,11 @@ impl PushRuleEvaluator { }; let result = match known_condition { - KnownCondition::EventMatch(event_match) => { - self.match_event_match(&event_match.key.clone(), &event_match.pattern.clone())? - } + KnownCondition::EventMatch(event_match) => self.match_event_match( + &self.flattened_keys, + &event_match.key.clone(), + &event_match.pattern.clone(), + )?, KnownCondition::EventMatchType(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 @@ -275,7 +277,7 @@ impl PushRuleEvaluator { _ => return Ok(false), }; - self.match_event_match(&event_match.key.clone(), pattern)? + self.match_event_match(&self.flattened_keys, &event_match.key.clone(), pattern)? } KnownCondition::ExactEventMatch(exact_event_match) => { self.match_exact_event_match(exact_event_match)? @@ -341,9 +343,14 @@ impl PushRuleEvaluator { } /// Evaluates a `event_match` condition. - fn match_event_match(&self, key: &str, pattern: &str) -> Result { + fn match_event_match( + &self, + flattened_event: &BTreeMap, + key: &str, + pattern: &str, + ) -> Result { let haystack = if let Some(JsonValue::Value(SimpleJsonValue::Str(haystack))) = - self.flattened_keys.get(&*key) + flattened_event.get(key) { haystack } else { @@ -440,23 +447,7 @@ impl PushRuleEvaluator { return Ok(false); }; - let haystack = - if let Some(JsonValue::Value(SimpleJsonValue::Str(haystack))) = event.get(&**key) { - haystack - } else { - return Ok(false); - }; - - // For the content.body we match against "words", but for everything - // else we match against the entire value. - let match_type = if key == "content.body" { - GlobMatchType::Word - } else { - GlobMatchType::Whole - }; - - let mut compiled_pattern = get_glob_matcher(pattern, match_type)?; - compiled_pattern.is_match(haystack) + self.match_event_match(event, key, pattern) } /// Evaluates a `exact_event_property_contains` condition. (MSC3758) From 12cf8be255b47f5a04c28d706cf33312a228f677 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 16:12:37 -0500 Subject: [PATCH 04/10] Separate pattern vs. pattern_type for related_event_match. --- rust/src/push/base_rules.rs | 11 +++--- rust/src/push/evaluator.rs | 67 ++++++++++++++++++++++--------------- rust/src/push/mod.rs | 15 ++++++++- 3 files changed, 59 insertions(+), 34 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index 91fddf91551c..1073a7bbc410 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -23,7 +23,7 @@ use serde_json::Value; use super::KnownCondition; use crate::push::EventMatchCondition; use crate::push::PushRule; -use crate::push::RelatedEventMatchCondition; +use crate::push::RelatedEventMatchTypeCondition; use crate::push::SetTweak; use crate::push::TweakValue; use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue}; @@ -129,11 +129,10 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/override/.im.nheko.msc3664.reply"), priority_class: 5, - conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::RelatedEventMatch( - RelatedEventMatchCondition { - key: Some(Cow::Borrowed("sender")), - pattern: None, - pattern_type: Some(Cow::Borrowed("user_id")), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::RelatedEventMatchType( + RelatedEventMatchTypeCondition { + key: Cow::Borrowed("sender"), + pattern_type: Cow::Borrowed("user_id"), rel_type: Cow::Borrowed("m.in_reply_to"), include_fallbacks: None, }, diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index c130c2659929..2ed693d33b7d 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; use crate::push::JsonValue; @@ -24,7 +25,7 @@ use regex::Regex; use super::{ utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, Action, Condition, ExactEventMatchCondition, FilteredPushRules, KnownCondition, - RelatedEventMatchCondition, SimpleJsonValue, + SimpleJsonValue, }; lazy_static! { @@ -282,8 +283,34 @@ impl PushRuleEvaluator { KnownCondition::ExactEventMatch(exact_event_match) => { self.match_exact_event_match(exact_event_match)? } - KnownCondition::RelatedEventMatch(event_match) => { - self.match_related_event_match(event_match, user_id)? + KnownCondition::RelatedEventMatch(event_match) => self.match_related_event_match( + &event_match.rel_type.clone(), + event_match.include_fallbacks, + event_match.key.clone(), + event_match.pattern.clone(), + )?, + KnownCondition::RelatedEventMatchType(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 { + return Ok(false); + }; + + let pattern = match &*event_match.pattern_type { + "user_id" => user_id, + "user_localpart" => get_localpart_from_id(user_id)?, + _ => return Ok(false), + }; + + self.match_related_event_match( + &event_match.rel_type.clone(), + event_match.include_fallbacks, + Some(event_match.key.clone()), + Some(Cow::Borrowed(pattern)), + )? } KnownCondition::ExactEventPropertyContains(exact_event_match) => { self.match_exact_event_property_contains(exact_event_match)? @@ -395,8 +422,10 @@ impl PushRuleEvaluator { /// Evaluates a `related_event_match` condition. (MSC3664) fn match_related_event_match( &self, - event_match: &RelatedEventMatchCondition, - user_id: Option<&str>, + rel_type: &str, + include_fallbacks: Option, + key: Option>, + pattern: Option>, ) -> Result { // First check if related event matching is enabled... if !self.related_event_match_enabled { @@ -404,7 +433,7 @@ impl PushRuleEvaluator { } // get the related event, fail if there is none. - let event = if let Some(event) = self.related_events_flattened.get(&*event_match.rel_type) { + let event = if let Some(event) = self.related_events_flattened.get(rel_type) { event } else { return Ok(false); @@ -412,42 +441,26 @@ impl PushRuleEvaluator { // If we are not matching fallbacks, don't match if our special key indicating this is a // fallback relation is not present. - if !event_match.include_fallbacks.unwrap_or(false) - && event.contains_key("im.vector.is_falling_back") - { + if !include_fallbacks.unwrap_or(false) && event.contains_key("im.vector.is_falling_back") { return Ok(false); } // if we have no key, accept the event as matching, if it existed without matching any // fields. - let key = if let Some(key) = &event_match.key { + let key = if let Some(key) = key { key } else { return Ok(true); }; - let pattern = if let Some(pattern) = &event_match.pattern { + // There was a key, so we *must* have a pattern to go with it. + let pattern = if let Some(pattern) = pattern { pattern - } else if let Some(pattern_type) = &event_match.pattern_type { - // 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 { - return Ok(false); - }; - - match &**pattern_type { - "user_id" => user_id, - "user_localpart" => get_localpart_from_id(user_id)?, - _ => return Ok(false), - } } else { return Ok(false); }; - self.match_event_match(event, key, pattern) + self.match_event_match(event, &key, &pattern) } /// Evaluates a `exact_event_property_contains` condition. (MSC3758) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index de2cc9ff552f..ce8910e01e03 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -335,6 +335,9 @@ pub enum KnownCondition { ExactEventMatch(ExactEventMatchCondition), #[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), #[serde(rename = "org.matrix.msc3952.is_user_mention")] @@ -395,8 +398,18 @@ pub struct RelatedEventMatchCondition { pub key: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub pattern: Option>, + pub rel_type: Cow<'static, str>, #[serde(skip_serializing_if = "Option::is_none")] - pub pattern_type: Option>, + pub include_fallbacks: Option, +} + +/// The body of a [`Condition::RelatedEventMatch`] that uses user_id or user_localpart as a pattern. +#[derive(Serialize, Debug, Clone)] +pub struct RelatedEventMatchTypeCondition { + // This is only used if pattern_type exists (and thus key must exist), so is + // a bit simpler than RelatedEventMatchCondition. + pub key: Cow<'static, str>, + pub pattern_type: Cow<'static, str>, pub rel_type: Cow<'static, str>, #[serde(skip_serializing_if = "Option::is_none")] pub include_fallbacks: Option, From 828c1e569db0cf6da6615837d806a838f0eb196c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 21 Feb 2023 11:45:49 -0500 Subject: [PATCH 05/10] Use an enum instead of magic values. --- rust/src/push/base_rules.rs | 8 ++++---- rust/src/push/evaluator.rs | 12 +++++------- rust/src/push/mod.rs | 11 +++++++++-- synapse/push/clientformat.py | 1 + 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index 1073a7bbc410..62de51d91528 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::EventMatchCondition; 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}; const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak { set_tweak: Cow::Borrowed("highlight"), @@ -106,7 +106,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ })), Condition::Known(KnownCondition::EventMatchType(EventMatchTypeCondition { key: Cow::Borrowed("state_key"), - pattern_type: Cow::Borrowed("user_id"), + pattern_type: Cow::Borrowed(&EventMatchPatternType::UserId), })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION, SOUND_ACTION]), @@ -132,7 +132,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::RelatedEventMatchType( RelatedEventMatchTypeCondition { key: Cow::Borrowed("sender"), - pattern_type: Cow::Borrowed("user_id"), + pattern_type: Cow::Borrowed(&EventMatchPatternType::UserId), rel_type: Cow::Borrowed("m.in_reply_to"), include_fallbacks: None, }, @@ -257,7 +257,7 @@ pub const BASE_APPEND_CONTENT_RULES: &[PushRule] = &[PushRule { conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatchType( EventMatchTypeCondition { key: Cow::Borrowed("content.body"), - pattern_type: Cow::Borrowed("user_localpart"), + pattern_type: Cow::Borrowed(&EventMatchPatternType::UserLocalpart), }, ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 2ed693d33b7d..1cfcf1b74982 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -15,7 +15,7 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; -use crate::push::JsonValue; +use crate::push::{EventMatchPatternType, JsonValue}; use anyhow::{Context, Error}; use lazy_static::lazy_static; use log::warn; @@ -273,9 +273,8 @@ impl PushRuleEvaluator { }; let pattern = match &*event_match.pattern_type { - "user_id" => user_id, - "user_localpart" => get_localpart_from_id(user_id)?, - _ => return Ok(false), + EventMatchPatternType::UserId => user_id, + EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?, }; self.match_event_match(&self.flattened_keys, &event_match.key.clone(), pattern)? @@ -300,9 +299,8 @@ impl PushRuleEvaluator { }; let pattern = match &*event_match.pattern_type { - "user_id" => user_id, - "user_localpart" => get_localpart_from_id(user_id)?, - _ => return Ok(false), + EventMatchPatternType::UserId => user_id, + EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?, }; self.match_related_event_match( diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index ce8910e01e03..a56c7d36f53d 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -375,13 +375,20 @@ pub struct EventMatchCondition { pub pattern: Cow<'static, str>, } +#[derive(Serialize, Debug, Clone)] +#[serde(rename_all = "snake_case")] +pub enum EventMatchPatternType { + UserId, + UserLocalpart, +} + /// The body of a [`Condition::EventMatch`] that uses user_id or user_localpart as a pattern. #[derive(Serialize, Debug, Clone)] pub struct EventMatchTypeCondition { 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 pattern_type: Cow<'static, str>, + pub pattern_type: Cow<'static, EventMatchPatternType>, } /// The body of a [`Condition::ExactEventMatch`] @@ -409,7 +416,7 @@ pub struct RelatedEventMatchTypeCondition { // This is only used if pattern_type exists (and thus key must exist), so is // a bit simpler than RelatedEventMatchCondition. pub key: Cow<'static, str>, - pub pattern_type: Cow<'static, str>, + pub pattern_type: Cow<'static, EventMatchPatternType>, pub rel_type: Cow<'static, str>, #[serde(skip_serializing_if = "Option::is_none")] pub include_fallbacks: Option, diff --git a/synapse/push/clientformat.py b/synapse/push/clientformat.py index bb76c169c6ce..f83854fea645 100644 --- a/synapse/push/clientformat.py +++ b/synapse/push/clientformat.py @@ -42,6 +42,7 @@ def format_push_rules_for_user( rulearray.append(template_rule) pattern_type = template_rule.pop("pattern_type", None) + print(pattern_type) if pattern_type == "user_id": template_rule["pattern"] = user.to_string() elif pattern_type == "user_localpart": From 546acb9d06ab6c8d511997d5e1540fec31139b26 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 16:23:37 -0500 Subject: [PATCH 06/10] Newsfragment --- changelog.d/15088.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15088.bugfix diff --git a/changelog.d/15088.bugfix b/changelog.d/15088.bugfix new file mode 100644 index 000000000000..15d5286f80f7 --- /dev/null +++ b/changelog.d/15088.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where Synapse handled an unspecced field on push rules. From c1506700b453892562eff620691c53f864797f03 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 24 Feb 2023 14:02:43 -0500 Subject: [PATCH 07/10] Remove unneeded clones. --- rust/src/push/evaluator.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 1cfcf1b74982..72f882890fa8 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -259,8 +259,8 @@ impl PushRuleEvaluator { let result = match known_condition { KnownCondition::EventMatch(event_match) => self.match_event_match( &self.flattened_keys, - &event_match.key.clone(), - &event_match.pattern.clone(), + &event_match.key, + &event_match.pattern, )?, KnownCondition::EventMatchType(event_match) => { // The `pattern_type` can either be "user_id" or "user_localpart", @@ -277,7 +277,7 @@ impl PushRuleEvaluator { EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?, }; - self.match_event_match(&self.flattened_keys, &event_match.key.clone(), pattern)? + self.match_event_match(&self.flattened_keys, &event_match.key, pattern)? } KnownCondition::ExactEventMatch(exact_event_match) => { self.match_exact_event_match(exact_event_match)? From e9e1a879a4317390ffe4eb880ad5dcf08c70ba75 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 24 Feb 2023 14:07:17 -0500 Subject: [PATCH 08/10] Reformat return type of match_related_event_match. --- rust/src/push/evaluator.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 72f882890fa8..a65c645cafe5 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -443,22 +443,14 @@ impl PushRuleEvaluator { return Ok(false); } - // if we have no key, accept the event as matching, if it existed without matching any - // fields. - let key = if let Some(key) = key { - key - } else { - return Ok(true); - }; - - // There was a key, so we *must* have a pattern to go with it. - let pattern = if let Some(pattern) = pattern { - pattern - } else { - return Ok(false); - }; - - self.match_event_match(event, &key, &pattern) + match (key, pattern) { + // if we have no key, accept the event as matching. + (None, _) => Ok(true), + // There was a key, so we *must* have a pattern to go with it. + (Some(_), None) => Ok(false), + // If there is a key & pattern, check if they're in the flattened event (given by rel_type). + (Some(key), Some(pattern)) => self.match_event_match(event, &key, &pattern), + } } /// Evaluates a `exact_event_property_contains` condition. (MSC3758) From 6f23bbbbf10daff50f7c7f4ab3431e048c39ee4f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 28 Feb 2023 07:03:23 -0500 Subject: [PATCH 09/10] Remove print statement. Co-authored-by: Erik Johnston --- synapse/push/clientformat.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/push/clientformat.py b/synapse/push/clientformat.py index f83854fea645..bb76c169c6ce 100644 --- a/synapse/push/clientformat.py +++ b/synapse/push/clientformat.py @@ -42,7 +42,6 @@ def format_push_rules_for_user( rulearray.append(template_rule) pattern_type = template_rule.pop("pattern_type", None) - print(pattern_type) if pattern_type == "user_id": template_rule["pattern"] = user.to_string() elif pattern_type == "user_localpart": From d9d927a1d1acca483bf665c16871beb5ed2428e3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 28 Feb 2023 07:28:33 -0500 Subject: [PATCH 10/10] Add more tests. --- rust/src/push/mod.rs | 59 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index a56c7d36f53d..97feb6efc924 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -614,7 +614,33 @@ fn test_serialize_condition() { fn test_deserialize_condition() { let json = r#"{"kind":"event_match","key":"content.body","pattern":"coffee"}"#; - let _: Condition = serde_json::from_str(json).unwrap(); + let condition: Condition = serde_json::from_str(json).unwrap(); + assert!(matches!( + condition, + Condition::Known(KnownCondition::EventMatch(_)) + )); +} + +#[test] +fn test_serialize_event_match_condition_with_pattern_type() { + let condition = Condition::Known(KnownCondition::EventMatchType(EventMatchTypeCondition { + key: "content.body".into(), + pattern_type: Cow::Owned(EventMatchPatternType::UserId), + })); + + let json = serde_json::to_string(&condition).unwrap(); + assert_eq!( + json, + r#"{"kind":"event_match","key":"content.body","pattern_type":"user_id"}"# + ) +} + +#[test] +fn test_cannot_deserialize_event_match_condition_with_pattern_type() { + let json = r#"{"kind":"event_match","key":"content.body","pattern_type":"user_id"}"#; + + let condition: Condition = serde_json::from_str(json).unwrap(); + assert!(matches!(condition, Condition::Unknown(_))); } #[test] @@ -628,6 +654,37 @@ fn test_deserialize_unstable_msc3664_condition() { )); } +#[test] +fn test_serialize_unstable_msc3664_condition_with_pattern_type() { + let condition = Condition::Known(KnownCondition::RelatedEventMatchType( + RelatedEventMatchTypeCondition { + key: "content.body".into(), + pattern_type: Cow::Owned(EventMatchPatternType::UserId), + rel_type: "m.in_reply_to".into(), + include_fallbacks: Some(true), + }, + )); + + let json = serde_json::to_string(&condition).unwrap(); + assert_eq!( + json, + r#"{"kind":"im.nheko.msc3664.related_event_match","key":"content.body","pattern_type":"user_id","rel_type":"m.in_reply_to","include_fallbacks":true}"# + ) +} + +#[test] +fn test_cannot_deserialize_unstable_msc3664_condition_with_pattern_type() { + let json = r#"{"kind":"im.nheko.msc3664.related_event_match","key":"content.body","pattern_type":"user_id","rel_type":"m.in_reply_to"}"#; + + let condition: Condition = serde_json::from_str(json).unwrap(); + // Since pattern is optional on RelatedEventMatch it deserializes it to that + // instead of RelatedEventMatchType. + assert!(matches!( + condition, + Condition::Known(KnownCondition::RelatedEventMatch(_)) + )); +} + #[test] fn test_deserialize_unstable_msc3931_condition() { let json =