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

Stabilize support for MSC3952: Intentional mentions. #15520

Merged
merged 1 commit into from
Jun 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/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