Implement MSC4210: Remove legacy mentions (#17783)

This commit is contained in:
Tulir Asokan 2024-10-14 15:24:28 +02:00 committed by GitHub
parent c5b379de66
commit 11bc9a1b3a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 37 additions and 2 deletions

View file

@ -0,0 +1 @@
Implement [MSC4210](https://github.com/matrix-org/matrix-spec-proposals/pull/4210): Remove legacy mentions. Contributed by @tulir @ Beeper.

View file

@ -60,6 +60,7 @@ fn bench_match_exact(b: &mut Bencher) {
true, true,
vec![], vec![],
false, false,
false,
) )
.unwrap(); .unwrap();
@ -105,6 +106,7 @@ fn bench_match_word(b: &mut Bencher) {
true, true,
vec![], vec![],
false, false,
false,
) )
.unwrap(); .unwrap();
@ -150,6 +152,7 @@ fn bench_match_word_miss(b: &mut Bencher) {
true, true,
vec![], vec![],
false, false,
false,
) )
.unwrap(); .unwrap();
@ -195,6 +198,7 @@ fn bench_eval_message(b: &mut Bencher) {
true, true,
vec![], vec![],
false, false,
false,
) )
.unwrap(); .unwrap();
@ -205,6 +209,7 @@ fn bench_eval_message(b: &mut Bencher) {
false, false,
false, false,
false, false,
false,
); );
b.iter(|| eval.run(&rules, Some("bob"), Some("person"))); b.iter(|| eval.run(&rules, Some("bob"), Some("person")));

View file

@ -105,6 +105,9 @@ pub struct PushRuleEvaluator {
/// If MSC3931 (room version feature flags) is enabled. Usually controlled by the same /// If MSC3931 (room version feature flags) is enabled. Usually controlled by the same
/// flag as MSC1767 (extensible events core). /// flag as MSC1767 (extensible events core).
msc3931_enabled: bool, msc3931_enabled: bool,
// If MSC4210 (remove legacy mentions) is enabled.
msc4210_enabled: bool,
} }
#[pymethods] #[pymethods]
@ -122,6 +125,7 @@ impl PushRuleEvaluator {
related_event_match_enabled, related_event_match_enabled,
room_version_feature_flags, room_version_feature_flags,
msc3931_enabled, msc3931_enabled,
msc4210_enabled,
))] ))]
pub fn py_new( pub fn py_new(
flattened_keys: BTreeMap<String, JsonValue>, flattened_keys: BTreeMap<String, JsonValue>,
@ -133,6 +137,7 @@ 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,
msc4210_enabled: bool,
) -> Result<Self, Error> { ) -> Result<Self, Error> {
let body = match flattened_keys.get("content.body") { let body = match flattened_keys.get("content.body") {
Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone().into_owned(), Some(JsonValue::Value(SimpleJsonValue::Str(s))) => s.clone().into_owned(),
@ -150,6 +155,7 @@ impl PushRuleEvaluator {
related_event_match_enabled, related_event_match_enabled,
room_version_feature_flags, room_version_feature_flags,
msc3931_enabled, msc3931_enabled,
msc4210_enabled,
}) })
} }
@ -176,7 +182,8 @@ impl PushRuleEvaluator {
// For backwards-compatibility the legacy mention rules are disabled // For backwards-compatibility the legacy mention rules are disabled
// if the event contains the 'm.mentions' property. // if the event contains the 'm.mentions' property.
if self.has_mentions // Additionally, MSC4210 always disables the legacy rules.
if (self.has_mentions || self.msc4210_enabled)
&& (rule_id == "global/override/.m.rule.contains_display_name" && (rule_id == "global/override/.m.rule.contains_display_name"
|| rule_id == "global/content/.m.rule.contains_user_name" || rule_id == "global/content/.m.rule.contains_user_name"
|| rule_id == "global/override/.m.rule.roomnotif") || rule_id == "global/override/.m.rule.roomnotif")
@ -526,6 +533,7 @@ fn push_rule_evaluator() {
true, true,
vec![], vec![],
true, true,
false,
) )
.unwrap(); .unwrap();
@ -555,6 +563,7 @@ fn test_requires_room_version_supports_condition() {
false, false,
flags, flags,
true, true,
false,
) )
.unwrap(); .unwrap();
@ -582,7 +591,7 @@ fn test_requires_room_version_supports_condition() {
}; };
let rules = PushRules::new(vec![custom_rule]); let rules = PushRules::new(vec![custom_rule]);
result = evaluator.run( result = evaluator.run(
&FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false), &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false, false),
None, None,
None, None,
); );

View file

@ -534,6 +534,7 @@ pub struct FilteredPushRules {
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc4028_push_encrypted_events: bool, msc4028_push_encrypted_events: bool,
msc4210_enabled: bool,
} }
#[pymethods] #[pymethods]
@ -546,6 +547,7 @@ impl FilteredPushRules {
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc4028_push_encrypted_events: bool, msc4028_push_encrypted_events: bool,
msc4210_enabled: bool,
) -> Self { ) -> Self {
Self { Self {
push_rules, push_rules,
@ -554,6 +556,7 @@ impl FilteredPushRules {
msc3381_polls_enabled, msc3381_polls_enabled,
msc3664_enabled, msc3664_enabled,
msc4028_push_encrypted_events, msc4028_push_encrypted_events,
msc4210_enabled,
} }
} }
@ -596,6 +599,14 @@ impl FilteredPushRules {
return false; return false;
} }
if self.msc4210_enabled
&& (rule.rule_id == "global/override/.m.rule.contains_display_name"
|| rule.rule_id == "global/content/.m.rule.contains_user_name"
|| rule.rule_id == "global/override/.m.rule.roomnotif")
{
return false;
}
true true
}) })
.map(|r| { .map(|r| {

View file

@ -447,3 +447,6 @@ class ExperimentalConfig(Config):
# MSC4151: Report room API (Client-Server API) # MSC4151: Report room API (Client-Server API)
self.msc4151_enabled: bool = experimental.get("msc4151_enabled", False) self.msc4151_enabled: bool = experimental.get("msc4151_enabled", False)
# MSC4210: Remove legacy mentions
self.msc4210_enabled: bool = experimental.get("msc4210_enabled", False)

View file

@ -436,6 +436,7 @@ 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.msc4210_enabled,
) )
for uid, rules in rules_by_user.items(): for uid, rules in rules_by_user.items():

View file

@ -109,6 +109,7 @@ def _load_rules(
msc3664_enabled=experimental_config.msc3664_enabled, msc3664_enabled=experimental_config.msc3664_enabled,
msc3381_polls_enabled=experimental_config.msc3381_polls_enabled, msc3381_polls_enabled=experimental_config.msc3381_polls_enabled,
msc4028_push_encrypted_events=experimental_config.msc4028_push_encrypted_events, msc4028_push_encrypted_events=experimental_config.msc4028_push_encrypted_events,
msc4210_enabled=experimental_config.msc4210_enabled,
) )
return filtered_rules return filtered_rules

View file

@ -48,6 +48,7 @@ class FilteredPushRules:
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc4028_push_encrypted_events: bool, msc4028_push_encrypted_events: bool,
msc4210_enabled: bool,
): ... ): ...
def rules(self) -> Collection[Tuple[PushRule, bool]]: ... def rules(self) -> Collection[Tuple[PushRule, bool]]: ...
@ -65,6 +66,7 @@ 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,
msc4210_enabled: bool,
): ... ): ...
def run( def run(
self, self,

View file

@ -149,6 +149,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
content: JsonMapping, content: JsonMapping,
*, *,
related_events: Optional[JsonDict] = None, related_events: Optional[JsonDict] = None,
msc4210: bool = False,
) -> PushRuleEvaluator: ) -> PushRuleEvaluator:
event = FrozenEvent( event = FrozenEvent(
{ {
@ -174,6 +175,7 @@ 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,
msc4210_enabled=msc4210,
) )
def test_display_name(self) -> None: def test_display_name(self) -> None: