Support the backwards compatibility features in MSC3952. (#14958)

If the feature is enabled and the event has a `m.mentions` property,
skip processing of the legacy mentions rules.
This commit is contained in:
Patrick Cloke 2023-02-03 11:28:20 -05:00 committed by GitHub
parent 0a686d1d13
commit 52700a0bcf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 190 additions and 65 deletions

View File

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

View File

@ -33,6 +33,7 @@ fn bench_match_exact(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new( let eval = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
false,
BTreeSet::new(), BTreeSet::new(),
false, false,
10, 10,
@ -71,6 +72,7 @@ fn bench_match_word(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new( let eval = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
false,
BTreeSet::new(), BTreeSet::new(),
false, false,
10, 10,
@ -109,6 +111,7 @@ fn bench_match_word_miss(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new( let eval = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
false,
BTreeSet::new(), BTreeSet::new(),
false, false,
10, 10,
@ -147,6 +150,7 @@ fn bench_eval_message(b: &mut Bencher) {
let eval = PushRuleEvaluator::py_new( let eval = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
false,
BTreeSet::new(), BTreeSet::new(),
false, false,
10, 10,

View File

@ -68,6 +68,8 @@ 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.
has_mentions: bool,
/// The user mentions that were part of the message. /// The user mentions that were part of the message.
user_mentions: BTreeSet<String>, user_mentions: BTreeSet<String>,
/// True if the message is a room message. /// True if the message is a room message.
@ -105,6 +107,7 @@ impl PushRuleEvaluator {
#[new] #[new]
pub fn py_new( pub fn py_new(
flattened_keys: BTreeMap<String, String>, flattened_keys: BTreeMap<String, String>,
has_mentions: bool,
user_mentions: BTreeSet<String>, user_mentions: BTreeSet<String>,
room_mention: bool, room_mention: bool,
room_member_count: u64, room_member_count: u64,
@ -123,6 +126,7 @@ impl PushRuleEvaluator {
Ok(PushRuleEvaluator { Ok(PushRuleEvaluator {
flattened_keys, flattened_keys,
body, body,
has_mentions,
user_mentions, user_mentions,
room_mention, room_mention,
room_member_count, room_member_count,
@ -155,6 +159,19 @@ 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
// 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 self.has_mentions
&& (rule_id == "global/override/.m.rule.contains_display_name"
|| rule_id == "global/content/.m.rule.contains_user_name"
|| rule_id == "global/override/.m.rule.roomnotif")
{
continue;
}
let extev_flag = &RoomVersionFeatures::ExtensibleEvents.as_str().to_string(); let extev_flag = &RoomVersionFeatures::ExtensibleEvents.as_str().to_string();
let supports_extensible_events = self.room_version_feature_flags.contains(extev_flag); let supports_extensible_events = self.room_version_feature_flags.contains(extev_flag);
let safe_from_rver_condition = SAFE_EXTENSIBLE_EVENTS_RULE_IDS.contains(rule_id); let safe_from_rver_condition = SAFE_EXTENSIBLE_EVENTS_RULE_IDS.contains(rule_id);
@ -441,6 +458,7 @@ fn push_rule_evaluator() {
flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string()); flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string());
let evaluator = PushRuleEvaluator::py_new( let evaluator = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
false,
BTreeSet::new(), BTreeSet::new(),
false, false,
10, 10,
@ -468,6 +486,7 @@ fn test_requires_room_version_supports_condition() {
let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()]; let flags = vec![RoomVersionFeatures::ExtensibleEvents.as_str().to_string()];
let evaluator = PushRuleEvaluator::py_new( let evaluator = PushRuleEvaluator::py_new(
flattened_keys, flattened_keys,
false,
BTreeSet::new(), BTreeSet::new(),
false, false,
10, 10,

View File

@ -56,6 +56,7 @@ class PushRuleEvaluator:
def __init__( def __init__(
self, self,
flattened_keys: Mapping[str, str], flattened_keys: Mapping[str, str],
has_mentions: bool,
user_mentions: Set[str], user_mentions: Set[str],
room_mention: bool, room_mention: bool,
room_member_count: int, room_member_count: int,

View File

@ -119,6 +119,9 @@ 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",
@ -364,9 +367,12 @@ class BulkPushRuleEvaluator:
# Pull out any user and room mentions. # Pull out any user and room mentions.
mentions = event.content.get(EventContentFields.MSC3952_MENTIONS) mentions = event.content.get(EventContentFields.MSC3952_MENTIONS)
has_mentions = self._intentional_mentions_enabled and isinstance(mentions, dict)
user_mentions: Set[str] = set() user_mentions: Set[str] = set()
room_mention = False room_mention = False
if isinstance(mentions, dict): if has_mentions:
# mypy seems to have lost the type even though it must be a dict here.
assert isinstance(mentions, dict)
# Remove out any non-string items and convert to a set. # Remove out any non-string items and convert to a set.
user_mentions_raw = mentions.get("user_ids") user_mentions_raw = mentions.get("user_ids")
if isinstance(user_mentions_raw, list): if isinstance(user_mentions_raw, list):
@ -378,6 +384,7 @@ class BulkPushRuleEvaluator:
evaluator = PushRuleEvaluator( evaluator = PushRuleEvaluator(
_flatten_dict(event, room_version=event.room_version), _flatten_dict(event, room_version=event.room_version),
has_mentions,
user_mentions, user_mentions,
room_mention, room_mention,
room_member_count, room_member_count,

View File

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Any from typing import Any, Optional
from unittest.mock import patch from unittest.mock import patch
from parameterized import parameterized from parameterized import parameterized
@ -25,7 +25,7 @@ from synapse.push.bulk_push_rule_evaluator import BulkPushRuleEvaluator
from synapse.rest import admin from synapse.rest import admin
from synapse.rest.client import login, register, room from synapse.rest.client import login, register, room
from synapse.server import HomeServer from synapse.server import HomeServer
from synapse.types import create_requester from synapse.types import JsonDict, create_requester
from synapse.util import Clock from synapse.util import Clock
from tests.test_utils import simple_async_mock from tests.test_utils import simple_async_mock
@ -196,19 +196,10 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self.get_success(bulk_evaluator.action_for_events_by_user([(event, context)])) self.get_success(bulk_evaluator.action_for_events_by_user([(event, context)]))
bulk_evaluator._action_for_event_by_user.assert_not_called() bulk_evaluator._action_for_event_by_user.assert_not_called()
@override_config({"experimental_features": {"msc3952_intentional_mentions": True}}) def _create_and_process(
def test_mentions(self) -> None: self, bulk_evaluator: BulkPushRuleEvaluator, content: Optional[JsonDict] = None
"""Test the behavior of an event which includes invalid mentions.""" ) -> bool:
bulk_evaluator = BulkPushRuleEvaluator(self.hs)
sentinel = object()
def create_and_process(mentions: Any = sentinel) -> bool:
"""Returns true iff the `mentions` trigger an event push action.""" """Returns true iff the `mentions` trigger an event push action."""
content = {}
if mentions is not sentinel:
content[EventContentFields.MSC3952_MENTIONS] = mentions
# Create a new message event which should cause a notification. # Create a new message event which should cause a notification.
event, context = self.get_success( event, context = self.get_success(
self.event_creation_handler.create_event( self.event_creation_handler.create_event(
@ -216,16 +207,14 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
{ {
"type": "test", "type": "test",
"room_id": self.room_id, "room_id": self.room_id,
"content": content, "content": content or {},
"sender": f"@bob:{self.hs.hostname}", "sender": f"@bob:{self.hs.hostname}",
}, },
) )
) )
# Ensure no actions are generated! # Execute the push rule machinery.
self.get_success( self.get_success(bulk_evaluator.action_for_events_by_user([(event, context)]))
bulk_evaluator.action_for_events_by_user([(event, context)])
)
# If any actions are generated for this event, return true. # If any actions are generated for this event, return true.
result = self.get_success( result = self.get_success(
@ -238,35 +227,113 @@ 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:
"""Test the behavior of an event which includes invalid user mentions."""
bulk_evaluator = BulkPushRuleEvaluator(self.hs)
# Not including the mentions field should not notify. # Not including the mentions field should not notify.
self.assertFalse(create_and_process()) self.assertFalse(self._create_and_process(bulk_evaluator))
# An empty mentions field should not notify. # An empty mentions field should not notify.
self.assertFalse(create_and_process({})) self.assertFalse(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {}}
)
)
# Non-dict mentions should be ignored. # Non-dict mentions should be ignored.
mentions: Any mentions: Any
for mentions in (None, True, False, 1, "foo", []): for mentions in (None, True, False, 1, "foo", []):
self.assertFalse(create_and_process(mentions)) self.assertFalse(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: mentions}
)
)
# A non-list should be ignored. # A non-list should be ignored.
for mentions in (None, True, False, 1, "foo", {}): for mentions in (None, True, False, 1, "foo", {}):
self.assertFalse(create_and_process({"user_ids": mentions})) self.assertFalse(
self._create_and_process(
# The Matrix ID appearing anywhere in the list should notify. bulk_evaluator,
self.assertTrue(create_and_process({"user_ids": [self.alice]})) {EventContentFields.MSC3952_MENTIONS: {"user_ids": mentions}},
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]})
) )
# 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]}},
)
)
self.assertTrue(
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
"user_ids": ["@another:test", self.alice]
}
},
)
)
# Duplicate user IDs should notify.
self.assertTrue(
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
"user_ids": [self.alice, self.alice]
}
},
)
)
# Invalid entries in the list are ignored.
self.assertFalse(
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
"user_ids": [None, True, False, {}, []]
}
},
)
)
self.assertTrue(
self._create_and_process(
bulk_evaluator,
{
EventContentFields.MSC3952_MENTIONS: {
"user_ids": [None, True, False, {}, [], self.alice]
}
},
)
)
# The legacy push rule should not mention if the mentions field exists.
self.assertFalse(
self._create_and_process(
bulk_evaluator,
{
"body": self.alice,
"msgtype": "m.text",
EventContentFields.MSC3952_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. # Room mentions from those without power should not notify.
self.assertFalse(create_and_process({"room": True})) self.assertFalse(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}}
)
)
# Room mentions from those with power should notify. # Room mentions from those with power should notify.
self.helper.send_state( self.helper.send_state(
@ -276,8 +343,30 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
self.token, self.token,
state_key="", state_key="",
) )
self.assertTrue(create_and_process({"room": True})) self.assertTrue(
self._create_and_process(
bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: {"room": True}}
)
)
# Invalid data should not notify. # Invalid data should not notify.
mentions: Any
for mentions in (None, False, 1, "foo", [], {}): for mentions in (None, False, 1, "foo", [], {}):
self.assertFalse(create_and_process({"room": mentions})) self.assertFalse(
self._create_and_process(
bulk_evaluator,
{EventContentFields.MSC3952_MENTIONS: {"room": mentions}},
)
)
# The legacy push rule should not mention if the mentions field exists.
self.assertFalse(
self._create_and_process(
bulk_evaluator,
{
"body": "@room",
"msgtype": "m.text",
EventContentFields.MSC3952_MENTIONS: {},
},
)
)

View File

@ -42,6 +42,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
self, self,
content: JsonMapping, content: JsonMapping,
*, *,
has_mentions: bool = False,
user_mentions: Optional[Set[str]] = None, user_mentions: Optional[Set[str]] = None,
room_mention: bool = False, room_mention: bool = False,
related_events: Optional[JsonDict] = None, related_events: Optional[JsonDict] = None,
@ -62,6 +63,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
power_levels: Dict[str, Union[int, Dict[str, int]]] = {} power_levels: Dict[str, Union[int, Dict[str, int]]] = {}
return PushRuleEvaluator( return PushRuleEvaluator(
_flatten_dict(event), _flatten_dict(event),
has_mentions,
user_mentions or set(), user_mentions or set(),
room_mention, room_mention,
room_member_count, room_member_count,
@ -102,19 +104,21 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
condition = {"kind": "org.matrix.msc3952.is_user_mention"} condition = {"kind": "org.matrix.msc3952.is_user_mention"}
# No mentions shouldn't match. # No mentions shouldn't match.
evaluator = self._get_evaluator({}) evaluator = self._get_evaluator({}, has_mentions=True)
self.assertFalse(evaluator.matches(condition, "@user:test", None)) self.assertFalse(evaluator.matches(condition, "@user:test", None))
# An empty set shouldn't match # An empty set shouldn't match
evaluator = self._get_evaluator({}, user_mentions=set()) evaluator = self._get_evaluator({}, has_mentions=True, user_mentions=set())
self.assertFalse(evaluator.matches(condition, "@user:test", None)) self.assertFalse(evaluator.matches(condition, "@user:test", None))
# The Matrix ID appearing anywhere in the mentions list should match # The Matrix ID appearing anywhere in the mentions list should match
evaluator = self._get_evaluator({}, user_mentions={"@user:test"}) evaluator = self._get_evaluator(
{}, has_mentions=True, user_mentions={"@user:test"}
)
self.assertTrue(evaluator.matches(condition, "@user:test", None)) self.assertTrue(evaluator.matches(condition, "@user:test", None))
evaluator = self._get_evaluator( evaluator = self._get_evaluator(
{}, user_mentions={"@another:test", "@user:test"} {}, has_mentions=True, user_mentions={"@another:test", "@user:test"}
) )
self.assertTrue(evaluator.matches(condition, "@user:test", None)) self.assertTrue(evaluator.matches(condition, "@user:test", None))
@ -126,16 +130,16 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
condition = {"kind": "org.matrix.msc3952.is_room_mention"} condition = {"kind": "org.matrix.msc3952.is_room_mention"}
# No room mention shouldn't match. # No room mention shouldn't match.
evaluator = self._get_evaluator({}) evaluator = self._get_evaluator({}, has_mentions=True)
self.assertFalse(evaluator.matches(condition, None, None)) self.assertFalse(evaluator.matches(condition, None, None))
# Room mention should match. # Room mention should match.
evaluator = self._get_evaluator({}, room_mention=True) evaluator = self._get_evaluator({}, has_mentions=True, room_mention=True)
self.assertTrue(evaluator.matches(condition, None, None)) self.assertTrue(evaluator.matches(condition, None, None))
# A room mention and user mention is valid. # A room mention and user mention is valid.
evaluator = self._get_evaluator( evaluator = self._get_evaluator(
{}, user_mentions={"@another:test"}, room_mention=True {}, has_mentions=True, user_mentions={"@another:test"}, room_mention=True
) )
self.assertTrue(evaluator.matches(condition, None, None)) self.assertTrue(evaluator.matches(condition, None, None))