Stabilize support for MSC3758: event_property_is push condition (#15185)

This removes the configuration flag & updates the identifiers to
use the stable version.
pull/15187/head
Patrick Cloke 2023-03-06 08:38:01 -05:00 committed by GitHub
parent 95876cf5f1
commit fd9cadcf53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 39 additions and 81 deletions

View File

@ -0,0 +1 @@
Stabilise support for [MSC3758](https://github.com/matrix-org/matrix-spec-proposals/pull/3758): `event_property_is` push condition.

View File

@ -53,7 +53,6 @@ fn bench_match_exact(b: &mut Bencher) {
vec![], vec![],
false, false,
false, false,
false,
) )
.unwrap(); .unwrap();
@ -100,7 +99,6 @@ fn bench_match_word(b: &mut Bencher) {
vec![], vec![],
false, false,
false, false,
false,
) )
.unwrap(); .unwrap();
@ -147,7 +145,6 @@ fn bench_match_word_miss(b: &mut Bencher) {
vec![], vec![],
false, false,
false, false,
false,
) )
.unwrap(); .unwrap();
@ -194,7 +191,6 @@ fn bench_eval_message(b: &mut Bencher) {
vec![], vec![],
false, false,
false, false,
false,
) )
.unwrap(); .unwrap();

View File

@ -24,10 +24,10 @@ use super::KnownCondition;
use crate::push::RelatedEventMatchTypeCondition; use crate::push::RelatedEventMatchTypeCondition;
use crate::push::SetTweak; use crate::push::SetTweak;
use crate::push::TweakValue; use crate::push::TweakValue;
use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue}; use crate::push::{Action, EventPropertyIsCondition, SimpleJsonValue};
use crate::push::{Condition, EventMatchTypeCondition}; use crate::push::{Condition, EventMatchTypeCondition};
use crate::push::{EventMatchCondition, EventMatchPatternType}; use crate::push::{EventMatchCondition, EventMatchPatternType};
use crate::push::{ExactEventMatchTypeCondition, PushRule}; use crate::push::{EventPropertyIsTypeCondition, PushRule};
const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak { const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak {
set_tweak: Cow::Borrowed("highlight"), set_tweak: Cow::Borrowed("highlight"),
@ -145,7 +145,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"), rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"),
priority_class: 5, priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known( conditions: Cow::Borrowed(&[Condition::Known(
KnownCondition::ExactEventPropertyContainsType(ExactEventMatchTypeCondition { KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition {
key: Cow::Borrowed("content.org.matrix.msc3952.mentions.user_ids"), key: Cow::Borrowed("content.org.matrix.msc3952.mentions.user_ids"),
value_type: Cow::Borrowed(&EventMatchPatternType::UserId), value_type: Cow::Borrowed(&EventMatchPatternType::UserId),
}), }),
@ -166,7 +166,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mention"), rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mention"),
priority_class: 5, priority_class: 5,
conditions: Cow::Borrowed(&[ conditions: Cow::Borrowed(&[
Condition::Known(KnownCondition::ExactEventMatch(ExactEventMatchCondition { Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition {
key: Cow::Borrowed("content.org.matrix.msc3952.mentions.room"), key: Cow::Borrowed("content.org.matrix.msc3952.mentions.room"),
value: Cow::Borrowed(&SimpleJsonValue::Bool(true)), value: Cow::Borrowed(&SimpleJsonValue::Bool(true)),
})), })),

View File

@ -23,7 +23,7 @@ use regex::Regex;
use super::{ use super::{
utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType},
Action, Condition, ExactEventMatchCondition, FilteredPushRules, KnownCondition, Action, Condition, EventPropertyIsCondition, FilteredPushRules, KnownCondition,
SimpleJsonValue, SimpleJsonValue,
}; };
use crate::push::{EventMatchPatternType, JsonValue}; use crate::push::{EventMatchPatternType, JsonValue};
@ -97,9 +97,6 @@ pub struct PushRuleEvaluator {
/// flag as MSC1767 (extensible events core). /// flag as MSC1767 (extensible events core).
msc3931_enabled: bool, msc3931_enabled: bool,
/// If MSC3758 (exact_event_match push rule condition) is enabled.
msc3758_exact_event_match: bool,
/// If MSC3966 (exact_event_property_contains push rule condition) is enabled. /// If MSC3966 (exact_event_property_contains push rule condition) is enabled.
msc3966_exact_event_property_contains: bool, msc3966_exact_event_property_contains: bool,
} }
@ -119,7 +116,6 @@ impl PushRuleEvaluator {
related_event_match_enabled: bool, related_event_match_enabled: bool,
room_version_feature_flags: Vec<String>, room_version_feature_flags: Vec<String>,
msc3931_enabled: bool, msc3931_enabled: bool,
msc3758_exact_event_match: bool,
msc3966_exact_event_property_contains: bool, msc3966_exact_event_property_contains: bool,
) -> Result<Self, Error> { ) -> Result<Self, Error> {
let body = match flattened_keys.get("content.body") { let body = match flattened_keys.get("content.body") {
@ -138,7 +134,6 @@ impl PushRuleEvaluator {
related_event_match_enabled, related_event_match_enabled,
room_version_feature_flags, room_version_feature_flags,
msc3931_enabled, msc3931_enabled,
msc3758_exact_event_match,
msc3966_exact_event_property_contains, msc3966_exact_event_property_contains,
}) })
} }
@ -275,8 +270,8 @@ impl PushRuleEvaluator {
self.match_event_match(&self.flattened_keys, &event_match.key, pattern)? self.match_event_match(&self.flattened_keys, &event_match.key, pattern)?
} }
KnownCondition::ExactEventMatch(exact_event_match) => { KnownCondition::EventPropertyIs(event_property_is) => {
self.match_exact_event_match(exact_event_match)? self.match_event_property_is(event_property_is)?
} }
KnownCondition::RelatedEventMatch(event_match) => self.match_related_event_match( KnownCondition::RelatedEventMatch(event_match) => self.match_related_event_match(
&event_match.rel_type.clone(), &event_match.rel_type.clone(),
@ -306,10 +301,10 @@ impl PushRuleEvaluator {
Some(Cow::Borrowed(pattern)), Some(Cow::Borrowed(pattern)),
)? )?
} }
KnownCondition::ExactEventPropertyContains(exact_event_match) => self KnownCondition::ExactEventPropertyContains(event_property_is) => self
.match_exact_event_property_contains( .match_exact_event_property_contains(
exact_event_match.key.clone(), event_property_is.key.clone(),
exact_event_match.value.clone(), event_property_is.value.clone(),
)?, )?,
KnownCondition::ExactEventPropertyContainsType(exact_event_match) => { KnownCondition::ExactEventPropertyContainsType(exact_event_match) => {
// The `pattern_type` can either be "user_id" or "user_localpart", // The `pattern_type` can either be "user_id" or "user_localpart",
@ -405,20 +400,15 @@ impl PushRuleEvaluator {
compiled_pattern.is_match(haystack) compiled_pattern.is_match(haystack)
} }
/// Evaluates a `exact_event_match` condition. (MSC3758) /// Evaluates a `event_property_is` condition.
fn match_exact_event_match( fn match_event_property_is(
&self, &self,
exact_event_match: &ExactEventMatchCondition, event_property_is: &EventPropertyIsCondition,
) -> Result<bool, Error> { ) -> Result<bool, Error> {
// First check if the feature is enabled. let value = &event_property_is.value;
if !self.msc3758_exact_event_match {
return Ok(false);
}
let value = &exact_event_match.value;
let haystack = if let Some(JsonValue::Value(haystack)) = let haystack = if let Some(JsonValue::Value(haystack)) =
self.flattened_keys.get(&*exact_event_match.key) self.flattened_keys.get(&*event_property_is.key)
{ {
haystack haystack
} else { } else {
@ -464,7 +454,7 @@ impl PushRuleEvaluator {
} }
} }
/// Evaluates a `exact_event_property_contains` condition. (MSC3758) /// Evaluates a `exact_event_property_contains` condition. (MSC3966)
fn match_exact_event_property_contains( fn match_exact_event_property_contains(
&self, &self,
key: Cow<str>, key: Cow<str>,
@ -526,7 +516,6 @@ fn push_rule_evaluator() {
vec![], vec![],
true, true,
true, true,
true,
) )
.unwrap(); .unwrap();
@ -557,7 +546,6 @@ fn test_requires_room_version_supports_condition() {
flags, flags,
true, true,
true, true,
true,
) )
.unwrap(); .unwrap();

View File

@ -331,21 +331,20 @@ pub enum KnownCondition {
// Identical to event_match but gives predefined patterns. Cannot be added by users. // Identical to event_match but gives predefined patterns. Cannot be added by users.
#[serde(skip_deserializing, rename = "event_match")] #[serde(skip_deserializing, rename = "event_match")]
EventMatchType(EventMatchTypeCondition), EventMatchType(EventMatchTypeCondition),
#[serde(rename = "com.beeper.msc3758.exact_event_match")] EventPropertyIs(EventPropertyIsCondition),
ExactEventMatch(ExactEventMatchCondition),
#[serde(rename = "im.nheko.msc3664.related_event_match")] #[serde(rename = "im.nheko.msc3664.related_event_match")]
RelatedEventMatch(RelatedEventMatchCondition), RelatedEventMatch(RelatedEventMatchCondition),
// Identical to related_event_match but gives predefined patterns. Cannot be added by users. // Identical to related_event_match but gives predefined patterns. Cannot be added by users.
#[serde(skip_deserializing, rename = "im.nheko.msc3664.related_event_match")] #[serde(skip_deserializing, rename = "im.nheko.msc3664.related_event_match")]
RelatedEventMatchType(RelatedEventMatchTypeCondition), RelatedEventMatchType(RelatedEventMatchTypeCondition),
#[serde(rename = "org.matrix.msc3966.exact_event_property_contains")] #[serde(rename = "org.matrix.msc3966.exact_event_property_contains")]
ExactEventPropertyContains(ExactEventMatchCondition), ExactEventPropertyContains(EventPropertyIsCondition),
// Identical to exact_event_property_contains but gives predefined patterns. Cannot be added by users. // Identical to exact_event_property_contains but gives predefined patterns. Cannot be added by users.
#[serde( #[serde(
skip_deserializing, skip_deserializing,
rename = "org.matrix.msc3966.exact_event_property_contains" rename = "org.matrix.msc3966.exact_event_property_contains"
)] )]
ExactEventPropertyContainsType(ExactEventMatchTypeCondition), ExactEventPropertyContainsType(EventPropertyIsTypeCondition),
ContainsDisplayName, ContainsDisplayName,
RoomMemberCount { RoomMemberCount {
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
@ -395,16 +394,16 @@ pub struct EventMatchTypeCondition {
pub pattern_type: Cow<'static, EventMatchPatternType>, pub pattern_type: Cow<'static, EventMatchPatternType>,
} }
/// The body of a [`Condition::ExactEventMatch`] /// The body of a [`Condition::EventPropertyIs`]
#[derive(Serialize, Deserialize, Debug, Clone)] #[derive(Serialize, Deserialize, Debug, Clone)]
pub struct ExactEventMatchCondition { pub struct EventPropertyIsCondition {
pub key: Cow<'static, str>, pub key: Cow<'static, str>,
pub value: Cow<'static, SimpleJsonValue>, pub value: Cow<'static, SimpleJsonValue>,
} }
/// The body of a [`Condition::ExactEventMatch`] that uses user_id or user_localpart as a pattern. /// The body of a [`Condition::EventPropertyIs`] that uses user_id or user_localpart as a pattern.
#[derive(Serialize, Debug, Clone)] #[derive(Serialize, Debug, Clone)]
pub struct ExactEventMatchTypeCondition { pub struct EventPropertyIsTypeCondition {
pub key: Cow<'static, str>, pub key: Cow<'static, str>,
// During serialization, the pattern_type property gets replaced with a // During serialization, the pattern_type property gets replaced with a
// pattern property of the correct value in synapse.push.clientformat.format_push_rules_for_user. // pattern property of the correct value in synapse.push.clientformat.format_push_rules_for_user.
@ -711,44 +710,41 @@ fn test_deserialize_unstable_msc3931_condition() {
} }
#[test] #[test]
fn test_deserialize_unstable_msc3758_condition() { fn test_deserialize_event_property_is_condition() {
// A string condition should work. // A string condition should work.
let json = let json = r#"{"kind":"event_property_is","key":"content.value","value":"foo"}"#;
r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":"foo"}"#;
let condition: Condition = serde_json::from_str(json).unwrap(); let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!( assert!(matches!(
condition, condition,
Condition::Known(KnownCondition::ExactEventMatch(_)) Condition::Known(KnownCondition::EventPropertyIs(_))
)); ));
// A boolean condition should work. // A boolean condition should work.
let json = let json = r#"{"kind":"event_property_is","key":"content.value","value":true}"#;
r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":true}"#;
let condition: Condition = serde_json::from_str(json).unwrap(); let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!( assert!(matches!(
condition, condition,
Condition::Known(KnownCondition::ExactEventMatch(_)) Condition::Known(KnownCondition::EventPropertyIs(_))
)); ));
// An integer condition should work. // An integer condition should work.
let json = r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":1}"#; let json = r#"{"kind":"event_property_is","key":"content.value","value":1}"#;
let condition: Condition = serde_json::from_str(json).unwrap(); let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!( assert!(matches!(
condition, condition,
Condition::Known(KnownCondition::ExactEventMatch(_)) Condition::Known(KnownCondition::EventPropertyIs(_))
)); ));
// A null condition should work // A null condition should work
let json = let json = r#"{"kind":"event_property_is","key":"content.value","value":null}"#;
r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":null}"#;
let condition: Condition = serde_json::from_str(json).unwrap(); let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!( assert!(matches!(
condition, condition,
Condition::Known(KnownCondition::ExactEventMatch(_)) Condition::Known(KnownCondition::EventPropertyIs(_))
)); ));
} }

View File

@ -65,7 +65,6 @@ class PushRuleEvaluator:
related_event_match_enabled: bool, related_event_match_enabled: bool,
room_version_feature_flags: Tuple[str, ...], room_version_feature_flags: Tuple[str, ...],
msc3931_enabled: bool, msc3931_enabled: bool,
msc3758_exact_event_match: bool,
msc3966_exact_event_property_contains: bool, msc3966_exact_event_property_contains: bool,
): ... ): ...
def run( def run(

View File

@ -169,11 +169,6 @@ class ExperimentalConfig(Config):
# MSC3925: do not replace events with their edits # MSC3925: do not replace events with their edits
self.msc3925_inhibit_edit = experimental.get("msc3925_inhibit_edit", False) self.msc3925_inhibit_edit = experimental.get("msc3925_inhibit_edit", False)
# MSC3758: exact_event_match push rule condition
self.msc3758_exact_event_match = experimental.get(
"msc3758_exact_event_match", False
)
# MSC3873: Disambiguate event_match keys. # MSC3873: Disambiguate event_match keys.
self.msc3873_escape_event_match_key = experimental.get( self.msc3873_escape_event_match_key = experimental.get(
"msc3873_escape_event_match_key", False "msc3873_escape_event_match_key", False
@ -184,10 +179,9 @@ class ExperimentalConfig(Config):
"msc3966_exact_event_property_contains", False "msc3966_exact_event_property_contains", False
) )
# MSC3952: Intentional mentions, this depends on MSC3758 and MSC3966. # MSC3952: Intentional mentions, this depends on MSC3966.
self.msc3952_intentional_mentions = ( self.msc3952_intentional_mentions = (
experimental.get("msc3952_intentional_mentions", False) experimental.get("msc3952_intentional_mentions", False)
and self.msc3758_exact_event_match
and self.msc3966_exact_event_property_contains and self.msc3966_exact_event_property_contains
) )

View File

@ -413,7 +413,6 @@ class BulkPushRuleEvaluator:
self._related_event_match_enabled, self._related_event_match_enabled,
event.room_version.msc3931_push_features, event.room_version.msc3931_push_features,
self.hs.config.experimental.msc1767_enabled, # MSC3931 flag self.hs.config.experimental.msc1767_enabled, # MSC3931 flag
self.hs.config.experimental.msc3758_exact_event_match,
self.hs.config.experimental.msc3966_exact_event_property_contains, self.hs.config.experimental.msc3966_exact_event_property_contains,
) )

View File

@ -231,7 +231,6 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
@override_config( @override_config(
{ {
"experimental_features": { "experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True, "msc3952_intentional_mentions": True,
"msc3966_exact_event_property_contains": True, "msc3966_exact_event_property_contains": True,
} }
@ -335,7 +334,6 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
@override_config( @override_config(
{ {
"experimental_features": { "experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True, "msc3952_intentional_mentions": True,
"msc3966_exact_event_property_contains": True, "msc3966_exact_event_property_contains": True,
} }

View File

@ -173,7 +173,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
related_event_match_enabled=True, related_event_match_enabled=True,
room_version_feature_flags=event.room_version.msc3931_push_features, room_version_feature_flags=event.room_version.msc3931_push_features,
msc3931_enabled=True, msc3931_enabled=True,
msc3758_exact_event_match=True,
msc3966_exact_event_property_contains=True, msc3966_exact_event_property_contains=True,
) )
@ -404,7 +403,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
# Test against a string value. # Test against a string value.
condition = { condition = {
"kind": "com.beeper.msc3758.exact_event_match", "kind": "event_property_is",
"key": "content.value", "key": "content.value",
"value": "foobaz", "value": "foobaz",
} }
@ -442,11 +441,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
"""Check that exact_event_match conditions work as expected for booleans.""" """Check that exact_event_match conditions work as expected for booleans."""
# Test against a True boolean value. # Test against a True boolean value.
condition = { condition = {"kind": "event_property_is", "key": "content.value", "value": True}
"kind": "com.beeper.msc3758.exact_event_match",
"key": "content.value",
"value": True,
}
self._assert_matches( self._assert_matches(
condition, condition,
{"value": True}, {"value": True},
@ -466,7 +461,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
# Test against a False boolean value. # Test against a False boolean value.
condition = { condition = {
"kind": "com.beeper.msc3758.exact_event_match", "kind": "event_property_is",
"key": "content.value", "key": "content.value",
"value": False, "value": False,
} }
@ -491,11 +486,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
def test_exact_event_match_null(self) -> None: def test_exact_event_match_null(self) -> None:
"""Check that exact_event_match conditions work as expected for null.""" """Check that exact_event_match conditions work as expected for null."""
condition = { condition = {"kind": "event_property_is", "key": "content.value", "value": None}
"kind": "com.beeper.msc3758.exact_event_match",
"key": "content.value",
"value": None,
}
self._assert_matches( self._assert_matches(
condition, condition,
{"value": None}, {"value": None},
@ -511,11 +502,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
def test_exact_event_match_integer(self) -> None: def test_exact_event_match_integer(self) -> None:
"""Check that exact_event_match conditions work as expected for integers.""" """Check that exact_event_match conditions work as expected for integers."""
condition = { condition = {"kind": "event_property_is", "key": "content.value", "value": 1}
"kind": "com.beeper.msc3758.exact_event_match",
"key": "content.value",
"value": 1,
}
self._assert_matches( self._assert_matches(
condition, condition,
{"value": 1}, {"value": 1},