Stabilize support for MSC3952: Intentional mentions. (#15520)

This commit is contained in:
Patrick Cloke 2023-06-06 04:11:07 -04:00 committed by GitHub
parent f9561b9e37
commit f880e64b11
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 27 additions and 64 deletions

View File

@ -0,0 +1 @@
Enable support for [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952): intentional mentions.

View File

@ -13,8 +13,6 @@
// limitations under the License. // limitations under the License.
#![feature(test)] #![feature(test)]
use std::collections::BTreeSet;
use synapse::push::{ use synapse::push::{
evaluator::PushRuleEvaluator, Condition, EventMatchCondition, FilteredPushRules, JsonValue, evaluator::PushRuleEvaluator, Condition, EventMatchCondition, FilteredPushRules, JsonValue,
PushRules, SimpleJsonValue, PushRules, SimpleJsonValue,
@ -197,7 +195,6 @@ 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

@ -142,11 +142,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default_enabled: true, default_enabled: true,
}, },
PushRule { PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"), rule_id: Cow::Borrowed("global/override/.m.is_user_mention"),
priority_class: 5, priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known( conditions: Cow::Borrowed(&[Condition::Known(
KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition { KnownCondition::ExactEventPropertyContainsType(EventPropertyIsTypeCondition {
key: Cow::Borrowed("content.org\\.matrix\\.msc3952\\.mentions.user_ids"), key: Cow::Borrowed("content.m\\.mentions.user_ids"),
value_type: Cow::Borrowed(&EventMatchPatternType::UserId), value_type: Cow::Borrowed(&EventMatchPatternType::UserId),
}), }),
)]), )]),
@ -163,11 +163,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default_enabled: true, default_enabled: true,
}, },
PushRule { PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mention"), rule_id: Cow::Borrowed("global/override/.m.is_room_mention"),
priority_class: 5, priority_class: 5,
conditions: Cow::Borrowed(&[ conditions: Cow::Borrowed(&[
Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition { Condition::Known(KnownCondition::EventPropertyIs(EventPropertyIsCondition {
key: Cow::Borrowed("content.org\\.matrix\\.msc3952\\.mentions.room"), key: Cow::Borrowed("content.m\\.mentions.room"),
value: Cow::Borrowed(&SimpleJsonValue::Bool(true)), value: Cow::Borrowed(&SimpleJsonValue::Bool(true)),
})), })),
Condition::Known(KnownCondition::SenderNotificationPermission { Condition::Known(KnownCondition::SenderNotificationPermission {

View File

@ -70,7 +70,9 @@ pub struct PushRuleEvaluator {
/// The "content.body", if any. /// The "content.body", if any.
body: String, body: String,
/// True if the event has a mentions property and MSC3952 support is enabled. /// True if the event has a m.mentions property. (Note that this is a separate
/// flag instead of checking flattened_keys since the m.mentions property
/// might be an empty map and not appear in flattened_keys.
has_mentions: bool, has_mentions: bool,
/// The number of users in the room. /// The number of users in the room.
@ -155,9 +157,7 @@ impl PushRuleEvaluator {
let rule_id = &push_rule.rule_id().to_string(); let rule_id = &push_rule.rule_id().to_string();
// 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 (and if the // if the event contains the 'm.mentions' property.
// experimental feature is enabled, both of these are represented
// by the has_mentions flag).
if self.has_mentions if self.has_mentions
&& (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"
@ -562,7 +562,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, false), &FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false),
None, None,
None, None,
); );

View File

@ -527,7 +527,6 @@ pub struct FilteredPushRules {
msc1767_enabled: bool, msc1767_enabled: bool,
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
msc3958_suppress_edits_enabled: bool, msc3958_suppress_edits_enabled: bool,
} }
@ -540,7 +539,6 @@ impl FilteredPushRules {
msc1767_enabled: bool, msc1767_enabled: bool,
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
msc3958_suppress_edits_enabled: bool, msc3958_suppress_edits_enabled: bool,
) -> Self { ) -> Self {
Self { Self {
@ -549,7 +547,6 @@ impl FilteredPushRules {
msc1767_enabled, msc1767_enabled,
msc3381_polls_enabled, msc3381_polls_enabled,
msc3664_enabled, msc3664_enabled,
msc3952_intentional_mentions,
msc3958_suppress_edits_enabled, msc3958_suppress_edits_enabled,
} }
} }
@ -587,10 +584,6 @@ impl FilteredPushRules {
return false; return false;
} }
if !self.msc3952_intentional_mentions && rule.rule_id.contains("org.matrix.msc3952")
{
return false;
}
if !self.msc3958_suppress_edits_enabled if !self.msc3958_suppress_edits_enabled
&& rule.rule_id == "global/override/.com.beeper.suppress_edits" && rule.rule_id == "global/override/.com.beeper.suppress_edits"
{ {

View File

@ -46,7 +46,6 @@ class FilteredPushRules:
msc1767_enabled: bool, msc1767_enabled: bool,
msc3381_polls_enabled: bool, msc3381_polls_enabled: bool,
msc3664_enabled: bool, msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
msc3958_suppress_edits_enabled: bool, msc3958_suppress_edits_enabled: bool,
): ... ): ...
def rules(self) -> Collection[Tuple[PushRule, bool]]: ... def rules(self) -> Collection[Tuple[PushRule, bool]]: ...

View File

@ -236,7 +236,7 @@ class EventContentFields:
AUTHORISING_USER: Final = "join_authorised_via_users_server" AUTHORISING_USER: Final = "join_authorised_via_users_server"
# Use for mentioning users. # Use for mentioning users.
MSC3952_MENTIONS: Final = "org.matrix.msc3952.mentions" MENTIONS: Final = "m.mentions"
# an unspecced field added to to-device messages to identify them uniquely-ish # an unspecced field added to to-device messages to identify them uniquely-ish
TO_DEVICE_MSGID: Final = "org.matrix.msgid" TO_DEVICE_MSGID: Final = "org.matrix.msgid"

View File

@ -358,11 +358,6 @@ class ExperimentalConfig(Config):
# MSC3391: Removing account data. # MSC3391: Removing account data.
self.msc3391_enabled = experimental.get("msc3391_enabled", False) self.msc3391_enabled = experimental.get("msc3391_enabled", False)
# MSC3952: Intentional mentions, this depends on MSC3966.
self.msc3952_intentional_mentions = experimental.get(
"msc3952_intentional_mentions", False
)
# MSC3959: Do not generate notifications for edits. # MSC3959: Do not generate notifications for edits.
self.msc3958_supress_edit_notifs = experimental.get( self.msc3958_supress_edit_notifs = experimental.get(
"msc3958_supress_edit_notifs", False "msc3958_supress_edit_notifs", False

View File

@ -134,13 +134,8 @@ class EventValidator:
) )
# If the event contains a mentions key, validate it. # If the event contains a mentions key, validate it.
if ( if EventContentFields.MENTIONS in event.content:
EventContentFields.MSC3952_MENTIONS in event.content validate_json_object(event.content[EventContentFields.MENTIONS], Mentions)
and config.experimental.msc3952_intentional_mentions
):
validate_json_object(
event.content[EventContentFields.MSC3952_MENTIONS], Mentions
)
def _validate_retention(self, event: EventBase) -> None: def _validate_retention(self, event: EventBase) -> None:
"""Checks that an event that defines the retention policy for a room respects the """Checks that an event that defines the retention policy for a room respects the

View File

@ -120,9 +120,6 @@ class BulkPushRuleEvaluator:
self.should_calculate_push_rules = self.hs.config.push.enable_push self.should_calculate_push_rules = self.hs.config.push.enable_push
self._related_event_match_enabled = self.hs.config.experimental.msc3664_enabled self._related_event_match_enabled = self.hs.config.experimental.msc3664_enabled
self._intentional_mentions_enabled = (
self.hs.config.experimental.msc3952_intentional_mentions
)
self.room_push_rule_cache_metrics = register_cache( self.room_push_rule_cache_metrics = register_cache(
"cache", "cache",
@ -390,10 +387,7 @@ class BulkPushRuleEvaluator:
del notification_levels[key] del notification_levels[key]
# Pull out any user and room mentions. # Pull out any user and room mentions.
has_mentions = ( has_mentions = EventContentFields.MENTIONS in event.content
self._intentional_mentions_enabled
and EventContentFields.MSC3952_MENTIONS in event.content
)
evaluator = PushRuleEvaluator( evaluator = PushRuleEvaluator(
_flatten_dict(event), _flatten_dict(event),

View File

@ -124,8 +124,6 @@ class VersionsRestServlet(RestServlet):
is not None, is not None,
# Adds support for relation-based redactions as per MSC3912. # Adds support for relation-based redactions as per MSC3912.
"org.matrix.msc3912": self.config.experimental.msc3912_enabled, "org.matrix.msc3912": self.config.experimental.msc3912_enabled,
# Adds support for unstable "intentional mentions" behaviour.
"org.matrix.msc3952_intentional_mentions": self.config.experimental.msc3952_intentional_mentions,
# Whether recursively provide relations is supported. # Whether recursively provide relations is supported.
"org.matrix.msc3981": self.config.experimental.msc3981_recurse_relations, "org.matrix.msc3981": self.config.experimental.msc3981_recurse_relations,
# Adds support for deleting account data. # Adds support for deleting account data.

View File

@ -88,7 +88,6 @@ def _load_rules(
msc1767_enabled=experimental_config.msc1767_enabled, msc1767_enabled=experimental_config.msc1767_enabled,
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,
msc3952_intentional_mentions=experimental_config.msc3952_intentional_mentions,
msc3958_suppress_edits_enabled=experimental_config.msc3958_supress_edit_notifs, msc3958_suppress_edits_enabled=experimental_config.msc3958_supress_edit_notifs,
) )

View File

@ -228,7 +228,6 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
) )
return len(result) > 0 return len(result) > 0
@override_config({"experimental_features": {"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)
@ -237,9 +236,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self.assertFalse(self._create_and_process(bulk_evaluator)) self.assertFalse(self._create_and_process(bulk_evaluator))
# An empty mentions field should not notify. # An empty mentions field should not notify.
self.assertFalse( self.assertFalse(
self._create_and_process( self._create_and_process(bulk_evaluator, {EventContentFields.MENTIONS: {}})
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {}}
)
) )
# Non-dict mentions should be ignored. # Non-dict mentions should be ignored.
@ -253,7 +250,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
for mentions in (None, True, False, 1, "foo", []): for mentions in (None, True, False, 1, "foo", []):
self.assertFalse( self.assertFalse(
self._create_and_process( self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: mentions} bulk_evaluator, {EventContentFields.MENTIONS: mentions}
) )
) )
@ -262,7 +259,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self.assertFalse( self.assertFalse(
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{EventContentFields.MSC3952_MENTIONS: {"user_ids": mentions}}, {EventContentFields.MENTIONS: {"user_ids": mentions}},
) )
) )
@ -270,14 +267,14 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self.assertTrue( self.assertTrue(
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{EventContentFields.MSC3952_MENTIONS: {"user_ids": [self.alice]}}, {EventContentFields.MENTIONS: {"user_ids": [self.alice]}},
) )
) )
self.assertTrue( self.assertTrue(
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{ {
EventContentFields.MSC3952_MENTIONS: { EventContentFields.MENTIONS: {
"user_ids": ["@another:test", self.alice] "user_ids": ["@another:test", self.alice]
} }
}, },
@ -288,11 +285,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self.assertTrue( self.assertTrue(
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{ {EventContentFields.MENTIONS: {"user_ids": [self.alice, self.alice]}},
EventContentFields.MSC3952_MENTIONS: {
"user_ids": [self.alice, self.alice]
}
},
) )
) )
@ -307,7 +300,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{ {
EventContentFields.MSC3952_MENTIONS: { EventContentFields.MENTIONS: {
"user_ids": [None, True, False, {}, []] "user_ids": [None, True, False, {}, []]
} }
}, },
@ -317,7 +310,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{ {
EventContentFields.MSC3952_MENTIONS: { EventContentFields.MENTIONS: {
"user_ids": [None, True, False, {}, [], self.alice] "user_ids": [None, True, False, {}, [], self.alice]
} }
}, },
@ -331,12 +324,11 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
{ {
"body": self.alice, "body": self.alice,
"msgtype": "m.text", "msgtype": "m.text",
EventContentFields.MSC3952_MENTIONS: {}, EventContentFields.MENTIONS: {},
}, },
) )
) )
@override_config({"experimental_features": {"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)
@ -344,7 +336,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
# Room mentions from those without power should not notify. # Room mentions from those without power should not notify.
self.assertFalse( self.assertFalse(
self._create_and_process( self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}} bulk_evaluator, {EventContentFields.MENTIONS: {"room": True}}
) )
) )
@ -358,7 +350,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
) )
self.assertTrue( self.assertTrue(
self._create_and_process( self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}} bulk_evaluator, {EventContentFields.MENTIONS: {"room": True}}
) )
) )
@ -374,7 +366,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self.assertFalse( self.assertFalse(
self._create_and_process( self._create_and_process(
bulk_evaluator, bulk_evaluator,
{EventContentFields.MSC3952_MENTIONS: {"room": mentions}}, {EventContentFields.MENTIONS: {"room": mentions}},
) )
) )
@ -385,7 +377,7 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
{ {
"body": "@room", "body": "@room",
"msgtype": "m.text", "msgtype": "m.text",
EventContentFields.MSC3952_MENTIONS: {}, EventContentFields.MENTIONS: {},
}, },
) )
) )