From 443368f3ef7b2c5d71b29e2b65a0570f5ff82090 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Sat, 27 Jan 2024 14:16:10 +0100 Subject: [PATCH 01/10] feat: time and day push rule condition rust --- changelog.d/16858.feature | 1 + rust/Cargo.toml | 1 + rust/src/push/evaluator.rs | 106 ++++++++++++++++++++++++++++++++++++- rust/src/push/mod.rs | 34 ++++++++++++ 4 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 changelog.d/16858.feature diff --git a/changelog.d/16858.feature b/changelog.d/16858.feature new file mode 100644 index 00000000000..260e5f7576a --- /dev/null +++ b/changelog.d/16858.feature @@ -0,0 +1 @@ +Experimental support for [MSC3767](https://github.com/matrix-org/matrix-spec-proposals/pull/3767): the `time_and_day` push rule condition. Contributed by @hanadi92. \ No newline at end of file diff --git a/rust/Cargo.toml b/rust/Cargo.toml index d89def1843a..aa48c8ba593 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -36,6 +36,7 @@ pythonize = "0.20.0" regex = "1.6.0" serde = { version = "1.0.144", features = ["derive"] } serde_json = "1.0.85" +chrono = "0.4.33" [features] extension-module = ["pyo3/extension-module"] diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 2f4b6d47bb2..319840f9dc8 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -23,6 +23,7 @@ use std::borrow::Cow; use std::collections::BTreeMap; use anyhow::{Context, Error}; +use chrono::Datelike; use lazy_static::lazy_static; use log::warn; use pyo3::prelude::*; @@ -31,7 +32,7 @@ use regex::Regex; use super::{ utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, Action, Condition, EventPropertyIsCondition, FilteredPushRules, KnownCondition, - SimpleJsonValue, + SimpleJsonValue, TimeAndDayIntervals, }; use crate::push::{EventMatchPatternType, JsonValue}; @@ -105,6 +106,9 @@ pub struct PushRuleEvaluator { /// If MSC3931 (room version feature flags) is enabled. Usually controlled by the same /// flag as MSC1767 (extensible events core). msc3931_enabled: bool, + + /// If MSC3767 (time based notification filtering push rule condition) is enabled + msc3767_time_and_day: bool, } #[pymethods] @@ -122,6 +126,7 @@ impl PushRuleEvaluator { related_event_match_enabled, room_version_feature_flags, msc3931_enabled, + msc3767_time_and_day, ))] pub fn py_new( flattened_keys: BTreeMap, @@ -133,6 +138,7 @@ impl PushRuleEvaluator { related_event_match_enabled: bool, room_version_feature_flags: Vec, msc3931_enabled: bool, + msc3767_time_and_day: bool, ) -> Result { let body = match flattened_keys.get("content.body") { Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone().into_owned(), @@ -150,6 +156,7 @@ impl PushRuleEvaluator { related_event_match_enabled, room_version_feature_flags, msc3931_enabled, + msc3767_time_and_day, }) } @@ -384,6 +391,13 @@ impl PushRuleEvaluator { && self.room_version_feature_flags.contains(&flag) } } + KnownCondition::TimeAndDay(time_and_day) => { + if !self.msc3767_time_and_day { + false + } else { + self.match_time_and_day(time_and_day.timezone.clone(), &time_and_day.intervals)? + } + } }; Ok(result) @@ -507,6 +521,31 @@ impl PushRuleEvaluator { Ok(matches) } + + /// + fn match_time_and_day( + &self, + _timezone: Option>, + intervals: &[TimeAndDayIntervals], + ) -> Result { + // Temp Notes from spec: + // The timezone to use for time comparison. This format allows for automatic DST handling. + // Intervals representing time periods in which the rule should match. Evaluated with an OR condition. + // + // time_of_day condition is met when the server's timezone-adjusted time is between the values of the tuple, + // or when no time_of_day is set on the interval. Values are inclusive. + // day_of_week condition is met when the server's timezone-adjusted day is included in the array. + // next step -> consider timezone if given + let now = chrono::Utc::now(); + let today = now.weekday().num_days_from_sunday(); + let current_time = now.time().format("%H:%M").to_string(); + let matches = intervals.iter().any(|interval| { + interval.day_of_week.contains(&today) + && interval.time_of_day.contains(current_time.to_string()) + }); + + Ok(matches) + } } #[test] @@ -526,6 +565,7 @@ fn push_rule_evaluator() { true, vec![], true, + true, ) .unwrap(); @@ -555,6 +595,7 @@ fn test_requires_room_version_supports_condition() { false, flags, true, + true, ) .unwrap(); @@ -588,3 +629,66 @@ fn test_requires_room_version_supports_condition() { ); assert_eq!(result.len(), 1); } + +#[test] +fn test_time_and_day_condition() { + use crate::push::{PushRule, PushRules, TimeAndDayCondition, TimeInterval}; + + let mut flattened_keys = BTreeMap::new(); + flattened_keys.insert( + "content.body".to_string(), + JsonValue::Value(SimpleJsonValue::Str(Cow::Borrowed("foo bar bob hello"))), + ); + let evaluator = PushRuleEvaluator::py_new( + flattened_keys, + false, + 10, + Some(0), + BTreeMap::new(), + BTreeMap::new(), + true, + vec![], + true, + true, + ) + .unwrap(); + + // smoke test: notify is working with other conditions + let result = evaluator.run( + &FilteredPushRules::default(), + Some("@bob:example.org"), + None, + ); + assert_eq!(result.len(), 3); + + // time and day condition in push rule + let custom_rule = PushRule { + rule_id: Cow::from(".m.rule.master"), + priority_class: 5, // override + conditions: Cow::from(vec![Condition::Known(KnownCondition::TimeAndDay( + TimeAndDayCondition { + timezone: None, + intervals: vec![TimeAndDayIntervals { + // for testing, make all days as dnd + time_of_day: TimeInterval { + start_time: Cow::from("00:01"), + end_time: Cow::from("23:59"), + }, + day_of_week: vec![0, 1, 2, 3, 4, 5, 6], + }], + }, + ))]), + actions: Cow::from(vec![Action::DontNotify]), + default: true, + default_enabled: true, + }; + let rules = PushRules::new(vec![custom_rule]); + let result = evaluator.run( + &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false), + None, + None, + ); + + // dnd time, dont_notify + assert_eq!(result.len(), 0); +} diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 7dedbf10b6a..72d619f6349 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -361,6 +361,8 @@ pub enum KnownCondition { RoomVersionSupports { feature: Cow<'static, str>, }, + #[serde(rename = "org.matrix.msc3767.time_and_day")] + TimeAndDay(TimeAndDayCondition), } impl IntoPy for Condition { @@ -438,6 +440,38 @@ pub struct RelatedEventMatchTypeCondition { pub include_fallbacks: Option, } +/// The body of [`KnownCondition::TimeAndDay`] +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct TimeAndDayCondition { + /// Timezone to use for time comparison + pub timezone: Option>, + /// Time periods in which the rule should match + pub intervals: Vec, +} + +/// +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct TimeAndDayIntervals { + /// Tuple of hh::mm representing start and end times of the day + pub time_of_day: TimeInterval, + /// 0 = Sunday, 1 = Monday, ..., 7 = Sunday + pub day_of_week: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone)] +pub struct TimeInterval { + start_time: Cow<'static, str>, + end_time: Cow<'static, str>, +} + +impl TimeInterval { + /// Checks whether the provided time is within the interval + pub fn contains(&self, time: String) -> bool { + // Since MSC specifies ISO 8601 which uses 24h, string comparison is valid. + time >= self.start_time.parse().unwrap() && time <= self.end_time.parse().unwrap() + } +} + /// The collection of push rules for a user. #[derive(Debug, Clone, Default)] #[pyclass(frozen)] From 933e8867c6e161f5936e4c264772a8107f3628d6 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Sat, 27 Jan 2024 19:50:15 +0100 Subject: [PATCH 02/10] feat: time and day push rule condition python --- synapse/config/experimental.py | 5 +++++ synapse/push/bulk_push_rule_evaluator.py | 1 + synapse/synapse_rust/push.pyi | 1 + tests/push/test_push_rule_evaluator.py | 19 +++++++++++++++++++ 4 files changed, 26 insertions(+) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d43d9da956c..1ea5b264142 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -430,3 +430,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.msc4069_profile_inhibit_propagation = experimental.get( "msc4069_profile_inhibit_propagation", False ) + + # MSC3767: time based notification filtering + self.msc3767_time_and_day = experimental.get( + "org.matrix.msc3767.time_and_day", False + ) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 34ab637c3d0..3fc1a4f59be 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -435,6 +435,7 @@ async def _action_for_event_by_user( self._related_event_match_enabled, event.room_version.msc3931_push_features, self.hs.config.experimental.msc1767_enabled, # MSC3931 flag + self.hs.config.experimental.msc3767_time_and_day, ) for uid, rules in rules_by_user.items(): diff --git a/synapse/synapse_rust/push.pyi b/synapse/synapse_rust/push.pyi index 27a974e1bbe..ba38ab0b35c 100644 --- a/synapse/synapse_rust/push.pyi +++ b/synapse/synapse_rust/push.pyi @@ -65,6 +65,7 @@ class PushRuleEvaluator: related_event_match_enabled: bool, room_version_feature_flags: Tuple[str, ...], msc3931_enabled: bool, + msc3767_time_and_day: bool, ): ... def run( self, diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index 420fbea9982..200e681ced7 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -174,6 +174,7 @@ def _get_evaluator( related_event_match_enabled=True, room_version_feature_flags=event.room_version.msc3931_push_features, msc3931_enabled=True, + msc3767_time_and_day=True, ) def test_display_name(self) -> None: @@ -802,6 +803,24 @@ def test_related_event_match_no_related_event(self) -> None: ) ) + def test_time_and_day_match(self) -> None: + # for testing, make whole day in do not disturb + condition = { + "kind": "org.matrix.msc3767.time_and_day", + "timezone": "", + "intervals": [ + { + "time_of_day": ["00:01", "23:59"], + "day_of_week": [0, 1, 2, 3, 4, 5, 6], + }, + ], + } + + evaluator = self._get_evaluator({"body": "foo bar baz"}) + action = evaluator.matches(condition, "@user:test", "user") + + self.assertTrue(action) + class TestBulkPushRuleEvaluator(unittest.HomeserverTestCase): """Tests for the bulk push rule evaluator""" From bbe92efd57c6744b434fb2bc8b3c8595e05c9c95 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Sun, 28 Jan 2024 08:40:25 +0100 Subject: [PATCH 03/10] fix: forgotten bench code --- rust/benches/evaluator.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index 4fea035b961..8f9101465a7 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -60,6 +60,7 @@ fn bench_match_exact(b: &mut Bencher) { true, vec![], false, + false, ) .unwrap(); @@ -105,6 +106,7 @@ fn bench_match_word(b: &mut Bencher) { true, vec![], false, + false, ) .unwrap(); @@ -150,6 +152,7 @@ fn bench_match_word_miss(b: &mut Bencher) { true, vec![], false, + false, ) .unwrap(); @@ -195,6 +198,7 @@ fn bench_eval_message(b: &mut Bencher) { true, vec![], false, + false, ) .unwrap(); From b38d85c79335ac1f7bec27ba00c7b06a8529f6a6 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Sun, 28 Jan 2024 10:09:17 +0100 Subject: [PATCH 04/10] feat: handle timezone --- rust/Cargo.toml | 1 + rust/src/push/evaluator.rs | 46 +++++++++++--------------- rust/src/push/mod.rs | 23 +++++++++---- rust/src/push/utils.rs | 13 ++++++++ tests/push/test_push_rule_evaluator.py | 3 +- 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/rust/Cargo.toml b/rust/Cargo.toml index aa48c8ba593..51d253014eb 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -37,6 +37,7 @@ regex = "1.6.0" serde = { version = "1.0.144", features = ["derive"] } serde_json = "1.0.85" chrono = "0.4.33" +chrono-tz = { version = "0.8.5", features = ["serde"]} [features] extension-module = ["pyo3/extension-module"] diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 319840f9dc8..6ab28d60c6c 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -23,14 +23,14 @@ use std::borrow::Cow; use std::collections::BTreeMap; use anyhow::{Context, Error}; -use chrono::Datelike; +use chrono_tz::Tz; use lazy_static::lazy_static; use log::warn; use pyo3::prelude::*; use regex::Regex; use super::{ - utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, + utils::{day_and_time_with_timezone, get_glob_matcher, get_localpart_from_id, GlobMatchType}, Action, Condition, EventPropertyIsCondition, FilteredPushRules, KnownCondition, SimpleJsonValue, TimeAndDayIntervals, }; @@ -395,7 +395,7 @@ impl PushRuleEvaluator { if !self.msc3767_time_and_day { false } else { - self.match_time_and_day(time_and_day.timezone.clone(), &time_and_day.intervals)? + self.match_time_and_day(time_and_day.timezone, &time_and_day.intervals)? } } }; @@ -522,26 +522,22 @@ impl PushRuleEvaluator { Ok(matches) } - /// + /// Match the current server time - adjusted with user timezone or default UTC - + /// against the intervals using Any/OR. fn match_time_and_day( &self, - _timezone: Option>, + timezone: Option, intervals: &[TimeAndDayIntervals], ) -> Result { - // Temp Notes from spec: - // The timezone to use for time comparison. This format allows for automatic DST handling. - // Intervals representing time periods in which the rule should match. Evaluated with an OR condition. - // - // time_of_day condition is met when the server's timezone-adjusted time is between the values of the tuple, - // or when no time_of_day is set on the interval. Values are inclusive. - // day_of_week condition is met when the server's timezone-adjusted day is included in the array. - // next step -> consider timezone if given - let now = chrono::Utc::now(); - let today = now.weekday().num_days_from_sunday(); - let current_time = now.time().format("%H:%M").to_string(); + // Evaluate today and current time with timezone + let (today, current_time) = day_and_time_with_timezone(timezone); + let matches = intervals.iter().any(|interval| { interval.day_of_week.contains(&today) - && interval.time_of_day.contains(current_time.to_string()) + && (interval.time_of_day.is_none() + || interval.time_of_day.as_ref().map_or(true, |time_interval| { + time_interval.contains(current_time.to_string()) || time_interval.is_empty() + })) }); Ok(matches) @@ -632,7 +628,7 @@ fn test_requires_room_version_supports_condition() { #[test] fn test_time_and_day_condition() { - use crate::push::{PushRule, PushRules, TimeAndDayCondition, TimeInterval}; + use crate::push::{PushRule, PushRules, TimeAndDayCondition}; let mut flattened_keys = BTreeMap::new(); flattened_keys.insert( @@ -669,16 +665,14 @@ fn test_time_and_day_condition() { TimeAndDayCondition { timezone: None, intervals: vec![TimeAndDayIntervals { - // for testing, make all days as dnd - time_of_day: TimeInterval { - start_time: Cow::from("00:01"), - end_time: Cow::from("23:59"), - }, + // for testing, to avoid flakiness, make all days as dnd + time_of_day: None, day_of_week: vec![0, 1, 2, 3, 4, 5, 6], }], }, ))]), - actions: Cow::from(vec![Action::DontNotify]), + // for testing, make this notify. Since DoNotNotify is filtered out. + actions: Cow::from(vec![Action::Notify]), default: true, default_enabled: true, }; @@ -689,6 +683,6 @@ fn test_time_and_day_condition() { None, ); - // dnd time, dont_notify - assert_eq!(result.len(), 0); + // dnd time, we made it Notify + assert_eq!(result.len(), 1); } diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 72d619f6349..35df4266dd4 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -62,6 +62,7 @@ use std::borrow::Cow; use std::collections::{BTreeMap, HashMap, HashSet}; use anyhow::{Context, Error}; +use chrono_tz::Tz; use log::warn; use pyo3::exceptions::PyTypeError; use pyo3::prelude::*; @@ -440,36 +441,44 @@ pub struct RelatedEventMatchTypeCondition { pub include_fallbacks: Option, } -/// The body of [`KnownCondition::TimeAndDay`] +/// The body of [`KnownCondition::TimeAndDay`]. #[derive(Serialize, Deserialize, Debug, Clone)] pub struct TimeAndDayCondition { - /// Timezone to use for time comparison - pub timezone: Option>, - /// Time periods in which the rule should match + /// Timezone to use for the server. + pub timezone: Option, + /// Time periods in which the rule should match. pub intervals: Vec, } -/// +/// Defines the intervals of `TimeAndDayCondition` #[derive(Serialize, Deserialize, Debug, Clone)] pub struct TimeAndDayIntervals { /// Tuple of hh::mm representing start and end times of the day - pub time_of_day: TimeInterval, + pub time_of_day: Option, /// 0 = Sunday, 1 = Monday, ..., 7 = Sunday pub day_of_week: Vec, } +/// Defines the time_of_day of `TimeAndDayIntervals` #[derive(Serialize, Deserialize, Debug, Clone)] pub struct TimeInterval { start_time: Cow<'static, str>, end_time: Cow<'static, str>, } +/// Implementation for `TimeInterval` impl TimeInterval { - /// Checks whether the provided time is within the interval + /// Checks if the provided time is within the interval. pub fn contains(&self, time: String) -> bool { // Since MSC specifies ISO 8601 which uses 24h, string comparison is valid. time >= self.start_time.parse().unwrap() && time <= self.end_time.parse().unwrap() } + + /// Checks if the interval is empty. + /// Considering if the two ends are equal or start is after end. + pub fn is_empty(&self) -> bool { + self.start_time >= self.end_time + } } /// The collection of push rules for a user. diff --git a/rust/src/push/utils.rs b/rust/src/push/utils.rs index 28ebed62c88..171e09ca2bb 100644 --- a/rust/src/push/utils.rs +++ b/rust/src/push/utils.rs @@ -22,6 +22,8 @@ use anyhow::bail; use anyhow::Context; use anyhow::Error; +use chrono::{Datelike, Utc}; +use chrono_tz::Tz; use lazy_static::lazy_static; use regex; use regex::Regex; @@ -165,6 +167,17 @@ impl Matcher { } } +/// Returns `today` and `current_time` based on the given timezone. Otherwise, Utc. +pub fn day_and_time_with_timezone(timezone: Option) -> (u32, String) { + let tz: Tz = timezone.unwrap_or(Tz::UTC); + let time_with_tz = Utc::now().with_timezone(&tz); + + let today_with_timezone = time_with_tz.weekday().num_days_from_sunday(); + let current_time_with_timezone = time_with_tz.time().format("%H:%M").to_string(); + + (today_with_timezone, current_time_with_timezone) +} + #[test] fn test_get_domain_from_id() { get_localpart_from_id("").unwrap_err(); diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index 200e681ced7..e330ac395a3 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -807,10 +807,9 @@ def test_time_and_day_match(self) -> None: # for testing, make whole day in do not disturb condition = { "kind": "org.matrix.msc3767.time_and_day", - "timezone": "", + "timezone": None, "intervals": [ { - "time_of_day": ["00:01", "23:59"], "day_of_week": [0, 1, 2, 3, 4, 5, 6], }, ], From fe9f9a81f01b3e9e2fd90dfac0976be210a63df9 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Wed, 8 May 2024 18:32:38 +0300 Subject: [PATCH 05/10] fix: update msc number --- changelog.d/16858.feature | 2 +- rust/src/push/evaluator.rs | 12 ++++++------ rust/src/push/mod.rs | 4 ++-- synapse/config/experimental.py | 6 +++--- synapse/push/bulk_push_rule_evaluator.py | 2 +- synapse/synapse_rust/push.pyi | 2 +- tests/push/test_push_rule_evaluator.py | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/changelog.d/16858.feature b/changelog.d/16858.feature index 260e5f7576a..616ac4cc058 100644 --- a/changelog.d/16858.feature +++ b/changelog.d/16858.feature @@ -1 +1 @@ -Experimental support for [MSC3767](https://github.com/matrix-org/matrix-spec-proposals/pull/3767): the `time_and_day` push rule condition. Contributed by @hanadi92. \ No newline at end of file +Experimental support for [MSC4141](https://github.com/matrix-org/matrix-spec-proposals/pull/4141): the `time_and_day` push rule condition. Contributed by @hanadi92. \ No newline at end of file diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 6ab28d60c6c..bc2bf427f9d 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -107,8 +107,8 @@ pub struct PushRuleEvaluator { /// flag as MSC1767 (extensible events core). msc3931_enabled: bool, - /// If MSC3767 (time based notification filtering push rule condition) is enabled - msc3767_time_and_day: bool, + /// If MSC4141 (time based notification filtering push rule condition) is enabled + msc4141_time_and_day: bool, } #[pymethods] @@ -126,7 +126,7 @@ impl PushRuleEvaluator { related_event_match_enabled, room_version_feature_flags, msc3931_enabled, - msc3767_time_and_day, + msc4141_time_and_day, ))] pub fn py_new( flattened_keys: BTreeMap, @@ -138,7 +138,7 @@ impl PushRuleEvaluator { related_event_match_enabled: bool, room_version_feature_flags: Vec, msc3931_enabled: bool, - msc3767_time_and_day: bool, + msc4141_time_and_day: bool, ) -> Result { let body = match flattened_keys.get("content.body") { Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone().into_owned(), @@ -156,7 +156,7 @@ impl PushRuleEvaluator { related_event_match_enabled, room_version_feature_flags, msc3931_enabled, - msc3767_time_and_day, + msc4141_time_and_day, }) } @@ -392,7 +392,7 @@ impl PushRuleEvaluator { } } KnownCondition::TimeAndDay(time_and_day) => { - if !self.msc3767_time_and_day { + if !self.msc4141_time_and_day { false } else { self.match_time_and_day(time_and_day.timezone, &time_and_day.intervals)? diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index d32b766b4b0..3d5259ad2d0 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -362,7 +362,7 @@ pub enum KnownCondition { RoomVersionSupports { feature: Cow<'static, str>, }, - #[serde(rename = "org.matrix.msc3767.time_and_day")] + #[serde(rename = "org.matrix.msc4141.time_and_day")] TimeAndDay(TimeAndDayCondition), } @@ -455,7 +455,7 @@ pub struct TimeAndDayCondition { pub struct TimeAndDayIntervals { /// Tuple of hh::mm representing start and end times of the day pub time_of_day: Option, - /// 0 = Sunday, 1 = Monday, ..., 7 = Sunday + /// 0 = Sunday, 1 = Monday, ..., 6 = Saturday pub day_of_week: Vec, } diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 403a2119dee..cf85dfe7f88 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -437,7 +437,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: "msc4115_membership_on_events", False ) - # MSC3767: time based notification filtering - self.msc3767_time_and_day = experimental.get( - "org.matrix.msc3767.time_and_day", False + # MSC4141: time based notification filtering + self.msc4141_time_and_day = experimental.get( + "org.matrix.msc4141.time_and_day", False ) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 3fc1a4f59be..865d05a8948 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -435,7 +435,7 @@ async def _action_for_event_by_user( self._related_event_match_enabled, event.room_version.msc3931_push_features, self.hs.config.experimental.msc1767_enabled, # MSC3931 flag - self.hs.config.experimental.msc3767_time_and_day, + self.hs.config.experimental.msc4141_time_and_day, ) for uid, rules in rules_by_user.items(): diff --git a/synapse/synapse_rust/push.pyi b/synapse/synapse_rust/push.pyi index ba38ab0b35c..2e557489ba6 100644 --- a/synapse/synapse_rust/push.pyi +++ b/synapse/synapse_rust/push.pyi @@ -65,7 +65,7 @@ class PushRuleEvaluator: related_event_match_enabled: bool, room_version_feature_flags: Tuple[str, ...], msc3931_enabled: bool, - msc3767_time_and_day: bool, + msc4141_time_and_day: bool, ): ... def run( self, diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index e330ac395a3..63408dfa1fb 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -174,7 +174,7 @@ def _get_evaluator( related_event_match_enabled=True, room_version_feature_flags=event.room_version.msc3931_push_features, msc3931_enabled=True, - msc3767_time_and_day=True, + msc4141_time_and_day=True, ) def test_display_name(self) -> None: @@ -806,7 +806,7 @@ def test_related_event_match_no_related_event(self) -> None: def test_time_and_day_match(self) -> None: # for testing, make whole day in do not disturb condition = { - "kind": "org.matrix.msc3767.time_and_day", + "kind": "org.matrix.msc4141.time_and_day", "timezone": None, "intervals": [ { From 717e7e995383c0682235f2a3cf5b906c8001b2de Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Thu, 9 May 2024 12:03:32 +0300 Subject: [PATCH 06/10] fix: use naive time instead of strings for time interval --- rust/src/push/evaluator.rs | 2 +- rust/src/push/mod.rs | 24 ++++++++++++++++++++---- rust/src/push/utils.rs | 6 +++--- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index bc2bf427f9d..7dfd901be34 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -536,7 +536,7 @@ impl PushRuleEvaluator { interval.day_of_week.contains(&today) && (interval.time_of_day.is_none() || interval.time_of_day.as_ref().map_or(true, |time_interval| { - time_interval.contains(current_time.to_string()) || time_interval.is_empty() + time_interval.contains(current_time) || time_interval.is_empty() })) }); diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 3d5259ad2d0..0724fb62837 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -62,6 +62,7 @@ use std::borrow::Cow; use std::collections::{BTreeMap, HashMap, HashSet}; use anyhow::{Context, Error}; +use chrono::NaiveTime; use chrono_tz::Tz; use log::warn; use pyo3::exceptions::PyTypeError; @@ -462,16 +463,22 @@ pub struct TimeAndDayIntervals { /// Defines the time_of_day of `TimeAndDayIntervals` #[derive(Serialize, Deserialize, Debug, Clone)] pub struct TimeInterval { - start_time: Cow<'static, str>, - end_time: Cow<'static, str>, + start_time: NaiveTime, + end_time: NaiveTime, } /// Implementation for `TimeInterval` impl TimeInterval { + pub fn new(start: &str, end: &str) -> Self { + TimeInterval { + start_time: start.parse::().expect("Invalid start time."), + end_time: end.parse::().expect("Invalid end time."), + } + } /// Checks if the provided time is within the interval. - pub fn contains(&self, time: String) -> bool { + pub fn contains(&self, time: NaiveTime) -> bool { // Since MSC specifies ISO 8601 which uses 24h, string comparison is valid. - time >= self.start_time.parse().unwrap() && time <= self.end_time.parse().unwrap() + time >= self.start_time && time <= self.end_time } /// Checks if the interval is empty. @@ -820,3 +827,12 @@ fn test_custom_action() { let new_json = serde_json::to_string(&action).unwrap(); assert_eq!(json, new_json); } + +#[test] +fn test_time_interval_contains() { + let interval = TimeInterval::new("08:00", "12:00"); + let time = "08:00".parse::().unwrap(); + let does_it = interval.contains(time); + + println!("{}", does_it) +} diff --git a/rust/src/push/utils.rs b/rust/src/push/utils.rs index 171e09ca2bb..d9d327644bd 100644 --- a/rust/src/push/utils.rs +++ b/rust/src/push/utils.rs @@ -22,7 +22,7 @@ use anyhow::bail; use anyhow::Context; use anyhow::Error; -use chrono::{Datelike, Utc}; +use chrono::{Datelike, NaiveTime, Utc}; use chrono_tz::Tz; use lazy_static::lazy_static; use regex; @@ -168,12 +168,12 @@ impl Matcher { } /// Returns `today` and `current_time` based on the given timezone. Otherwise, Utc. -pub fn day_and_time_with_timezone(timezone: Option) -> (u32, String) { +pub fn day_and_time_with_timezone(timezone: Option) -> (u32, NaiveTime) { let tz: Tz = timezone.unwrap_or(Tz::UTC); let time_with_tz = Utc::now().with_timezone(&tz); let today_with_timezone = time_with_tz.weekday().num_days_from_sunday(); - let current_time_with_timezone = time_with_tz.time().format("%H:%M").to_string(); + let current_time_with_timezone = time_with_tz.time(); (today_with_timezone, current_time_with_timezone) } From bc0560dfed395a8a25bd7f40857051c27d4b0578 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Thu, 9 May 2024 12:03:57 +0300 Subject: [PATCH 07/10] chore: add serde for chrono package --- rust/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/Cargo.toml b/rust/Cargo.toml index d2ee73aa668..b0d499ca013 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -43,7 +43,7 @@ sha2 = "0.10.8" serde = { version = "1.0.144", features = ["derive"] } serde_json = "1.0.85" ulid = "1.1.2" -chrono = "0.4.33" +chrono = { version = "0.4.33", features = ["serde"] } chrono-tz = { version = "0.8.5", features = ["serde"]} [features] From d599a6131c58465c94b84fc8248860ef55a9b0a2 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Thu, 9 May 2024 12:04:05 +0300 Subject: [PATCH 08/10] chore: update cargo lock --- Cargo.lock | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 167 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 59d43ece2d8..98381e4efae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,6 +11,21 @@ dependencies = [ "memchr", ] +[[package]] +name = "android-tzdata" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e999941b234f3131b00bc13c22d06e8c5ff726d1b6318ac7eb276997bbb4fef0" + +[[package]] +name = "android_system_properties" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" +dependencies = [ + "libc", +] + [[package]] name = "anyhow" version = "1.0.83" @@ -71,12 +86,62 @@ version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "514de17de45fdb8dc022b1a7975556c53c86f9f0aa5f534b98977b171857c2c9" +[[package]] +name = "cc" +version = "1.0.97" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "099a5357d84c4c61eb35fc8eafa9a79a902c2f76911e5747ced4e032edd8d9b4" + [[package]] name = "cfg-if" version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "chrono" +version = "0.4.38" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a21f936df1771bf62b77f047b726c4625ff2e8aa607c01ec06e5a05bd8463401" +dependencies = [ + "android-tzdata", + "iana-time-zone", + "js-sys", + "num-traits", + "serde", + "wasm-bindgen", + "windows-targets", +] + +[[package]] +name = "chrono-tz" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d59ae0466b83e838b81a54256c39d5d7c20b9d7daa10510a242d9b75abd5936e" +dependencies = [ + "chrono", + "chrono-tz-build", + "phf", + "serde", +] + +[[package]] +name = "chrono-tz-build" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "433e39f13c9a060046954e0592a8d0a4bcb1040125cbf91cb8ee58964cfb350f" +dependencies = [ + "parse-zoneinfo", + "phf", + "phf_codegen", +] + +[[package]] +name = "core-foundation-sys" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06ea2b9bc92be3c2baa9334a323ebca2d6f074ff852cd1d7b11064035cd3868f" + [[package]] name = "cpufeatures" version = "0.2.12" @@ -189,6 +254,29 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" +[[package]] +name = "iana-time-zone" +version = "0.1.60" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7ffbb5a1b541ea2561f8c41c087286cc091e21e556a4f09a8f6cbf17b69b141" +dependencies = [ + "android_system_properties", + "core-foundation-sys", + "iana-time-zone-haiku", + "js-sys", + "wasm-bindgen", + "windows-core", +] + +[[package]] +name = "iana-time-zone-haiku" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" +dependencies = [ + "cc", +] + [[package]] name = "indoc" version = "2.0.5" @@ -259,6 +347,15 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", +] + [[package]] name = "once_cell" version = "1.19.0" @@ -288,6 +385,53 @@ dependencies = [ "windows-targets", ] +[[package]] +name = "parse-zoneinfo" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f2a05b18d44e2957b88f96ba460715e295bc1d7510468a2f3d3b44535d26c24" +dependencies = [ + "regex", +] + +[[package]] +name = "phf" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ade2d8b8f33c7333b51bcf0428d37e217e9f32192ae4772156f65063b8ce03dc" +dependencies = [ + "phf_shared", +] + +[[package]] +name = "phf_codegen" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8d39688d359e6b34654d328e262234662d16cc0f60ec8dcbe5e718709342a5a" +dependencies = [ + "phf_generator", + "phf_shared", +] + +[[package]] +name = "phf_generator" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "48e4cc64c2ad9ebe670cb8fd69dd50ae301650392e81c05f9bfcb2d5bdbc24b0" +dependencies = [ + "phf_shared", + "rand", +] + +[[package]] +name = "phf_shared" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "90fcb95eef784c2ac79119d1dd819e162b5da872ce6f3c3abe1e8ca1c082f72b" +dependencies = [ + "siphasher", +] + [[package]] name = "portable-atomic" version = "1.6.0" @@ -485,18 +629,18 @@ checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" [[package]] name = "serde" -version = "1.0.200" +version = "1.0.201" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ddc6f9cc94d67c0e21aaf7eda3a010fd3af78ebf6e096aa6e2e13c79749cce4f" +checksum = "780f1cebed1629e4753a1a38a3c72d30b97ec044f0aef68cb26650a3c5cf363c" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.200" +version = "1.0.201" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "856f046b9400cee3c8c94ed572ecdb752444c24528c035cd35882aad6f492bcb" +checksum = "c5e405930b9796f1c00bee880d03fc7e0bb4b9a11afc776885ffe84320da2865" dependencies = [ "proc-macro2", "quote", @@ -505,9 +649,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.116" +version = "1.0.117" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e17db7126d17feb94eb3fad46bf1a96b034e8aacbc2e775fe81505f8b0b2813" +checksum = "455182ea6142b14f93f4bc5320a2b31c1f266b66a4a5c858b013302a5d8cbfc3" dependencies = [ "itoa", "ryu", @@ -536,6 +680,12 @@ dependencies = [ "digest", ] +[[package]] +name = "siphasher" +version = "0.3.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "38b58827f4464d87d377d175e90bf58eb00fd8716ff0a62f80356b5e61555d0d" + [[package]] name = "smallvec" version = "1.13.2" @@ -567,6 +717,8 @@ dependencies = [ "base64", "blake2", "bytes", + "chrono", + "chrono-tz", "headers", "hex", "http", @@ -694,6 +846,15 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "windows-core" +version = "0.52.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33ab640c8d7e35bf8ba19b884ba838ceb4fba93a4e8c65a9059d08afcfc683d9" +dependencies = [ + "windows-targets", +] + [[package]] name = "windows-targets" version = "0.52.5" From f74b965e64e6cead753629cdc5f896683040d3a1 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Thu, 9 May 2024 12:05:36 +0300 Subject: [PATCH 09/10] tests: unit test time interval contains --- rust/src/push/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 0724fb62837..ec4e41abb80 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -831,8 +831,10 @@ fn test_custom_action() { #[test] fn test_time_interval_contains() { let interval = TimeInterval::new("08:00", "12:00"); - let time = "08:00".parse::().unwrap(); - let does_it = interval.contains(time); - println!("{}", does_it) + let contains = interval.contains("09:00".parse::().unwrap()); + assert!(contains); + + let not_contains = interval.contains("20:00".parse::().unwrap()); + assert!(!not_contains); } From 40bf733b933314bbcbc21594e1721942837e4a74 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Thu, 9 May 2024 12:09:50 +0300 Subject: [PATCH 10/10] fix: improve comments --- rust/src/push/evaluator.rs | 4 +++- rust/src/push/mod.rs | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 7dfd901be34..e462b9d5f7d 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -665,7 +665,9 @@ fn test_time_and_day_condition() { TimeAndDayCondition { timezone: None, intervals: vec![TimeAndDayIntervals { - // for testing, to avoid flakiness, make all days as dnd + // for testing, to avoid flakiness, make all days/times as dnd + // the logic for time_of_day is tested in a unit test + // see test_time_interval_contains time_of_day: None, day_of_week: vec![0, 1, 2, 3, 4, 5, 6], }], diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index ec4e41abb80..eed167f3cda 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -454,7 +454,7 @@ pub struct TimeAndDayCondition { /// Defines the intervals of `TimeAndDayCondition` #[derive(Serialize, Deserialize, Debug, Clone)] pub struct TimeAndDayIntervals { - /// Tuple of hh::mm representing start and end times of the day + /// Optional Struct of NaiveTime representing start and end times of the day pub time_of_day: Option, /// 0 = Sunday, 1 = Monday, ..., 6 = Saturday pub day_of_week: Vec, @@ -469,6 +469,7 @@ pub struct TimeInterval { /// Implementation for `TimeInterval` impl TimeInterval { + /// Creates a new TimeInterval from passed start and end times strings pub fn new(start: &str, end: &str) -> Self { TimeInterval { start_time: start.parse::().expect("Invalid start time."), @@ -477,7 +478,6 @@ impl TimeInterval { } /// Checks if the provided time is within the interval. pub fn contains(&self, time: NaiveTime) -> bool { - // Since MSC specifies ISO 8601 which uses 24h, string comparison is valid. time >= self.start_time && time <= self.end_time }