From 4be2958944c3db13973bec4c35988def3de8b0a7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 9 May 2022 14:26:22 -0400 Subject: [PATCH 1/5] Move _condition_checker into PushRuleEvaluator. --- synapse/push/bulk_push_rule_evaluator.py | 30 ++---------------------- synapse/push/push_rule_evaluator.py | 25 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index b07cf2eee705..456b0cfd254c 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -247,8 +247,8 @@ async def action_for_event_by_user( if "enabled" in rule and not rule["enabled"]: continue - matches = _condition_checker( - evaluator, rule["conditions"], uid, display_name, condition_cache + matches = evaluator.check_conditions( + rule["conditions"], uid, display_name, condition_cache ) if matches: actions = [x for x in rule["actions"] if x != "dont_notify"] @@ -267,32 +267,6 @@ async def action_for_event_by_user( ) -def _condition_checker( - evaluator: PushRuleEvaluatorForEvent, - conditions: List[dict], - uid: str, - display_name: Optional[str], - cache: Dict[str, bool], -) -> bool: - for cond in conditions: - _cache_key = cond.get("_cache_key", None) - if _cache_key: - res = cache.get(_cache_key, None) - if res is False: - return False - elif res is True: - continue - - res = evaluator.matches(cond, uid, display_name) - if _cache_key: - cache[_cache_key] = bool(res) - - if not res: - return False - - return True - - MemberMap = Dict[str, Optional[EventIdMembership]] Rule = Dict[str, dict] RulesByUser = Dict[str, List[Rule]] diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index f617c759e6cf..c5ca498071bf 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -129,6 +129,31 @@ def __init__( # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) + def check_conditions( + self, + conditions: List[dict], + uid: str, + display_name: Optional[str], + cache: Dict[str, bool], + ) -> bool: + for cond in conditions: + _cache_key = cond.get("_cache_key", None) + if _cache_key: + res = cache.get(_cache_key, None) + if res is False: + return False + elif res is True: + continue + + res = self.matches(cond, uid, display_name) + if _cache_key: + cache[_cache_key] = bool(res) + + if not res: + return False + + return True + def matches( self, condition: Dict[str, Any], user_id: str, display_name: Optional[str] ) -> bool: From f899bf3bfec797a1269ebecb72f12ad334468bca Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 9 May 2022 14:28:11 -0400 Subject: [PATCH 2/5] Move the condition cache into PushRuleEvaluator. --- synapse/push/bulk_push_rule_evaluator.py | 4 +--- synapse/push/push_rule_evaluator.py | 13 ++++++------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 456b0cfd254c..85ddb56c6eb4 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -208,8 +208,6 @@ async def action_for_event_by_user( event, len(room_members), sender_power_level, power_levels ) - condition_cache: Dict[str, bool] = {} - # If the event is not a state event check if any users ignore the sender. if not event.is_state(): ignorers = await self.store.ignored_by(event.sender) @@ -248,7 +246,7 @@ async def action_for_event_by_user( continue matches = evaluator.check_conditions( - rule["conditions"], uid, display_name, condition_cache + rule["conditions"], uid, display_name ) if matches: actions = [x for x in rule["actions"] if x != "dont_notify"] diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index c5ca498071bf..f4b5460b7bfb 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -129,17 +129,16 @@ def __init__( # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) + # Maps cache keys to final values. + self._condition_cache: Dict[str, bool] = {} + def check_conditions( - self, - conditions: List[dict], - uid: str, - display_name: Optional[str], - cache: Dict[str, bool], + self, conditions: List[dict], uid: str, display_name: Optional[str] ) -> bool: for cond in conditions: _cache_key = cond.get("_cache_key", None) if _cache_key: - res = cache.get(_cache_key, None) + res = self._condition_cache.get(_cache_key, None) if res is False: return False elif res is True: @@ -147,7 +146,7 @@ def check_conditions( res = self.matches(cond, uid, display_name) if _cache_key: - cache[_cache_key] = bool(res) + self._condition_cache[_cache_key] = bool(res) if not res: return False From c2755bbf1145cd99cf502c2e3946e2603d17e72a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 9 May 2022 14:34:11 -0400 Subject: [PATCH 3/5] Add some docstrings. --- synapse/push/push_rule_evaluator.py | 41 +++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index f4b5460b7bfb..5bf46accf7a3 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -135,6 +135,17 @@ def __init__( def check_conditions( self, conditions: List[dict], uid: str, display_name: Optional[str] ) -> bool: + """ + Returns true if a user's conditions/user ID/display name match the event. + + Args: + conditions: The user's conditions to match. + uid: The user's MXID. + display_name: The display name. + + Returns: + True if all conditions match the event, False otherwise. + """ for cond in conditions: _cache_key = cond.get("_cache_key", None) if _cache_key: @@ -156,6 +167,17 @@ def check_conditions( def matches( self, condition: Dict[str, Any], user_id: str, display_name: Optional[str] ) -> bool: + """ + Returns true if a user's condition/user ID/display name match the event. + + Args: + condition: The user's condition to match. + uid: The user's MXID. + display_name: The display name, or None if there is not one. + + Returns: + True if the condition matches the event, False otherwise. + """ if condition["kind"] == "event_match": return self._event_match(condition, user_id) elif condition["kind"] == "contains_display_name": @@ -170,6 +192,16 @@ def matches( return True def _event_match(self, condition: dict, user_id: str) -> bool: + """ + Check an "event_match" push rule condition. + + Args: + condition: The "event_match" push rule condition to match. + user_id: The user's MXID. + + Returns: + True if the condition matches the event, False otherwise. + """ pattern = condition.get("pattern", None) if not pattern: @@ -198,6 +230,15 @@ def _event_match(self, condition: dict, user_id: str) -> bool: return _glob_matches(pattern, haystack) def _contains_display_name(self, display_name: Optional[str]) -> bool: + """ + Check an "event_match" push rule condition. + + Args: + display_name: The display name, or None if there is not one. + + Returns: + True if the display name is found in the event body, False otherwise. + """ if not display_name: return False From f0806509250ec3de9719303e4f993473d538951c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 9 May 2022 14:41:41 -0400 Subject: [PATCH 4/5] Inline a method which is only used once. --- synapse/push/push_rule_evaluator.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 5bf46accf7a3..54db6b5612a3 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -223,7 +223,7 @@ def _event_match(self, condition: dict, user_id: str) -> bool: return _glob_matches(pattern, body, word_boundary=True) else: - haystack = self._get_value(condition["key"]) + haystack = self._value_cache.get(condition["key"], None) if haystack is None: return False @@ -256,9 +256,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( From cf31140764095f0db1b102318b62886e04be5afc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 9 May 2022 14:54:29 -0400 Subject: [PATCH 5/5] Newsfragment --- changelog.d/12677.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12677.misc diff --git a/changelog.d/12677.misc b/changelog.d/12677.misc new file mode 100644 index 000000000000..eed12e69e9ba --- /dev/null +++ b/changelog.d/12677.misc @@ -0,0 +1 @@ +Refactor functions to on `PushRuleEvaluatorForEvent`.