From c2d0c2773a41183ac2796baef371b44bab9b7ad3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Sun, 4 Sep 2022 18:40:32 +0100 Subject: [PATCH 01/16] Implement push rules as Rust native types. --- .rustfmt.toml | 1 + changelog.d/13720.misc | 1 + rust/Cargo.toml | 10 +- rust/src/lib.rs | 8 +- rust/src/push/base_rules.rs | 320 ++++++++++++++ rust/src/push/mod.rs | 411 ++++++++++++++++++ .../__init__.pyi} | 0 stubs/synapse/synapse_rust/push.pyi | 30 ++ synapse/push/bulk_push_rule_evaluator.py | 9 +- synapse/push/clientformat.py | 5 +- synapse/storage/databases/main/push_rule.py | 24 +- tests/handlers/test_deactivate_account.py | 27 +- 12 files changed, 815 insertions(+), 31 deletions(-) create mode 100644 .rustfmt.toml create mode 100644 changelog.d/13720.misc create mode 100644 rust/src/push/base_rules.rs create mode 100644 rust/src/push/mod.rs rename stubs/synapse/{synapse_rust.pyi => synapse_rust/__init__.pyi} (100%) create mode 100644 stubs/synapse/synapse_rust/push.pyi diff --git a/.rustfmt.toml b/.rustfmt.toml new file mode 100644 index 000000000000..bf96e7743de0 --- /dev/null +++ b/.rustfmt.toml @@ -0,0 +1 @@ +group_imports = "StdExternalCrate" diff --git a/changelog.d/13720.misc b/changelog.d/13720.misc new file mode 100644 index 000000000000..af5de96b0447 --- /dev/null +++ b/changelog.d/13720.misc @@ -0,0 +1 @@ +Speed up push rule evaluation in rooms with large numbers of local users. diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 0a9760cafcd1..fd4bdfe8e713 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -18,4 +18,12 @@ crate-type = ["cdylib"] name = "synapse.synapse_rust" [dependencies] -pyo3 = { version = "0.16.5", features = ["extension-module", "macros", "abi3", "abi3-py37"] } +anyhow = "1.0.63" +lazy_static = "1.4.0" +log = "0.4.17" +pyo3 = { version = "0.17.1", features = ["extension-module", "macros", "anyhow", "abi3", "abi3-py37"] } +pyo3-log = "0.7.0" +pythonize = "0.17.0" +regex = "1.6.0" +serde = { version = "1.0.144", features = ["derive"] } +serde_json = "1.0.85" diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 142fc2ed93af..2bd76c1aa86b 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -1,5 +1,7 @@ use pyo3::prelude::*; +pub mod push; + /// Formats the sum of two numbers as string. #[pyfunction] #[pyo3(text_signature = "(a, b, /)")] @@ -9,8 +11,12 @@ fn sum_as_string(a: usize, b: usize) -> PyResult { /// The entry point for defining the Python module. #[pymodule] -fn synapse_rust(_py: Python<'_>, m: &PyModule) -> PyResult<()> { +fn synapse_rust(py: Python<'_>, m: &PyModule) -> PyResult<()> { + pyo3_log::init(); + m.add_function(wrap_pyfunction!(sum_as_string, m)?)?; + push::register_module(py, m)?; + Ok(()) } diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs new file mode 100644 index 000000000000..eed3ae51b18c --- /dev/null +++ b/rust/src/push/base_rules.rs @@ -0,0 +1,320 @@ +// Copyright 2022 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Contains the definitions of the "base" push rules. + +use std::borrow::Cow; +use std::collections::HashMap; + +use lazy_static::lazy_static; +use serde_json::Value; + +use crate::push::Action; +use crate::push::Condition; +use crate::push::EventMatchCondition; +use crate::push::PushRule; +use crate::push::SetTweak; +use crate::push::TweakValue; + +const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak { + set_tweak: Cow::Borrowed("highlight"), + value: None, + other_keys: Value::Null, +}); + +const HIGHLIGHT_FALSE_ACTION: Action = Action::SetTweak(SetTweak { + set_tweak: Cow::Borrowed("highlight"), + value: Some(TweakValue::Other(Value::Bool(false))), + other_keys: Value::Null, +}); + +const SOUND_ACTION: Action = Action::SetTweak(SetTweak { + set_tweak: Cow::Borrowed("sound"), + value: Some(TweakValue::String(Cow::Borrowed("default"))), + other_keys: Value::Null, +}); + +const RING_ACTION: Action = Action::SetTweak(SetTweak { + set_tweak: Cow::Borrowed("sound"), + value: Some(TweakValue::String(Cow::Borrowed("ring"))), + other_keys: Value::Null, +}); + +pub const BASE_PREPEND_OVERRIDE_RULES: &[PushRule] = &[PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.master"), + priority_class: 5, + conditions: Cow::Borrowed(&[]), + actions: Cow::Borrowed(&[Action::DontNotify]), + default: true, + default_enabled: false, +}]; + +pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ + PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.suppress_notices"), + priority_class: 5, + conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("content.msgtype"), + pattern: Some(Cow::Borrowed("m.notice")), + pattern_type: None, + })]), + actions: Cow::Borrowed(&[Action::DontNotify]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.invite_for_me"), + priority_class: 5, + conditions: Cow::Borrowed(&[ + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.member")), + pattern_type: None, + }), + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("content.membership"), + pattern: Some(Cow::Borrowed("invite")), + pattern_type: None, + }), + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("state_key"), + pattern: None, + pattern_type: Some(Cow::Borrowed("user_id")), + }), + ]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION, SOUND_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.member_event"), + priority_class: 5, + conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.member")), + pattern_type: None, + })]), + actions: Cow::Borrowed(&[Action::DontNotify]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.contains_display_name"), + priority_class: 5, + conditions: Cow::Borrowed(&[Condition::ContainsDisplayName]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.roomnotif"), + priority_class: 5, + conditions: Cow::Borrowed(&[ + Condition::SenderNotificationPermission { + key: Cow::Borrowed("room"), + }, + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("content.body"), + pattern: Some(Cow::Borrowed("@room")), + pattern_type: None, + }), + ]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.tombstone"), + priority_class: 5, + conditions: Cow::Borrowed(&[ + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.tombstone")), + pattern_type: None, + }), + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("state_key"), + pattern: Some(Cow::Borrowed("")), + pattern_type: None, + }), + ]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/override/.m.rule.reaction"), + priority_class: 5, + conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.reaction")), + pattern_type: None, + })]), + actions: Cow::Borrowed(&[Action::DontNotify]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/override/.org.matrix.msc3786.rule.room.server_acl"), + priority_class: 5, + conditions: Cow::Borrowed(&[ + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.server_acl")), + pattern_type: None, + }), + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("state_key"), + pattern: Some(Cow::Borrowed("")), + pattern_type: None, + }), + ]), + actions: Cow::Borrowed(&[]), + default: true, + default_enabled: true, + }, +]; + +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::EventMatch(EventMatchCondition { + key: Cow::Borrowed("content.body"), + pattern: None, + pattern_type: Some(Cow::Borrowed("user_localpart")), + })]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), + default: true, + default_enabled: true, +}]; + +pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ + PushRule { + rule_id: Cow::Borrowed("global/underride/.m.rule.call"), + priority_class: 1, + conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.call.invite")), + pattern_type: None, + })]), + actions: Cow::Borrowed(&[Action::Notify, RING_ACTION, HIGHLIGHT_FALSE_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/underride/.m.rule.room_one_to_one"), + priority_class: 1, + conditions: Cow::Borrowed(&[ + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.message")), + pattern_type: None, + }), + Condition::RoomMemberCount { + is: Some(Cow::Borrowed("2")), + }, + ]), + actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION, HIGHLIGHT_FALSE_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/underride/.m.rule.encrypted_room_one_to_one"), + priority_class: 1, + conditions: Cow::Borrowed(&[ + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.encrypted")), + pattern_type: None, + }), + Condition::RoomMemberCount { + is: Some(Cow::Borrowed("2")), + }, + ]), + actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION, HIGHLIGHT_FALSE_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/underride/.org.matrix.msc3772.thread_reply"), + priority_class: 1, + conditions: Cow::Borrowed(&[Condition::RelationMatch { + rel_type: Cow::Borrowed("m.thread"), + sender: None, + sender_type: Some(Cow::Borrowed("user_id")), + }]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/underride/.m.rule.message"), + priority_class: 1, + conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.message")), + pattern_type: None, + })]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/underride/.m.rule.encrypted"), + priority_class: 1, + conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.encrypted")), + pattern_type: None, + })]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), + default: true, + default_enabled: true, + }, + PushRule { + rule_id: Cow::Borrowed("global/underride/.im.vector.jitsi"), + priority_class: 1, + conditions: Cow::Borrowed(&[ + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("im.vector.modular.widgets")), + pattern_type: None, + }), + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("content.type"), + pattern: Some(Cow::Borrowed("jitsi")), + pattern_type: None, + }), + Condition::EventMatch(EventMatchCondition { + key: Cow::Borrowed("state_key"), + pattern: Some(Cow::Borrowed("*")), + pattern_type: None, + }), + ]), + actions: Cow::Borrowed(&[HIGHLIGHT_FALSE_ACTION]), + default: true, + default_enabled: true, + }, +]; + +lazy_static! { + pub static ref BASE_RULES_BY_ID: HashMap<&'static str, &'static PushRule> = + BASE_PREPEND_OVERRIDE_RULES + .iter() + .chain(BASE_APPEND_OVERRIDE_RULES.iter()) + .chain(BASE_APPEND_CONTENT_RULES.iter()) + .chain(BASE_APPEND_UNDERRIDE_RULES.iter()) + .map(|rule| { (&*rule.rule_id, rule) }) + .collect(); +} diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs new file mode 100644 index 000000000000..f9d3db8182b1 --- /dev/null +++ b/rust/src/push/mod.rs @@ -0,0 +1,411 @@ +// Copyright 2022 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! An implementation of Matrix push rules. +//! +//! The `Cow<_>` type is used extensively within this module to allow creating +//! the base rules as constants (in Rust constants can't require explicit +//! allocation atm). + +use std::borrow::Cow; +use std::collections::{BTreeMap, HashMap}; + +use anyhow::{Context, Error}; +use log::warn; +use pyo3::prelude::*; +use pythonize::pythonize; +use serde::de::Error as _; +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +mod base_rules; + +/// Called when registering modules with python. +pub fn register_module(py: Python<'_>, m: &PyModule) -> PyResult<()> { + let child_module = PyModule::new(py, "push")?; + child_module.add_class::()?; + child_module.add_class::()?; + child_module.add_class::()?; + m.add_submodule(child_module)?; + + // We need to manually add the module to sys.modules to make `from + // synapse.synapse_rust import push` work. + py.import("sys")? + .getattr("modules")? + .set_item("synapse.synapse_rust.push", child_module)?; + + Ok(()) +} + +/// A single push rule for a user. +#[derive(Debug, Clone)] +#[pyclass(frozen)] +pub struct PushRule { + pub rule_id: Cow<'static, str>, + #[pyo3(get)] + pub priority_class: i32, + pub conditions: Cow<'static, [Condition]>, + pub actions: Cow<'static, [Action]>, + #[pyo3(get)] + pub default: bool, + #[pyo3(get)] + pub default_enabled: bool, +} + +#[pymethods] +impl PushRule { + #[staticmethod] + pub fn from_db( + rule_id: String, + priority_class: i32, + conditions: &str, + actions: &str, + ) -> Result { + let conditions = serde_json::from_str(conditions).context("parsing conditions")?; + let actions = serde_json::from_str(actions).context("parsing actions")?; + + Ok(PushRule { + rule_id: Cow::Owned(rule_id), + priority_class, + conditions, + actions, + default: false, + default_enabled: true, + }) + } + + #[getter] + fn rule_id(&self) -> &str { + &self.rule_id + } + + #[getter] + fn actions(&self) -> Vec { + self.actions.clone().into_owned() + } + + #[getter] + fn conditions(&self) -> Vec { + self.conditions.clone().into_owned() + } + + fn __repr__(&self) -> String { + format!( + "", + self.rule_id, self.conditions, self.actions + ) + } +} + +/// The "action" Synapse should perform for a matching push rule. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Action { + DontNotify, + Notify, + Coalesce, + SetTweak(SetTweak), +} + +impl IntoPy for Action { + fn into_py(self, py: Python<'_>) -> PyObject { + pythonize(py, &self).expect("valid action") + } +} + +/// The body of a `SetTweak` push action. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +pub struct SetTweak { + set_tweak: Cow<'static, str>, + + #[serde(skip_serializing_if = "Option::is_none")] + value: Option, + + // This picks saves any other fields that may have been added as clients. + // These get added when we convert the `Action` to a python object. + #[serde(flatten)] + other_keys: Value, +} + +/// The value of a `set_tweak`. +/// +/// We need this (rather than using `TweakValue` directly) so that we can use +/// `&'static str` in the value when defining the constant base rules. +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] +#[serde(untagged)] +pub enum TweakValue { + String(Cow<'static, str>), + Other(Value), +} + +impl Serialize for Action { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match self { + Action::DontNotify => serializer.serialize_str("dont_notify"), + Action::Notify => serializer.serialize_str("notify"), + Action::Coalesce => serializer.serialize_str("coalesce"), + Action::SetTweak(tweak) => tweak.serialize(serializer), + } + } +} + +/// Simple helper class for deserializing Action from JSON. +#[derive(Deserialize)] +#[serde(untagged)] +enum ActionDeserializeHelper { + Str(String), + SetTweak(SetTweak), +} + +impl<'de> Deserialize<'de> for Action { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let helper: ActionDeserializeHelper = Deserialize::deserialize(deserializer)?; + match helper { + ActionDeserializeHelper::Str(s) => match &*s { + "dont_notify" => Ok(Action::DontNotify), + "notify" => Ok(Action::Notify), + "coalesce" => Ok(Action::Coalesce), + _ => Err(D::Error::custom("unrecognized action")), + }, + ActionDeserializeHelper::SetTweak(set_tweak) => Ok(Action::SetTweak(set_tweak)), + } + } +} + +/// A condition used in push rules to match against an event. +#[derive(Serialize, Deserialize, Debug, Clone)] +#[serde(rename_all = "snake_case")] +#[serde(tag = "kind")] +pub enum Condition { + EventMatch(EventMatchCondition), + ContainsDisplayName, + RoomMemberCount { + #[serde(skip_serializing_if = "Option::is_none")] + is: Option>, + }, + SenderNotificationPermission { + key: Cow<'static, str>, + }, + #[serde(rename = "org.matrix.msc3772.relation_match")] + RelationMatch { + rel_type: Cow<'static, str>, + #[serde(skip_serializing_if = "Option::is_none")] + sender: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + sender_type: Option>, + }, +} + +impl IntoPy for Condition { + fn into_py(self, py: Python<'_>) -> PyObject { + pythonize(py, &self).expect("valid condition") + } +} + +/// The body of a [`Condition::EventMatch`] +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct EventMatchCondition { + key: Cow<'static, str>, + #[serde(skip_serializing_if = "Option::is_none")] + pattern: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pattern_type: Option>, +} + +/// The collection of push rules for a user. +#[derive(Debug, Clone, Default)] +#[pyclass(frozen)] +struct PushRules { + /// Custom push rules that override a base rule. + overridden_base_rules: HashMap, PushRule>, + + /// Custom rules that come between the prepend/append override base rules. + override_rules: Vec, + /// Custom rules that come before the base content rules. + content: Vec, + /// Custom rules that come before the base room rules. + room: Vec, + /// Custom rules that come before the base sender rules. + sender: Vec, + /// Custom rules that come before the base underride rules. + underride: Vec, +} + +#[pymethods] +impl PushRules { + #[new] + fn new(rules: Vec) -> PushRules { + let mut push_rules: PushRules = Default::default(); + + for rule in rules { + if let Some(&o) = base_rules::BASE_RULES_BY_ID.get(&*rule.rule_id) { + push_rules.overridden_base_rules.insert( + rule.rule_id.clone(), + PushRule { + actions: rule.actions.clone(), + ..o.clone() + }, + ); + + continue; + } + + match rule.priority_class { + 5 => push_rules.override_rules.push(rule), + 4 => push_rules.content.push(rule), + 3 => push_rules.room.push(rule), + 2 => push_rules.sender.push(rule), + 1 => push_rules.underride.push(rule), + _ => { + warn!( + "Unrecognized priority class for rule {}: {}", + rule.rule_id, rule.priority_class + ); + } + } + } + + push_rules + } + + /// Returns the list of all rules, including base rules, in the order they + /// should be executed in. + fn rules(&self) -> Vec { + self.iter().cloned().collect() + } +} + +impl PushRules { + /// Iterates over all the rules, including base rules, in the order they + /// should be executed in. + pub fn iter(&self) -> impl Iterator { + base_rules::BASE_PREPEND_OVERRIDE_RULES + .iter() + .chain(self.override_rules.iter()) + .chain(base_rules::BASE_APPEND_OVERRIDE_RULES.iter()) + .chain(self.content.iter()) + .chain(base_rules::BASE_APPEND_CONTENT_RULES.iter()) + .chain(self.room.iter()) + .chain(self.sender.iter()) + .chain(self.underride.iter()) + .chain(base_rules::BASE_APPEND_UNDERRIDE_RULES.iter()) + .map(|rule| { + self.overridden_base_rules + .get(&*rule.rule_id) + .unwrap_or(rule) + }) + } +} + +/// A wrapper around `PushRules` that checks the enabled state of rules and +/// filters out disabled experimental rules. +#[derive(Debug, Clone, Default)] +#[pyclass(frozen)] +pub struct FilteredPushRules { + push_rules: PushRules, + enabled_map: BTreeMap, + msc3786_enabled: bool, + msc3772_enabled: bool, +} + +#[pymethods] +impl FilteredPushRules { + #[new] + fn py_new( + push_rules: PushRules, + enabled_map: BTreeMap, + msc3786_enabled: bool, + msc3772_enabled: bool, + ) -> Self { + Self { + push_rules, + enabled_map, + msc3786_enabled, + msc3772_enabled, + } + } + + /// Returns the list of all rules and their enabled state, including base + /// rules, in the order they should be executed in. + fn rules(&self) -> Vec<(PushRule, bool)> { + self.iter().map(|(r, e)| (r.clone(), e)).collect() + } +} + +impl FilteredPushRules { + /// Iterates over all the rules and their enabled state, including base + /// rules, in the order they should be executed in. + fn iter(&self) -> impl Iterator { + self.push_rules + .iter() + .filter(|rule| { + // Ignore disabled experimental push rules + if !self.msc3786_enabled + && rule.rule_id == "global/override/.org.matrix.msc3786.rule.room.server_acl" + { + return false; + } + + if !self.msc3772_enabled + && rule.rule_id == "global/underride/.org.matrix.msc3772.thread_reply" + { + return false; + } + + true + }) + .map(|r| { + let enabled = *self + .enabled_map + .get(&*r.rule_id) + .unwrap_or(&r.default_enabled); + (r, enabled) + }) + } +} + +#[test] +fn test_erialize_condition() { + let condition = Condition::EventMatch(EventMatchCondition { + key: "content.body".into(), + pattern: Some("coffee".into()), + pattern_type: None, + }); + + let json = serde_json::to_string(&condition).unwrap(); + assert_eq!( + json, + r#"{"kind":"event_match","key":"content.body","pattern":"coffee"}"# + ) +} + +#[test] +fn test_deserialize_condition() { + let json = r#"{"kind":"event_match","key":"content.body","pattern":"coffee"}"#; + + let _: Condition = serde_json::from_str(json).unwrap(); +} + +#[test] +fn test_deserialize_action() { + let _: Action = serde_json::from_str(r#""notify""#).unwrap(); + let _: Action = serde_json::from_str(r#""dont_notify""#).unwrap(); + let _: Action = serde_json::from_str(r#""coalesce""#).unwrap(); + let _: Action = serde_json::from_str(r#"{"set_tweak": "highlight"}"#).unwrap(); +} diff --git a/stubs/synapse/synapse_rust.pyi b/stubs/synapse/synapse_rust/__init__.pyi similarity index 100% rename from stubs/synapse/synapse_rust.pyi rename to stubs/synapse/synapse_rust/__init__.pyi diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi new file mode 100644 index 000000000000..06336c8899b2 --- /dev/null +++ b/stubs/synapse/synapse_rust/push.pyi @@ -0,0 +1,30 @@ +from typing import Any, Collection, Dict, Mapping, Sequence, Tuple, Union + +from synapse.types import JsonDict + +class PushRule: + rule_id: str + priority_class: int + conditions: Sequence[Mapping[str, str]] + actions: Sequence[Union[Mapping[str, Any], str]] + default: bool + default_enabled: bool + + @staticmethod + def from_db( + rule_id: str, priority_class: int, conditions: str, actions: str + ) -> "PushRule": ... + +class PushRules: + def __init__(self, rules: Collection[PushRule]): ... + def rules(self) -> Collection[PushRule]: ... + +class FilteredPushRules: + def __init__( + self, + push_rules: PushRules, + enabled_map: Dict[str, bool], + msc3786_enabled: bool, + msc3772_enabled: bool, + ): ... + def rules(self) -> Collection[Tuple[PushRule, bool]]: ... diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index d1caf8a0f7a0..b976508ef3d8 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -34,14 +34,15 @@ from synapse.event_auth import auth_types_for_event, get_user_power_level from synapse.events import EventBase, relation_from_event from synapse.events.snapshot import EventContext +from synapse.push.push_rule_evaluator import _flatten_dict from synapse.state import POWER_KEY from synapse.storage.databases.main.roommember import EventIdMembership from synapse.storage.state import StateFilter +from synapse.synapse_rust.push import FilteredPushRules, PushRule from synapse.util.caches import register_cache from synapse.util.metrics import measure_func from synapse.visibility import filter_event_for_clients_with_state -from .baserules import FilteredPushRules, PushRule from .push_rule_evaluator import PushRuleEvaluatorForEvent if TYPE_CHECKING: @@ -282,9 +283,11 @@ async def action_for_event_by_user( ) = await self._get_power_levels_and_sender_level(event, context) relations = await self._get_mutual_relations( - event, itertools.chain(*rules_by_user.values()) + event, itertools.chain(*(r.rules() for r in rules_by_user.values())) ) + logger.info("Flatten map: %s", _flatten_dict(event)) + logger.info("room_member_count: %s", room_member_count) evaluator = PushRuleEvaluatorForEvent( event, room_member_count, @@ -333,7 +336,7 @@ async def action_for_event_by_user( # current user, it'll be added to the dict later. actions_by_user[uid] = [] - for rule, enabled in rules: + for rule, enabled in rules.rules(): if not enabled: continue diff --git a/synapse/push/clientformat.py b/synapse/push/clientformat.py index 73618d9234bf..ebc13beda1bc 100644 --- a/synapse/push/clientformat.py +++ b/synapse/push/clientformat.py @@ -16,10 +16,9 @@ from typing import Any, Dict, List, Optional from synapse.push.rulekinds import PRIORITY_CLASS_INVERSE_MAP, PRIORITY_CLASS_MAP +from synapse.synapse_rust.push import FilteredPushRules, PushRule from synapse.types import UserID -from .baserules import FilteredPushRules, PushRule - def format_push_rules_for_user( user: UserID, ruleslist: FilteredPushRules @@ -34,7 +33,7 @@ def format_push_rules_for_user( rules["global"] = _add_empty_priority_class_arrays(rules["global"]) - for r, enabled in ruleslist: + for r, enabled in ruleslist.rules(): template_name = _priority_class_to_template_name(r.priority_class) rulearray = rules["global"][template_name] diff --git a/synapse/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py index 5079edd1e083..9d76e9b87979 100644 --- a/synapse/storage/databases/main/push_rule.py +++ b/synapse/storage/databases/main/push_rule.py @@ -30,9 +30,8 @@ from synapse.api.errors import StoreError from synapse.config.homeserver import ExperimentalConfig -from synapse.push.baserules import FilteredPushRules, PushRule, compile_push_rules from synapse.replication.slave.storage._slaved_id_tracker import SlavedIdTracker -from synapse.storage._base import SQLBaseStore, db_to_json +from synapse.storage._base import SQLBaseStore from synapse.storage.database import ( DatabasePool, LoggingDatabaseConnection, @@ -51,6 +50,7 @@ IdGenerator, StreamIdGenerator, ) +from synapse.synapse_rust.push import FilteredPushRules, PushRule, PushRules from synapse.types import JsonDict from synapse.util import json_encoder from synapse.util.caches.descriptors import cached, cachedList @@ -72,18 +72,26 @@ def _load_rules( """ ruleslist = [ - PushRule( + PushRule.from_db( rule_id=rawrule["rule_id"], priority_class=rawrule["priority_class"], - conditions=db_to_json(rawrule["conditions"]), - actions=db_to_json(rawrule["actions"]), + conditions=rawrule["conditions"], + actions=rawrule["actions"], ) for rawrule in rawrules ] - push_rules = compile_push_rules(ruleslist) + push_rules = PushRules( + ruleslist, + ) - filtered_rules = FilteredPushRules(push_rules, enabled_map, experimental_config) + # TODO: Experimental config + filtered_rules = FilteredPushRules( + push_rules, + enabled_map, + msc3786_enabled=experimental_config.msc3786_enabled, + msc3772_enabled=experimental_config.msc3772_enabled, + ) return filtered_rules @@ -845,7 +853,7 @@ async def copy_push_rules_from_room_to_room_for_user( user_push_rules = await self.get_push_rules_for_user(user_id) # Get rules relating to the old room and copy them to the new room - for rule, enabled in user_push_rules: + for rule, enabled in user_push_rules.rules(): if not enabled: continue diff --git a/tests/handlers/test_deactivate_account.py b/tests/handlers/test_deactivate_account.py index 7b9b711521d8..bce65fab7d84 100644 --- a/tests/handlers/test_deactivate_account.py +++ b/tests/handlers/test_deactivate_account.py @@ -15,11 +15,11 @@ from twisted.test.proto_helpers import MemoryReactor from synapse.api.constants import AccountDataTypes -from synapse.push.baserules import PushRule from synapse.push.rulekinds import PRIORITY_CLASS_MAP from synapse.rest import admin from synapse.rest.client import account, login from synapse.server import HomeServer +from synapse.synapse_rust.push import PushRule from synapse.util import Clock from tests.unittest import HomeserverTestCase @@ -161,20 +161,15 @@ def test_push_rules_deleted_upon_account_deactivation(self) -> None: self._store.get_push_rules_for_user(self.user) ) # Filter out default rules; we don't care - push_rules = [r for r, _ in filtered_push_rules if self._is_custom_rule(r)] + push_rules = [ + r for r, _ in filtered_push_rules.rules() if self._is_custom_rule(r) + ] # Check our rule made it - self.assertEqual( - push_rules, - [ - PushRule( - rule_id="personal.override.rule1", - priority_class=5, - conditions=[], - actions=[], - ) - ], - push_rules, - ) + self.assertEqual(len(push_rules), 1) + self.assertEqual(push_rules[0].rule_id, "personal.override.rule1") + self.assertEqual(push_rules[0].priority_class, 5) + self.assertEqual(push_rules[0].conditions, []) + self.assertEqual(push_rules[0].actions, []) # Request the deactivation of our account self._deactivate_my_account() @@ -183,7 +178,9 @@ def test_push_rules_deleted_upon_account_deactivation(self) -> None: self._store.get_push_rules_for_user(self.user) ) # Filter out default rules; we don't care - push_rules = [r for r, _ in filtered_push_rules if self._is_custom_rule(r)] + push_rules = [ + r for r, _ in filtered_push_rules.rules() if self._is_custom_rule(r) + ] # Check our rule no longer exists self.assertEqual(push_rules, [], push_rules) From dcea8d82b92111853aa04a888b5d6163388aae21 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 9 Sep 2022 15:28:02 +0100 Subject: [PATCH 02/16] Remove python baserules --- rust/src/push/mod.rs | 9 +- stubs/synapse/synapse_rust/push.pyi | 2 + synapse/handlers/push_rules.py | 5 +- synapse/push/baserules.py | 583 ---------------------------- 4 files changed, 14 insertions(+), 585 deletions(-) delete mode 100644 synapse/push/baserules.py diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index f9d3db8182b1..6df5382126c0 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -19,7 +19,7 @@ //! allocation atm). use std::borrow::Cow; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use anyhow::{Context, Error}; use log::warn; @@ -37,6 +37,8 @@ pub fn register_module(py: Python<'_>, m: &PyModule) -> PyResult<()> { child_module.add_class::()?; child_module.add_class::()?; child_module.add_class::()?; + child_module.add_function(wrap_pyfunction!(get_base_rule_ids, m)?)?; + m.add_submodule(child_module)?; // We need to manually add the module to sys.modules to make `from @@ -48,6 +50,11 @@ pub fn register_module(py: Python<'_>, m: &PyModule) -> PyResult<()> { Ok(()) } +#[pyfunction] +fn get_base_rule_ids() -> HashSet<&'static str> { + base_rules::BASE_RULES_BY_ID.keys().copied().collect() +} + /// A single push rule for a user. #[derive(Debug, Clone)] #[pyclass(frozen)] diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi index 06336c8899b2..cb59bf9d8af9 100644 --- a/stubs/synapse/synapse_rust/push.pyi +++ b/stubs/synapse/synapse_rust/push.pyi @@ -28,3 +28,5 @@ class FilteredPushRules: msc3772_enabled: bool, ): ... def rules(self) -> Collection[Tuple[PushRule, bool]]: ... + +def get_base_rule_ids() -> Collection[str]: ... diff --git a/synapse/handlers/push_rules.py b/synapse/handlers/push_rules.py index 2599160bcc00..1219672a59dc 100644 --- a/synapse/handlers/push_rules.py +++ b/synapse/handlers/push_rules.py @@ -16,14 +16,17 @@ import attr from synapse.api.errors import SynapseError, UnrecognizedRequestError -from synapse.push.baserules import BASE_RULE_IDS from synapse.storage.push_rule import RuleNotFoundException +from synapse.synapse_rust.push import get_base_rule_ids from synapse.types import JsonDict if TYPE_CHECKING: from synapse.server import HomeServer +BASE_RULE_IDS = get_base_rule_ids() + + @attr.s(slots=True, frozen=True, auto_attribs=True) class RuleSpec: scope: str diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py deleted file mode 100644 index 440205e80c05..000000000000 --- a/synapse/push/baserules.py +++ /dev/null @@ -1,583 +0,0 @@ -# Copyright 2015, 2016 OpenMarket Ltd -# Copyright 2017 New Vector Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -""" -Push rules is the system used to determine which events trigger a push (and a -bump in notification counts). - -This consists of a list of "push rules" for each user, where a push rule is a -pair of "conditions" and "actions". When a user receives an event Synapse -iterates over the list of push rules until it finds one where all the conditions -match the event, at which point "actions" describe the outcome (e.g. notify, -highlight, etc). - -Push rules are split up into 5 different "kinds" (aka "priority classes"), which -are run in order: - 1. Override — highest priority rules, e.g. always ignore notices - 2. Content — content specific rules, e.g. @ notifications - 3. Room — per room rules, e.g. enable/disable notifications for all messages - in a room - 4. Sender — per sender rules, e.g. never notify for messages from a given - user - 5. Underride — the lowest priority "default" rules, e.g. notify for every - message. - -The set of "base rules" are the list of rules that every user has by default. A -user can modify their copy of the push rules in one of three ways: - - 1. Adding a new push rule of a certain kind - 2. Changing the actions of a base rule - 3. Enabling/disabling a base rule. - -The base rules are split into whether they come before or after a particular -kind, so the order of push rule evaluation would be: base rules for before -"override" kind, user defined "override" rules, base rules after "override" -kind, etc, etc. -""" - -import itertools -import logging -from typing import Dict, Iterator, List, Mapping, Sequence, Tuple, Union - -import attr - -from synapse.config.experimental import ExperimentalConfig -from synapse.push.rulekinds import PRIORITY_CLASS_MAP - -logger = logging.getLogger(__name__) - - -@attr.s(auto_attribs=True, slots=True, frozen=True) -class PushRule: - """A push rule - - Attributes: - rule_id: a unique ID for this rule - priority_class: what "kind" of push rule this is (see - `PRIORITY_CLASS_MAP` for mapping between int and kind) - conditions: the sequence of conditions that all need to match - actions: the actions to apply if all conditions are met - default: is this a base rule? - default_enabled: is this enabled by default? - """ - - rule_id: str - priority_class: int - conditions: Sequence[Mapping[str, str]] - actions: Sequence[Union[str, Mapping]] - default: bool = False - default_enabled: bool = True - - -@attr.s(auto_attribs=True, slots=True, frozen=True, weakref_slot=False) -class PushRules: - """A collection of push rules for an account. - - Can be iterated over, producing push rules in priority order. - """ - - # A mapping from rule ID to push rule that overrides a base rule. These will - # be returned instead of the base rule. - overriden_base_rules: Dict[str, PushRule] = attr.Factory(dict) - - # The following stores the custom push rules at each priority class. - # - # We keep these separate (rather than combining into one big list) to avoid - # copying the base rules around all the time. - override: List[PushRule] = attr.Factory(list) - content: List[PushRule] = attr.Factory(list) - room: List[PushRule] = attr.Factory(list) - sender: List[PushRule] = attr.Factory(list) - underride: List[PushRule] = attr.Factory(list) - - def __iter__(self) -> Iterator[PushRule]: - # When iterating over the push rules we need to return the base rules - # interspersed at the correct spots. - for rule in itertools.chain( - BASE_PREPEND_OVERRIDE_RULES, - self.override, - BASE_APPEND_OVERRIDE_RULES, - self.content, - BASE_APPEND_CONTENT_RULES, - self.room, - self.sender, - self.underride, - BASE_APPEND_UNDERRIDE_RULES, - ): - # Check if a base rule has been overriden by a custom rule. If so - # return that instead. - override_rule = self.overriden_base_rules.get(rule.rule_id) - if override_rule: - yield override_rule - else: - yield rule - - def __len__(self) -> int: - # The length is mostly used by caches to get a sense of "size" / amount - # of memory this object is using, so we only count the number of custom - # rules. - return ( - len(self.overriden_base_rules) - + len(self.override) - + len(self.content) - + len(self.room) - + len(self.sender) - + len(self.underride) - ) - - -@attr.s(auto_attribs=True, slots=True, frozen=True, weakref_slot=False) -class FilteredPushRules: - """A wrapper around `PushRules` that filters out disabled experimental push - rules, and includes the "enabled" state for each rule when iterated over. - """ - - push_rules: PushRules - enabled_map: Dict[str, bool] - experimental_config: ExperimentalConfig - - def __iter__(self) -> Iterator[Tuple[PushRule, bool]]: - for rule in self.push_rules: - if not _is_experimental_rule_enabled( - rule.rule_id, self.experimental_config - ): - continue - - enabled = self.enabled_map.get(rule.rule_id, rule.default_enabled) - - yield rule, enabled - - def __len__(self) -> int: - return len(self.push_rules) - - -DEFAULT_EMPTY_PUSH_RULES = PushRules() - - -def compile_push_rules(rawrules: List[PushRule]) -> PushRules: - """Given a set of custom push rules return a `PushRules` instance (which - includes the base rules). - """ - - if not rawrules: - # Fast path to avoid allocating empty lists when there are no custom - # rules for the user. - return DEFAULT_EMPTY_PUSH_RULES - - rules = PushRules() - - for rule in rawrules: - # We need to decide which bucket each custom push rule goes into. - - # If it has the same ID as a base rule then it overrides that... - overriden_base_rule = BASE_RULES_BY_ID.get(rule.rule_id) - if overriden_base_rule: - rules.overriden_base_rules[rule.rule_id] = attr.evolve( - overriden_base_rule, actions=rule.actions - ) - continue - - # ... otherwise it gets added to the appropriate priority class bucket - collection: List[PushRule] - if rule.priority_class == 5: - collection = rules.override - elif rule.priority_class == 4: - collection = rules.content - elif rule.priority_class == 3: - collection = rules.room - elif rule.priority_class == 2: - collection = rules.sender - elif rule.priority_class == 1: - collection = rules.underride - elif rule.priority_class <= 0: - logger.info( - "Got rule with priority class less than zero, but doesn't override a base rule: %s", - rule, - ) - continue - else: - # We log and continue here so as not to break event sending - logger.error("Unknown priority class: %", rule.priority_class) - continue - - collection.append(rule) - - return rules - - -def _is_experimental_rule_enabled( - rule_id: str, experimental_config: ExperimentalConfig -) -> bool: - """Used by `FilteredPushRules` to filter out experimental rules when they - have not been enabled. - """ - if ( - rule_id == "global/override/.org.matrix.msc3786.rule.room.server_acl" - and not experimental_config.msc3786_enabled - ): - return False - if ( - rule_id == "global/underride/.org.matrix.msc3772.thread_reply" - and not experimental_config.msc3772_enabled - ): - return False - return True - - -BASE_APPEND_CONTENT_RULES = [ - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["content"], - rule_id="global/content/.m.rule.contains_user_name", - conditions=[ - { - "kind": "event_match", - "key": "content.body", - # Match the localpart of the requester's MXID. - "pattern_type": "user_localpart", - } - ], - actions=[ - "notify", - {"set_tweak": "sound", "value": "default"}, - {"set_tweak": "highlight"}, - ], - ) -] - - -BASE_PREPEND_OVERRIDE_RULES = [ - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.master", - default_enabled=False, - conditions=[], - actions=["dont_notify"], - ) -] - - -BASE_APPEND_OVERRIDE_RULES = [ - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.suppress_notices", - conditions=[ - { - "kind": "event_match", - "key": "content.msgtype", - "pattern": "m.notice", - "_cache_key": "_suppress_notices", - } - ], - actions=["dont_notify"], - ), - # NB. .m.rule.invite_for_me must be higher prio than .m.rule.member_event - # otherwise invites will be matched by .m.rule.member_event - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.invite_for_me", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.member", - "_cache_key": "_member", - }, - { - "kind": "event_match", - "key": "content.membership", - "pattern": "invite", - "_cache_key": "_invite_member", - }, - # Match the requester's MXID. - {"kind": "event_match", "key": "state_key", "pattern_type": "user_id"}, - ], - actions=[ - "notify", - {"set_tweak": "sound", "value": "default"}, - {"set_tweak": "highlight", "value": False}, - ], - ), - # Will we sometimes want to know about people joining and leaving? - # Perhaps: if so, this could be expanded upon. Seems the most usual case - # is that we don't though. We add this override rule so that even if - # the room rule is set to notify, we don't get notifications about - # join/leave/avatar/displayname events. - # See also: https://matrix.org/jira/browse/SYN-607 - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.member_event", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.member", - "_cache_key": "_member", - } - ], - actions=["dont_notify"], - ), - # This was changed from underride to override so it's closer in priority - # to the content rules where the user name highlight rule lives. This - # way a room rule is lower priority than both but a custom override rule - # is higher priority than both. - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.contains_display_name", - conditions=[{"kind": "contains_display_name"}], - actions=[ - "notify", - {"set_tweak": "sound", "value": "default"}, - {"set_tweak": "highlight"}, - ], - ), - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.roomnotif", - conditions=[ - { - "kind": "event_match", - "key": "content.body", - "pattern": "@room", - "_cache_key": "_roomnotif_content", - }, - { - "kind": "sender_notification_permission", - "key": "room", - "_cache_key": "_roomnotif_pl", - }, - ], - actions=["notify", {"set_tweak": "highlight", "value": True}], - ), - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.tombstone", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.tombstone", - "_cache_key": "_tombstone", - }, - { - "kind": "event_match", - "key": "state_key", - "pattern": "", - "_cache_key": "_tombstone_statekey", - }, - ], - actions=["notify", {"set_tweak": "highlight", "value": True}], - ), - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.m.rule.reaction", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.reaction", - "_cache_key": "_reaction", - } - ], - actions=["dont_notify"], - ), - # XXX: This is an experimental rule that is only enabled if msc3786_enabled - # is enabled, if it is not the rule gets filtered out in _load_rules() in - # PushRulesWorkerStore - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["override"], - rule_id="global/override/.org.matrix.msc3786.rule.room.server_acl", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.server_acl", - "_cache_key": "_room_server_acl", - }, - { - "kind": "event_match", - "key": "state_key", - "pattern": "", - "_cache_key": "_room_server_acl_state_key", - }, - ], - actions=[], - ), -] - - -BASE_APPEND_UNDERRIDE_RULES = [ - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["underride"], - rule_id="global/underride/.m.rule.call", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.call.invite", - "_cache_key": "_call", - } - ], - actions=[ - "notify", - {"set_tweak": "sound", "value": "ring"}, - {"set_tweak": "highlight", "value": False}, - ], - ), - # XXX: once m.direct is standardised everywhere, we should use it to detect - # a DM from the user's perspective rather than this heuristic. - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["underride"], - rule_id="global/underride/.m.rule.room_one_to_one", - conditions=[ - {"kind": "room_member_count", "is": "2", "_cache_key": "member_count"}, - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.message", - "_cache_key": "_message", - }, - ], - actions=[ - "notify", - {"set_tweak": "sound", "value": "default"}, - {"set_tweak": "highlight", "value": False}, - ], - ), - # XXX: this is going to fire for events which aren't m.room.messages - # but are encrypted (e.g. m.call.*)... - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["underride"], - rule_id="global/underride/.m.rule.encrypted_room_one_to_one", - conditions=[ - {"kind": "room_member_count", "is": "2", "_cache_key": "member_count"}, - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.encrypted", - "_cache_key": "_encrypted", - }, - ], - actions=[ - "notify", - {"set_tweak": "sound", "value": "default"}, - {"set_tweak": "highlight", "value": False}, - ], - ), - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["underride"], - rule_id="global/underride/.org.matrix.msc3772.thread_reply", - conditions=[ - { - "kind": "org.matrix.msc3772.relation_match", - "rel_type": "m.thread", - # Match the requester's MXID. - "sender_type": "user_id", - } - ], - actions=["notify", {"set_tweak": "highlight", "value": False}], - ), - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["underride"], - rule_id="global/underride/.m.rule.message", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.message", - "_cache_key": "_message", - } - ], - actions=["notify", {"set_tweak": "highlight", "value": False}], - ), - # XXX: this is going to fire for events which aren't m.room.messages - # but are encrypted (e.g. m.call.*)... - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["underride"], - rule_id="global/underride/.m.rule.encrypted", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "m.room.encrypted", - "_cache_key": "_encrypted", - } - ], - actions=["notify", {"set_tweak": "highlight", "value": False}], - ), - PushRule( - default=True, - priority_class=PRIORITY_CLASS_MAP["underride"], - rule_id="global/underride/.im.vector.jitsi", - conditions=[ - { - "kind": "event_match", - "key": "type", - "pattern": "im.vector.modular.widgets", - "_cache_key": "_type_modular_widgets", - }, - { - "kind": "event_match", - "key": "content.type", - "pattern": "jitsi", - "_cache_key": "_content_type_jitsi", - }, - { - "kind": "event_match", - "key": "state_key", - "pattern": "*", - "_cache_key": "_is_state_event", - }, - ], - actions=["notify", {"set_tweak": "highlight", "value": False}], - ), -] - - -BASE_RULE_IDS = set() - -BASE_RULES_BY_ID: Dict[str, PushRule] = {} - -for r in BASE_APPEND_CONTENT_RULES: - BASE_RULE_IDS.add(r.rule_id) - BASE_RULES_BY_ID[r.rule_id] = r - -for r in BASE_PREPEND_OVERRIDE_RULES: - BASE_RULE_IDS.add(r.rule_id) - BASE_RULES_BY_ID[r.rule_id] = r - -for r in BASE_APPEND_OVERRIDE_RULES: - BASE_RULE_IDS.add(r.rule_id) - BASE_RULES_BY_ID[r.rule_id] = r - -for r in BASE_APPEND_UNDERRIDE_RULES: - BASE_RULE_IDS.add(r.rule_id) - BASE_RULES_BY_ID[r.rule_id] = r From 62caed98a839626b7eeeadf938915c3772e91065 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 9 Sep 2022 15:49:35 +0100 Subject: [PATCH 03/16] Newsfile --- changelog.d/13768.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13768.misc diff --git a/changelog.d/13768.misc b/changelog.d/13768.misc new file mode 100644 index 000000000000..28bddb705967 --- /dev/null +++ b/changelog.d/13768.misc @@ -0,0 +1 @@ +Port push rules to using Rust. From 071953f49a0112513a1a2c2c0e20bd5dbf525af7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 12 Sep 2022 17:54:18 +0100 Subject: [PATCH 04/16] Update rust/src/push/mod.rs Co-authored-by: David Robertson --- rust/src/push/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 6df5382126c0..2a5fb625244f 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -388,7 +388,7 @@ impl FilteredPushRules { } #[test] -fn test_erialize_condition() { +fn test_serialize_condition() { let condition = Condition::EventMatch(EventMatchCondition { key: "content.body".into(), pattern: Some("coffee".into()), From 4d9734950f44d54b31be29d8426820405de21ffd Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 13 Sep 2022 08:13:44 +0100 Subject: [PATCH 05/16] Code review --- rust/src/push/mod.rs | 3 +++ stubs/synapse/synapse_rust/push.pyi | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 2a5fb625244f..dcbe9aeec0f0 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -126,6 +126,9 @@ pub enum Action { impl IntoPy for Action { fn into_py(self, py: Python<'_>) -> PyObject { + // When we pass the `Action` struct to Python we want it to be converted + // to a dict. We use `pythonize`, which converts the struct using the + // `serde` serialization. pythonize(py, &self).expect("valid action") } } diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi index cb59bf9d8af9..93c4e69d424f 100644 --- a/stubs/synapse/synapse_rust/push.pyi +++ b/stubs/synapse/synapse_rust/push.pyi @@ -3,13 +3,18 @@ from typing import Any, Collection, Dict, Mapping, Sequence, Tuple, Union from synapse.types import JsonDict class PushRule: - rule_id: str - priority_class: int - conditions: Sequence[Mapping[str, str]] - actions: Sequence[Union[Mapping[str, Any], str]] - default: bool - default_enabled: bool - + @property + def rule_id(self) -> str: ... + @property + def priority_class(self) -> int: ... + @property + def conditions(self) -> Sequence[Mapping[str, str]]: ... + @property + def actions(self) -> Sequence[Union[Mapping[str, Any], str]]: ... + @property + def default(self) -> bool: ... + @property + def default_enabled(self) -> bool: ... @staticmethod def from_db( rule_id: str, priority_class: int, conditions: str, actions: str From e828b34e4925c949927e7fb505d695a386cd1907 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:17:16 +0100 Subject: [PATCH 06/16] Apply suggestions from code review Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- rust/src/push/base_rules.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index eed3ae51b18c..aaedb2aab10f 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -130,7 +130,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ pattern_type: None, }), ]), - actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION]), default: true, default_enabled: true, }, @@ -302,7 +302,7 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ pattern_type: None, }), ]), - actions: Cow::Borrowed(&[HIGHLIGHT_FALSE_ACTION]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), default: true, default_enabled: true, }, From 47a3dcf90701678c2ead4edbad8832c6700a86c8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:17:55 +0100 Subject: [PATCH 07/16] Apply suggestions from code review Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- rust/src/push/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index dcbe9aeec0f0..848c61ff1b6f 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -59,13 +59,19 @@ fn get_base_rule_ids() -> HashSet<&'static str> { #[derive(Debug, Clone)] #[pyclass(frozen)] pub struct PushRule { + /// A unique ID for this rule pub rule_id: Cow<'static, str>, + /// The "kind" of push rule this is (see `PRIORITY_CLASS_MAP` in Python) #[pyo3(get)] pub priority_class: i32, + /// The conditions that must all match for actions to be applied pub conditions: Cow<'static, [Condition]>, + /// The actions to apply if all conditions are met pub actions: Cow<'static, [Action]>, + /// Whether this is a base rule #[pyo3(get)] pub default: bool, + /// Whether this is enabled by default #[pyo3(get)] pub default_enabled: bool, } From 68689fa8f57e721e94d8b6adce3fba1626679307 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:18:11 +0100 Subject: [PATCH 08/16] Apply suggestions from code review Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- rust/src/push/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 848c61ff1b6f..6e0c1034a189 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -147,7 +147,7 @@ pub struct SetTweak { #[serde(skip_serializing_if = "Option::is_none")] value: Option, - // This picks saves any other fields that may have been added as clients. + // This picks up any other fields that may have been added by clients. // These get added when we convert the `Action` to a python object. #[serde(flatten)] other_keys: Value, From a109a7756560b4d3442690005804fb1678689c7a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:18:54 +0100 Subject: [PATCH 09/16] Remove newsfile --- changelog.d/13720.misc | 1 - 1 file changed, 1 deletion(-) delete mode 100644 changelog.d/13720.misc diff --git a/changelog.d/13720.misc b/changelog.d/13720.misc deleted file mode 100644 index af5de96b0447..000000000000 --- a/changelog.d/13720.misc +++ /dev/null @@ -1 +0,0 @@ -Speed up push rule evaluation in rooms with large numbers of local users. From dbeb21b9dfe5084e14f948dd690b827c447c03c7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:20:03 +0100 Subject: [PATCH 10/16] Remove old todo --- synapse/storage/databases/main/push_rule.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py index 9d76e9b87979..ed17b2e70c04 100644 --- a/synapse/storage/databases/main/push_rule.py +++ b/synapse/storage/databases/main/push_rule.py @@ -85,7 +85,6 @@ def _load_rules( ruleslist, ) - # TODO: Experimental config filtered_rules = FilteredPushRules( push_rules, enabled_map, From 0e10bb36d3daa05c5139b793e47cc94566d2ad91 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:21:15 +0100 Subject: [PATCH 11/16] Swap actions --- rust/src/push/base_rules.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index aaedb2aab10f..1fd09e5fcff3 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -194,7 +194,7 @@ pub const BASE_APPEND_CONTENT_RULES: &[PushRule] = &[PushRule { pattern: None, pattern_type: Some(Cow::Borrowed("user_localpart")), })]), - actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), + actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION, HIGHLIGHT_ACTION]), default: true, default_enabled: true, }]; From 1e0584325d277551817ed50744949860d0077a51 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:22:26 +0100 Subject: [PATCH 12/16] Add back module docstring --- rust/src/push/mod.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 6e0c1034a189..2dff2202c2b3 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -17,6 +17,40 @@ //! The `Cow<_>` type is used extensively within this module to allow creating //! the base rules as constants (in Rust constants can't require explicit //! allocation atm). +//! +//! --- +//! +//! Push rules is the system used to determine which events trigger a push (and a +//! bump in notification counts). +//! +//! This consists of a list of "push rules" for each user, where a push rule is a +//! pair of "conditions" and "actions". When a user receives an event Synapse +//! iterates over the list of push rules until it finds one where all the conditions +//! match the event, at which point "actions" describe the outcome (e.g. notify, +//! highlight, etc). +//! +//! Push rules are split up into 5 different "kinds" (aka "priority classes"), which +//! are run in order: +//! 1. Override — highest priority rules, e.g. always ignore notices +//! 2. Content — content specific rules, e.g. @ notifications +//! 3. Room — per room rules, e.g. enable/disable notifications for all messages +//! in a room +//! 4. Sender — per sender rules, e.g. never notify for messages from a given +//! user +//! 5. Underride — the lowest priority "default" rules, e.g. notify for every +//! message. +//! +//! The set of "base rules" are the list of rules that every user has by default. A +//! user can modify their copy of the push rules in one of three ways: +//! +//! 1. Adding a new push rule of a certain kind +//! 2. Changing the actions of a base rule +//! 3. Enabling/disabling a base rule. +//! +//! The base rules are split into whether they come before or after a particular +//! kind, so the order of push rule evaluation would be: base rules for before +//! "override" kind, user defined "override" rules, base rules after "override" +//! kind, etc, etc. use std::borrow::Cow; use std::collections::{BTreeMap, HashMap, HashSet}; From 415bbd691fb10d44634522beaeabe30b69882fb8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:23:58 +0100 Subject: [PATCH 13/16] Remove debug logging --- synapse/push/bulk_push_rule_evaluator.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index f82f15ec4492..dafd4730f246 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -287,8 +287,6 @@ async def action_for_event_by_user( if relation.rel_type == RelationTypes.THREAD: thread_id = relation.parent_id - logger.info("Flatten map: %s", _flatten_dict(event)) - logger.info("room_member_count: %s", room_member_count) evaluator = PushRuleEvaluatorForEvent( event, room_member_count, From 86515a9e54f02cb06659a02e093cdc0e7972ef11 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 10:27:43 +0100 Subject: [PATCH 14/16] Handle unrecognized actions --- rust/src/push/mod.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 2dff2202c2b3..c8c9c15da7ac 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -162,6 +162,9 @@ pub enum Action { Notify, Coalesce, SetTweak(SetTweak), + + // An unrecognized custom action. + Unknown(Value), } impl IntoPy for Action { @@ -208,6 +211,7 @@ impl Serialize for Action { Action::Notify => serializer.serialize_str("notify"), Action::Coalesce => serializer.serialize_str("coalesce"), Action::SetTweak(tweak) => tweak.serialize(serializer), + Action::Unknown(value) => value.serialize(serializer), } } } @@ -218,6 +222,7 @@ impl Serialize for Action { enum ActionDeserializeHelper { Str(String), SetTweak(SetTweak), + Unknown(Value), } impl<'de> Deserialize<'de> for Action { @@ -234,6 +239,7 @@ impl<'de> Deserialize<'de> for Action { _ => Err(D::Error::custom("unrecognized action")), }, ActionDeserializeHelper::SetTweak(set_tweak) => Ok(Action::SetTweak(set_tweak)), + ActionDeserializeHelper::Unknown(value) => Ok(Action::Unknown(value)), } } } @@ -459,3 +465,14 @@ fn test_deserialize_action() { let _: Action = serde_json::from_str(r#""coalesce""#).unwrap(); let _: Action = serde_json::from_str(r#"{"set_tweak": "highlight"}"#).unwrap(); } + +#[test] +fn test_custom_action() { + let json = r#"{"some_custom":"action_fields"}"#; + + let action: Action = serde_json::from_str(json).unwrap(); + assert!(matches!(action, Action::Unknown(_))); + + let new_json = serde_json::to_string(&action).unwrap(); + assert_eq!(json, new_json); +} From b57be51f5a6adcd5fdb2ad9d9dce4d75b296cfdc Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 11:00:31 +0100 Subject: [PATCH 15/16] Correctly handle unknwon conditions --- rust/src/push/base_rules.rs | 157 ++++++++++++++++++++---------------- rust/src/push/mod.rs | 30 ++++++- 2 files changed, 113 insertions(+), 74 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index 1fd09e5fcff3..7c62bc48490f 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -20,6 +20,7 @@ use std::collections::HashMap; use lazy_static::lazy_static; use serde_json::Value; +use super::KnownCondition; use crate::push::Action; use crate::push::Condition; use crate::push::EventMatchCondition; @@ -64,11 +65,13 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/override/.m.rule.suppress_notices"), priority_class: 5, - conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { - key: Cow::Borrowed("content.msgtype"), - pattern: Some(Cow::Borrowed("m.notice")), - pattern_type: None, - })]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { + key: Cow::Borrowed("content.msgtype"), + pattern: Some(Cow::Borrowed("m.notice")), + pattern_type: None, + }, + ))]), actions: Cow::Borrowed(&[Action::DontNotify]), default: true, default_enabled: true, @@ -77,21 +80,21 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/override/.m.rule.invite_for_me"), priority_class: 5, conditions: Cow::Borrowed(&[ - Condition::EventMatch(EventMatchCondition { + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), pattern: Some(Cow::Borrowed("m.room.member")), pattern_type: None, - }), - Condition::EventMatch(EventMatchCondition { + })), + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("content.membership"), pattern: Some(Cow::Borrowed("invite")), pattern_type: None, - }), - Condition::EventMatch(EventMatchCondition { + })), + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("state_key"), pattern: None, pattern_type: Some(Cow::Borrowed("user_id")), - }), + })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION, SOUND_ACTION]), default: true, @@ -100,11 +103,13 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/override/.m.rule.member_event"), priority_class: 5, - conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { - key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.member")), - pattern_type: None, - })]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.member")), + pattern_type: None, + }, + ))]), actions: Cow::Borrowed(&[Action::DontNotify]), default: true, default_enabled: true, @@ -112,7 +117,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/override/.m.rule.contains_display_name"), priority_class: 5, - conditions: Cow::Borrowed(&[Condition::ContainsDisplayName]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::ContainsDisplayName)]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), default: true, default_enabled: true, @@ -121,14 +126,14 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/override/.m.rule.roomnotif"), priority_class: 5, conditions: Cow::Borrowed(&[ - Condition::SenderNotificationPermission { + Condition::Known(KnownCondition::SenderNotificationPermission { key: Cow::Borrowed("room"), - }, - Condition::EventMatch(EventMatchCondition { + }), + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("content.body"), pattern: Some(Cow::Borrowed("@room")), pattern_type: None, - }), + })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION]), default: true, @@ -138,16 +143,16 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/override/.m.rule.tombstone"), priority_class: 5, conditions: Cow::Borrowed(&[ - Condition::EventMatch(EventMatchCondition { + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), pattern: Some(Cow::Borrowed("m.room.tombstone")), pattern_type: None, - }), - Condition::EventMatch(EventMatchCondition { + })), + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("state_key"), pattern: Some(Cow::Borrowed("")), pattern_type: None, - }), + })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION]), default: true, @@ -156,11 +161,13 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/override/.m.rule.reaction"), priority_class: 5, - conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { - key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.reaction")), - pattern_type: None, - })]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.reaction")), + pattern_type: None, + }, + ))]), actions: Cow::Borrowed(&[Action::DontNotify]), default: true, default_enabled: true, @@ -169,16 +176,16 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/override/.org.matrix.msc3786.rule.room.server_acl"), priority_class: 5, conditions: Cow::Borrowed(&[ - Condition::EventMatch(EventMatchCondition { + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), pattern: Some(Cow::Borrowed("m.room.server_acl")), pattern_type: None, - }), - Condition::EventMatch(EventMatchCondition { + })), + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("state_key"), pattern: Some(Cow::Borrowed("")), pattern_type: None, - }), + })), ]), actions: Cow::Borrowed(&[]), default: true, @@ -189,12 +196,14 @@ 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::EventMatch(EventMatchCondition { - key: Cow::Borrowed("content.body"), - pattern: None, - pattern_type: Some(Cow::Borrowed("user_localpart")), - })]), - actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION, HIGHLIGHT_ACTION]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { + key: Cow::Borrowed("content.body"), + pattern: None, + pattern_type: Some(Cow::Borrowed("user_localpart")), + }, + ))]), + actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), default: true, default_enabled: true, }]; @@ -203,11 +212,13 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/underride/.m.rule.call"), priority_class: 1, - conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { - key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.call.invite")), - pattern_type: None, - })]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.call.invite")), + pattern_type: None, + }, + ))]), actions: Cow::Borrowed(&[Action::Notify, RING_ACTION, HIGHLIGHT_FALSE_ACTION]), default: true, default_enabled: true, @@ -216,14 +227,14 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/underride/.m.rule.room_one_to_one"), priority_class: 1, conditions: Cow::Borrowed(&[ - Condition::EventMatch(EventMatchCondition { + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), pattern: Some(Cow::Borrowed("m.room.message")), pattern_type: None, - }), - Condition::RoomMemberCount { + })), + Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), - }, + }), ]), actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION, HIGHLIGHT_FALSE_ACTION]), default: true, @@ -233,14 +244,14 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/underride/.m.rule.encrypted_room_one_to_one"), priority_class: 1, conditions: Cow::Borrowed(&[ - Condition::EventMatch(EventMatchCondition { + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), pattern: Some(Cow::Borrowed("m.room.encrypted")), pattern_type: None, - }), - Condition::RoomMemberCount { + })), + Condition::Known(KnownCondition::RoomMemberCount { is: Some(Cow::Borrowed("2")), - }, + }), ]), actions: Cow::Borrowed(&[Action::Notify, SOUND_ACTION, HIGHLIGHT_FALSE_ACTION]), default: true, @@ -249,11 +260,11 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/underride/.org.matrix.msc3772.thread_reply"), priority_class: 1, - conditions: Cow::Borrowed(&[Condition::RelationMatch { + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::RelationMatch { rel_type: Cow::Borrowed("m.thread"), sender: None, sender_type: Some(Cow::Borrowed("user_id")), - }]), + })]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), default: true, default_enabled: true, @@ -261,11 +272,13 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/underride/.m.rule.message"), priority_class: 1, - conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { - key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.message")), - pattern_type: None, - })]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.message")), + pattern_type: None, + }, + ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), default: true, default_enabled: true, @@ -273,11 +286,13 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/underride/.m.rule.encrypted"), priority_class: 1, - conditions: Cow::Borrowed(&[Condition::EventMatch(EventMatchCondition { - key: Cow::Borrowed("type"), - pattern: Some(Cow::Borrowed("m.room.encrypted")), - pattern_type: None, - })]), + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( + EventMatchCondition { + key: Cow::Borrowed("type"), + pattern: Some(Cow::Borrowed("m.room.encrypted")), + pattern_type: None, + }, + ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), default: true, default_enabled: true, @@ -286,21 +301,21 @@ pub const BASE_APPEND_UNDERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/underride/.im.vector.jitsi"), priority_class: 1, conditions: Cow::Borrowed(&[ - Condition::EventMatch(EventMatchCondition { + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("type"), pattern: Some(Cow::Borrowed("im.vector.modular.widgets")), pattern_type: None, - }), - Condition::EventMatch(EventMatchCondition { + })), + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("content.type"), pattern: Some(Cow::Borrowed("jitsi")), pattern_type: None, - }), - Condition::EventMatch(EventMatchCondition { + })), + Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: Cow::Borrowed("state_key"), pattern: Some(Cow::Borrowed("*")), pattern_type: None, - }), + })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_FALSE_ACTION]), default: true, diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index c8c9c15da7ac..de6764e7c5c7 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -245,10 +245,23 @@ impl<'de> Deserialize<'de> for Action { } /// A condition used in push rules to match against an event. +/// +/// We need this split as `serde` doesn't give us the ability to have a +/// "catchall" variant in tagged enums. +#[derive(Serialize, Deserialize, Debug, Clone)] +#[serde(untagged)] +pub enum Condition { + /// A recognized condition that we can match against + Known(KnownCondition), + /// An unrecognized condition that we ignore. + Unknown(Value), +} + +/// The set of "known" conditions that we can handle. #[derive(Serialize, Deserialize, Debug, Clone)] #[serde(rename_all = "snake_case")] #[serde(tag = "kind")] -pub enum Condition { +pub enum KnownCondition { EventMatch(EventMatchCondition), ContainsDisplayName, RoomMemberCount { @@ -438,11 +451,11 @@ impl FilteredPushRules { #[test] fn test_serialize_condition() { - let condition = Condition::EventMatch(EventMatchCondition { + let condition = Condition::Known(KnownCondition::EventMatch(EventMatchCondition { key: "content.body".into(), pattern: Some("coffee".into()), pattern_type: None, - }); + })); let json = serde_json::to_string(&condition).unwrap(); assert_eq!( @@ -458,6 +471,17 @@ fn test_deserialize_condition() { let _: Condition = serde_json::from_str(json).unwrap(); } +#[test] +fn test_deserialize_custom_condition() { + let json = r#"{"kind":"custom_tag"}"#; + + let condition: Condition = serde_json::from_str(json).unwrap(); + assert!(matches!(condition, Condition::Unknown(_))); + + let new_json = serde_json::to_string(&condition).unwrap(); + assert_eq!(json, new_json); +} + #[test] fn test_deserialize_action() { let _: Action = serde_json::from_str(r#""notify""#).unwrap(); From a691a2dbdce27cbbecada278c384c74132e3b914 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 19 Sep 2022 11:01:08 +0100 Subject: [PATCH 16/16] Lint --- synapse/push/bulk_push_rule_evaluator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index dafd4730f246..404379ef67d3 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -34,7 +34,6 @@ from synapse.event_auth import auth_types_for_event, get_user_power_level from synapse.events import EventBase, relation_from_event from synapse.events.snapshot import EventContext -from synapse.push.push_rule_evaluator import _flatten_dict from synapse.state import POWER_KEY from synapse.storage.databases.main.roommember import EventIdMembership from synapse.storage.state import StateFilter