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

Implement MSC3952: Intentional mentions #14823

Merged
merged 6 commits into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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/14823.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Experimental support for [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952): intentional mentions.
16 changes: 16 additions & 0 deletions rust/src/push/base_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default: true,
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_user_mentioned"),
priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::IsUserMention)]),
actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]),
default: true,
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed("global/override/.m.rule.contains_display_name"),
priority_class: 5,
Expand All @@ -139,6 +147,14 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
default: true,
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed(".org.matrix.msc3952.is_room_mentioned"),
priority_class: 5,
conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::IsRoomMention)]),
actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]),
default: true,
default_enabled: true,
},
PushRule {
rule_id: Cow::Borrowed("global/override/.m.rule.roomnotif"),
priority_class: 5,
Expand Down
25 changes: 23 additions & 2 deletions rust/src/push/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};

use anyhow::{Context, Error};
use lazy_static::lazy_static;
Expand Down Expand Up @@ -68,6 +68,11 @@ pub struct PushRuleEvaluator {
/// The "content.body", if any.
body: String,

/// The user mentions that were part of the message.
user_mentions: BTreeSet<String>,
/// True if the message is a room message.
room_mention: bool,

/// The number of users in the room.
room_member_count: u64,

Expand Down Expand Up @@ -100,6 +105,8 @@ impl PushRuleEvaluator {
#[new]
pub fn py_new(
flattened_keys: BTreeMap<String, String>,
user_mentions: BTreeSet<String>,
room_mention: bool,
room_member_count: u64,
sender_power_level: Option<i64>,
notification_power_levels: BTreeMap<String, i64>,
Expand All @@ -116,6 +123,8 @@ impl PushRuleEvaluator {
Ok(PushRuleEvaluator {
flattened_keys,
body,
user_mentions,
room_mention,
room_member_count,
notification_power_levels,
sender_power_level,
Expand Down Expand Up @@ -229,6 +238,14 @@ impl PushRuleEvaluator {
KnownCondition::RelatedEventMatch(event_match) => {
self.match_related_event_match(event_match, user_id)?
}
KnownCondition::IsUserMention => {
if let Some(uid) = user_id {
self.user_mentions.contains(uid)
} else {
false
}
}
KnownCondition::IsRoomMention => self.room_mention,
KnownCondition::ContainsDisplayName => {
if let Some(dn) = display_name {
if !dn.is_empty() {
Expand Down Expand Up @@ -424,6 +441,8 @@ fn push_rule_evaluator() {
flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string());
let evaluator = PushRuleEvaluator::py_new(
flattened_keys,
BTreeSet::new(),
false,
10,
Some(0),
BTreeMap::new(),
Expand All @@ -449,6 +468,8 @@ fn test_requires_room_version_supports_condition() {
let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()];
let evaluator = PushRuleEvaluator::py_new(
flattened_keys,
BTreeSet::new(),
false,
10,
Some(0),
BTreeMap::new(),
Expand Down Expand Up @@ -483,7 +504,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),
&FilteredPushRules::py_new(rules, BTreeMap::new(), true, false, true, false),
None,
None,
);
Expand Down
34 changes: 34 additions & 0 deletions rust/src/push/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@ pub enum KnownCondition {
EventMatch(EventMatchCondition),
#[serde(rename = "im.nheko.msc3664.related_event_match")]
RelatedEventMatch(RelatedEventMatchCondition),
#[serde(rename = "org.matrix.msc3952.is_user_mention")]
IsUserMention,
#[serde(rename = "org.matrix.msc3952.is_room_mention")]
IsRoomMention,
ContainsDisplayName,
RoomMemberCount {
#[serde(skip_serializing_if = "Option::is_none")]
Expand Down Expand Up @@ -414,6 +418,7 @@ pub struct FilteredPushRules {
msc1767_enabled: bool,
msc3381_polls_enabled: bool,
msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
}

#[pymethods]
Expand All @@ -425,13 +430,15 @@ impl FilteredPushRules {
msc1767_enabled: bool,
msc3381_polls_enabled: bool,
msc3664_enabled: bool,
msc3952_intentional_mentions: bool,
) -> Self {
Self {
push_rules,
enabled_map,
msc1767_enabled,
msc3381_polls_enabled,
msc3664_enabled,
msc3952_intentional_mentions,
}
}

Expand Down Expand Up @@ -465,6 +472,11 @@ impl FilteredPushRules {
return false;
}

if !self.msc3952_intentional_mentions && rule.rule_id.contains("org.matrix.msc3952")
{
return false;
}

true
})
.map(|r| {
Expand Down Expand Up @@ -522,6 +534,28 @@ fn test_deserialize_unstable_msc3931_condition() {
));
}

#[test]
fn test_deserialize_unstable_msc3952_user_condition() {
let json = r#"{"kind":"org.matrix.msc3952.is_user_mention"}"#;

let condition: Condition = serde_json::from_str(json).unwrap();
assert!(matches!(
condition,
Condition::Known(KnownCondition::IsUserMention)
));
}
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

#[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]
fn test_deserialize_custom_condition() {
let json = r#"{"kind":"custom_tag"}"#;
Expand Down
5 changes: 4 additions & 1 deletion stubs/synapse/synapse_rust/push.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Tuple, Union
from typing import Any, Collection, Dict, Mapping, Optional, Sequence, Set, Tuple, Union

from synapse.types import JsonDict

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

Expand All @@ -55,6 +56,8 @@ class PushRuleEvaluator:
def __init__(
self,
flattened_keys: Mapping[str, str],
user_mentions: Set[str],
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
room_mention: bool,
room_member_count: int,
sender_power_level: Optional[int],
notification_power_levels: Mapping[str, int],
Expand Down
3 changes: 3 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ class EventContentFields:
# The authorising user for joining a restricted room.
AUTHORISING_USER: Final = "join_authorised_via_users_server"

# Use for mentioning users.
MSC3952_MENTIONS: Final = "org.matrix.msc3952.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: 5 additions & 0 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:

# MSC3925: do not replace events with their edits
self.msc3925_inhibit_edit = experimental.get("msc3925_inhibit_edit", False)

# MSC3952: Intentional mentions
self.msc3952_intentional_mentions = experimental.get(
"msc3952_intentional_mentions", False
)
25 changes: 24 additions & 1 deletion synapse/push/bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,20 @@
List,
Mapping,
Optional,
Set,
Tuple,
Union,
)

from prometheus_client import Counter

from synapse.api.constants import MAIN_TIMELINE, EventTypes, Membership, RelationTypes
from synapse.api.constants import (
MAIN_TIMELINE,
EventContentFields,
EventTypes,
Membership,
RelationTypes,
)
from synapse.api.room_versions import PushRuleRoomFlag, RoomVersion
from synapse.event_auth import auth_types_for_event, get_user_power_level
from synapse.events import EventBase, relation_from_event
Expand Down Expand Up @@ -342,8 +349,24 @@ async def _action_for_event_by_user(
for user_id, level in notification_levels.items():
notification_levels[user_id] = int(level)

# Pull out any user and room mentions.
mentions = event.content.get(EventContentFields.MSC3952_MENTIONS)
user_mentions: Set[str] = set()
room_mention = False
if isinstance(mentions, dict):
# Remove out any non-string items and convert to a set.
user_mentions_raw = mentions.get("user_ids")
if isinstance(user_mentions_raw, list):
user_mentions = set(
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(
_flatten_dict(event, room_version=event.room_version),
user_mentions,
room_mention,
room_member_count,
sender_power_level,
notification_levels,
Expand Down
1 change: 1 addition & 0 deletions synapse/storage/databases/main/push_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ 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,
)

return filtered_rules
Expand Down
76 changes: 76 additions & 0 deletions tests/push/test_bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Any
from unittest.mock import patch

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import EventContentFields
from synapse.api.room_versions import RoomVersions
from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator
from synapse.rest import admin
Expand Down Expand Up @@ -126,3 +128,77 @@ def test_action_for_event_by_user_disabled_by_config(self) -> None:
# Ensure no actions are generated!
self.get_success(bulk_evaluator.action_for_events_by_user([(event, context)]))
bulk_evaluator._action_for_event_by_user.assert_not_called()

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

sentinel = object()

def create_and_process(mentions: Any = sentinel) -> bool:
content = {}
clokep marked this conversation as resolved.
Show resolved Hide resolved
if mentions is not sentinel:
content[EventContentFields.MSC3952_MENTIONS] = mentions

# Create a new message event which should cause a notification.
event, context = self.get_success(
self.event_creation_handler.create_event(
self.requester,
{
"type": "test",
"room_id": self.room_id,
"content": content,
"sender": f"@bob:{self.hs.hostname}",
},
)
)

# Ensure no actions are generated!
self.get_success(
bulk_evaluator.action_for_events_by_user([(event, context)])
)

# If any actions are generated for this event, return true.
result = self.get_success(
self.hs.get_datastores().main.db_pool.simple_select_list(
table="event_push_actions_staging",
keyvalues={"event_id": event.event_id},
retcols=("*",),
desc="get_event_push_actions_staging",
)
)
return len(result) > 0

# Not including the mentions field should not notify.
self.assertFalse(create_and_process())
# An empty mentions field should not notify.
self.assertFalse(create_and_process({}))

# Non-dict mentions should be ignored.
mentions: Any
for mentions in (None, True, False, 1, "foo", []):
self.assertFalse(create_and_process(mentions))

# A non-list should be ignored.
for mentions in (None, True, False, 1, "foo", {}):
self.assertFalse(create_and_process({"user_ids": mentions}))

# The Matrix ID appearing anywhere in the list should notify.
self.assertTrue(create_and_process({"user_ids": [self.alice]}))
self.assertTrue(create_and_process({"user_ids": ["@another:test", self.alice]}))

# Duplicate user IDs should notify.
self.assertTrue(create_and_process({"user_ids": [self.alice, self.alice]}))

# Invalid entries in the list are ignored.
self.assertFalse(create_and_process({"user_ids": [None, True, False, {}, []]}))
self.assertTrue(
create_and_process({"user_ids": [None, True, False, {}, [], self.alice]})
)

# Room mentions should notify.
self.assertTrue(create_and_process({"room": True}))
# A non-True should not notify.
for mentions in (None, False, 1, "foo", [], {}):
self.assertFalse(create_and_process({"room": mentions}))
Loading