From 9c1f853d58440d1f924fa55bc242b248c410dd7c Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Wed, 13 Jan 2016 13:08:59 +0000 Subject: Rename 'user_name' to 'user_id' in push to make it consistent with the rest of the code --- synapse/push/__init__.py | 36 ++++++++++++++++++------------------ synapse/push/baserules.py | 12 ++++++------ synapse/push/httppusher.py | 6 +++--- synapse/push/push_rule_evaluator.py | 24 ++++++++++++------------ synapse/push/pusherpool.py | 30 +++++++++++++++--------------- 5 files changed, 54 insertions(+), 54 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/__init__.py b/synapse/push/__init__.py index c5ddfb564c..a5dc84160c 100644 --- a/synapse/push/__init__.py +++ b/synapse/push/__init__.py @@ -35,7 +35,7 @@ class Pusher(object): MAX_BACKOFF = 60 * 60 * 1000 GIVE_UP_AFTER = 24 * 60 * 60 * 1000 - def __init__(self, _hs, profile_tag, user_name, app_id, + def __init__(self, _hs, profile_tag, user_id, app_id, app_display_name, device_display_name, pushkey, pushkey_ts, data, last_token, last_success, failing_since): self.hs = _hs @@ -43,7 +43,7 @@ class Pusher(object): self.store = self.hs.get_datastore() self.clock = self.hs.get_clock() self.profile_tag = profile_tag - self.user_name = user_name + self.user_id = user_id self.app_id = app_id self.app_display_name = app_display_name self.device_display_name = device_display_name @@ -92,15 +92,15 @@ class Pusher(object): # we fail to dispatch the push) config = PaginationConfig(from_token=None, limit='1') chunk = yield self.evStreamHandler.get_stream( - self.user_name, config, timeout=0, affect_presence=False, + self.user_id, config, timeout=0, affect_presence=False, only_room_events=True ) self.last_token = chunk['end'] self.store.update_pusher_last_token( - self.app_id, self.pushkey, self.user_name, self.last_token + self.app_id, self.pushkey, self.user_id, self.last_token ) logger.info("Pusher %s for user %s starting from token %s", - self.pushkey, self.user_name, self.last_token) + self.pushkey, self.user_id, self.last_token) wait = 0 while self.alive: @@ -125,7 +125,7 @@ class Pusher(object): config = PaginationConfig(from_token=from_tok, limit='1') timeout = (300 + random.randint(-60, 60)) * 1000 chunk = yield self.evStreamHandler.get_stream( - self.user_name, config, timeout=timeout, affect_presence=False, + self.user_id, config, timeout=timeout, affect_presence=False, only_room_events=True ) @@ -142,7 +142,7 @@ class Pusher(object): yield self.store.update_pusher_last_token( self.app_id, self.pushkey, - self.user_name, + self.user_id, self.last_token ) return @@ -153,8 +153,8 @@ class Pusher(object): processed = False rule_evaluator = yield \ - push_rule_evaluator.evaluator_for_user_name_and_profile_tag( - self.user_name, self.profile_tag, single_event['room_id'], self.store + push_rule_evaluator.evaluator_for_user_id_and_profile_tag( + self.user_id, self.profile_tag, single_event['room_id'], self.store ) actions = yield rule_evaluator.actions_for_event(single_event) @@ -179,7 +179,7 @@ class Pusher(object): pk ) yield self.hs.get_pusherpool().remove_pusher( - self.app_id, pk, self.user_name + self.app_id, pk, self.user_id ) else: processed = True @@ -193,7 +193,7 @@ class Pusher(object): yield self.store.update_pusher_last_token_and_success( self.app_id, self.pushkey, - self.user_name, + self.user_id, self.last_token, self.clock.time_msec() ) @@ -202,7 +202,7 @@ class Pusher(object): yield self.store.update_pusher_failing_since( self.app_id, self.pushkey, - self.user_name, + self.user_id, self.failing_since) else: if not self.failing_since: @@ -210,7 +210,7 @@ class Pusher(object): yield self.store.update_pusher_failing_since( self.app_id, self.pushkey, - self.user_name, + self.user_id, self.failing_since ) @@ -222,13 +222,13 @@ class Pusher(object): # of old notifications. logger.warn("Giving up on a notification to user %s, " "pushkey %s", - self.user_name, self.pushkey) + self.user_id, self.pushkey) self.backoff_delay = Pusher.INITIAL_BACKOFF self.last_token = chunk['end'] yield self.store.update_pusher_last_token( self.app_id, self.pushkey, - self.user_name, + self.user_id, self.last_token ) @@ -236,14 +236,14 @@ class Pusher(object): yield self.store.update_pusher_failing_since( self.app_id, self.pushkey, - self.user_name, + self.user_id, self.failing_since ) else: logger.warn("Failed to dispatch push for user %s " "(failing for %dms)." "Trying again in %dms", - self.user_name, + self.user_id, self.clock.time_msec() - self.failing_since, self.backoff_delay) yield synapse.util.async.sleep(self.backoff_delay / 1000.0) @@ -280,7 +280,7 @@ class Pusher(object): if last_active > self.last_last_active_time: self.last_last_active_time = last_active if self.has_unread: - logger.info("Resetting badge count for %s", self.user_name) + logger.info("Resetting badge count for %s", self.user_id) self.reset_badge_count() self.has_unread = False diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 57de0e34b4..8bac7fd6af 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -15,27 +15,27 @@ from synapse.push.rulekinds import PRIORITY_CLASS_MAP, PRIORITY_CLASS_INVERSE_MAP -def list_with_base_rules(rawrules, user_name): +def list_with_base_rules(rawrules, user_id): ruleslist = [] # shove the server default rules for each kind onto the end of each current_prio_class = PRIORITY_CLASS_INVERSE_MAP.keys()[-1] ruleslist.extend(make_base_prepend_rules( - user_name, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] + user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) for r in rawrules: if r['priority_class'] < current_prio_class: while r['priority_class'] < current_prio_class: ruleslist.extend(make_base_append_rules( - user_name, + user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) current_prio_class -= 1 if current_prio_class > 0: ruleslist.extend(make_base_prepend_rules( - user_name, + user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) @@ -43,13 +43,13 @@ def list_with_base_rules(rawrules, user_name): while current_prio_class > 0: ruleslist.extend(make_base_append_rules( - user_name, + user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) current_prio_class -= 1 if current_prio_class > 0: ruleslist.extend(make_base_prepend_rules( - user_name, + user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 7866db6a24..28f1fab0e4 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -23,13 +23,13 @@ logger = logging.getLogger(__name__) class HttpPusher(Pusher): - def __init__(self, _hs, profile_tag, user_name, app_id, + def __init__(self, _hs, profile_tag, user_id, app_id, app_display_name, device_display_name, pushkey, pushkey_ts, data, last_token, last_success, failing_since): super(HttpPusher, self).__init__( _hs, profile_tag, - user_name, + user_id, app_id, app_display_name, device_display_name, @@ -87,7 +87,7 @@ class HttpPusher(Pusher): } if event['type'] == 'm.room.member': d['notification']['membership'] = event['content']['membership'] - d['notification']['user_is_target'] = event['state_key'] == self.user_name + d['notification']['user_is_target'] = event['state_key'] == self.user_id if 'content' in event: d['notification']['content'] = event['content'] diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 705ab8c967..b0283743a2 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -27,17 +27,17 @@ logger = logging.getLogger(__name__) @defer.inlineCallbacks -def evaluator_for_user_name_and_profile_tag(user_name, profile_tag, room_id, store): - rawrules = yield store.get_push_rules_for_user(user_name) - enabled_map = yield store.get_push_rules_enabled_for_user(user_name) +def evaluator_for_user_id_and_profile_tag(user_id, profile_tag, room_id, store): + rawrules = yield store.get_push_rules_for_user(user_id) + enabled_map = yield store.get_push_rules_enabled_for_user(user_id) our_member_event = yield store.get_current_state( room_id=room_id, event_type='m.room.member', - state_key=user_name, + state_key=user_id, ) defer.returnValue(PushRuleEvaluator( - user_name, profile_tag, rawrules, enabled_map, + user_id, profile_tag, rawrules, enabled_map, room_id, our_member_event, store )) @@ -46,9 +46,9 @@ class PushRuleEvaluator: DEFAULT_ACTIONS = [] INEQUALITY_EXPR = re.compile("^([=<>]*)([0-9]*)$") - def __init__(self, user_name, profile_tag, raw_rules, enabled_map, room_id, + def __init__(self, user_id, profile_tag, raw_rules, enabled_map, room_id, our_member_event, store): - self.user_name = user_name + self.user_id = user_id self.profile_tag = profile_tag self.room_id = room_id self.our_member_event = our_member_event @@ -61,7 +61,7 @@ class PushRuleEvaluator: rule['actions'] = json.loads(raw_rule['actions']) rules.append(rule) - user = UserID.from_string(self.user_name) + user = UserID.from_string(self.user_id) self.rules = baserules.list_with_base_rules(rules, user) self.enabled_map = enabled_map @@ -83,7 +83,7 @@ class PushRuleEvaluator: has configured both globally and per-room when we have the ability to do such things. """ - if ev['user_id'] == self.user_name: + if ev['user_id'] == self.user_id: # let's assume you probably know about messages you sent yourself defer.returnValue([]) @@ -124,13 +124,13 @@ class PushRuleEvaluator: if len(actions) == 0: logger.warn( "Ignoring rule id %s with no actions for user %s", - r['rule_id'], self.user_name + r['rule_id'], self.user_id ) continue if matches: logger.info( "%s matches for user %s, event %s", - r['rule_id'], self.user_name, ev['event_id'] + r['rule_id'], self.user_id, ev['event_id'] ) # filter out dont_notify as we treat an empty actions list @@ -141,7 +141,7 @@ class PushRuleEvaluator: logger.info( "No rules match for user %s, event %s", - self.user_name, ev['event_id'] + self.user_id, ev['event_id'] ) defer.returnValue(PushRuleEvaluator.DEFAULT_ACTIONS) diff --git a/synapse/push/pusherpool.py b/synapse/push/pusherpool.py index 4208e5c76c..12c4af14bd 100644 --- a/synapse/push/pusherpool.py +++ b/synapse/push/pusherpool.py @@ -38,12 +38,12 @@ class PusherPool: @defer.inlineCallbacks def user_presence_changed(self, user, state): - user_name = user.to_string() + user_id = user.to_string() # until we have read receipts, pushers use this to reset a user's # badge counters to zero for p in self.pushers.values(): - if p.user_name == user_name: + if p.user_id == user_id: yield p.presence_changed(state) @defer.inlineCallbacks @@ -52,14 +52,14 @@ class PusherPool: self._start_pushers(pushers) @defer.inlineCallbacks - def add_pusher(self, user_name, access_token, profile_tag, kind, app_id, + def add_pusher(self, user_id, access_token, profile_tag, kind, app_id, app_display_name, device_display_name, pushkey, lang, data): # we try to create the pusher just to validate the config: it # will then get pulled out of the database, # recreated, added and started: this means we have only one # code path adding pushers. self._create_pusher({ - "user_name": user_name, + "user_name": user_id, "kind": kind, "profile_tag": profile_tag, "app_id": app_id, @@ -74,7 +74,7 @@ class PusherPool: "failing_since": None }) yield self._add_pusher_to_store( - user_name, access_token, profile_tag, kind, app_id, + user_id, access_token, profile_tag, kind, app_id, app_display_name, device_display_name, pushkey, lang, data ) @@ -109,11 +109,11 @@ class PusherPool: self.remove_pusher(p['app_id'], p['pushkey'], p['user_name']) @defer.inlineCallbacks - def _add_pusher_to_store(self, user_name, access_token, profile_tag, kind, + def _add_pusher_to_store(self, user_id, access_token, profile_tag, kind, app_id, app_display_name, device_display_name, pushkey, lang, data): yield self.store.add_pusher( - user_name=user_name, + user_id=user_id, access_token=access_token, profile_tag=profile_tag, kind=kind, @@ -125,14 +125,14 @@ class PusherPool: lang=lang, data=data, ) - self._refresh_pusher(app_id, pushkey, user_name) + self._refresh_pusher(app_id, pushkey, user_id) def _create_pusher(self, pusherdict): if pusherdict['kind'] == 'http': return HttpPusher( self.hs, profile_tag=pusherdict['profile_tag'], - user_name=pusherdict['user_name'], + user_id=pusherdict['user_name'], app_id=pusherdict['app_id'], app_display_name=pusherdict['app_display_name'], device_display_name=pusherdict['device_display_name'], @@ -150,14 +150,14 @@ class PusherPool: ) @defer.inlineCallbacks - def _refresh_pusher(self, app_id, pushkey, user_name): + def _refresh_pusher(self, app_id, pushkey, user_id): resultlist = yield self.store.get_pushers_by_app_id_and_pushkey( app_id, pushkey ) p = None for r in resultlist: - if r['user_name'] == user_name: + if r['user_name'] == user_id: p = r if p: @@ -186,12 +186,12 @@ class PusherPool: logger.info("Started pushers") @defer.inlineCallbacks - def remove_pusher(self, app_id, pushkey, user_name): - fullid = "%s:%s:%s" % (app_id, pushkey, user_name) + def remove_pusher(self, app_id, pushkey, user_id): + fullid = "%s:%s:%s" % (app_id, pushkey, user_id) if fullid in self.pushers: logger.info("Stopping pusher %s", fullid) self.pushers[fullid].stop() del self.pushers[fullid] - yield self.store.delete_pusher_by_app_id_pushkey_user_name( - app_id, pushkey, user_name + yield self.store.delete_pusher_by_app_id_pushkey_user_id( + app_id, pushkey, user_id ) -- cgit 1.4.1 From fcb05b4c8269ac7cb2e153e0a68c040a965d8279 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 13 Jan 2016 17:37:09 +0000 Subject: Temporarily disable notification branch --- synapse/handlers/federation.py | 13 +++++++------ synapse/handlers/sync.py | 5 ++++- synapse/push/action_generator.py | 3 +++ 3 files changed, 14 insertions(+), 7 deletions(-) (limited to 'synapse/push') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 5dee24e21d..2f6359c768 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -36,7 +36,7 @@ from synapse.events.utils import prune_event from synapse.util.retryutils import NotRetryingDestination -from synapse.push.action_generator import ActionGenerator +# from synapse.push.action_generator import ActionGenerator from twisted.internet import defer @@ -244,11 +244,12 @@ class FederationHandler(BaseHandler): user = UserID.from_string(event.state_key) yield user_joined_room(self.distributor, user, event.room_id) - if not backfilled and not event.internal_metadata.is_outlier(): - action_generator = ActionGenerator(self.store) - yield action_generator.handle_push_actions_for_event( - event, self - ) + # Temporarily disable notifications due to performance concerns. + # if not backfilled and not event.internal_metadata.is_outlier(): + # action_generator = ActionGenerator(self.store) + # yield action_generator.handle_push_actions_for_event( + # event, self + # ) @defer.inlineCallbacks def _filter_events_for_server(self, server_name, room_id, events): diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 33c1a4512c..69707c47ed 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -840,7 +840,10 @@ class SyncHandler(BaseHandler): return False @defer.inlineCallbacks - def unread_notifs_for_room_id(self, room_id, sync_config, ephemeral_by_room): + def unreadnotifs_for_room_id(self, room_id, sync_config, ephemeral_by_room): + # Temporarily disable notifications due to performance concerns. + return + last_unread_event_id = self.last_read_event_id_for_room_and_user( room_id, sync_config.user.to_string(), ephemeral_by_room ) diff --git a/synapse/push/action_generator.py b/synapse/push/action_generator.py index 4cf94f6c61..73467f3adc 100644 --- a/synapse/push/action_generator.py +++ b/synapse/push/action_generator.py @@ -36,6 +36,9 @@ class ActionGenerator: @defer.inlineCallbacks def handle_push_actions_for_event(self, event, handler): + # Temporarily disable notifications due to performance concerns. + return + if event.type == EventTypes.Redaction and event.redacts is not None: yield self.store.remove_push_actions_for_event_id( event.room_id, event.redacts -- cgit 1.4.1 From f59b56450797746230046137b2e2008cb66cb604 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 14:09:47 +0000 Subject: Make notifications go quicker --- synapse/push/bulk_push_rule_evaluator.py | 116 +++++++++------- synapse/push/push_rule_evaluator.py | 226 ++++++++++++++++++++----------- synapse/storage/push_rule.py | 23 +++- synapse/storage/registration.py | 26 +++- 4 files changed, 260 insertions(+), 131 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index ce244fa959..b9f78fd598 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -14,16 +14,16 @@ # limitations under the License. import logging -import simplejson as json +import ujson as json from twisted.internet import defer -from synapse.types import UserID - import baserules -from push_rule_evaluator import PushRuleEvaluator +from push_rule_evaluator import PushRuleEvaluatorForEvent + +from synapse.api.constants import EventTypes +from synapse.types import UserID -from synapse.events.utils import serialize_event logger = logging.getLogger(__name__) @@ -35,28 +35,25 @@ def decode_rule_json(rule): @defer.inlineCallbacks -def evaluator_for_room_id(room_id, store): - users = yield store.get_users_in_room(room_id) - rules_by_user = yield store.bulk_get_push_rules(users) +def _get_rules(room_id, user_ids, store): + rules_by_user = yield store.bulk_get_push_rules(user_ids) rules_by_user = { uid: baserules.list_with_base_rules( - [decode_rule_json(rule_list) for rule_list in rules_by_user[uid]] - if uid in rules_by_user else [], + [decode_rule_json(rule_list) for rule_list in rules_by_user.get(uid, [])], UserID.from_string(uid), ) - for uid in users + for uid in user_ids } - member_events = yield store.get_current_state( - room_id=room_id, - event_type='m.room.member', - ) - display_names = {} - for ev in member_events: - if ev.content.get("displayname"): - display_names[ev.state_key] = ev.content.get("displayname") + defer.returnValue(rules_by_user) + + +@defer.inlineCallbacks +def evaluator_for_room_id(room_id, store): + users = yield store.get_users_in_room(room_id) + rules_by_user = yield _get_rules(room_id, users, store) defer.returnValue(BulkPushRuleEvaluator( - room_id, rules_by_user, display_names, users, store + room_id, rules_by_user, users, store )) @@ -69,10 +66,9 @@ class BulkPushRuleEvaluator: the same logic to run the actual rules, but could be optimised further (see https://matrix.org/jira/browse/SYN-562) """ - def __init__(self, room_id, rules_by_user, display_names, users_in_room, store): + def __init__(self, room_id, rules_by_user, users_in_room, store): self.room_id = room_id self.rules_by_user = rules_by_user - self.display_names = display_names self.users_in_room = users_in_room self.store = store @@ -80,15 +76,30 @@ class BulkPushRuleEvaluator: def action_for_event_by_user(self, event, handler): actions_by_user = {} + users_dict = yield self.store.are_guests(self.rules_by_user.keys()) + + filtered_by_user = yield handler._filter_events_for_clients( + users_dict.items(), [event] + ) + + evaluator = PushRuleEvaluatorForEvent.create(event, len(self.users_in_room)) + + condition_cache = {} + + member_state = yield self.store.get_state_for_event( + event.event_id, + ) + + display_names = {} + for ev in member_state.values(): + nm = ev.content.get("displayname", None) + if nm and ev.type == EventTypes.Member: + display_names[ev.state_key] = nm + for uid, rules in self.rules_by_user.items(): - display_name = None - if uid in self.display_names: - display_name = self.display_names[uid] - - is_guest = yield self.store.is_guest(UserID.from_string(uid)) - filtered = yield handler._filter_events_for_client( - uid, [event], is_guest=is_guest - ) + display_name = display_names.get(uid, None) + + filtered = filtered_by_user[uid] if len(filtered) == 0: continue @@ -96,29 +107,32 @@ class BulkPushRuleEvaluator: if 'enabled' in rule and not rule['enabled']: continue - # XXX: profile tags - if BulkPushRuleEvaluator.event_matches_rule( - event, rule, - display_name, len(self.users_in_room), None - ): + matches = _condition_checker( + evaluator, rule['conditions'], display_name, condition_cache + ) + if matches: actions = [x for x in rule['actions'] if x != 'dont_notify'] - if len(actions) > 0: + if actions: actions_by_user[uid] = actions break defer.returnValue(actions_by_user) - @staticmethod - def event_matches_rule(event, rule, - display_name, room_member_count, profile_tag): - matches = True - - # passing the clock all the way into here is extremely awkward and push - # rules do not care about any of the relative timestamps, so we just - # pass 0 for the current time. - client_event = serialize_event(event, 0) - - for cond in rule['conditions']: - matches &= PushRuleEvaluator._event_fulfills_condition( - client_event, cond, display_name, room_member_count, profile_tag - ) - return matches + +def _condition_checker(evaluator, conditions, display_name, cache): + for cond in conditions: + _id = cond.get("_id", None) + if _id: + res = cache.get(_id, None) + if res is False: + break + elif res is True: + continue + + res = evaluator.matches(cond, display_name, None) + if _id: + cache[_id] = res + + if res is False: + return False + + return True diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index b0283743a2..bbc8308c2d 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -15,17 +15,22 @@ from twisted.internet import defer -from synapse.types import UserID - import baserules import logging import simplejson as json import re +from synapse.types import UserID + logger = logging.getLogger(__name__) +GLOB_REGEX = re.compile(r'\\\[(\\\!|)(.*)\\\]') +IS_GLOB = re.compile(r'[\?\*\[\]]') +INEQUALITY_EXPR = re.compile("^([=<>]*)([0-9]*)$") + + @defer.inlineCallbacks def evaluator_for_user_id_and_profile_tag(user_id, profile_tag, room_id, store): rawrules = yield store.get_push_rules_for_user(user_id) @@ -42,9 +47,34 @@ def evaluator_for_user_id_and_profile_tag(user_id, profile_tag, room_id, store): )) +def _room_member_count(ev, condition, room_member_count): + if 'is' not in condition: + return False + m = INEQUALITY_EXPR.match(condition['is']) + if not m: + return False + ineq = m.group(1) + rhs = m.group(2) + if not rhs.isdigit(): + return False + rhs = int(rhs) + + if ineq == '' or ineq == '==': + return room_member_count == rhs + elif ineq == '<': + return room_member_count < rhs + elif ineq == '>': + return room_member_count > rhs + elif ineq == '>=': + return room_member_count >= rhs + elif ineq == '<=': + return room_member_count <= rhs + else: + return False + + class PushRuleEvaluator: DEFAULT_ACTIONS = [] - INEQUALITY_EXPR = re.compile("^([=<>]*)([0-9]*)$") def __init__(self, user_id, profile_tag, raw_rules, enabled_map, room_id, our_member_event, store): @@ -98,6 +128,8 @@ class PushRuleEvaluator: room_members = yield self.store.get_users_in_room(room_id) room_member_count = len(room_members) + evaluator = PushRuleEvaluatorForEvent.create(ev, room_member_count) + for r in self.rules: if r['rule_id'] in self.enabled_map: r['enabled'] = self.enabled_map[r['rule_id']] @@ -105,21 +137,10 @@ class PushRuleEvaluator: r['enabled'] = True if not r['enabled']: continue - matches = True conditions = r['conditions'] actions = r['actions'] - for c in conditions: - matches &= self._event_fulfills_condition( - ev, c, display_name=my_display_name, - room_member_count=room_member_count, - profile_tag=self.profile_tag - ) - logger.debug( - "Rule %s %s", - r['rule_id'], "matches" if matches else "doesn't match" - ) # ignore rules with no actions (we have an explict 'dont_notify') if len(actions) == 0: logger.warn( @@ -127,6 +148,18 @@ class PushRuleEvaluator: r['rule_id'], self.user_id ) continue + + matches = True + for c in conditions: + matches = evaluator.matches(c, my_display_name, self.profile_tag) + if not matches: + break + + logger.debug( + "Rule %s %s", + r['rule_id'], "matches" if matches else "doesn't match" + ) + if matches: logger.info( "%s matches for user %s, event %s", @@ -145,81 +178,84 @@ class PushRuleEvaluator: ) defer.returnValue(PushRuleEvaluator.DEFAULT_ACTIONS) - @staticmethod - def _glob_to_regexp(glob): - r = re.escape(glob) - r = re.sub(r'\\\*', r'.*?', r) - r = re.sub(r'\\\?', r'.', r) - # handle [abc], [a-z] and [!a-z] style ranges. - r = re.sub(r'\\\[(\\\!|)(.*)\\\]', - lambda x: ('[%s%s]' % (x.group(1) and '^' or '', - re.sub(r'\\\-', '-', x.group(2)))), r) - return r +class PushRuleEvaluatorForEvent(object): + WORD_BOUNDARY = re.compile(r'\b') + + def __init__(self, event, body_parts, room_member_count): + self._event = event + self._body_parts = body_parts + self._room_member_count = room_member_count + + self._value_cache = _flatten_dict(event) @staticmethod - def _event_fulfills_condition(ev, condition, - display_name, room_member_count, profile_tag): - if condition['kind'] == 'event_match': - if 'pattern' not in condition: - logger.warn("event_match condition with no pattern") - return False - # XXX: optimisation: cache our pattern regexps - if condition['key'] == 'content.body': - r = r'\b%s\b' % PushRuleEvaluator._glob_to_regexp(condition['pattern']) - else: - r = r'^%s$' % PushRuleEvaluator._glob_to_regexp(condition['pattern']) - val = _value_for_dotted_key(condition['key'], ev) - if val is None: - return False - return re.search(r, val, flags=re.IGNORECASE) is not None + def create(event, room_member_count): + body = event.get("content", {}).get("body", None) + if body: + body_parts = PushRuleEvaluatorForEvent.WORD_BOUNDARY.split(body) + body_parts[:] = [ + part.lower() for part in body_parts + ] + else: + body_parts = [] + + return PushRuleEvaluatorForEvent(event, body_parts, room_member_count) + def matches(self, condition, display_name, profile_tag): + if condition['kind'] == 'event_match': + return self._event_match(condition) elif condition['kind'] == 'device': if 'profile_tag' not in condition: return True return condition['profile_tag'] == profile_tag - elif condition['kind'] == 'contains_display_name': - # This is special because display names can be different - # between rooms and so you can't really hard code it in a rule. - # Optimisation: we should cache these names and update them from - # the event stream. - if 'content' not in ev or 'body' not in ev['content']: - return False - if not display_name: - return False - return re.search( - r"\b%s\b" % re.escape(display_name), ev['content']['body'], - flags=re.IGNORECASE - ) is not None - + return self._contains_display_name(display_name) elif condition['kind'] == 'room_member_count': - if 'is' not in condition: - return False - m = PushRuleEvaluator.INEQUALITY_EXPR.match(condition['is']) - if not m: - return False - ineq = m.group(1) - rhs = m.group(2) - if not rhs.isdigit(): - return False - rhs = int(rhs) - - if ineq == '' or ineq == '==': - return room_member_count == rhs - elif ineq == '<': - return room_member_count < rhs - elif ineq == '>': - return room_member_count > rhs - elif ineq == '>=': - return room_member_count >= rhs - elif ineq == '<=': - return room_member_count <= rhs - else: - return False + return _room_member_count( + self._event, condition, self._room_member_count + ) else: return True + def _event_match(self, condition): + pattern = condition.get('pattern', None) + + if not pattern: + logger.warn("event_match condition with no pattern") + return False + + # XXX: optimisation: cache our pattern regexps + if condition['key'] == 'content.body': + matcher = _glob_to_matcher(pattern) + + for part in self._body_parts: + if matcher(part): + return True + return False + else: + haystack = self._get_value(condition['key']) + if haystack is None: + return False + + matcher = _glob_to_matcher(pattern) + + return matcher(haystack.lower()) + + def _contains_display_name(self, display_name): + if not display_name: + return False + + lower_display_name = display_name.lower() + for part in self._body_parts: + if part == lower_display_name: + return True + + return False + + def _get_value(self, dotted_key): + return self._value_cache.get(dotted_key, None) + def _value_for_dotted_key(dotted_key, event): parts = dotted_key.split(".") @@ -229,4 +265,42 @@ def _value_for_dotted_key(dotted_key, event): return None val = val[parts[0]] parts = parts[1:] + return val + + +def _glob_to_matcher(glob): + glob = glob.lower() + + if not IS_GLOB.search(glob): + return lambda value: value == glob + + r = re.escape(glob) + + r = r.replace(r'\*', '.*?') + r = r.replace(r'\?', '.') + + # handle [abc], [a-z] and [!a-z] style ranges. + r = GLOB_REGEX.sub( + lambda x: ( + '[%s%s]' % ( + x.group(1) and '^' or '', + x.group(2).replace(r'\\\-', '-') + ) + ), + r, + ) + + r = r + "$" + r = re.compile(r) + return lambda value: r.match(value) + + +def _flatten_dict(d, prefix=[], result={}): + for key, value in d.items(): + if isinstance(value, basestring): + result[".".join(prefix + [key])] = value.lower() + elif hasattr(value, "items"): + _flatten_dict(value, prefix=(prefix+[key]), result=result) + + return result diff --git a/synapse/storage/push_rule.py b/synapse/storage/push_rule.py index 2adfefd994..1adf28b893 100644 --- a/synapse/storage/push_rule.py +++ b/synapse/storage/push_rule.py @@ -14,7 +14,7 @@ # limitations under the License. from ._base import SQLBaseStore -from synapse.util.caches.descriptors import cachedInlineCallbacks +from synapse.util.caches.descriptors import cached, cachedInlineCallbacks, cachedList from twisted.internet import defer import logging @@ -60,6 +60,27 @@ class PushRuleStore(SQLBaseStore): r['rule_id']: False if r['enabled'] == 0 else True for r in results }) + @cached() + def _get_push_rules_enabled_for_user(self, user_id): + def f(txn): + sql = ( + "SELECT pr.*" + " FROM push_rules AS pr" + " LEFT JOIN push_rules_enable AS pre" + " ON pr.user_name = pre.user_name AND pr.rule_id = pre.rule_id" + " WHERE pr.user_name = ?" + " AND (pre.enabled IS NULL OR pre.enabled = 1)" + " ORDER BY pr.priority_class DESC, pr.priority DESC" + ) + txn.execute(sql, (user_id,)) + return self.cursor_to_dict(txn) + + return self.runInteraction( + "_get_push_rules_enabled_for_user", f + ) + + # @cachedList(cache=_get_push_rules_enabled_for_user.cache, list_name="user_ids", + # num_args=1, inlineCallbacks=True) @defer.inlineCallbacks def bulk_get_push_rules(self, user_ids): if not user_ids: diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 999b710fbb..70cde0d04d 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -18,7 +18,7 @@ from twisted.internet import defer from synapse.api.errors import StoreError, Codes from ._base import SQLBaseStore -from synapse.util.caches.descriptors import cached, cachedInlineCallbacks +from synapse.util.caches.descriptors import cached, cachedInlineCallbacks, cachedList class RegistrationStore(SQLBaseStore): @@ -256,10 +256,10 @@ class RegistrationStore(SQLBaseStore): defer.returnValue(res if res else False) @cachedInlineCallbacks() - def is_guest(self, user): + def is_guest(self, user_id): res = yield self._simple_select_one_onecol( table="users", - keyvalues={"name": user.to_string()}, + keyvalues={"name": user_id}, retcol="is_guest", allow_none=True, desc="is_guest", @@ -267,6 +267,26 @@ class RegistrationStore(SQLBaseStore): defer.returnValue(res if res else False) + @cachedList(cache=is_guest.cache, list_name="user_ids", num_args=1, + inlineCallbacks=True) + def are_guests(self, user_ids): + sql = "SELECT name, is_guest FROM users WHERE name IN (%s)" % ( + ",".join("?" for _ in user_ids), + ) + + rows = yield self._execute( + "are_guests", self.cursor_to_dict, sql, *user_ids + ) + + result = {user_id: False for user_id in user_ids} + + result.update({ + row["name"]: bool(row["is_guest"]) + for row in rows + }) + + defer.returnValue(result) + def _query_for_auth(self, txn, token): sql = ( "SELECT users.name, users.is_guest, access_tokens.id as token_id" -- cgit 1.4.1 From 63485b30298f906efbdbe39311dd7503101e7602 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 14:11:43 +0000 Subject: Re-enable urnead notifications --- synapse/handlers/federation.py | 13 ++++++------- synapse/handlers/sync.py | 3 --- synapse/push/action_generator.py | 3 --- 3 files changed, 6 insertions(+), 13 deletions(-) (limited to 'synapse/push') diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 26402ea9cd..4b94940e99 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -36,7 +36,7 @@ from synapse.events.utils import prune_event from synapse.util.retryutils import NotRetryingDestination -# from synapse.push.action_generator import ActionGenerator +from synapse.push.action_generator import ActionGenerator from twisted.internet import defer @@ -244,12 +244,11 @@ class FederationHandler(BaseHandler): user = UserID.from_string(event.state_key) yield user_joined_room(self.distributor, user, event.room_id) - # Temporarily disable notifications due to performance concerns. - # if not backfilled and not event.internal_metadata.is_outlier(): - # action_generator = ActionGenerator(self.store) - # yield action_generator.handle_push_actions_for_event( - # event, self - # ) + if not backfilled and not event.internal_metadata.is_outlier(): + action_generator = ActionGenerator(self.store) + yield action_generator.handle_push_actions_for_event( + event, self + ) @defer.inlineCallbacks def _filter_events_for_server(self, server_name, room_id, events): diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 1942268c3c..52202d8e63 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -841,9 +841,6 @@ class SyncHandler(BaseHandler): @defer.inlineCallbacks def unread_notifs_for_room_id(self, room_id, sync_config, ephemeral_by_room): - # Temporarily disable notifications due to performance concerns. - defer.returnValue([]) - last_unread_event_id = self.last_read_event_id_for_room_and_user( room_id, sync_config.user.to_string(), ephemeral_by_room ) diff --git a/synapse/push/action_generator.py b/synapse/push/action_generator.py index 73467f3adc..4cf94f6c61 100644 --- a/synapse/push/action_generator.py +++ b/synapse/push/action_generator.py @@ -36,9 +36,6 @@ class ActionGenerator: @defer.inlineCallbacks def handle_push_actions_for_event(self, event, handler): - # Temporarily disable notifications due to performance concerns. - return - if event.type == EventTypes.Redaction and event.redacts is not None: yield self.store.remove_push_actions_for_event_id( event.room_id, event.redacts -- cgit 1.4.1 From 345ff2196a8b381a3dfea537ab659aac0837045a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 14:39:34 +0000 Subject: Don't edit ruleset --- synapse/push/push_rule_evaluator.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index bbc8308c2d..60d9f1f239 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -131,11 +131,10 @@ class PushRuleEvaluator: evaluator = PushRuleEvaluatorForEvent.create(ev, room_member_count) for r in self.rules: - if r['rule_id'] in self.enabled_map: - r['enabled'] = self.enabled_map[r['rule_id']] - elif 'enabled' not in r: - r['enabled'] = True - if not r['enabled']: + if self.enabled_map.get(r['rule_id'], None) is False: + continue + + if not r.get("enabled", True): continue conditions = r['conditions'] -- cgit 1.4.1 From d1f56f732e1c213e203f287945d84966c3eec6f3 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 10:09:14 +0000 Subject: Use static for const dicts --- synapse/push/baserules.py | 364 +++++++++++++++---------------- synapse/push/bulk_push_rule_evaluator.py | 15 +- synapse/push/push_rule_evaluator.py | 20 +- synapse/rest/client/v1/push_rule.py | 8 +- 4 files changed, 209 insertions(+), 198 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 8bac7fd6af..d8a0eda9fa 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -15,27 +15,25 @@ from synapse.push.rulekinds import PRIORITY_CLASS_MAP, PRIORITY_CLASS_INVERSE_MAP -def list_with_base_rules(rawrules, user_id): +def list_with_base_rules(rawrules): ruleslist = [] # shove the server default rules for each kind onto the end of each current_prio_class = PRIORITY_CLASS_INVERSE_MAP.keys()[-1] ruleslist.extend(make_base_prepend_rules( - user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] + PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) for r in rawrules: if r['priority_class'] < current_prio_class: while r['priority_class'] < current_prio_class: ruleslist.extend(make_base_append_rules( - user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) current_prio_class -= 1 if current_prio_class > 0: ruleslist.extend(make_base_prepend_rules( - user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) @@ -43,28 +41,26 @@ def list_with_base_rules(rawrules, user_id): while current_prio_class > 0: ruleslist.extend(make_base_append_rules( - user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) current_prio_class -= 1 if current_prio_class > 0: ruleslist.extend(make_base_prepend_rules( - user_id, PRIORITY_CLASS_INVERSE_MAP[current_prio_class] )) return ruleslist -def make_base_append_rules(user, kind): +def make_base_append_rules(kind): rules = [] if kind == 'override': - rules = make_base_append_override_rules() + rules = BASE_APPEND_OVRRIDE_RULES elif kind == 'underride': - rules = make_base_append_underride_rules(user) + rules = BASE_APPEND_UNDERRIDE_RULES elif kind == 'content': - rules = make_base_append_content_rules(user) + rules = BASE_APPEND_CONTENT_RULES for r in rules: r['priority_class'] = PRIORITY_CLASS_MAP[kind] @@ -73,11 +69,11 @@ def make_base_append_rules(user, kind): return rules -def make_base_prepend_rules(user, kind): +def make_base_prepend_rules(kind): rules = [] if kind == 'override': - rules = make_base_prepend_override_rules() + rules = BASE_PREPEND_OVERRIDE_RULES for r in rules: r['priority_class'] = PRIORITY_CLASS_MAP[kind] @@ -86,180 +82,182 @@ def make_base_prepend_rules(user, kind): return rules -def make_base_append_content_rules(user): - return [ - { - 'rule_id': 'global/content/.m.rule.contains_user_name', - 'conditions': [ - { - 'kind': 'event_match', - 'key': 'content.body', - 'pattern': user.localpart, # Matrix ID match - } - ], - 'actions': [ - 'notify', - { - 'set_tweak': 'sound', - 'value': 'default', - }, { - 'set_tweak': 'highlight' - } - ] - }, - ] +BASE_APPEND_CONTENT_RULES = [ + { + 'rule_id': 'global/content/.m.rule.contains_user_name', + 'conditions': [ + { + 'kind': 'event_match', + 'key': 'content.body', + 'pattern_type': 'user_localpart' + } + ], + 'actions': [ + 'notify', + { + 'set_tweak': 'sound', + 'value': 'default', + }, { + 'set_tweak': 'highlight' + } + ] + }, +] -def make_base_prepend_override_rules(): - return [ - { - 'rule_id': 'global/override/.m.rule.master', - 'enabled': False, - 'conditions': [], - 'actions': [ - "dont_notify" - ] - } - ] +BASE_PREPEND_OVERRIDE_RULES = [ + { + 'rule_id': 'global/override/.m.rule.master', + 'enabled': False, + 'conditions': [], + 'actions': [ + "dont_notify" + ] + } +] -def make_base_append_override_rules(): - return [ - { - 'rule_id': 'global/override/.m.rule.suppress_notices', - 'conditions': [ - { - 'kind': 'event_match', - 'key': 'content.msgtype', - 'pattern': 'm.notice', - } - ], - 'actions': [ - 'dont_notify', - ] - } - ] +BASE_APPEND_OVRRIDE_RULES = [ + { + 'rule_id': 'global/override/.m.rule.suppress_notices', + 'conditions': [ + { + 'kind': 'event_match', + 'key': 'content.msgtype', + 'pattern': 'm.notice', + '_id': '_suppress_notices', + } + ], + 'actions': [ + 'dont_notify', + ] + } +] -def make_base_append_underride_rules(user): - return [ - { - 'rule_id': 'global/underride/.m.rule.call', - 'conditions': [ - { - 'kind': 'event_match', - 'key': 'type', - 'pattern': 'm.call.invite', - } - ], - 'actions': [ - 'notify', - { - 'set_tweak': 'sound', - 'value': 'ring' - }, { - 'set_tweak': 'highlight', - 'value': False - } - ] - }, - { - 'rule_id': 'global/underride/.m.rule.contains_display_name', - 'conditions': [ - { - 'kind': 'contains_display_name' - } - ], - 'actions': [ - 'notify', - { - 'set_tweak': 'sound', - 'value': 'default' - }, { - 'set_tweak': 'highlight' - } - ] - }, - { - 'rule_id': 'global/underride/.m.rule.room_one_to_one', - 'conditions': [ - { - 'kind': 'room_member_count', - 'is': '2' - } - ], - 'actions': [ - 'notify', - { - 'set_tweak': 'sound', - 'value': 'default' - }, { - 'set_tweak': 'highlight', - 'value': False - } - ] - }, - { - 'rule_id': 'global/underride/.m.rule.invite_for_me', - 'conditions': [ - { - 'kind': 'event_match', - 'key': 'type', - 'pattern': 'm.room.member', - }, - { - 'kind': 'event_match', - 'key': 'content.membership', - 'pattern': 'invite', - }, - { - 'kind': 'event_match', - 'key': 'state_key', - 'pattern': user.to_string(), - }, - ], - 'actions': [ - 'notify', - { - 'set_tweak': 'sound', - 'value': 'default' - }, { - 'set_tweak': 'highlight', - 'value': False - } - ] - }, - { - 'rule_id': 'global/underride/.m.rule.member_event', - 'conditions': [ - { - 'kind': 'event_match', - 'key': 'type', - 'pattern': 'm.room.member', - } - ], - 'actions': [ - 'notify', { - 'set_tweak': 'highlight', - 'value': False - } - ] - }, - { - 'rule_id': 'global/underride/.m.rule.message', - 'enabled': False, - 'conditions': [ - { - 'kind': 'event_match', - 'key': 'type', - 'pattern': 'm.room.message', - } - ], - 'actions': [ - 'notify', { - 'set_tweak': 'highlight', - 'value': False - } - ] - } - ] +BASE_APPEND_UNDERRIDE_RULES = [ + { + 'rule_id': 'global/underride/.m.rule.call', + 'conditions': [ + { + 'kind': 'event_match', + 'key': 'type', + 'pattern': 'm.call.invite', + '_id': '_call', + } + ], + 'actions': [ + 'notify', + { + 'set_tweak': 'sound', + 'value': 'ring' + }, { + 'set_tweak': 'highlight', + 'value': False + } + ] + }, + { + 'rule_id': 'global/underride/.m.rule.contains_display_name', + 'conditions': [ + { + 'kind': 'contains_display_name' + } + ], + 'actions': [ + 'notify', + { + 'set_tweak': 'sound', + 'value': 'default' + }, { + 'set_tweak': 'highlight' + } + ] + }, + { + 'rule_id': 'global/underride/.m.rule.room_one_to_one', + 'conditions': [ + { + 'kind': 'room_member_count', + 'is': '2' + } + ], + 'actions': [ + 'notify', + { + 'set_tweak': 'sound', + 'value': 'default' + }, { + 'set_tweak': 'highlight', + 'value': False + } + ] + }, + { + 'rule_id': 'global/underride/.m.rule.invite_for_me', + 'conditions': [ + { + 'kind': 'event_match', + 'key': 'type', + 'pattern': 'm.room.member', + '_id': '_invite_type', + }, + { + 'kind': 'event_match', + 'key': 'content.membership', + 'pattern': 'invite', + '_id': '_invite_member', + }, + { + 'kind': 'event_match', + 'key': 'state_key', + 'pattern_type': 'user_id' + }, + ], + 'actions': [ + 'notify', + { + 'set_tweak': 'sound', + 'value': 'default' + }, { + 'set_tweak': 'highlight', + 'value': False + } + ] + }, + { + 'rule_id': 'global/underride/.m.rule.member_event', + 'conditions': [ + { + 'kind': 'event_match', + 'key': 'type', + 'pattern': 'm.room.member', + '_id': '_member', + } + ], + 'actions': [ + 'notify', { + 'set_tweak': 'highlight', + 'value': False + } + ] + }, + { + 'rule_id': 'global/underride/.m.rule.message', + 'enabled': False, + 'conditions': [ + { + 'kind': 'event_match', + 'key': 'type', + 'pattern': 'm.room.message', + '_id': '_message', + } + ], + '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 b9f78fd598..f1910f7da7 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -22,7 +22,6 @@ import baserules from push_rule_evaluator import PushRuleEvaluatorForEvent from synapse.api.constants import EventTypes -from synapse.types import UserID logger = logging.getLogger(__name__) @@ -38,10 +37,10 @@ def decode_rule_json(rule): def _get_rules(room_id, user_ids, store): rules_by_user = yield store.bulk_get_push_rules(user_ids) rules_by_user = { - uid: baserules.list_with_base_rules( - [decode_rule_json(rule_list) for rule_list in rules_by_user.get(uid, [])], - UserID.from_string(uid), - ) + uid: baserules.list_with_base_rules([ + decode_rule_json(rule_list) + for rule_list in rules_by_user.get(uid, []) + ]) for uid in user_ids } defer.returnValue(rules_by_user) @@ -108,7 +107,7 @@ class BulkPushRuleEvaluator: continue matches = _condition_checker( - evaluator, rule['conditions'], display_name, condition_cache + evaluator, rule['conditions'], uid, display_name, condition_cache ) if matches: actions = [x for x in rule['actions'] if x != 'dont_notify'] @@ -118,7 +117,7 @@ class BulkPushRuleEvaluator: defer.returnValue(actions_by_user) -def _condition_checker(evaluator, conditions, display_name, cache): +def _condition_checker(evaluator, conditions, uid, display_name, cache): for cond in conditions: _id = cond.get("_id", None) if _id: @@ -128,7 +127,7 @@ def _condition_checker(evaluator, conditions, display_name, cache): elif res is True: continue - res = evaluator.matches(cond, display_name, None) + res = evaluator.matches(cond, uid, display_name, None) if _id: cache[_id] = res diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 60d9f1f239..9332fe5c5e 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -91,8 +91,7 @@ class PushRuleEvaluator: rule['actions'] = json.loads(raw_rule['actions']) rules.append(rule) - user = UserID.from_string(self.user_id) - self.rules = baserules.list_with_base_rules(rules, user) + self.rules = baserules.list_with_base_rules(rules) self.enabled_map = enabled_map @@ -150,7 +149,9 @@ class PushRuleEvaluator: matches = True for c in conditions: - matches = evaluator.matches(c, my_display_name, self.profile_tag) + matches = evaluator.matches( + c, self.user_id, my_display_name, self.profile_tag + ) if not matches: break @@ -201,9 +202,9 @@ class PushRuleEvaluatorForEvent(object): return PushRuleEvaluatorForEvent(event, body_parts, room_member_count) - def matches(self, condition, display_name, profile_tag): + def matches(self, condition, user_id, display_name, profile_tag): if condition['kind'] == 'event_match': - return self._event_match(condition) + return self._event_match(condition, user_id) elif condition['kind'] == 'device': if 'profile_tag' not in condition: return True @@ -217,9 +218,16 @@ class PushRuleEvaluatorForEvent(object): else: return True - def _event_match(self, condition): + def _event_match(self, condition, user_id): pattern = condition.get('pattern', None) + if not pattern: + pattern_type = condition.get('pattern_type', None) + if pattern_type == "user_id": + pattern = user_id + elif pattern_type == "user_localpart": + pattern = UserID.from_string(user_id).localpart + if not pattern: logger.warn("event_match condition with no pattern") return False diff --git a/synapse/rest/client/v1/push_rule.py b/synapse/rest/client/v1/push_rule.py index b176efd8a8..aa861e7033 100644 --- a/synapse/rest/client/v1/push_rule.py +++ b/synapse/rest/client/v1/push_rule.py @@ -126,7 +126,7 @@ class PushRuleRestServlet(ClientV1RestServlet): rule["actions"] = json.loads(rawrule["actions"]) ruleslist.append(rule) - ruleslist = baserules.list_with_base_rules(ruleslist, user) + ruleslist = baserules.list_with_base_rules(ruleslist) rules = {'global': {}, 'device': {}} @@ -144,6 +144,12 @@ class PushRuleRestServlet(ClientV1RestServlet): for c in r["conditions"]: c.pop("_id", None) + pattern_type = c.pop("pattern_type", None) + if pattern_type == "user_id": + c["pattern"] = user.to_string() + elif pattern_type == "user_localpart": + c["pattern"] = user.localpart + if r['priority_class'] > PRIORITY_CLASS_MAP['override']: # per-device rule profile_tag = _profile_tag_from_conditions(r["conditions"]) -- cgit 1.4.1 From 866fe27e783697f74f45573d0eb202e3803fe756 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 15:29:41 +0000 Subject: Do for loop once at start --- synapse/push/baserules.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index d8a0eda9fa..8be47f0975 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -62,10 +62,6 @@ def make_base_append_rules(kind): elif kind == 'content': rules = BASE_APPEND_CONTENT_RULES - for r in rules: - r['priority_class'] = PRIORITY_CLASS_MAP[kind] - r['default'] = True # Deprecated, left for backwards compat - return rules @@ -75,10 +71,6 @@ def make_base_prepend_rules(kind): if kind == 'override': rules = BASE_PREPEND_OVERRIDE_RULES - for r in rules: - r['priority_class'] = PRIORITY_CLASS_MAP[kind] - r['default'] = True # Deprecated, left for backwards compat - return rules @@ -261,3 +253,20 @@ BASE_APPEND_UNDERRIDE_RULES = [ ] } ] + + +for r in BASE_APPEND_CONTENT_RULES: + r['priority_class'] = PRIORITY_CLASS_MAP['content'] + r['default'] = True + +for r in BASE_PREPEND_OVERRIDE_RULES: + r['priority_class'] = PRIORITY_CLASS_MAP['override'] + r['default'] = True + +for r in BASE_APPEND_OVRRIDE_RULES: + r['priority_class'] = PRIORITY_CLASS_MAP['override'] + r['default'] = True + +for r in BASE_APPEND_UNDERRIDE_RULES: + r['priority_class'] = PRIORITY_CLASS_MAP['underride'] + r['default'] = True -- cgit 1.4.1 From 7dd14e5d1c8950f50279efccce83b9ff30f0bcfb Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 15:42:23 +0000 Subject: Add comments and remove dead code --- synapse/push/push_rule_evaluator.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 9332fe5c5e..eab7fc7d5c 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -184,9 +184,14 @@ class PushRuleEvaluatorForEvent(object): def __init__(self, event, body_parts, room_member_count): self._event = event + + # This is a list of words of the content.body (if event has one). Each + # word has been converted to lower case. self._body_parts = body_parts + self._room_member_count = room_member_count + # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) @staticmethod @@ -264,19 +269,13 @@ class PushRuleEvaluatorForEvent(object): return self._value_cache.get(dotted_key, None) -def _value_for_dotted_key(dotted_key, event): - parts = dotted_key.split(".") - val = event - while len(parts) > 0: - if parts[0] not in val: - return None - val = val[parts[0]] - parts = parts[1:] - - return val - - def _glob_to_matcher(glob): + """Takes a glob and returns a `func(string) -> bool`, which returns if the + string matches the glob. Assumes given string is lower case. + + The matcher returned is either a simple string comparison for globs without + wildcards, or a regex matcher for globs with wildcards. + """ glob = glob.lower() if not IS_GLOB.search(glob): -- cgit 1.4.1 From d16dcf642e56e2ad3e4e6fb5834844251f5383f4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 15:44:04 +0000 Subject: Drop log levels --- synapse/push/push_rule_evaluator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index eab7fc7d5c..0816b632b4 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -161,7 +161,7 @@ class PushRuleEvaluator: ) if matches: - logger.info( + logger.debug( "%s matches for user %s, event %s", r['rule_id'], self.user_id, ev['event_id'] ) @@ -172,7 +172,7 @@ class PushRuleEvaluator: defer.returnValue(actions) - logger.info( + logger.debug( "No rules match for user %s, event %s", self.user_id, ev['event_id'] ) -- cgit 1.4.1 From 29c353c5536520cd149e1aacf6bd42c7c3f8f4e7 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 16:48:17 +0000 Subject: Don't split at word boundaries, actually use regex --- synapse/push/bulk_push_rule_evaluator.py | 2 +- synapse/push/push_rule_evaluator.py | 109 +++++++++++++------------------ 2 files changed, 48 insertions(+), 63 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index f1910f7da7..b0b3a38db7 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -81,7 +81,7 @@ class BulkPushRuleEvaluator: users_dict.items(), [event] ) - evaluator = PushRuleEvaluatorForEvent.create(event, len(self.users_in_room)) + evaluator = PushRuleEvaluatorForEvent(event, len(self.users_in_room)) condition_cache = {} diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 0816b632b4..78d4b564d4 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -127,7 +127,7 @@ class PushRuleEvaluator: room_members = yield self.store.get_users_in_room(room_id) room_member_count = len(room_members) - evaluator = PushRuleEvaluatorForEvent.create(ev, room_member_count) + evaluator = PushRuleEvaluatorForEvent(ev, room_member_count) for r in self.rules: if self.enabled_map.get(r['rule_id'], None) is False: @@ -180,33 +180,13 @@ class PushRuleEvaluator: class PushRuleEvaluatorForEvent(object): - WORD_BOUNDARY = re.compile(r'\b') - - def __init__(self, event, body_parts, room_member_count): + def __init__(self, event, room_member_count): self._event = event - - # This is a list of words of the content.body (if event has one). Each - # word has been converted to lower case. - self._body_parts = body_parts - self._room_member_count = room_member_count # Maps strings of e.g. 'content.body' -> event["content"]["body"] self._value_cache = _flatten_dict(event) - @staticmethod - def create(event, room_member_count): - body = event.get("content", {}).get("body", None) - if body: - body_parts = PushRuleEvaluatorForEvent.WORD_BOUNDARY.split(body) - body_parts[:] = [ - part.lower() for part in body_parts - ] - else: - body_parts = [] - - return PushRuleEvaluatorForEvent(event, body_parts, room_member_count) - def matches(self, condition, user_id, display_name, profile_tag): if condition['kind'] == 'event_match': return self._event_match(condition, user_id) @@ -239,67 +219,72 @@ class PushRuleEvaluatorForEvent(object): # XXX: optimisation: cache our pattern regexps if condition['key'] == 'content.body': - matcher = _glob_to_matcher(pattern) + body = self._event["content"].get("body", None) + if not body: + return False - for part in self._body_parts: - if matcher(part): - return True - return False + return _glob_matches(pattern, body, word_boundary=True) else: haystack = self._get_value(condition['key']) if haystack is None: return False - matcher = _glob_to_matcher(pattern) - - return matcher(haystack.lower()) + return _glob_matches(pattern, haystack) def _contains_display_name(self, display_name): if not display_name: return False - lower_display_name = display_name.lower() - for part in self._body_parts: - if part == lower_display_name: - return True + body = self._event["content"].get("body", None) + if not body: + return False - return False + return _glob_matches(display_name, body, word_boundary=True) def _get_value(self, dotted_key): return self._value_cache.get(dotted_key, None) -def _glob_to_matcher(glob): - """Takes a glob and returns a `func(string) -> bool`, which returns if the - string matches the glob. Assumes given string is lower case. - - The matcher returned is either a simple string comparison for globs without - wildcards, or a regex matcher for globs with wildcards. - """ - glob = glob.lower() - - if not IS_GLOB.search(glob): - return lambda value: value == glob +def _glob_matches(glob, value, word_boundary=False): + """Tests if value matches glob. - r = re.escape(glob) + Args: + glob (string) + value (string): String to test against glob. + word_boundary (bool): Whether to match against word boundaries or entire + string. Defaults to False. - r = r.replace(r'\*', '.*?') - r = r.replace(r'\?', '.') + Returns: + bool + """ + if IS_GLOB.search(glob): + r = re.escape(glob) + + r = r.replace(r'\*', '.*?') + r = r.replace(r'\?', '.') + + # handle [abc], [a-z] and [!a-z] style ranges. + r = GLOB_REGEX.sub( + lambda x: ( + '[%s%s]' % ( + x.group(1) and '^' or '', + x.group(2).replace(r'\\\-', '-') + ) + ), + r, + ) + r = r + "$" + r = re.compile(r, flags=re.IGNORECASE) - # handle [abc], [a-z] and [!a-z] style ranges. - r = GLOB_REGEX.sub( - lambda x: ( - '[%s%s]' % ( - x.group(1) and '^' or '', - x.group(2).replace(r'\\\-', '-') - ) - ), - r, - ) + return r.match(value) + elif word_boundary: + r = re.escape(glob) + r = "\b%s\b" % (r,) + r = re.compile(r, flags=re.IGNORECASE) - r = r + "$" - r = re.compile(r) - return lambda value: r.match(value) + return r.search(value) + else: + return value.lower() == glob.lower() def _flatten_dict(d, prefix=[], result={}): -- cgit 1.4.1 From 47f82e4408763d834a0097dceb8b2cca4b0ba4d5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 17:04:36 +0000 Subject: Fix branch didn't check word_boundary --- synapse/push/push_rule_evaluator.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 78d4b564d4..86d8ac5c4f 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -273,10 +273,16 @@ def _glob_matches(glob, value, word_boundary=False): ), r, ) - r = r + "$" - r = re.compile(r, flags=re.IGNORECASE) + if word_boundary: + r = "\b%s\b" % (r,) + r = re.compile(r, flags=re.IGNORECASE) + + return r.search(value) + else: + r = r + "$" + r = re.compile(r, flags=re.IGNORECASE) - return r.match(value) + return r.match(value) elif word_boundary: r = re.escape(glob) r = "\b%s\b" % (r,) -- cgit 1.4.1 From a284ad4092871d55dfb213398d5a1994bb666a78 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 17:20:44 +0000 Subject: You need to escape backslashes --- synapse/push/push_rule_evaluator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 86d8ac5c4f..2524922131 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -274,7 +274,7 @@ def _glob_matches(glob, value, word_boundary=False): r, ) if word_boundary: - r = "\b%s\b" % (r,) + r = r"\b%s\b" % (r,) r = re.compile(r, flags=re.IGNORECASE) return r.search(value) @@ -285,7 +285,7 @@ def _glob_matches(glob, value, word_boundary=False): return r.match(value) elif word_boundary: r = re.escape(glob) - r = "\b%s\b" % (r,) + r = r"\b%s\b" % (r,) r = re.compile(r, flags=re.IGNORECASE) return r.search(value) -- cgit 1.4.1 From 003853e702a9ce1c58da7d326402a1f2cfa69e1f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Jan 2016 17:34:02 +0000 Subject: Preserve truthiness --- synapse/push/push_rule_evaluator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'synapse/push') diff --git a/synapse/push/push_rule_evaluator.py b/synapse/push/push_rule_evaluator.py index 2524922131..379652c513 100644 --- a/synapse/push/push_rule_evaluator.py +++ b/synapse/push/push_rule_evaluator.py @@ -130,7 +130,8 @@ class PushRuleEvaluator: evaluator = PushRuleEvaluatorForEvent(ev, room_member_count) for r in self.rules: - if self.enabled_map.get(r['rule_id'], None) is False: + enabled = self.enabled_map.get(r['rule_id'], None) + if enabled is not None and not enabled: continue if not r.get("enabled", True): -- cgit 1.4.1 From f750a442f738ef9bdc2994810cc3220586f8c765 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 19 Jan 2016 10:14:53 +0000 Subject: Update _id --- synapse/push/baserules.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'synapse/push') diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py index 8be47f0975..e1217b5c52 100644 --- a/synapse/push/baserules.py +++ b/synapse/push/baserules.py @@ -171,7 +171,8 @@ BASE_APPEND_UNDERRIDE_RULES = [ 'conditions': [ { 'kind': 'room_member_count', - 'is': '2' + 'is': '2', + '_id': 'member_count', } ], 'actions': [ @@ -192,7 +193,7 @@ BASE_APPEND_UNDERRIDE_RULES = [ 'kind': 'event_match', 'key': 'type', 'pattern': 'm.room.member', - '_id': '_invite_type', + '_id': '_member', }, { 'kind': 'event_match', -- cgit 1.4.1