From 820f02b70badfc04d35c95f8ffb9682c8310e91e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Mar 2023 10:06:02 -0500 Subject: [PATCH] Stabilize support for MSC3966: event_property_contains push condition. (#15187) This removes the configuration flag & updates the identifiers to use the stable version. --- changelog.d/15187.feature | 1 + rust/benches/evaluator.rs | 4 ---- rust/src/push/evaluator.rs | 22 +++++---------------- rust/src/push/mod.rs | 8 ++------ stubs/synapse/synapse_rust/push.pyi | 1 - synapse/config/experimental.py | 10 ++-------- synapse/push/bulk_push_rule_evaluator.py | 1 - tests/push/test_bulk_push_rule_evaluator.py | 18 ++--------------- tests/push/test_push_rule_evaluator.py | 3 +-- 9 files changed, 13 insertions(+), 55 deletions(-) create mode 100644 changelog.d/15187.feature diff --git a/changelog.d/15187.feature b/changelog.d/15187.feature new file mode 100644 index 0000000000..f2b7689255 --- /dev/null +++ b/changelog.d/15187.feature @@ -0,0 +1 @@ +Stabilise support for [MSC3966](https://github.com/matrix-org/matrix-spec-proposals/pull/3966): `event_property_contains` push condition. diff --git a/rust/benches/evaluator.rs b/rust/benches/evaluator.rs index 79b553dbb0..64e13f6486 100644 --- a/rust/benches/evaluator.rs +++ b/rust/benches/evaluator.rs @@ -52,7 +52,6 @@ fn bench_match_exact(b: &mut Bencher) { true, vec![], false, - false, ) .unwrap(); @@ -98,7 +97,6 @@ fn bench_match_word(b: &mut Bencher) { true, vec![], false, - false, ) .unwrap(); @@ -144,7 +142,6 @@ fn bench_match_word_miss(b: &mut Bencher) { true, vec![], false, - false, ) .unwrap(); @@ -190,7 +187,6 @@ fn bench_eval_message(b: &mut Bencher) { true, vec![], false, - false, ) .unwrap(); diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 67fe6a4823..6941c61ea4 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -96,9 +96,6 @@ 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 MSC3966 (exact_event_property_contains push rule condition) is enabled. - msc3966_exact_event_property_contains: bool, } #[pymethods] @@ -116,7 +113,6 @@ impl PushRuleEvaluator { related_event_match_enabled: bool, room_version_feature_flags: Vec, msc3931_enabled: bool, - msc3966_exact_event_property_contains: bool, ) -> Result { let body = match flattened_keys.get("content.body") { Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone(), @@ -134,7 +130,6 @@ impl PushRuleEvaluator { related_event_match_enabled, room_version_feature_flags, msc3931_enabled, - msc3966_exact_event_property_contains, }) } @@ -301,8 +296,8 @@ impl PushRuleEvaluator { Some(Cow::Borrowed(pattern)), )? } - KnownCondition::ExactEventPropertyContains(event_property_is) => self - .match_exact_event_property_contains( + KnownCondition::EventPropertyContains(event_property_is) => self + .match_event_property_contains( event_property_is.key.clone(), event_property_is.value.clone(), )?, @@ -321,7 +316,7 @@ impl PushRuleEvaluator { EventMatchPatternType::UserLocalpart => get_localpart_from_id(user_id)?, }; - self.match_exact_event_property_contains( + self.match_event_property_contains( exact_event_match.key.clone(), Cow::Borrowed(&SimpleJsonValue::Str(pattern.to_string())), )? @@ -454,17 +449,12 @@ impl PushRuleEvaluator { } } - /// Evaluates a `exact_event_property_contains` condition. (MSC3966) - fn match_exact_event_property_contains( + /// Evaluates a `event_property_contains` condition. + fn match_event_property_contains( &self, key: Cow, value: Cow, ) -> Result { - // First check if the feature is enabled. - if !self.msc3966_exact_event_property_contains { - return Ok(false); - } - let haystack = if let Some(JsonValue::Array(haystack)) = self.flattened_keys.get(&*key) { haystack } else { @@ -515,7 +505,6 @@ fn push_rule_evaluator() { true, vec![], true, - true, ) .unwrap(); @@ -545,7 +534,6 @@ fn test_requires_room_version_supports_condition() { false, flags, true, - true, ) .unwrap(); diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 7fde88e825..575a1c1e68 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -337,13 +337,9 @@ pub enum KnownCondition { // Identical to related_event_match but gives predefined patterns. Cannot be added by users. #[serde(skip_deserializing, rename = "im.nheko.msc3664.related_event_match")] RelatedEventMatchType(RelatedEventMatchTypeCondition), - #[serde(rename = "org.matrix.msc3966.exact_event_property_contains")] - ExactEventPropertyContains(EventPropertyIsCondition), + EventPropertyContains(EventPropertyIsCondition), // Identical to exact_event_property_contains but gives predefined patterns. Cannot be added by users. - #[serde( - skip_deserializing, - rename = "org.matrix.msc3966.exact_event_property_contains" - )] + #[serde(skip_deserializing, rename = "event_property_contains")] ExactEventPropertyContainsType(EventPropertyIsTypeCondition), ContainsDisplayName, RoomMemberCount { diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi index c040944aac..5d0ce4b1a4 100644 --- a/stubs/synapse/synapse_rust/push.pyi +++ b/stubs/synapse/synapse_rust/push.pyi @@ -65,7 +65,6 @@ class PushRuleEvaluator: related_event_match_enabled: bool, room_version_feature_flags: Tuple[str, ...], msc3931_enabled: bool, - msc3966_exact_event_property_contains: bool, ): ... def run( self, diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 489f2601ac..9ff382ccc3 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -171,15 +171,9 @@ class ExperimentalConfig(Config): "msc3873_escape_event_match_key", False ) - # MSC3966: exact_event_property_contains push rule condition. - self.msc3966_exact_event_property_contains = experimental.get( - "msc3966_exact_event_property_contains", False - ) - # MSC3952: Intentional mentions, this depends on MSC3966. - self.msc3952_intentional_mentions = ( - experimental.get("msc3952_intentional_mentions", False) - and self.msc3966_exact_event_property_contains + self.msc3952_intentional_mentions = experimental.get( + "msc3952_intentional_mentions", False ) # MSC3959: Do not generate notifications for edits. diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index ba12b6d79a..45622a9e9b 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -413,7 +413,6 @@ class BulkPushRuleEvaluator: self._related_event_match_enabled, event.room_version.msc3931_push_features, self.hs.config.experimental.msc1767_enabled, # MSC3931 flag - self.hs.config.experimental.msc3966_exact_event_property_contains, ) users = rules_by_user.keys() diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py index c6591c50de..46df0102f7 100644 --- a/tests/push/test_bulk_push_rule_evaluator.py +++ b/tests/push/test_bulk_push_rule_evaluator.py @@ -228,14 +228,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase): ) return len(result) > 0 - @override_config( - { - "experimental_features": { - "msc3952_intentional_mentions": True, - "msc3966_exact_event_property_contains": True, - } - } - ) + @override_config({"experimental_features": {"msc3952_intentional_mentions": True}}) def test_user_mentions(self) -> None: """Test the behavior of an event which includes invalid user mentions.""" bulk_evaluator = BulkPushRuleEvaluator(self.hs) @@ -331,14 +324,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase): ) ) - @override_config( - { - "experimental_features": { - "msc3952_intentional_mentions": True, - "msc3966_exact_event_property_contains": True, - } - } - ) + @override_config({"experimental_features": {"msc3952_intentional_mentions": True}}) def test_room_mentions(self) -> None: """Test the behavior of an event which includes invalid room mentions.""" bulk_evaluator = BulkPushRuleEvaluator(self.hs) diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index ff5a9a66f5..6deee0fd02 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -173,7 +173,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): related_event_match_enabled=True, room_version_feature_flags=event.room_version.msc3931_push_features, msc3931_enabled=True, - msc3966_exact_event_property_contains=True, ) def test_display_name(self) -> None: @@ -526,7 +525,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): """Check that exact_event_property_contains conditions work as expected.""" condition = { - "kind": "org.matrix.msc3966.exact_event_property_contains", + "kind": "event_property_contains", "key": "content.value", "value": "foobaz", }