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

Commit

Permalink
Stabilize support for MSC3952: Intentional mentions.
Browse files Browse the repository at this point in the history
  • Loading branch information
clokep committed May 24, 2023
1 parent ca5c4be commit 6c94dc9
Show file tree
Hide file tree
Showing 13 changed files with 27 additions and 64 deletions.
1 change: 1 addition & 0 deletions changelog.d/15520.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enable support for [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952): intentional mentions.
3 changes: 0 additions & 3 deletions rust/benches/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
// limitations under the License.

#![feature(test)]
use std::collections::BTreeSet;

use synapse::push::{
evaluator::PushRuleEvaluator, Condition, EventMatchCondition, FilteredPushRules, JsonValue,
PushRules, SimpleJsonValue,
Expand Down Expand Up @@ -197,7 +195,6 @@ fn bench_eval_message(b: &mut Bencher) {
false,
false,
false,
false,
);

b.iter(|| eval.run(&rules, Some("bob"), Some("person")));
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 @@ -142,11 +142,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mention"),
rule_id: Cow::Borrowed("global/override/.m.is_user_mention"),
priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known(
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),
}),
)]),
Expand All @@ -163,11 +163,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mention"),
rule_id: Cow::Borrowed("global/override/.m.is_room_mention"),
priority_class: 5,
conditions: Cow::Borrowed(&[
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)),
})),
Condition::Known(KnownCondition::SenderNotificationPermission {
Expand Down
10 changes: 5 additions & 5 deletions rust/src/push/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ pub struct PushRuleEvaluator {
/// The "content.body", if any.
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,

/// The number of users in the room.
Expand Down Expand Up @@ -155,9 +157,7 @@ impl PushRuleEvaluator {
let rule_id = &push_rule.rule_id().to_string();

// For backwards-compatibility the legacy mention rules are disabled
// if the event contains the 'm.mentions' property (and if the
// experimental feature is enabled, both of these are represented
// by the has_mentions flag).
// if the event contains the 'm.mentions' property.
if self.has_mentions
&& (rule_id == "global/override/.m.rule.contains_display_name"
|| rule_id == "global/content/.m.rule.contains_user_name"
Expand Down Expand Up @@ -562,7 +562,7 @@ fn test_requires_room_version_supports_condition() {
};
let rules = PushRules::new(vec![custom_rule]);
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,
);
Expand Down
7 changes: 0 additions & 7 deletions rust/src/push/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,6 @@ pub struct FilteredPushRules {
msc1767_enabled: bool,
msc3381_polls_enabled: bool,
msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
msc3958_suppress_edits_enabled: bool,
}

Expand All @@ -540,7 +539,6 @@ impl FilteredPushRules {
msc1767_enabled: bool,
msc3381_polls_enabled: bool,
msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
msc3958_suppress_edits_enabled: bool,
) -> Self {
Self {
Expand All @@ -549,7 +547,6 @@ impl FilteredPushRules {
msc1767_enabled,
msc3381_polls_enabled,
msc3664_enabled,
msc3952_intentional_mentions,
msc3958_suppress_edits_enabled,
}
}
Expand Down Expand Up @@ -587,10 +584,6 @@ impl FilteredPushRules {
return false;
}

if !self.msc3952_intentional_mentions && rule.rule_id.contains("org.matrix.msc3952")
{
return false;
}
if !self.msc3958_suppress_edits_enabled
&& rule.rule_id == "global/override/.com.beeper.suppress_edits"
{
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 @@ -46,7 +46,6 @@ class FilteredPushRules:
msc1767_enabled: bool,
msc3381_polls_enabled: bool,
msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
msc3958_suppress_edits_enabled: bool,
): ...
def rules(self) -> Collection[Tuple[PushRule, bool]]: ...
Expand Down
2 changes: 1 addition & 1 deletion synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ class EventContentFields:
AUTHORISING_USER: Final = "join_authorised_via_users_server"

# 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
TO_DEVICE_MSGID: Final = "org.matrix.msgid"
Expand Down
5 changes: 0 additions & 5 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# MSC3391: Removing account data.
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.
self.msc3958_supress_edit_notifs = experimental.get(
"msc3958_supress_edit_notifs", False
Expand Down
9 changes: 2 additions & 7 deletions synapse/events/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,8 @@ def validate_new(self, event: EventBase, config: HomeServerConfig) -> None:
)

# If the event contains a mentions key, validate it.
if (
EventContentFields.MSC3952_MENTIONS in event.content
and config.experimental.msc3952_intentional_mentions
):
validate_json_object(
event.content[EventContentFields.MSC3952_MENTIONS], Mentions
)
if EventContentFields.MENTIONS in event.content:
validate_json_object(event.content[EventContentFields.MENTIONS], Mentions)

def _validate_retention(self, event: EventBase) -> None:
"""Checks that an event that defines the retention policy for a room respects the
Expand Down
8 changes: 1 addition & 7 deletions synapse/push/bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ def __init__(self, hs: "HomeServer"):
self.should_calculate_push_rules = self.hs.config.push.enable_push

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(
"cache",
Expand Down Expand Up @@ -390,10 +387,7 @@ async def _action_for_event_by_user(
del notification_levels[key]

# Pull out any user and room mentions.
has_mentions = (
self._intentional_mentions_enabled
and EventContentFields.MSC3952_MENTIONS in event.content
)
has_mentions = EventContentFields.MENTIONS in event.content

evaluator = PushRuleEvaluator(
_flatten_dict(event),
Expand Down
2 changes: 0 additions & 2 deletions synapse/rest/client/versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,6 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
is not None,
# Adds support for relation-based redactions as per MSC3912.
"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.
"org.matrix.msc3981": self.config.experimental.msc3981_recurse_relations,
# Adds support for deleting account data.
Expand Down
1 change: 0 additions & 1 deletion synapse/storage/databases/main/push_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ def _load_rules(
msc1767_enabled=experimental_config.msc1767_enabled,
msc3664_enabled=experimental_config.msc3664_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,
)

Expand Down
34 changes: 13 additions & 21 deletions tests/push/test_bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ def _create_and_process(
)
return len(result) > 0

@override_config({"experimental_features": {"msc3952_intentional_mentions": True}})
def test_user_mentions(self) -> None:
"""Test the behavior of an event which includes invalid user mentions."""
bulk_evaluator = BulkPushRuleEvaluator(self.hs)
Expand All @@ -237,9 +236,7 @@ def test_user_mentions(self) -> None:
self.assertFalse(self._create_and_process(bulk_evaluator))
# An empty mentions field should not notify.
self.assertFalse(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {}}
)
self._create_and_process(bulk_evaluator, {EventContentFields.MENTIONS: {}})
)

# Non-dict mentions should be ignored.
Expand All @@ -253,7 +250,7 @@ def test_user_mentions(self) -> None:
for mentions in (None, True, False, 1, "foo", []):
self.assertFalse(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: mentions}
bulk_evaluator, {EventContentFields.MENTIONS: mentions}
)
)

Expand All @@ -262,22 +259,22 @@ def test_user_mentions(self) -> None:
self.assertFalse(
self._create_and_process(
bulk_evaluator,
{EventContentFields.MSC3952_MENTIONS: {"user_ids": mentions}},
{EventContentFields.MENTIONS: {"user_ids": mentions}},
)
)

# The Matrix ID appearing anywhere in the list should notify.
self.assertTrue(
self._create_and_process(
bulk_evaluator,
{EventContentFields.MSC3952_MENTIONS: {"user_ids": [self.alice]}},
{EventContentFields.MENTIONS: {"user_ids": [self.alice]}},
)
)
self.assertTrue(
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
EventContentFields.MENTIONS: {
"user_ids": ["@another:test", self.alice]
}
},
Expand All @@ -288,11 +285,7 @@ def test_user_mentions(self) -> None:
self.assertTrue(
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
"user_ids": [self.alice, self.alice]
}
},
{EventContentFields.MENTIONS: {"user_ids": [self.alice, self.alice]}},
)
)

Expand All @@ -307,7 +300,7 @@ def test_user_mentions(self) -> None:
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
EventContentFields.MENTIONS: {
"user_ids": [None, True, False, {}, []]
}
},
Expand All @@ -317,7 +310,7 @@ def test_user_mentions(self) -> None:
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
EventContentFields.MENTIONS: {
"user_ids": [None, True, False, {}, [], self.alice]
}
},
Expand All @@ -331,20 +324,19 @@ def test_user_mentions(self) -> None:
{
"body": self.alice,
"msgtype": "m.text",
EventContentFields.MSC3952_MENTIONS: {},
EventContentFields.MENTIONS: {},
},
)
)

@override_config({"experimental_features": {"msc3952_intentional_mentions": True}})
def test_room_mentions(self) -> None:
"""Test the behavior of an event which includes invalid room mentions."""
bulk_evaluator = BulkPushRuleEvaluator(self.hs)

# Room mentions from those without power should not notify.
self.assertFalse(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}}
bulk_evaluator, {EventContentFields.MENTIONS: {"room": True}}
)
)

Expand All @@ -358,7 +350,7 @@ def test_room_mentions(self) -> None:
)
self.assertTrue(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}}
bulk_evaluator, {EventContentFields.MENTIONS: {"room": True}}
)
)

Expand All @@ -374,7 +366,7 @@ def test_room_mentions(self) -> None:
self.assertFalse(
self._create_and_process(
bulk_evaluator,
{EventContentFields.MSC3952_MENTIONS: {"room": mentions}},
{EventContentFields.MENTIONS: {"room": mentions}},
)
)

Expand All @@ -385,7 +377,7 @@ def test_room_mentions(self) -> None:
{
"body": "@room",
"msgtype": "m.text",
EventContentFields.MSC3952_MENTIONS: {},
EventContentFields.MENTIONS: {},
},
)
)
Expand Down

0 comments on commit 6c94dc9

Please sign in to comment.