From fa969cfdde72a2d136eba08eb99e00d47ddb5cdf Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Oct 2017 12:39:18 +0100 Subject: [PATCH 01/12] Support for channel notifications Add condition type to check the sender's power level and add a base rule using it for @channel notifications. --- synapse/push/baserules.py | 23 ++++++++++++++++++++++ synapse/push/bulk_push_rule_evaluator.py | 19 +++++++++++++++++- synapse/push/push_rule_evaluator.py | 25 +++++++++++++++++------- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 85effdfa46..354b1f4493 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -1,4 +1,5 @@ # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2017 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -238,6 +239,28 @@ BASE_APPEND_OVERRIDE_RULES = [ } ] }, + { + 'rule_id': 'global/underride/.m.rule.channelnotif', + 'conditions': [ + { + 'kind': 'event_match', + 'key': 'content.body', + 'pattern': '*@channel*', + '_id': '_channelnotif_content', + }, + { + 'kind': 'sender_power_level', + 'is': '>=50', + '_id': '_channelnotif_pl', + }, + ], + 'actions': [ + 'notify', { + 'set_tweak': 'highlight', + 'value': True, + } + ] + } ] diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index b0d64aa6c4..6459eec225 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -19,6 +19,7 @@ from twisted.internet import defer from .push_rule_evaluator import PushRuleEvaluatorForEvent +from synapse.event_auth import get_user_power_level from synapse.api.constants import EventTypes, Membership from synapse.metrics import get_metrics_for from synapse.util.caches import metrics as cache_metrics @@ -59,6 +60,7 @@ class BulkPushRuleEvaluator(object): def __init__(self, hs): self.hs = hs self.store = hs.get_datastore() + self.auth = hs.get_auth() self.room_push_rule_cache_metrics = cache_metrics.register_cache( "cache", @@ -108,6 +110,17 @@ class BulkPushRuleEvaluator(object): self.room_push_rule_cache_metrics, ) + @defer.inlineCallbacks + def _get_sender_power_level(self, event, context): + auth_events_ids = yield self.auth.compute_auth_events( + event, context.prev_state_ids, for_verification=False, + ) + auth_events = yield self.store.get_events(auth_events_ids) + auth_events = { + (e.type, e.state_key): e for e in auth_events.values() + } + defer.returnValue(get_user_power_level(event.sender, auth_events)) + @defer.inlineCallbacks def action_for_event_by_user(self, event, context): """Given an event and context, evaluate the push rules and return @@ -123,7 +136,11 @@ class BulkPushRuleEvaluator(object): event, context ) - evaluator = PushRuleEvaluatorForEvent(event, len(room_members)) + sender_power_level = yield self._get_sender_power_level(event, context) + + evaluator = PushRuleEvaluatorForEvent( + event, len(room_members), sender_power_level + ) condition_cache = {} diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 65f9a63fd8..9cf3f9c632 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2015 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -29,6 +30,12 @@ INEQUALITY_EXPR = re.compile("^([=<>]*)([0-9]*)$") def _room_member_count(ev, condition, room_member_count): + return _test_ineq_condition(condition, room_member_count) + +def _sender_power_level(ev, condition, power_level): + return _test_ineq_condition(condition, power_level) + +def _test_ineq_condition(condition, number): if 'is' not in condition: return False m = INEQUALITY_EXPR.match(condition['is']) @@ -41,19 +48,18 @@ def _room_member_count(ev, condition, room_member_count): rhs = int(rhs) if ineq == '' or ineq == '==': - return room_member_count == rhs + return number == rhs elif ineq == '<': - return room_member_count < rhs + return number < rhs elif ineq == '>': - return room_member_count > rhs + return number > rhs elif ineq == '>=': - return room_member_count >= rhs + return number >= rhs elif ineq == '<=': - return room_member_count <= rhs + return number <= rhs else: return False - def tweaks_for_actions(actions): tweaks = {} for a in actions: @@ -65,9 +71,10 @@ def tweaks_for_actions(actions): class PushRuleEvaluatorForEvent(object): - def __init__(self, event, room_member_count): + def __init__(self, event, room_member_count, sender_power_level): self._event = event self._room_member_count = room_member_count + self._sender_power_level = sender_power_level # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) @@ -81,6 +88,10 @@ class PushRuleEvaluatorForEvent(object): return _room_member_count( self._event, condition, self._room_member_count ) + elif condition['kind'] == 'sender_power_level': + return _sender_power_level( + self._event, condition, self._sender_power_level + ) else: return True From b9b9714fd5c3fa4c209d3ca1f6ddde365373ec98 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Oct 2017 13:02:19 +0100 Subject: [PATCH 02/12] Get rule type right --- synapse/push/baserules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 354b1f4493..3b290a22a1 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -240,7 +240,7 @@ BASE_APPEND_OVERRIDE_RULES = [ ] }, { - 'rule_id': 'global/underride/.m.rule.channelnotif', + 'rule_id': 'global/override/.m.rule.channelnotif', 'conditions': [ { 'kind': 'event_match', From 985ce80375b601d500ae58faa032874a794e942a Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Oct 2017 13:03:44 +0100 Subject: [PATCH 03/12] They're called rooms --- synapse/push/baserules.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 3b290a22a1..71f9ab6b25 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -240,18 +240,18 @@ BASE_APPEND_OVERRIDE_RULES = [ ] }, { - 'rule_id': 'global/override/.m.rule.channelnotif', + 'rule_id': 'global/override/.m.rule.roomnotif', 'conditions': [ { 'kind': 'event_match', 'key': 'content.body', - 'pattern': '*@channel*', - '_id': '_channelnotif_content', + 'pattern': '*@room*', + '_id': '_roomnotif_content', }, { 'kind': 'sender_power_level', 'is': '>=50', - '_id': '_channelnotif_pl', + '_id': '_roomnotif_pl', }, ], 'actions': [ From e433393c4fded875cc38b09b02b453b7a014a9af Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Oct 2017 13:08:02 +0100 Subject: [PATCH 04/12] pep8 --- synapse/push/push_rule_evaluator.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 9cf3f9c632..a91dc0ee08 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -32,9 +32,11 @@ INEQUALITY_EXPR = re.compile("^([=<>]*)([0-9]*)$") def _room_member_count(ev, condition, room_member_count): return _test_ineq_condition(condition, room_member_count) + def _sender_power_level(ev, condition, power_level): return _test_ineq_condition(condition, power_level) + def _test_ineq_condition(condition, number): if 'is' not in condition: return False @@ -60,6 +62,7 @@ def _test_ineq_condition(condition, number): else: return False + def tweaks_for_actions(actions): tweaks = {} for a in actions: From ed80c6b6cc6da27849038a1b83bec7fa1ac54b3e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Oct 2017 13:20:22 +0100 Subject: [PATCH 05/12] Add fastpath optimisation --- synapse/push/bulk_push_rule_evaluator.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 6459eec225..ca3b5af807 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -112,9 +112,15 @@ class BulkPushRuleEvaluator(object): @defer.inlineCallbacks def _get_sender_power_level(self, event, context): - auth_events_ids = yield self.auth.compute_auth_events( - event, context.prev_state_ids, for_verification=False, - ) + pl_event_key = (EventTypes.PowerLevels, "", ) + if pl_event_key in context.prev_state_ids: + # fastpath: if there's a power level event, that's all we need, and + # not having a power level event is an extreme edge case + auth_events_ids = [context.prev_state_ids[pl_event_key]] + else: + auth_events_ids = yield self.auth.compute_auth_events( + event, context.prev_state_ids, for_verification=False, + ) auth_events = yield self.store.get_events(auth_events_ids) auth_events = { (e.type, e.state_key): e for e in auth_events.values() From 269af961e9910e254f5abc979f0bd293687414f3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 5 Oct 2017 13:27:12 +0100 Subject: [PATCH 06/12] Make be faster --- synapse/push/bulk_push_rule_evaluator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index ca3b5af807..df16d5ce9e 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -112,11 +112,11 @@ class BulkPushRuleEvaluator(object): @defer.inlineCallbacks def _get_sender_power_level(self, event, context): - pl_event_key = (EventTypes.PowerLevels, "", ) - if pl_event_key in context.prev_state_ids: + pl_event_id = context.prev_state_ids.get((EventTypes.PowerLevels, "",)) + if pl_event_id: # fastpath: if there's a power level event, that's all we need, and # not having a power level event is an extreme edge case - auth_events_ids = [context.prev_state_ids[pl_event_key]] + auth_events_ids = [pl_event_id] else: auth_events_ids = yield self.auth.compute_auth_events( event, context.prev_state_ids, for_verification=False, From 707374d5dc8ed0ed077ed525262678ebcd583090 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 10 Oct 2017 11:21:41 +0100 Subject: [PATCH 07/12] What year is it!? Who's the president!? --- synapse/push/push_rule_evaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index a91dc0ee08..7cf777f16f 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd -# Copyright 2015 New Vector Ltd +# Copyright 2017 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From a9f9d686316da9efa3e165275fb20066c0367649 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 10 Oct 2017 11:38:31 +0100 Subject: [PATCH 08/12] More optimisation --- synapse/push/bulk_push_rule_evaluator.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index df16d5ce9e..66e8a68a05 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -112,19 +112,22 @@ class BulkPushRuleEvaluator(object): @defer.inlineCallbacks def _get_sender_power_level(self, event, context): - pl_event_id = context.prev_state_ids.get((EventTypes.PowerLevels, "",)) + pl_event_key = (EventTypes.PowerLevels, "", ) + pl_event_id = context.prev_state_ids.get(pl_event_key) if pl_event_id: # fastpath: if there's a power level event, that's all we need, and # not having a power level event is an extreme edge case - auth_events_ids = [pl_event_id] + pl_event = yield self.store.get_event(pl_event_id) + auth_events = { pl_event_key: pl_event } else: auth_events_ids = yield self.auth.compute_auth_events( event, context.prev_state_ids, for_verification=False, ) - auth_events = yield self.store.get_events(auth_events_ids) - auth_events = { - (e.type, e.state_key): e for e in auth_events.values() - } + auth_events = yield self.store.get_events(auth_events_ids) + auth_events = { + (e.type, e.state_key): e for e in auth_events.itervalues() + } + defer.returnValue(get_user_power_level(event.sender, auth_events)) @defer.inlineCallbacks From c9f034b4acb0a48915d1680e310b692816eed713 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 10 Oct 2017 11:47:10 +0100 Subject: [PATCH 09/12] There was already a constant for this also update copyright --- synapse/push/bulk_push_rule_evaluator.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 66e8a68a05..adc99bd4f6 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015 OpenMarket Ltd +# Copyright 2017 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -25,6 +26,7 @@ from synapse.metrics import get_metrics_for from synapse.util.caches import metrics as cache_metrics from synapse.util.caches.descriptors import cached from synapse.util.async import Linearizer +from synapse.state import POWER_KEY from collections import namedtuple @@ -112,13 +114,12 @@ class BulkPushRuleEvaluator(object): @defer.inlineCallbacks def _get_sender_power_level(self, event, context): - pl_event_key = (EventTypes.PowerLevels, "", ) - pl_event_id = context.prev_state_ids.get(pl_event_key) + pl_event_id = context.prev_state_ids.get(POWER_KEY) if pl_event_id: # fastpath: if there's a power level event, that's all we need, and # not having a power level event is an extreme edge case pl_event = yield self.store.get_event(pl_event_id) - auth_events = { pl_event_key: pl_event } + auth_events = { POWER_KEY: pl_event } else: auth_events_ids = yield self.auth.compute_auth_events( event, context.prev_state_ids, for_verification=False, From 0f1eb3e914a1e47e441bd8bfb7d523882646fb6e Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 10 Oct 2017 15:23:00 +0100 Subject: [PATCH 10/12] Use notification levels in power_levels Rather than making the condition directly require a specific power level. This way the level require to notify a room can be configured per room. --- synapse/push/baserules.py | 4 ++-- synapse/push/bulk_push_rule_evaluator.py | 10 ++++++---- synapse/push/push_rule_evaluator.py | 20 ++++++++++++++------ 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 71f9ab6b25..9dce99ebec 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -249,8 +249,8 @@ BASE_APPEND_OVERRIDE_RULES = [ '_id': '_roomnotif_content', }, { - 'kind': 'sender_power_level', - 'is': '>=50', + 'kind': 'sender_notification_permission', + 'key': 'room', '_id': '_roomnotif_pl', }, ], diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index adc99bd4f6..db07a97a94 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -113,7 +113,7 @@ class BulkPushRuleEvaluator(object): ) @defer.inlineCallbacks - def _get_sender_power_level(self, event, context): + def _get_power_levels_and_sender_level(self, event, context): pl_event_id = context.prev_state_ids.get(POWER_KEY) if pl_event_id: # fastpath: if there's a power level event, that's all we need, and @@ -129,7 +129,9 @@ class BulkPushRuleEvaluator(object): (e.type, e.state_key): e for e in auth_events.itervalues() } - defer.returnValue(get_user_power_level(event.sender, auth_events)) + sender_level = get_user_power_level(event.sender, auth_events) + + defer.returnValue((auth_events[POWER_KEY].content, sender_level)) @defer.inlineCallbacks def action_for_event_by_user(self, event, context): @@ -146,10 +148,10 @@ class BulkPushRuleEvaluator(object): event, context ) - sender_power_level = yield self._get_sender_power_level(event, context) + (power_levels, sender_power_level) = yield self._get_power_levels_and_sender_level(event, context) evaluator = PushRuleEvaluatorForEvent( - event, len(room_members), sender_power_level + event, len(room_members), sender_power_level, power_levels, ) condition_cache = {} diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 7cf777f16f..5011bef4f1 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -33,8 +33,15 @@ def _room_member_count(ev, condition, room_member_count): return _test_ineq_condition(condition, room_member_count) -def _sender_power_level(ev, condition, power_level): - return _test_ineq_condition(condition, power_level) +def _sender_notification_permission(ev, condition, sender_power_level, power_levels): + notif_level_key = condition.get('key') + if notif_level_key is None: + return False + + notif_levels = power_levels.get('notifications', {}) + room_notif_level = notif_levels.get(notif_level_key, 50) + + return sender_power_level >= room_notif_level; def _test_ineq_condition(condition, number): @@ -74,10 +81,11 @@ def tweaks_for_actions(actions): class PushRuleEvaluatorForEvent(object): - def __init__(self, event, room_member_count, sender_power_level): + def __init__(self, event, room_member_count, sender_power_level, power_levels): self._event = event self._room_member_count = room_member_count self._sender_power_level = sender_power_level + self._power_levels = power_levels # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) @@ -91,9 +99,9 @@ class PushRuleEvaluatorForEvent(object): return _room_member_count( self._event, condition, self._room_member_count ) - elif condition['kind'] == 'sender_power_level': - return _sender_power_level( - self._event, condition, self._sender_power_level + elif condition['kind'] == 'sender_notification_permission': + return _sender_notification_permission( + self._event, condition, self._sender_power_level, self._power_levels, ) else: return True From ab1bc9bf5fcc8875c61e4ec7357d6e43abc76a55 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 10 Oct 2017 15:34:05 +0100 Subject: [PATCH 11/12] Don't KeyError if no power_levels event --- synapse/push/bulk_push_rule_evaluator.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index db07a97a94..05c1c5165f 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -131,7 +131,9 @@ class BulkPushRuleEvaluator(object): sender_level = get_user_power_level(event.sender, auth_events) - defer.returnValue((auth_events[POWER_KEY].content, sender_level)) + pl_event = auth_events.get(POWER_KEY) + + defer.returnValue((pl_event.content if pl_event else {}, sender_level)) @defer.inlineCallbacks def action_for_event_by_user(self, event, context): From 81a5e0073cbca1ed469fdae94150de3059e5c3e3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 10 Oct 2017 15:53:34 +0100 Subject: [PATCH 12/12] pep8 --- synapse/push/bulk_push_rule_evaluator.py | 6 ++++-- synapse/push/push_rule_evaluator.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index 05c1c5165f..425a017bdf 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -119,7 +119,7 @@ class BulkPushRuleEvaluator(object): # fastpath: if there's a power level event, that's all we need, and # not having a power level event is an extreme edge case pl_event = yield self.store.get_event(pl_event_id) - auth_events = { POWER_KEY: pl_event } + auth_events = {POWER_KEY: pl_event} else: auth_events_ids = yield self.auth.compute_auth_events( event, context.prev_state_ids, for_verification=False, @@ -150,7 +150,9 @@ class BulkPushRuleEvaluator(object): event, context ) - (power_levels, sender_power_level) = yield self._get_power_levels_and_sender_level(event, context) + (power_levels, sender_power_level) = ( + yield self._get_power_levels_and_sender_level(event, context) + ) evaluator = PushRuleEvaluatorForEvent( event, len(room_members), sender_power_level, power_levels, diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 5011bef4f1..3601f2d365 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -41,7 +41,7 @@ def _sender_notification_permission(ev, condition, sender_power_level, power_lev notif_levels = power_levels.get('notifications', {}) room_notif_level = notif_levels.get(notif_level_key, 50) - return sender_power_level >= room_notif_level; + return sender_power_level >= room_notif_level def _test_ineq_condition(condition, number):