From dc500539f03b90f713bd0008d77fd4846df4ff9a Mon Sep 17 00:00:00 2001 From: Brad Murray Date: Tue, 11 Jan 2022 14:18:01 +0000 Subject: [PATCH 01/15] DES-1255 Implement related event match push rule --- synapse/push/baserules.py | 26 ++++++++++ synapse/push/bulk_push_rule_evaluator.py | 8 ++- synapse/push/push_rule_evaluator.py | 29 ++++++++--- tests/push/test_push_rule_evaluator.py | 64 +++++++++++++++++++++++- 4 files changed, 116 insertions(+), 11 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 621150699069..9b39904fe9fe 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -427,6 +427,32 @@ def make_base_prepend_rules( ], "actions": ["notify", {"set_tweak": "sound", "value": "ring"}], }, + # Enable notifications for reactions + { + "rule_id": "global/override/com.beeper.reaction", + "conditions": [ + { + "kind": "event_match", + "key": "type", + "pattern": "m.reaction", + "_id": "_reaction", + }, + # Only send reaction notifications for smaller rooms (under 20 members) + { + "kind": "room_member_count", + "is": "<20", + "_id": "_member_count", + }, + # Only send notification if the reaction is to your message + { + "kind": "related_event_match", + "key": "sender", + "pattern_type": "user_id", + "_id": "_sender", + }, + ], + "actions": ["notify", {"set_tweak": "highlight", "value": False}], + }, ] diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index bee660893be2..3b7948c882ef 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -36,7 +36,6 @@ logger = logging.getLogger(__name__) - push_rules_invalidation_counter = Counter( "synapse_push_bulk_push_rule_evaluator_push_rules_invalidation_counter", "" ) @@ -203,8 +202,13 @@ async def action_for_event_by_user( sender_power_level, ) = await self._get_power_levels_and_sender_level(event, context) + related_event_id = event.content.get("m.relates_to", {}).get("event_id") + related_event = ( + (await self.store.get_event(related_event_id)) if related_event_id else None + ) + evaluator = PushRuleEvaluatorForEvent( - event, len(room_members), sender_power_level, power_levels + event, len(room_members), sender_power_level, power_levels, related_event ) condition_cache: Dict[str, bool] = {} diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 659a53805df1..536c4389a030 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -120,6 +120,7 @@ def __init__( room_member_count: int, sender_power_level: int, power_levels: Dict[str, Union[int, Dict[str, int]]], + related_event: Optional[EventBase], ): self._event = event self._room_member_count = room_member_count @@ -129,11 +130,22 @@ def __init__( # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) + self._related_event = related_event + self._related_event_value_cache = ( + _flatten_dict(related_event) if related_event else None + ) + def matches( self, condition: Dict[str, Any], user_id: str, display_name: Optional[str] ) -> bool: if condition["kind"] == "event_match": - return self._event_match(condition, user_id) + return self._event_match(condition, user_id, self._event, self._value_cache) + elif condition["kind"] == "related_event_match": + if not self._related_event: + return False + return self._event_match( + condition, user_id, self._related_event, self._related_event_value_cache + ) elif condition["kind"] == "contains_display_name": return self._contains_display_name(display_name) elif condition["kind"] == "room_member_count": @@ -145,7 +157,13 @@ def matches( else: return True - def _event_match(self, condition: dict, user_id: str) -> bool: + def _event_match( + self, + condition: dict, + user_id: str, + event: EventBase, + event_value_cache: Dict[str, str], + ) -> bool: pattern = condition.get("pattern", None) if not pattern: @@ -161,13 +179,13 @@ def _event_match(self, condition: dict, user_id: str) -> bool: # XXX: optimisation: cache our pattern regexps if condition["key"] == "content.body": - body = self._event.content.get("body", None) + body = event.content.get("body", None) if not body or not isinstance(body, str): return False return _glob_matches(pattern, body, word_boundary=True) else: - haystack = self._get_value(condition["key"]) + haystack = event_value_cache.get(condition["key"], None) if haystack is None: return False @@ -191,9 +209,6 @@ def _contains_display_name(self, display_name: Optional[str]) -> bool: return bool(r.search(body)) - def _get_value(self, dotted_key: str) -> Optional[str]: - return self._value_cache.get(dotted_key, None) - # Caches (string, is_glob, word_boundary) -> regex for push. See _glob_matches regex_cache: LruCache[Tuple[str, bool, bool], Pattern] = LruCache( diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index a52e89e4074b..32501a374b5b 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -23,7 +23,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): - def _get_evaluator(self, content): + def _get_evaluator(self, content, related_event=None): event = FrozenEvent( { "event_id": "$event_id", @@ -39,7 +39,7 @@ def _get_evaluator(self, content): sender_power_level = 0 power_levels = {} return PushRuleEvaluatorForEvent( - event, room_member_count, sender_power_level, power_levels + event, room_member_count, sender_power_level, power_levels, related_event ) def test_display_name(self): @@ -266,3 +266,63 @@ def test_tweaks_for_actions(self): push_rule_evaluator.tweaks_for_actions(actions), {"sound": "default", "highlight": True}, ) + + def test_related_event_match(self): + evaluator = self._get_evaluator( + { + "m.relates_to": { + "event_id": "$parent_event_id", + "key": "\ud83d\udc4d\ufe0f", + "rel_type": "m.annotation", + } + }, + FrozenEvent( + { + "event_id": "$parent.event_id", + "type": "m.room.message", + "sender": "@other_user:test", + "state_key": "", + "room_id": "#room:test", + "content": {"msgtype": "m.text", "body": "Original message"}, + }, + RoomVersions.V1, + ), + ) + self.assertFalse( + evaluator.matches( + { + "kind": "related_event_match", + "key": "sender", + "pattern_type": "user_id", + }, + "@user:test", + "display_name", + ) + ) + self.assertTrue( + evaluator.matches( + { + "kind": "related_event_match", + "key": "sender", + "pattern_type": "user_id", + }, + "@other_user:test", + "display_name", + ) + ) + + def test_related_event_match_no_related_event(self): + evaluator = self._get_evaluator( + {"msgtype": "m.text", "body": "Message without related event"} + ) + self.assertFalse( + evaluator.matches( + { + "kind": "related_event_match", + "key": "sender", + "pattern_type": "user_id", + }, + "@user:test", + "display_name", + ) + ) From a637984e9505a8da22795df570b1cb94ba2f5e2e Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 08:09:07 +0100 Subject: [PATCH 02/15] Make relation push rules match MSC3664 Signed-off-by: Nicolas Werner --- synapse/push/baserules.py | 45 ++++----- synapse/push/bulk_push_rule_evaluator.py | 15 ++- synapse/push/push_rule_evaluator.py | 37 +++++-- tests/push/test_push_rule_evaluator.py | 117 +++++++++++++++++++---- 4 files changed, 159 insertions(+), 55 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 9b39904fe9fe..9fa537231f03 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -363,6 +363,25 @@ def make_base_prepend_rules( ], "actions": ["notify", {"set_tweak": "sound", "value": "default"}], }, + # Enable notifications for replies without fallback + { + "rule_id": "global/override/im.nheko.msc3664.reply", + "conditions": [ + # Only send notification if the reply is to your message + { + "kind": "related_event_match", + "key": "sender", + "rel_type": "m.in_reply_to", + "pattern_type": "user_id", + "_id": "_sender", + }, + ], + "actions": [ + "notify", + {"set_tweak": "sound", "value": "default"}, + {"set_tweak": "highlight"}, + ], + }, { "rule_id": "global/override/.m.rule.contains_display_name", "conditions": [{"kind": "contains_display_name"}], @@ -427,32 +446,6 @@ def make_base_prepend_rules( ], "actions": ["notify", {"set_tweak": "sound", "value": "ring"}], }, - # Enable notifications for reactions - { - "rule_id": "global/override/com.beeper.reaction", - "conditions": [ - { - "kind": "event_match", - "key": "type", - "pattern": "m.reaction", - "_id": "_reaction", - }, - # Only send reaction notifications for smaller rooms (under 20 members) - { - "kind": "room_member_count", - "is": "<20", - "_id": "_member_count", - }, - # Only send notification if the reaction is to your message - { - "kind": "related_event_match", - "key": "sender", - "pattern_type": "user_id", - "_id": "_sender", - }, - ], - "actions": ["notify", {"set_tweak": "highlight", "value": False}], - }, ] diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 3b7948c882ef..7941d7ee9c56 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -202,13 +202,22 @@ async def action_for_event_by_user( sender_power_level, ) = await self._get_power_levels_and_sender_level(event, context) + related_events: Dict[str, EventBase] = {} related_event_id = event.content.get("m.relates_to", {}).get("event_id") - related_event = ( - (await self.store.get_event(related_event_id)) if related_event_id else None + relation_type = event.content.get("m.relates_to", {}).get("rel_type") + if related_event_id is not None and relation_type is not None: + related_events[relation_type] = await self.store.get_event(related_event_id) + + reply_event_id = ( + event.content.get("m.relates_to", {}) + .get("m.in_reply_to", {}) + .get("event_id") ) + if reply_event_id is not None: + related_events["m.in_reply_to"] = await self.store.get_event(reply_event_id) evaluator = PushRuleEvaluatorForEvent( - event, len(room_members), sender_power_level, power_levels, related_event + event, len(room_members), sender_power_level, power_levels, related_events ) condition_cache: Dict[str, bool] = {} diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 536c4389a030..4dd86e287927 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -120,7 +120,7 @@ def __init__( room_member_count: int, sender_power_level: int, power_levels: Dict[str, Union[int, Dict[str, int]]], - related_event: Optional[EventBase], + related_events: Dict[str, EventBase], ): self._event = event self._room_member_count = room_member_count @@ -130,21 +130,34 @@ def __init__( # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) - self._related_event = related_event - self._related_event_value_cache = ( - _flatten_dict(related_event) if related_event else None - ) + self._related_events = related_events + self._related_events_value_cache = { + k: _flatten_dict(v) for k, v in related_events.items() + } def matches( self, condition: Dict[str, Any], user_id: str, display_name: Optional[str] ) -> bool: if condition["kind"] == "event_match": return self._event_match(condition, user_id, self._event, self._value_cache) - elif condition["kind"] == "related_event_match": - if not self._related_event: + elif condition["kind"] == "im.nheko.msc3664.related_event_match": + # If we have no related event, the pattern will never match + if not self._related_events: return False + + related_event = self._related_events.get(condition["rel_type"]) + related_event_value_cache = self._related_events_value_cache.get( + condition["rel_type"] + ) + if not related_event or not related_event_value_cache: + return False + + # we have a related event, but we only want to match for existence + if not condition.get("key"): + return True + return self._event_match( - condition, user_id, self._related_event, self._related_event_value_cache + condition, user_id, related_event, related_event_value_cache ) elif condition["kind"] == "contains_display_name": return self._contains_display_name(display_name) @@ -174,8 +187,12 @@ def _event_match( pattern = UserID.from_string(user_id).localpart if not pattern: - logger.warning("event_match condition with no pattern") - return False + # msc3664 specifies that key without pattern matches field existence + if condition["kind"] == "im.nheko.msc3664.related_event_match": + return condition["key"] in event_value_cache + else: + logger.warning("event_match condition with no pattern") + return False # XXX: optimisation: cache our pattern regexps if condition["key"] == "content.body": diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index 32501a374b5b..55f3bcec6d23 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -23,7 +23,7 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): - def _get_evaluator(self, content, related_event=None): + def _get_evaluator(self, content, related_events=None): event = FrozenEvent( { "event_id": "$event_id", @@ -39,7 +39,11 @@ def _get_evaluator(self, content, related_event=None): sender_power_level = 0 power_levels = {} return PushRuleEvaluatorForEvent( - event, room_member_count, sender_power_level, power_levels, related_event + event, + room_member_count, + sender_power_level, + power_levels, + {} if related_events is None else related_events, ) def test_display_name(self): @@ -274,25 +278,40 @@ def test_related_event_match(self): "event_id": "$parent_event_id", "key": "\ud83d\udc4d\ufe0f", "rel_type": "m.annotation", + "m.in_reply_to": { + "event_id": "$parent_event_id", + }, } }, - FrozenEvent( - { - "event_id": "$parent.event_id", - "type": "m.room.message", - "sender": "@other_user:test", - "state_key": "", - "room_id": "#room:test", - "content": {"msgtype": "m.text", "body": "Original message"}, - }, - RoomVersions.V1, - ), + { + "m.in_reply_to": FrozenEvent( + { + "event_id": "$parent_event_id", + "type": "m.room.message", + "sender": "@other_user:test", + "room_id": "#room:test", + "content": {"msgtype": "m.text", "body": "Original message"}, + }, + RoomVersions.V1, + ), + "m.annotation": FrozenEvent( + { + "event_id": "$parent_event_id", + "type": "m.room.message", + "sender": "@other_user:test", + "room_id": "#room:test", + "content": {"msgtype": "m.text", "body": "Original message"}, + }, + RoomVersions.V1, + ), + }, ) self.assertFalse( evaluator.matches( { - "kind": "related_event_match", + "kind": "im.nheko.msc3664.related_event_match", "key": "sender", + "rel_type": "m.in_reply_to", "pattern_type": "user_id", }, "@user:test", @@ -302,14 +321,58 @@ def test_related_event_match(self): self.assertTrue( evaluator.matches( { - "kind": "related_event_match", + "kind": "im.nheko.msc3664.related_event_match", "key": "sender", + "rel_type": "m.in_reply_to", + "pattern_type": "user_id", + }, + "@other_user:test", + "display_name", + ) + ) + self.assertTrue( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "key": "sender", + "rel_type": "m.annotation", "pattern_type": "user_id", }, "@other_user:test", "display_name", ) ) + self.assertTrue( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "key": "sender", + "rel_type": "m.in_reply_to", + }, + "@user:test", + "display_name", + ) + ) + self.assertTrue( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "rel_type": "m.in_reply_to", + }, + "@user:test", + "display_name", + ) + ) + self.assertFalse( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "rel_type": "m.replace", + }, + "@other_user:test", + "display_name", + ) + ) def test_related_event_match_no_related_event(self): evaluator = self._get_evaluator( @@ -318,11 +381,33 @@ def test_related_event_match_no_related_event(self): self.assertFalse( evaluator.matches( { - "kind": "related_event_match", + "kind": "im.nheko.msc3664.related_event_match", "key": "sender", + "rel_type": "m.in_reply_to", "pattern_type": "user_id", }, "@user:test", "display_name", ) ) + self.assertFalse( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "key": "sender", + "rel_type": "m.in_reply_to", + }, + "@user:test", + "display_name", + ) + ) + self.assertFalse( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "rel_type": "m.in_reply_to", + }, + "@user:test", + "display_name", + ) + ) From e40ccde3839b1b0f38214bb7d3849da8c91e319a Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 08:54:42 +0100 Subject: [PATCH 03/15] Add newsfragment --- changelog.d/11804.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11804.feature diff --git a/changelog.d/11804.feature b/changelog.d/11804.feature new file mode 100644 index 000000000000..64203935416f --- /dev/null +++ b/changelog.d/11804.feature @@ -0,0 +1 @@ +Implement [MSC3664](https://github.com/matrix-org/matrix-doc/pull/3664). Contributed by Nico. From 8f04118ee69f8c4d73c2f108ea421dfe3bfbed64 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 10:26:58 +0100 Subject: [PATCH 04/15] Fix some namespacing mistakes Signed-off-by: Nicolas Werner --- synapse/push/baserules.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 9fa537231f03..924e648d4d90 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -238,6 +238,25 @@ def make_base_prepend_rules( ], "actions": ["dont_notify"], }, + # Enable notifications for replies without fallback + { + "rule_id": "global/override/im.nheko.msc3664.reply", + "conditions": [ + # Only send notification if the reply is to your message + { + "kind": "im.nheko.msc3664.related_event_match", + "key": "sender", + "rel_type": "m.in_reply_to", + "pattern_type": "user_id", + "_id": "_reply", + }, + ], + "actions": [ + "notify", + {"set_tweak": "sound", "value": "default"}, + {"set_tweak": "highlight"}, + ], + }, # This was changed from underride to override so it's closer in priority # to the content rules where the user name highlight rule lives. This # way a room rule is lower priority than both but a custom override rule @@ -369,11 +388,11 @@ def make_base_prepend_rules( "conditions": [ # Only send notification if the reply is to your message { - "kind": "related_event_match", + "kind": "im.nheko.msc3664.related_event_match", "key": "sender", "rel_type": "m.in_reply_to", "pattern_type": "user_id", - "_id": "_sender", + "_id": "_reply", }, ], "actions": [ From c64565a56ac99220a669f9f1a7fe44e7be521009 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 13:54:56 +0100 Subject: [PATCH 05/15] Remove special case specific to related event matches for pattern Signed-off-by: Nicolas Werner --- synapse/push/push_rule_evaluator.py | 8 ++------ tests/push/test_push_rule_evaluator.py | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 4dd86e287927..34c424182b79 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -187,12 +187,8 @@ def _event_match( pattern = UserID.from_string(user_id).localpart if not pattern: - # msc3664 specifies that key without pattern matches field existence - if condition["kind"] == "im.nheko.msc3664.related_event_match": - return condition["key"] in event_value_cache - else: - logger.warning("event_match condition with no pattern") - return False + logger.warning("event_match condition with no pattern") + return False # XXX: optimisation: cache our pattern regexps if condition["key"] == "content.body": diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index 55f3bcec6d23..f857a5e8ea7b 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -342,7 +342,7 @@ def test_related_event_match(self): "display_name", ) ) - self.assertTrue( + self.assertFalse( evaluator.matches( { "kind": "im.nheko.msc3664.related_event_match", From 2b944772bfc936c46795edf892123e131ee4336d Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Tue, 25 Jan 2022 18:27:47 +0100 Subject: [PATCH 06/15] Don't fail on events we don't have Signed-off-by: Nicolas Werner --- synapse/push/bulk_push_rule_evaluator.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 7941d7ee9c56..36aa2eec1c61 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -206,7 +206,11 @@ async def action_for_event_by_user( related_event_id = event.content.get("m.relates_to", {}).get("event_id") relation_type = event.content.get("m.relates_to", {}).get("rel_type") if related_event_id is not None and relation_type is not None: - related_events[relation_type] = await self.store.get_event(related_event_id) + related_event = await self.store.get_event( + related_event_id, allow_none=True + ) + if related_event is not None: + related_events[relation_type] = related_event reply_event_id = ( event.content.get("m.relates_to", {}) @@ -214,7 +218,9 @@ async def action_for_event_by_user( .get("event_id") ) if reply_event_id is not None: - related_events["m.in_reply_to"] = await self.store.get_event(reply_event_id) + related_event = await self.store.get_event(reply_event_id, allow_none=True) + if related_event is not None: + related_events["m.in_reply_to"] = related_event evaluator = PushRuleEvaluatorForEvent( event, len(room_members), sender_power_level, power_levels, related_events From c9786d555635fd2f9f36560bf2b25880ce8cc199 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Wed, 26 Jan 2022 01:18:09 +0100 Subject: [PATCH 07/15] Default rules should start with a dot Signed-off-by: Nicolas Werner --- synapse/push/baserules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 924e648d4d90..d72d6a16f1e8 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -240,7 +240,7 @@ def make_base_prepend_rules( }, # Enable notifications for replies without fallback { - "rule_id": "global/override/im.nheko.msc3664.reply", + "rule_id": "global/override/.im.nheko.msc3664.reply", "conditions": [ # Only send notification if the reply is to your message { @@ -384,7 +384,7 @@ def make_base_prepend_rules( }, # Enable notifications for replies without fallback { - "rule_id": "global/override/im.nheko.msc3664.reply", + "rule_id": "global/override/.im.nheko.msc3664.reply", "conditions": [ # Only send notification if the reply is to your message { From c57d2073e268c8bfa78c74ffbf22f38f80b53393 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Wed, 19 Oct 2022 22:18:06 +0200 Subject: [PATCH 08/15] Fixup rust pushrules for related events --- rust/src/push/base_rules.rs | 7 +++-- rust/src/push/evaluator.rs | 42 +++++++++++--------------- rust/src/push/mod.rs | 17 ++++++++--- tests/push/test_push_rule_evaluator.py | 34 +++++++++------------ 4 files changed, 50 insertions(+), 50 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index b957b2ac41ac..aebf302810b9 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -24,6 +24,7 @@ use super::KnownCondition; use crate::push::Action; use crate::push::Condition; use crate::push::EventMatchCondition; +use crate::push::RelatedEventMatchCondition; use crate::push::PushRule; use crate::push::SetTweak; use crate::push::TweakValue; @@ -118,11 +119,11 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ rule_id: Cow::Borrowed("global/override/.im.nheko.msc3664.reply"), priority_class: 5, conditions: Cow::Borrowed(&[ - Condition::Known(KnownCondition::RelatedEventMatch(EventMatchCondition { - key: Cow::Borrowed("state_key"), + Condition::Known(KnownCondition::RelatedEventMatch(RelatedEventMatchCondition { + key: Some(Cow::Borrowed("state_key")), pattern: None, pattern_type: Some(Cow::Borrowed("user_id")), - rel_type: Some(Cow::Borrowed("m.in_reply_to")), + rel_type: Cow::Borrowed("m.in_reply_to"), })), ]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 894d857d0c60..af759ffcf4e1 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -22,7 +22,7 @@ use regex::Regex; use super::{ utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, - Action, Condition, EventMatchCondition, FilteredPushRules, KnownCondition, + Action, Condition, EventMatchCondition, RelatedEventMatchCondition, FilteredPushRules, KnownCondition, }; lazy_static! { @@ -254,6 +254,20 @@ impl PushRuleEvaluator { event_match: &RelatedEventMatchCondition, user_id: Option<&str>, ) -> Result { + let event = if let Some(event) = self.related_events_flattened.get(&*event_match.rel_type) { + event + } else { + return Ok(false); + }; + + // if we have no key, accept the event as matching, if it existed without matching any + // fields. + let key = if let Some(key) = &event_match.key { + key + } else { + return Ok(true); + }; + let pattern = if let Some(pattern) = &event_match.pattern { pattern } else if let Some(pattern_type) = &event_match.pattern_type { @@ -275,27 +289,7 @@ impl PushRuleEvaluator { return Ok(false); }; - //let relation_type = if let Some(relation_type) = &event_match.rel_type { - // relation_type - //} else { - // return Ok(false); - //} - - let event = if let Some(event) = self.related_events_flattened.get(&*event_match.rel_type) { - event - } else { - return Ok(false); - }; - - // if we have no key, accept the event as matching, if it existed without matching any - // fields. - let key = if let Some(key) = &event_match.key { - key - } else { - return Ok(true); - } - - let haystack = if let Some(haystack) = event.get(&*event_match.key) { + let haystack = if let Some(haystack) = event.get(&**key) { haystack } else { return Ok(false); @@ -303,7 +297,7 @@ impl PushRuleEvaluator { // For the content.body we match against "words", but for everything // else we match against the entire value. - let match_type = if event_match.key == "content.body" { + let match_type = if key == "content.body" { GlobMatchType::Word } else { GlobMatchType::Whole @@ -342,7 +336,7 @@ fn push_rule_evaluator() { let mut flattened_keys = BTreeMap::new(); flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string()); let evaluator = - PushRuleEvaluator::py_new(flattened_keys, 10, Some(0), BTreeMap::new()).unwrap(); + PushRuleEvaluator::py_new(flattened_keys, 10, Some(0), BTreeMap::new(), BTreeMap::new()).unwrap(); let result = evaluator.run(&FilteredPushRules::default(), None, Some("bob")); assert_eq!(result.len(), 3); diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 3f01b0228d5f..366e33b98968 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -267,6 +267,7 @@ pub enum Condition { #[serde(tag = "kind")] pub enum KnownCondition { EventMatch(EventMatchCondition), + #[serde(rename = "im.nheko.msc3664.related_event_match")] RelatedEventMatch(RelatedEventMatchCondition), ContainsDisplayName, RoomMemberCount { @@ -302,14 +303,14 @@ pub struct EventMatchCondition { /// The body of a [`Condition::RelatedEventMatch`] #[derive(Serialize, Deserialize, Debug, Clone)] -pub struct EventMatchCondition { - pub key: Cow<'static, str>, +pub struct RelatedEventMatchCondition { + #[serde(skip_serializing_if = "Option::is_none")] + pub key: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub pattern: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub pattern_type: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - pub rel_type: Option>, + pub rel_type: Cow<'static, str>, } /// The collection of push rules for a user. @@ -459,6 +460,14 @@ fn test_deserialize_condition() { let _: Condition = serde_json::from_str(json).unwrap(); } +#[test] +fn test_deserialize_unstable_condition() { + let json = r#"{"kind":"im.nheko.msc3664.related_event_match","key":"content.body","pattern":"coffee","rel_type":"m.in_reply_to"}"#; + + let condition: Condition = serde_json::from_str(json).unwrap(); + assert!(matches!(condition, Condition::Known(KnownCondition::RelatedEventMatch(_)))); +} + #[test] fn test_deserialize_custom_condition() { let json = r#"{"kind":"custom_tag"}"#; diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index f81e1694d13d..d4b4ca745605 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -298,7 +298,7 @@ def test_related_event_match(self): { "m.relates_to": { "event_id": "$parent_event_id", - "key": "\ud83d\udc4d\ufe0f", + "key": "😀", "rel_type": "m.annotation", "m.in_reply_to": { "event_id": "$parent_event_id", @@ -306,47 +306,43 @@ def test_related_event_match(self): } }, { - "m.in_reply_to": FrozenEvent( - { + "m.in_reply_to": { "event_id": "$parent_event_id", "type": "m.room.message", "sender": "@other_user:test", - "room_id": "#room:test", - "content": {"msgtype": "m.text", "body": "Original message"}, + "room_id": "!room:test", + "content.msgtype": "m.text", + "content.body": "Original message", }, - RoomVersions.V1, - ), - "m.annotation": FrozenEvent( - { + "m.annotation": { "event_id": "$parent_event_id", "type": "m.room.message", "sender": "@other_user:test", - "room_id": "#room:test", - "content": {"msgtype": "m.text", "body": "Original message"}, + "room_id": "!room:test", + "content.msgtype": "m.text", + "content.body": "Original message", }, - RoomVersions.V1, - ), }, ) - self.assertFalse( + self.assertTrue( evaluator.matches( { "kind": "im.nheko.msc3664.related_event_match", "key": "sender", "rel_type": "m.in_reply_to", - "pattern_type": "user_id", + "pattern": "@other_user:test", }, "@user:test", "display_name", ) ) - self.assertTrue( + self.assertFalse( evaluator.matches( { "kind": "im.nheko.msc3664.related_event_match", "key": "sender", "rel_type": "m.in_reply_to", - "pattern_type": "user_id", + "pattern": "@user:test", }, "@other_user:test", "display_name", @@ -358,7 +354,7 @@ def test_related_event_match(self): "kind": "im.nheko.msc3664.related_event_match", "key": "sender", "rel_type": "m.annotation", - "pattern_type": "user_id", + "pattern": "@other_user:test", }, "@other_user:test", "display_name", @@ -406,7 +402,7 @@ def test_related_event_match_no_related_event(self): "kind": "im.nheko.msc3664.related_event_match", "key": "sender", "rel_type": "m.in_reply_to", - "pattern_type": "user_id", + "pattern": "@other_user:test", }, "@user:test", "display_name", From 1c1fbf1e8c4d11202c681d0ff7446ad450b692d7 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Wed, 19 Oct 2022 23:14:13 +0200 Subject: [PATCH 09/15] Add feature flag for MSC3664 --- rust/src/push/evaluator.rs | 13 +++++- rust/src/push/mod.rs | 20 +++++++-- stubs/synapse/synapse_rust/push.pyi | 6 ++- synapse/config/experimental.py | 3 ++ synapse/push/bulk_push_rule_evaluator.py | 45 +++++++++++++-------- synapse/rest/client/capabilities.py | 5 +++ synapse/storage/databases/main/push_rule.py | 15 +++++-- tests/push/test_push_rule_evaluator.py | 34 +++++++++------- 8 files changed, 100 insertions(+), 41 deletions(-) diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index af759ffcf4e1..7b362fb89484 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -53,6 +53,9 @@ pub struct PushRuleEvaluator { /// The related events, indexed by relation type. Flattened in the same manner as /// `flattened_keys`. related_events_flattened: BTreeMap>, + + /// If msc3664, push rules for related events, is enabled. + related_event_match_enabled: bool, } #[pymethods] @@ -65,6 +68,7 @@ impl PushRuleEvaluator { sender_power_level: Option, notification_power_levels: BTreeMap, related_events_flattened: BTreeMap>, + related_event_match_enabled: bool, ) -> Result { let body = flattened_keys .get("content.body") @@ -78,6 +82,7 @@ impl PushRuleEvaluator { notification_power_levels, sender_power_level, related_events_flattened, + related_event_match_enabled, }) } @@ -248,12 +253,18 @@ impl PushRuleEvaluator { compiled_pattern.is_match(haystack) } - /// Evaluates a `related_event_match` condition. + /// Evaluates a `related_event_match` condition. (MSC3664) fn match_related_event_match( &self, event_match: &RelatedEventMatchCondition, user_id: Option<&str>, ) -> Result { + // First check if related event matching is enabled... + if !self.related_event_match_enabled { + return Ok(false); + } + + // get the related event, fail if there is none. let event = if let Some(event) = self.related_events_flattened.get(&*event_match.rel_type) { event } else { diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 366e33b98968..466182b4679f 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -405,15 +405,17 @@ impl PushRules { pub struct FilteredPushRules { push_rules: PushRules, enabled_map: BTreeMap, + msc3664_enabled: bool, } #[pymethods] impl FilteredPushRules { #[new] - pub fn py_new(push_rules: PushRules, enabled_map: BTreeMap) -> Self { + pub fn py_new(push_rules: PushRules, enabled_map: BTreeMap, msc3664_enabled: bool) -> Self { Self { push_rules, enabled_map, + msc3664_enabled } } @@ -427,8 +429,20 @@ impl FilteredPushRules { impl FilteredPushRules { /// Iterates over all the rules and their enabled state, including base /// rules, in the order they should be executed in. - fn iter(&self) -> impl Iterator { - self.push_rules.iter().map(|r| { + fn iter(&self) -> impl Iterator { + self.push_rules + .iter() + .filter(|rule| { + // Ignore disabled experimental push rules + if !self.msc3664_enabled + && rule.rule_id == "global/override/.im.nheko.msc3664.reply" + { + return false; + } + + true + }) + .map(|r| { let enabled = *self .enabled_map .get(&*r.rule_id) diff --git a/stubs/synapse/synapse_rust/push.pyi b/stubs/synapse/synapse_rust/push.pyi index f2a61df6604b..f3b6d6c9333a 100644 --- a/stubs/synapse/synapse_rust/push.pyi +++ b/stubs/synapse/synapse_rust/push.pyi @@ -25,7 +25,9 @@ class PushRules: def rules(self) -> Collection[PushRule]: ... class FilteredPushRules: - def __init__(self, push_rules: PushRules, enabled_map: Dict[str, bool]): ... + def __init__( + self, push_rules: PushRules, enabled_map: Dict[str, bool], msc3664_enabled: bool + ): ... def rules(self) -> Collection[Tuple[PushRule, bool]]: ... def get_base_rule_ids() -> Collection[str]: ... @@ -37,6 +39,8 @@ class PushRuleEvaluator: room_member_count: int, sender_power_level: Optional[int], notification_power_levels: Mapping[str, int], + related_events_flattened: Mapping[str, Mapping[str, str]], + related_event_match_enabled: bool, ): ... def run( self, diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 4009add01db0..d9bdd66d552d 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -98,6 +98,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # MSC3773: Thread notifications self.msc3773_enabled: bool = experimental.get("msc3773_enabled", False) + # MSC3664: Pushrules to match on related events + self.msc3664_enabled: bool = experimental.get("msc3664_enabled", False) + # MSC3848: Introduce errcodes for specific event sending failures self.msc3848_enabled: bool = experimental.get("msc3848_enabled", False) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 8ce4afa04615..c74cfd92b104 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -106,6 +106,8 @@ def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() self._event_auth_handler = hs.get_event_auth_handler() + self._related_event_match_enabled = self.hs.config.experimental.msc3664_enabled + self.room_push_rule_cache_metrics = register_cache( "cache", "room_push_rule_cache", @@ -225,24 +227,32 @@ async def action_for_event_by_user( ) = await self._get_power_levels_and_sender_level(event, context) related_events: Dict[str, Dict[str, str]] = {} - related_event_id = event.content.get("m.relates_to", {}).get("event_id") - relation_type = event.content.get("m.relates_to", {}).get("rel_type") - if related_event_id is not None and relation_type is not None: - related_event = await self.store.get_event( - related_event_id, allow_none=True + if self._related_event_match_enabled: + related_event_id = event.content.get("m.relates_to", {}).get("event_id") + relation_type = event.content.get("m.relates_to", {}).get("rel_type") + if related_event_id is not None and relation_type is not None: + related_event = await self.store.get_event( + related_event_id, allow_none=True + ) + if related_event is not None: + related_events[relation_type] = _flatten_dict(related_event) + + reply_event_id = ( + event.content.get("m.relates_to", {}) + .get("m.in_reply_to", {}) + .get("event_id") ) - if related_event is not None: - related_events[relation_type] = _flatten_dict(related_event) - - reply_event_id = ( - event.content.get("m.relates_to", {}) - .get("m.in_reply_to", {}) - .get("event_id") - ) - if reply_event_id is not None and (relation_type is not "m.thread" or not event.content.get("m.relates_to", {}).get("is_falling_back", False)): - related_event = await self.store.get_event(reply_event_id, allow_none=True) - if related_event is not None: - related_events["m.in_reply_to"] = _flatten_dict(related_event) + if reply_event_id is not None and ( + relation_type != "m.thread" + or not event.content.get("m.relates_to", {}).get( + "is_falling_back", False + ) + ): + related_event = await self.store.get_event( + reply_event_id, allow_none=True + ) + if related_event is not None: + related_events["m.in_reply_to"] = _flatten_dict(related_event) # Find the event's thread ID. relation = relation_from_event(event) @@ -270,6 +280,7 @@ async def action_for_event_by_user( sender_power_level, notification_levels, related_events, + self._related_event_match_enabled, ) users = rules_by_user.keys() diff --git a/synapse/rest/client/capabilities.py b/synapse/rest/client/capabilities.py index 4237071c61bd..e84dde31b118 100644 --- a/synapse/rest/client/capabilities.py +++ b/synapse/rest/client/capabilities.py @@ -77,6 +77,11 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: "enabled": True, } + if self.config.experimental.msc3664_enabled: + response["capabilities"]["im.nheko.msc3664.related_event_match"] = { + "enabled": self.config.experimental.msc3664_enabled, + } + return HTTPStatus.OK, response diff --git a/synapse/storage/databases/main/push_rule.py b/synapse/storage/databases/main/push_rule.py index 51416b22366a..b6c15f29f8f3 100644 --- a/synapse/storage/databases/main/push_rule.py +++ b/synapse/storage/databases/main/push_rule.py @@ -29,6 +29,7 @@ ) from synapse.api.errors import StoreError +from synapse.config.homeserver import ExperimentalConfig from synapse.replication.slave.storage._slaved_id_tracker import SlavedIdTracker from synapse.storage._base import SQLBaseStore from synapse.storage.database import ( @@ -62,7 +63,9 @@ def _load_rules( - rawrules: List[JsonDict], enabled_map: Dict[str, bool] + rawrules: List[JsonDict], + enabled_map: Dict[str, bool], + experimental_config: ExperimentalConfig, ) -> FilteredPushRules: """Take the DB rows returned from the DB and convert them into a full `FilteredPushRules` object. @@ -80,7 +83,9 @@ def _load_rules( push_rules = PushRules(ruleslist) - filtered_rules = FilteredPushRules(push_rules, enabled_map) + filtered_rules = FilteredPushRules( + push_rules, enabled_map, msc3664_enabled=experimental_config.msc3664_enabled + ) return filtered_rules @@ -160,7 +165,7 @@ async def get_push_rules_for_user(self, user_id: str) -> FilteredPushRules: enabled_map = await self.get_push_rules_enabled_for_user(user_id) - return _load_rules(rows, enabled_map) + return _load_rules(rows, enabled_map, self.hs.config.experimental) async def get_push_rules_enabled_for_user(self, user_id: str) -> Dict[str, bool]: results = await self.db_pool.simple_select_list( @@ -219,7 +224,9 @@ async def bulk_get_push_rules( results: Dict[str, FilteredPushRules] = {} for user_id, rules in raw_rules.items(): - results[user_id] = _load_rules(rules, enabled_map_by_user.get(user_id, {})) + results[user_id] = _load_rules( + rules, enabled_map_by_user.get(user_id, {}), self.hs.config.experimental + ) return results diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index d4b4ca745605..d73e834275d9 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -38,7 +38,9 @@ class PushRuleEvaluatorTestCase(unittest.TestCase): - def _get_evaluator(self, content: JsonDict, related_events=None) -> PushRuleEvaluator: + def _get_evaluator( + self, content: JsonDict, related_events=None + ) -> PushRuleEvaluator: event = FrozenEvent( { "event_id": "$event_id", @@ -59,6 +61,7 @@ def _get_evaluator(self, content: JsonDict, related_events=None) -> PushRuleEval sender_power_level, power_levels.get("notifications", {}), {} if related_events is None else related_events, + True, ) def test_display_name(self) -> None: @@ -307,21 +310,21 @@ def test_related_event_match(self): }, { "m.in_reply_to": { - "event_id": "$parent_event_id", - "type": "m.room.message", - "sender": "@other_user:test", - "room_id": "!room:test", - "content.msgtype": "m.text", - "content.body": "Original message", - }, + "event_id": "$parent_event_id", + "type": "m.room.message", + "sender": "@other_user:test", + "room_id": "!room:test", + "content.msgtype": "m.text", + "content.body": "Original message", + }, "m.annotation": { - "event_id": "$parent_event_id", - "type": "m.room.message", - "sender": "@other_user:test", - "room_id": "!room:test", - "content.msgtype": "m.text", - "content.body": "Original message", - }, + "event_id": "$parent_event_id", + "type": "m.room.message", + "sender": "@other_user:test", + "room_id": "!room:test", + "content.msgtype": "m.text", + "content.body": "Original message", + }, }, ) self.assertTrue( @@ -430,6 +433,7 @@ def test_related_event_match_no_related_event(self): ) ) + class TestBulkPushRuleEvaluator(unittest.HomeserverTestCase): """Tests for the bulk push rule evaluator""" From 954c73a21c377617151f0dcbd4d9b71dfcfce60e Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 20 Oct 2022 01:43:23 +0200 Subject: [PATCH 10/15] Fix default rule --- rust/src/push/base_rules.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index aebf302810b9..e42a2c6dc6fa 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -120,7 +120,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ priority_class: 5, conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::RelatedEventMatch(RelatedEventMatchCondition { - key: Some(Cow::Borrowed("state_key")), + key: Some(Cow::Borrowed("sender")), pattern: None, pattern_type: Some(Cow::Borrowed("user_id")), rel_type: Cow::Borrowed("m.in_reply_to"), From 0826a6482921749175432a1423471e87ff3c6fcb Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 20 Oct 2022 01:58:47 +0200 Subject: [PATCH 11/15] cargo fmt --- rust/src/push/base_rules.rs | 10 +++++----- rust/src/push/evaluator.rs | 13 ++++++++++--- rust/src/push/mod.rs | 27 +++++++++++++++++---------- 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index e42a2c6dc6fa..bbb30dc20d02 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -24,8 +24,8 @@ use super::KnownCondition; use crate::push::Action; use crate::push::Condition; use crate::push::EventMatchCondition; -use crate::push::RelatedEventMatchCondition; use crate::push::PushRule; +use crate::push::RelatedEventMatchCondition; use crate::push::SetTweak; use crate::push::TweakValue; @@ -118,14 +118,14 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ PushRule { rule_id: Cow::Borrowed("global/override/.im.nheko.msc3664.reply"), priority_class: 5, - conditions: Cow::Borrowed(&[ - Condition::Known(KnownCondition::RelatedEventMatch(RelatedEventMatchCondition { + conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::RelatedEventMatch( + RelatedEventMatchCondition { key: Some(Cow::Borrowed("sender")), pattern: None, pattern_type: Some(Cow::Borrowed("user_id")), rel_type: Cow::Borrowed("m.in_reply_to"), - })), - ]), + }, + ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), default: true, default_enabled: true, diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 7b362fb89484..5ddba1dde2d7 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -22,7 +22,8 @@ use regex::Regex; use super::{ utils::{get_glob_matcher, get_localpart_from_id, GlobMatchType}, - Action, Condition, EventMatchCondition, RelatedEventMatchCondition, FilteredPushRules, KnownCondition, + Action, Condition, EventMatchCondition, FilteredPushRules, KnownCondition, + RelatedEventMatchCondition, }; lazy_static! { @@ -346,8 +347,14 @@ impl PushRuleEvaluator { fn push_rule_evaluator() { let mut flattened_keys = BTreeMap::new(); flattened_keys.insert("content.body".to_string(), "foo bar bob hello".to_string()); - let evaluator = - PushRuleEvaluator::py_new(flattened_keys, 10, Some(0), BTreeMap::new(), BTreeMap::new()).unwrap(); + let evaluator = PushRuleEvaluator::py_new( + flattened_keys, + 10, + Some(0), + BTreeMap::new(), + BTreeMap::new(), + ) + .unwrap(); let result = evaluator.run(&FilteredPushRules::default(), None, Some("bob")); assert_eq!(result.len(), 3); diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 466182b4679f..3430bf5cf624 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -411,11 +411,15 @@ pub struct FilteredPushRules { #[pymethods] impl FilteredPushRules { #[new] - pub fn py_new(push_rules: PushRules, enabled_map: BTreeMap, msc3664_enabled: bool) -> Self { + pub fn py_new( + push_rules: PushRules, + enabled_map: BTreeMap, + msc3664_enabled: bool, + ) -> Self { Self { push_rules, enabled_map, - msc3664_enabled + msc3664_enabled, } } @@ -429,7 +433,7 @@ impl FilteredPushRules { impl FilteredPushRules { /// Iterates over all the rules and their enabled state, including base /// rules, in the order they should be executed in. - fn iter(&self) -> impl Iterator { + fn iter(&self) -> impl Iterator { self.push_rules .iter() .filter(|rule| { @@ -443,12 +447,12 @@ impl FilteredPushRules { true }) .map(|r| { - let enabled = *self - .enabled_map - .get(&*r.rule_id) - .unwrap_or(&r.default_enabled); - (r, enabled) - }) + let enabled = *self + .enabled_map + .get(&*r.rule_id) + .unwrap_or(&r.default_enabled); + (r, enabled) + }) } } @@ -479,7 +483,10 @@ fn test_deserialize_unstable_condition() { let json = r#"{"kind":"im.nheko.msc3664.related_event_match","key":"content.body","pattern":"coffee","rel_type":"m.in_reply_to"}"#; let condition: Condition = serde_json::from_str(json).unwrap(); - assert!(matches!(condition, Condition::Known(KnownCondition::RelatedEventMatch(_)))); + assert!(matches!( + condition, + Condition::Known(KnownCondition::RelatedEventMatch(_)) + )); } #[test] From 15305769103a00f60d0f43d9b638d933b51cc2ce Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 20 Oct 2022 02:13:36 +0200 Subject: [PATCH 12/15] Adapt related push rules tests to config options --- rust/src/push/evaluator.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 5ddba1dde2d7..4d62d5ba843b 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -353,6 +353,7 @@ fn push_rule_evaluator() { Some(0), BTreeMap::new(), BTreeMap::new(), + true, ) .unwrap(); From 502a9cf82d51e870c27a890dd35b68990b096bd1 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Mon, 24 Oct 2022 14:00:08 +0000 Subject: [PATCH 13/15] Update rust/src/push/mod.rs Co-authored-by: Erik Johnston --- rust/src/push/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 3430bf5cf624..676f479baae6 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -479,7 +479,7 @@ fn test_deserialize_condition() { } #[test] -fn test_deserialize_unstable_condition() { +fn test_deserialize_unstable_msc3664_condition() { let json = r#"{"kind":"im.nheko.msc3664.related_event_match","key":"content.body","pattern":"coffee","rel_type":"m.in_reply_to"}"#; let condition: Condition = serde_json::from_str(json).unwrap(); From f9b3c398fa5717f448df45211463e85bac8a4a13 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Tue, 25 Oct 2022 04:04:07 +0200 Subject: [PATCH 14/15] Add fallback matching in relation push condition Also move some code to a separate function. We pass a pseudo toplevel key in the flattened dict for simplicity. This field will never exist, right? The field indicates if the relation was a fallback or not. --- rust/src/push/base_rules.rs | 1 + rust/src/push/evaluator.rs | 8 +++ rust/src/push/mod.rs | 2 + synapse/push/bulk_push_rule_evaluator.py | 72 +++++++++++++++--------- tests/push/test_push_rule_evaluator.py | 72 ++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 28 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index bbb30dc20d02..49802fa4eb93 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -124,6 +124,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ pattern: None, pattern_type: Some(Cow::Borrowed("user_id")), rel_type: Cow::Borrowed("m.in_reply_to"), + include_fallbacks: None, }, ))]), actions: Cow::Borrowed(&[Action::Notify, HIGHLIGHT_ACTION, SOUND_ACTION]), diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs index 4d62d5ba843b..cedd42c54d05 100644 --- a/rust/src/push/evaluator.rs +++ b/rust/src/push/evaluator.rs @@ -272,6 +272,14 @@ impl PushRuleEvaluator { return Ok(false); }; + // If we are not matching fallbacks, don't match if our special key indicating this is a + // fallback relation is not present. + if !event_match.include_fallbacks.unwrap_or(false) + && event.contains_key("im.vector.is_falling_back") + { + return Ok(false); + } + // if we have no key, accept the event as matching, if it existed without matching any // fields. let key = if let Some(key) = &event_match.key { diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs index 676f479baae6..d57800aa4afe 100644 --- a/rust/src/push/mod.rs +++ b/rust/src/push/mod.rs @@ -311,6 +311,8 @@ pub struct RelatedEventMatchCondition { #[serde(skip_serializing_if = "Option::is_none")] pub pattern_type: Option>, pub rel_type: Cow<'static, str>, + #[serde(skip_serializing_if = "Option::is_none")] + pub include_fallbacks: Option, } /// The collection of push rules for a user. diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index c74cfd92b104..920505a72e66 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -195,6 +195,48 @@ async def _get_power_levels_and_sender_level( return pl_event.content if pl_event else {}, sender_level + async def _related_events(self, event: EventBase) -> dict[str, dict[str, str]]: + """Fetches the related events for 'event'. Sets the im.vector.is_falling_back key if the event is from a fallback relation + + Returns: + Mapping of relation type to flattened events. + """ + related_events: dict[str, dict[str, str]] = {} + if self._related_event_match_enabled: + related_event_id = event.content.get("m.relates_to", {}).get("event_id") + relation_type = event.content.get("m.relates_to", {}).get("rel_type") + if related_event_id is not None and relation_type is not None: + related_event = await self.store.get_event( + related_event_id, allow_none=True + ) + if related_event is not None: + related_events[relation_type] = _flatten_dict(related_event) + + reply_event_id = ( + event.content.get("m.relates_to", {}) + .get("m.in_reply_to", {}) + .get("event_id") + ) + + # convert replies to pseudo relations + if reply_event_id is not None: + related_event = await self.store.get_event( + reply_event_id, allow_none=True + ) + + if related_event is not None: + related_events["m.in_reply_to"] = _flatten_dict(related_event) + + # indicate that this is from a fallback relation. + if relation_type == "m.thread" and event.content.get( + "m.relates_to", {} + ).get("is_falling_back", False): + related_events["m.in_reply_to"][ + "im.vector.is_falling_back" + ] = "" + + return related_events + @measure_func("action_for_event_by_user") async def action_for_event_by_user( self, event: EventBase, context: EventContext @@ -226,34 +268,6 @@ async def action_for_event_by_user( sender_power_level, ) = await self._get_power_levels_and_sender_level(event, context) - related_events: Dict[str, Dict[str, str]] = {} - if self._related_event_match_enabled: - related_event_id = event.content.get("m.relates_to", {}).get("event_id") - relation_type = event.content.get("m.relates_to", {}).get("rel_type") - if related_event_id is not None and relation_type is not None: - related_event = await self.store.get_event( - related_event_id, allow_none=True - ) - if related_event is not None: - related_events[relation_type] = _flatten_dict(related_event) - - reply_event_id = ( - event.content.get("m.relates_to", {}) - .get("m.in_reply_to", {}) - .get("event_id") - ) - if reply_event_id is not None and ( - relation_type != "m.thread" - or not event.content.get("m.relates_to", {}).get( - "is_falling_back", False - ) - ): - related_event = await self.store.get_event( - reply_event_id, allow_none=True - ) - if related_event is not None: - related_events["m.in_reply_to"] = _flatten_dict(related_event) - # Find the event's thread ID. relation = relation_from_event(event) # If the event does not have a relation, then it cannot have a thread ID. @@ -267,6 +281,8 @@ async def action_for_event_by_user( # the parent is part of a thread. thread_id = await self.store.get_thread_id(relation.parent_id) + related_events = await self._related_events(event) + # It's possible that old room versions have non-integer power levels (floats or # strings). Workaround this by explicitly converting to int. notification_levels = power_levels.get("notifications", {}) diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index d73e834275d9..fe7c145840d6 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -395,6 +395,78 @@ def test_related_event_match(self): ) ) + def test_related_event_match_with_fallback(self): + evaluator = self._get_evaluator( + { + "m.relates_to": { + "event_id": "$parent_event_id", + "key": "😀", + "rel_type": "m.thread", + "is_falling_back": True, + "m.in_reply_to": { + "event_id": "$parent_event_id", + }, + } + }, + { + "m.in_reply_to": { + "event_id": "$parent_event_id", + "type": "m.room.message", + "sender": "@other_user:test", + "room_id": "!room:test", + "content.msgtype": "m.text", + "content.body": "Original message", + "im.vector.is_falling_back": "", + }, + "m.thread": { + "event_id": "$parent_event_id", + "type": "m.room.message", + "sender": "@other_user:test", + "room_id": "!room:test", + "content.msgtype": "m.text", + "content.body": "Original message", + }, + }, + ) + self.assertTrue( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "key": "sender", + "rel_type": "m.in_reply_to", + "pattern": "@other_user:test", + "include_fallbacks": True, + }, + "@user:test", + "display_name", + ) + ) + self.assertFalse( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "key": "sender", + "rel_type": "m.in_reply_to", + "pattern": "@other_user:test", + "include_fallbacks": False, + }, + "@user:test", + "display_name", + ) + ) + self.assertFalse( + evaluator.matches( + { + "kind": "im.nheko.msc3664.related_event_match", + "key": "sender", + "rel_type": "m.in_reply_to", + "pattern": "@other_user:test", + }, + "@user:test", + "display_name", + ) + ) + def test_related_event_match_no_related_event(self): evaluator = self._get_evaluator( {"msgtype": "m.text", "body": "Message without related event"} From c6a57ab7b6c6663ddddd5670158f04f2643039df Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Tue, 25 Oct 2022 12:08:03 +0200 Subject: [PATCH 15/15] Try to fix type error --- synapse/push/bulk_push_rule_evaluator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index bb04541eb2f1..75b7e126cae1 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -219,13 +219,13 @@ async def _get_power_levels_and_sender_level( return pl_event.content if pl_event else {}, sender_level - async def _related_events(self, event: EventBase) -> dict[str, dict[str, str]]: + async def _related_events(self, event: EventBase) -> Dict[str, Dict[str, str]]: """Fetches the related events for 'event'. Sets the im.vector.is_falling_back key if the event is from a fallback relation Returns: Mapping of relation type to flattened events. """ - related_events: dict[str, dict[str, str]] = {} + related_events: Dict[str, Dict[str, str]] = {} if self._related_event_match_enabled: related_event_id = event.content.get("m.relates_to", {}).get("event_id") relation_type = event.content.get("m.relates_to", {}).get("rel_type")