Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Stabilize support for MSC3758: event_property_is push condition #15185

Merged
merged 3 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/15185.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Stabilise support for [MSC3758](https://github.com/matrix-org/matrix-spec-proposals/pull/3758): `event_property_is` push condition.
4 changes: 0 additions & 4 deletions rust/benches/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ fn bench_match_exact(b: &mut Bencher) {
vec![],
false,
false,
false,
)
.unwrap();

Expand Down Expand Up @@ -99,7 +98,6 @@ fn bench_match_word(b: &mut Bencher) {
vec![],
false,
false,
false,
)
.unwrap();

Expand Down Expand Up @@ -146,7 +144,6 @@ fn bench_match_word_miss(b: &mut Bencher) {
vec![],
false,
false,
false,
)
.unwrap();

Expand Down Expand Up @@ -193,7 +190,6 @@ fn bench_eval_message(b: &mut Bencher) {
vec![],
false,
false,
false,
)
.unwrap();

Expand Down
8 changes: 4 additions & 4 deletions rust/src/push/base_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ use super::KnownCondition;
use crate::push::RelatedEventMatchTypeCondition;
use crate::push::SetTweak;
use crate::push::TweakValue;
use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue};
use crate::push::{Action, EventPropertyIsCondition, SimpleJsonValue};
use crate::push::{Condition, EventMatchTypeCondition};
use crate::push::{EventMatchCondition, EventMatchPatternType};
use crate::push::{ExactEventMatchTypeCondition, PushRule};
use crate::push::{EventPropertyIsTypeCondition, PushRule};

const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak {
set_tweak: Cow::Borrowed("highlight"),
Expand Down Expand Up @@ -145,7 +145,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"),
priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known(
KnownCondition::ExactEventPropertyContainsType(ExactEventMatchTypeCondition {
KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition {
key: Cow::Borrowed("content.org.matrix.msc3952.mentions.user_ids"),
value_type: Cow::Borrowed(&EventMatchPatternType::UserId),
}),
Expand All @@ -166,7 +166,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mention"),
priority_class: 5,
conditions: Cow::Borrowed(&[
Condition::Known(KnownCondition::ExactEventMatch(ExactEventMatchCondition {
Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition {
key: Cow::Borrowed("content.org.matrix.msc3952.mentions.room"),
value: Cow::Borrowed(&SimpleJsonValue::Bool(true)),
})),
Expand Down
36 changes: 12 additions & 24 deletions rust/src/push/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use regex::Regex;

use super::{
utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType},
Action, Condition, ExactEventMatchCondition, FilteredPushRules, KnownCondition,
Action, Condition, EventPropertyIsCondition, FilteredPushRules, KnownCondition,
SimpleJsonValue,
};

Expand Down Expand Up @@ -97,9 +97,6 @@ pub struct PushRuleEvaluator {
/// flag as MSC1767 (extensible events core).
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.
msc3966_exact_event_property_contains: bool,
}
Expand All @@ -119,7 +116,6 @@ impl PushRuleEvaluator {
related_event_match_enabled: bool,
room_version_feature_flags: Vec<String>,
msc3931_enabled: bool,
msc3758_exact_event_match: bool,
msc3966_exact_event_property_contains: bool,
) -> Result<Self, Error> {
let body = match flattened_keys.get("content.body") {
Expand All @@ -138,7 +134,6 @@ impl PushRuleEvaluator {
related_event_match_enabled,
room_version_feature_flags,
msc3931_enabled,
msc3758_exact_event_match,
msc3966_exact_event_property_contains,
})
}
Expand Down Expand Up @@ -275,8 +270,8 @@ impl PushRuleEvaluator {

self.match_event_match(&self.flattened_keys, &event_match.key, pattern)?
}
KnownCondition::ExactEventMatch(exact_event_match) => {
self.match_exact_event_match(exact_event_match)?
KnownCondition::EventPropertyIs(event_property_is) => {
self.match_event_property_is(event_property_is)?
}
KnownCondition::RelatedEventMatch(event_match) => self.match_related_event_match(
&event_match.rel_type.clone(),
Expand Down Expand Up @@ -306,10 +301,10 @@ impl PushRuleEvaluator {
Some(Cow::Borrowed(pattern)),
)?
}
KnownCondition::ExactEventPropertyContains(exact_event_match) => self
KnownCondition::ExactEventPropertyContains(event_property_is) => self
.match_exact_event_property_contains(
exact_event_match.key.clone(),
exact_event_match.value.clone(),
event_property_is.key.clone(),
event_property_is.value.clone(),
)?,
KnownCondition::ExactEventPropertyContainsType(exact_event_match) => {
// The `pattern_type` can either be "user_id" or "user_localpart",
Expand Down Expand Up @@ -405,20 +400,15 @@ impl PushRuleEvaluator {
compiled_pattern.is_match(haystack)
}

/// Evaluates a `exact_event_match` condition. (MSC3758)
fn match_exact_event_match(
/// Evaluates a `event_property_is` condition.
fn match_event_property_is(
&self,
exact_event_match: &ExactEventMatchCondition,
event_property_is: &EventPropertyIsCondition,
) -> Result<bool, Error> {
// First check if the feature is enabled.
if !self.msc3758_exact_event_match {
return Ok(false);
}

let value = &exact_event_match.value;
let value = &event_property_is.value;

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
} else {
Expand Down Expand Up @@ -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(
&self,
key: Cow<str>,
Expand Down Expand Up @@ -526,7 +516,6 @@ fn push_rule_evaluator() {
vec![],
true,
true,
true,
)
.unwrap();

Expand Down Expand Up @@ -557,7 +546,6 @@ fn test_requires_room_version_supports_condition() {
flags,
true,
true,
true,
)
.unwrap();

Expand Down
36 changes: 16 additions & 20 deletions rust/src/push/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,21 +331,20 @@ pub enum KnownCondition {
// Identical to event_match but gives predefined patterns. Cannot be added by users.
#[serde(skip_deserializing, rename = "event_match")]
EventMatchType(EventMatchTypeCondition),
#[serde(rename = "com.beeper.msc3758.exact_event_match")]
ExactEventMatch(ExactEventMatchCondition),
EventPropertyIs(EventPropertyIsCondition),
#[serde(rename = "im.nheko.msc3664.related_event_match")]
RelatedEventMatch(RelatedEventMatchCondition),
// 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(ExactEventMatchCondition),
ExactEventPropertyContains(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"
)]
ExactEventPropertyContainsType(ExactEventMatchTypeCondition),
ExactEventPropertyContainsType(EventPropertyIsTypeCondition),
ContainsDisplayName,
RoomMemberCount {
#[serde(skip_serializing_if = "Option::is_none")]
Expand Down Expand Up @@ -395,16 +394,16 @@ pub struct EventMatchTypeCondition {
pub pattern_type: Cow<'static, EventMatchPatternType>,
}

/// The body of a [`Condition::ExactEventMatch`]
/// The body of a [`Condition::EventPropertyIs`]
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct ExactEventMatchCondition {
pub struct EventPropertyIsCondition {
pub key: Cow<'static, str>,
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)]
pub struct ExactEventMatchTypeCondition {
pub struct EventPropertyIsTypeCondition {
pub key: Cow<'static, str>,
// 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.
Expand Down Expand Up @@ -711,44 +710,41 @@ fn test_deserialize_unstable_msc3931_condition() {
}

#[test]
fn test_deserialize_unstable_msc3758_condition() {
fn test_deserialize_event_property_is_condition() {
// A string condition should work.
let json =
r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":"foo"}"#;
let json = r#"{"kind":"event_property_is","key":"content.value","value":"foo"}"#;

let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!(
condition,
Condition::Known(KnownCondition::ExactEventMatch(_))
Condition::Known(KnownCondition::EventPropertyIs(_))
));

// A boolean condition should work.
let json =
r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":true}"#;
let json = r#"{"kind":"event_property_is","key":"content.value","value":true}"#;

let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!(
condition,
Condition::Known(KnownCondition::ExactEventMatch(_))
Condition::Known(KnownCondition::EventPropertyIs(_))
));

// 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();
assert!(matches!(
condition,
Condition::Known(KnownCondition::ExactEventMatch(_))
Condition::Known(KnownCondition::EventPropertyIs(_))
));

// A null condition should work
let json =
r#"{"kind":"com.beeper.msc3758.exact_event_match","key":"content.value","value":null}"#;
let json = r#"{"kind":"event_property_is","key":"content.value","value":null}"#;

let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!(
condition,
Condition::Known(KnownCondition::ExactEventMatch(_))
Condition::Known(KnownCondition::EventPropertyIs(_))
));
}

Expand Down
1 change: 0 additions & 1 deletion stubs/synapse/synapse_rust/push.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class PushRuleEvaluator:
related_event_match_enabled: bool,
room_version_feature_flags: Tuple[str, ...],
msc3931_enabled: bool,
msc3758_exact_event_match: bool,
msc3966_exact_event_property_contains: bool,
): ...
def run(
Expand Down
8 changes: 1 addition & 7 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# MSC3925: do not replace events with their edits
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.
self.msc3873_escape_event_match_key = experimental.get(
"msc3873_escape_event_match_key", False
Expand All @@ -184,10 +179,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
"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 = (
experimental.get("msc3952_intentional_mentions", False)
and self.msc3758_exact_event_match
and self.msc3966_exact_event_property_contains
)

Expand Down
1 change: 0 additions & 1 deletion synapse/push/bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.msc3758_exact_event_match,
self.hs.config.experimental.msc3966_exact_event_property_contains,
)

Expand Down
2 changes: 0 additions & 2 deletions tests/push/test_bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ def _create_and_process(
@override_config(
{
"experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True,
"msc3966_exact_event_property_contains": True,
}
Expand Down Expand Up @@ -335,7 +334,6 @@ def test_user_mentions(self) -> None:
@override_config(
{
"experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True,
"msc3966_exact_event_property_contains": True,
}
Expand Down
23 changes: 5 additions & 18 deletions tests/push/test_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
msc3758_exact_event_match=True,
msc3966_exact_event_property_contains=True,
)

Expand Down Expand Up @@ -404,7 +403,7 @@ def test_exact_event_match_string(self) -> None:

# Test against a string value.
condition = {
"kind": "com.beeper.msc3758.exact_event_match",
"kind": "event_property_is",
"key": "content.value",
"value": "foobaz",
}
Expand Down Expand Up @@ -442,11 +441,7 @@ def test_exact_event_match_boolean(self) -> None:
"""Check that exact_event_match conditions work as expected for booleans."""

# Test against a True boolean value.
condition = {
"kind": "com.beeper.msc3758.exact_event_match",
"key": "content.value",
"value": True,
}
condition = {"kind": "event_property_is", "key": "content.value", "value": True}
self._assert_matches(
condition,
{"value": True},
Expand All @@ -466,7 +461,7 @@ def test_exact_event_match_boolean(self) -> None:

# Test against a False boolean value.
condition = {
"kind": "com.beeper.msc3758.exact_event_match",
"kind": "event_property_is",
"key": "content.value",
"value": False,
}
Expand All @@ -491,11 +486,7 @@ def test_exact_event_match_boolean(self) -> None:
def test_exact_event_match_null(self) -> None:
"""Check that exact_event_match conditions work as expected for null."""

condition = {
"kind": "com.beeper.msc3758.exact_event_match",
"key": "content.value",
"value": None,
}
condition = {"kind": "event_property_is", "key": "content.value", "value": None}
self._assert_matches(
condition,
{"value": None},
Expand All @@ -511,11 +502,7 @@ def test_exact_event_match_null(self) -> None:
def test_exact_event_match_integer(self) -> None:
"""Check that exact_event_match conditions work as expected for integers."""

condition = {
"kind": "com.beeper.msc3758.exact_event_match",
"key": "content.value",
"value": 1,
}
condition = {"kind": "event_property_is", "key": "content.value", "value": 1}
self._assert_matches(
condition,
{"value": 1},
Expand Down