Update intentional mentions (MSC3952) to depend on `exact_event_match` (MSC3758). (#15037)

This replaces the specific `is_room_mention` push rule condition
used in MSC3952 with the generic `exact_event_match` push rule
condition from MSC3758.

No functionality changes due to this.
This commit is contained in:
Patrick Cloke 2023-02-16 09:51:22 -05:00 committed by GitHub
parent d1efc47925
commit 979f237b28
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 26 additions and 59 deletions

1
changelog.d/15037.misc Normal file
View File

@ -0,0 +1 @@
Update [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952) support based on changes to the MSC.

View File

@ -45,7 +45,6 @@ fn bench_match_exact(b: &mut Bencher) {
flattened_keys, flattened_keys,
false, false,
BTreeSet::new(), BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
Default::default(), Default::default(),
@ -95,7 +94,6 @@ fn bench_match_word(b: &mut Bencher) {
flattened_keys, flattened_keys,
false, false,
BTreeSet::new(), BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
Default::default(), Default::default(),
@ -145,7 +143,6 @@ fn bench_match_word_miss(b: &mut Bencher) {
flattened_keys, flattened_keys,
false, false,
BTreeSet::new(), BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
Default::default(), Default::default(),
@ -195,7 +192,6 @@ fn bench_eval_message(b: &mut Bencher) {
flattened_keys, flattened_keys,
false, false,
BTreeSet::new(), BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
Default::default(), Default::default(),

View File

@ -21,13 +21,13 @@ use lazy_static::lazy_static;
use serde_json::Value; use serde_json::Value;
use super::KnownCondition; use super::KnownCondition;
use crate::push::Action;
use crate::push::Condition; use crate::push::Condition;
use crate::push::EventMatchCondition; use crate::push::EventMatchCondition;
use crate::push::PushRule; use crate::push::PushRule;
use crate::push::RelatedEventMatchCondition; use crate::push::RelatedEventMatchCondition;
use crate::push::SetTweak; use crate::push::SetTweak;
use crate::push::TweakValue; use crate::push::TweakValue;
use crate::push::{Action, ExactEventMatchCondition, SimpleJsonValue};
const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak { const HIGHLIGHT_ACTION: Action = Action::SetTweak(SetTweak {
set_tweak: Cow::Borrowed("highlight"), set_tweak: Cow::Borrowed("highlight"),
@ -168,7 +168,10 @@ 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::IsRoomMention), Condition::Known(KnownCondition::ExactEventMatch(ExactEventMatchCondition {
key: Cow::Borrowed("content.org.matrix.msc3952.mentions.room"),
value: Cow::Borrowed(&SimpleJsonValue::Bool(true)),
})),
Condition::Known(KnownCondition::SenderNotificationPermission { Condition::Known(KnownCondition::SenderNotificationPermission {
key: Cow::Borrowed("room"), key: Cow::Borrowed("room"),
}), }),

View File

@ -73,8 +73,6 @@ pub struct PushRuleEvaluator {
has_mentions: bool, has_mentions: bool,
/// The user mentions that were part of the message. /// The user mentions that were part of the message.
user_mentions: BTreeSet<String>, user_mentions: BTreeSet<String>,
/// True if the message is a room message.
room_mention: bool,
/// The number of users in the room. /// The number of users in the room.
room_member_count: u64, room_member_count: u64,
@ -116,7 +114,6 @@ impl PushRuleEvaluator {
flattened_keys: BTreeMap<String, JsonValue>, flattened_keys: BTreeMap<String, JsonValue>,
has_mentions: bool, has_mentions: bool,
user_mentions: BTreeSet<String>, user_mentions: BTreeSet<String>,
room_mention: bool,
room_member_count: u64, room_member_count: u64,
sender_power_level: Option<i64>, sender_power_level: Option<i64>,
notification_power_levels: BTreeMap<String, i64>, notification_power_levels: BTreeMap<String, i64>,
@ -137,7 +134,6 @@ impl PushRuleEvaluator {
body, body,
has_mentions, has_mentions,
user_mentions, user_mentions,
room_mention,
room_member_count, room_member_count,
notification_power_levels, notification_power_levels,
sender_power_level, sender_power_level,
@ -279,7 +275,6 @@ impl PushRuleEvaluator {
false false
} }
} }
KnownCondition::IsRoomMention => self.room_mention,
KnownCondition::ContainsDisplayName => { KnownCondition::ContainsDisplayName => {
if let Some(dn) = display_name { if let Some(dn) = display_name {
if !dn.is_empty() { if !dn.is_empty() {
@ -529,7 +524,6 @@ fn push_rule_evaluator() {
flattened_keys, flattened_keys,
false, false,
BTreeSet::new(), BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
BTreeMap::new(), BTreeMap::new(),
@ -562,7 +556,6 @@ fn test_requires_room_version_supports_condition() {
flattened_keys, flattened_keys,
false, false,
BTreeSet::new(), BTreeSet::new(),
false,
10, 10,
Some(0), Some(0),
BTreeMap::new(), BTreeMap::new(),

View File

@ -336,8 +336,6 @@ pub enum KnownCondition {
ExactEventPropertyContains(ExactEventMatchCondition), ExactEventPropertyContains(ExactEventMatchCondition),
#[serde(rename = "org.matrix.msc3952.is_user_mention")] #[serde(rename = "org.matrix.msc3952.is_user_mention")]
IsUserMention, IsUserMention,
#[serde(rename = "org.matrix.msc3952.is_room_mention")]
IsRoomMention,
ContainsDisplayName, ContainsDisplayName,
RoomMemberCount { RoomMemberCount {
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
@ -667,17 +665,6 @@ fn test_deserialize_unstable_msc3952_user_condition() {
)); ));
} }
#[test]
fn test_deserialize_unstable_msc3952_room_condition() {
let json = r#"{"kind":"org.matrix.msc3952.is_room_mention"}"#;
let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!(
condition,
Condition::Known(KnownCondition::IsRoomMention)
));
}
#[test] #[test]
fn test_deserialize_custom_condition() { fn test_deserialize_custom_condition() {
let json = r#"{"kind":"custom_tag"}"#; let json = r#"{"kind":"custom_tag"}"#;

View File

@ -59,7 +59,6 @@ class PushRuleEvaluator:
flattened_keys: Mapping[str, JsonValue], flattened_keys: Mapping[str, JsonValue],
has_mentions: bool, has_mentions: bool,
user_mentions: Set[str], user_mentions: Set[str],
room_mention: bool,
room_member_count: int, room_member_count: int,
sender_power_level: Optional[int], sender_power_level: Optional[int],
notification_power_levels: Mapping[str, int], notification_power_levels: Mapping[str, int],

View File

@ -179,9 +179,10 @@ class ExperimentalConfig(Config):
"msc3783_escape_event_match_key", False "msc3783_escape_event_match_key", False
) )
# MSC3952: Intentional mentions # MSC3952: Intentional mentions, this depends on MSC3758.
self.msc3952_intentional_mentions = experimental.get( self.msc3952_intentional_mentions = (
"msc3952_intentional_mentions", False experimental.get("msc3952_intentional_mentions", False)
and self.msc3758_exact_event_match
) )
# MSC3959: Do not generate notifications for edits. # MSC3959: Do not generate notifications for edits.

View File

@ -400,7 +400,6 @@ class BulkPushRuleEvaluator:
mentions = event.content.get(EventContentFields.MSC3952_MENTIONS) mentions = event.content.get(EventContentFields.MSC3952_MENTIONS)
has_mentions = self._intentional_mentions_enabled and isinstance(mentions, dict) has_mentions = self._intentional_mentions_enabled and isinstance(mentions, dict)
user_mentions: Set[str] = set() user_mentions: Set[str] = set()
room_mention = False
if has_mentions: if has_mentions:
# mypy seems to have lost the type even though it must be a dict here. # mypy seems to have lost the type even though it must be a dict here.
assert isinstance(mentions, dict) assert isinstance(mentions, dict)
@ -410,8 +409,6 @@ class BulkPushRuleEvaluator:
user_mentions = set( user_mentions = set(
filter(lambda item: isinstance(item, str), user_mentions_raw) filter(lambda item: isinstance(item, str), user_mentions_raw)
) )
# Room mention is only true if the value is exactly true.
room_mention = mentions.get("room") is True
evaluator = PushRuleEvaluator( evaluator = PushRuleEvaluator(
_flatten_dict( _flatten_dict(
@ -420,7 +417,6 @@ class BulkPushRuleEvaluator:
), ),
has_mentions, has_mentions,
user_mentions, user_mentions,
room_mention,
room_member_count, room_member_count,
sender_power_level, sender_power_level,
notification_levels, notification_levels,

View File

@ -227,7 +227,14 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
) )
return len(result) > 0 return len(result) > 0
@override_config({"experimental_features": {"msc3952_intentional_mentions": True}}) @override_config(
{
"experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True,
}
}
)
def test_user_mentions(self) -> None: def test_user_mentions(self) -> None:
"""Test the behavior of an event which includes invalid user mentions.""" """Test the behavior of an event which includes invalid user mentions."""
bulk_evaluator = BulkPushRuleEvaluator(self.hs) bulk_evaluator = BulkPushRuleEvaluator(self.hs)
@ -323,7 +330,14 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
) )
) )
@override_config({"experimental_features": {"msc3952_intentional_mentions": True}}) @override_config(
{
"experimental_features": {
"msc3758_exact_event_match": True,
"msc3952_intentional_mentions": True,
}
}
)
def test_room_mentions(self) -> None: def test_room_mentions(self) -> None:
"""Test the behavior of an event which includes invalid room mentions.""" """Test the behavior of an event which includes invalid room mentions."""
bulk_evaluator = BulkPushRuleEvaluator(self.hs) bulk_evaluator = BulkPushRuleEvaluator(self.hs)

View File

@ -149,7 +149,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
*, *,
has_mentions: bool = False, has_mentions: bool = False,
user_mentions: Optional[Set[str]] = None, user_mentions: Optional[Set[str]] = None,
room_mention: bool = False,
related_events: Optional[JsonDict] = None, related_events: Optional[JsonDict] = None,
) -> PushRuleEvaluator: ) -> PushRuleEvaluator:
event = FrozenEvent( event = FrozenEvent(
@ -170,7 +169,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
_flatten_dict(event), _flatten_dict(event),
has_mentions, has_mentions,
user_mentions or set(), user_mentions or set(),
room_mention,
room_member_count, room_member_count,
sender_power_level, sender_power_level,
cast(Dict[str, int], power_levels.get("notifications", {})), cast(Dict[str, int], power_levels.get("notifications", {})),
@ -232,27 +230,6 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
# Note that invalid data is tested at tests.push.test_bulk_push_rule_evaluator.TestBulkPushRuleEvaluator.test_mentions # Note that invalid data is tested at tests.push.test_bulk_push_rule_evaluator.TestBulkPushRuleEvaluator.test_mentions
# since the BulkPushRuleEvaluator is what handles data sanitisation. # since the BulkPushRuleEvaluator is what handles data sanitisation.
def test_room_mentions(self) -> None:
"""Check for room mentions."""
condition = {"kind": "org.matrix.msc3952.is_room_mention"}
# No room mention shouldn't match.
evaluator = self._get_evaluator({}, has_mentions=True)
self.assertFalse(evaluator.matches(condition, None, None))
# Room mention should match.
evaluator = self._get_evaluator({}, has_mentions=True, room_mention=True)
self.assertTrue(evaluator.matches(condition, None, None))
# A room mention and user mention is valid.
evaluator = self._get_evaluator(
{}, has_mentions=True, user_mentions={"@another:test"}, room_mention=True
)
self.assertTrue(evaluator.matches(condition, None, None))
# Note that invalid data is tested at tests.push.test_bulk_push_rule_evaluator.TestBulkPushRuleEvaluator.test_mentions
# since the BulkPushRuleEvaluator is what handles data sanitisation.
def _assert_matches( def _assert_matches(
self, condition: JsonDict, content: JsonMapping, msg: Optional[str] = None self, condition: JsonDict, content: JsonMapping, msg: Optional[str] = None
) -> None: ) -> None: