Stabilize support for MSC3966: event_property_contains push condition. (

#15187) This removes the configuration flag & updates the identifiers to use the stable version.
matrix-org · Mar 7, 2023 · 820f02b · 820f02b
1 parent 2af1a98
commit 820f02b
Show file tree

Hide file tree

Showing 9 changed files with 13 additions and 55 deletions.
diff --git a/changelog.d/15187.feature 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
@@ -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
@@ -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<String>,
         msc3931_enabled: bool,
-        msc3966_exact_event_property_contains: bool,
     ) -> Result<Self, Error> {
         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<str>,
         value: Cow<SimpleJsonValue>,
     ) -> Result<bool, Error> {
-        // 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
@@ -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
@@ -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
@@ -171,15 +171,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
             "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
@@ -413,7 +413,6 @@ 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.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
@@ -228,14 +228,7 @@ def _create_and_process(
         )
         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 @@ def test_user_mentions(self) -> None:
             )
         )
 
-    @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
@@ -173,7 +173,6 @@ def _get_evaluator(
             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 @@ def test_exact_event_property_contains(self) -> None:
         """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",
         }