From db6e26bb8c29556b43ca2e8c52510158a6f71204 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 22 Jan 2016 16:03:55 +0000 Subject: [PATCH 1/4] Don't mutate cached values --- synapse/push/bulk_push_rule_evaluator.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 06250d2d9..ec917c239 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -55,11 +55,15 @@ def _get_rules(room_id, user_ids, store): user_enabled_map = rules_enabled_by_user[uid] - for rule in rules_by_user[uid]: + for i, rule in enumerate(rules_by_user[uid]): rule_id = rule['rule_id'] if rule_id in user_enabled_map: - rule['enabled'] = user_enabled_map[rule_id] + if rule.get('enabled', True) != bool(user_enabled_map[rule_id]): + # Rules are cached across users. + rule = dict(rule) + rule['enabled'] = bool(user_enabled_map[rule_id]) + rules_by_user[uid][i] = rule defer.returnValue(rules_by_user) From b4a41aa542203c03bb8a6c93097b94bc5d167265 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 22 Jan 2016 16:56:48 +0000 Subject: [PATCH 2/4] Don't add notifications to the table unless there's actually a 'notify' action --- synapse/push/bulk_push_rule_evaluator.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 06250d2d9..c4cc85eb4 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -135,8 +135,16 @@ class BulkPushRuleEvaluator: evaluator, rule['conditions'], uid, display_name, condition_cache ) if matches: + notify = False + actions = [] + for a in rule['actions']: + if a != 'dont_notify': + actions.append(a) + elif a == 'notify': + notify = True + actions = [x for x in rule['actions'] if x != 'dont_notify'] - if actions: + if actions and notify: actions_by_user[uid] = actions break defer.returnValue(actions_by_user) From 60965bd7e5a27d3952116c431f54786380643d05 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 22 Jan 2016 17:21:15 +0000 Subject: [PATCH 3/4] Revert b4a41aa542203c03bb8a6c93097b94bc5d167265 as it's just broken. --- synapse/push/bulk_push_rule_evaluator.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index c4cc85eb4..06250d2d9 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -135,16 +135,8 @@ class BulkPushRuleEvaluator: evaluator, rule['conditions'], uid, display_name, condition_cache ) if matches: - notify = False - actions = [] - for a in rule['actions']: - if a != 'dont_notify': - actions.append(a) - elif a == 'notify': - notify = True - actions = [x for x in rule['actions'] if x != 'dont_notify'] - if actions and notify: + if actions: actions_by_user[uid] = actions break defer.returnValue(actions_by_user) From 3fe8c5673605074035f132185a143bcaded1a734 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 22 Jan 2016 17:21:58 +0000 Subject: [PATCH 4/4] Better fix for actions with both dont_notify and tweaks --- synapse/push/bulk_push_rule_evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 06250d2d9..5abab80e6 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -136,7 +136,7 @@ class BulkPushRuleEvaluator: ) if matches: actions = [x for x in rule['actions'] if x != 'dont_notify'] - if actions: + if actions and 'notify' in actions: actions_by_user[uid] = actions break defer.returnValue(actions_by_user)