Remove dead code for setting device specific rules.

It wasn't possible to hit the code from the API because of a typo
in parsing the request path. Since no-one was using the feature
we might as well remove the dead code.
This commit is contained in:
Mark Haines 2016-02-18 16:05:13 +00:00 committed by review.rocks
parent 35cda2e692
commit b9977ea667
10 changed files with 45 additions and 141 deletions

View file

@ -47,14 +47,13 @@ class Pusher(object):
MAX_BACKOFF = 60 * 60 * 1000 MAX_BACKOFF = 60 * 60 * 1000
GIVE_UP_AFTER = 24 * 60 * 60 * 1000 GIVE_UP_AFTER = 24 * 60 * 60 * 1000
def __init__(self, _hs, profile_tag, user_id, app_id, def __init__(self, _hs, user_id, app_id,
app_display_name, device_display_name, pushkey, pushkey_ts, app_display_name, device_display_name, pushkey, pushkey_ts,
data, last_token, last_success, failing_since): data, last_token, last_success, failing_since):
self.hs = _hs self.hs = _hs
self.evStreamHandler = self.hs.get_handlers().event_stream_handler self.evStreamHandler = self.hs.get_handlers().event_stream_handler
self.store = self.hs.get_datastore() self.store = self.hs.get_datastore()
self.clock = self.hs.get_clock() self.clock = self.hs.get_clock()
self.profile_tag = profile_tag
self.user_id = user_id self.user_id = user_id
self.app_id = app_id self.app_id = app_id
self.app_display_name = app_display_name self.app_display_name = app_display_name
@ -186,8 +185,8 @@ class Pusher(object):
processed = False processed = False
rule_evaluator = yield \ rule_evaluator = yield \
push_rule_evaluator.evaluator_for_user_id_and_profile_tag( push_rule_evaluator.evaluator_for_user_id(
self.user_id, self.profile_tag, single_event['room_id'], self.store self.user_id, single_event['room_id'], self.store
) )
actions = yield rule_evaluator.actions_for_event(single_event) actions = yield rule_evaluator.actions_for_event(single_event)

View file

@ -44,5 +44,5 @@ class ActionGenerator:
) )
context.push_actions = [ context.push_actions = [
(uid, None, actions) for uid, actions in actions_by_user.items() (uid, actions) for uid, actions in actions_by_user.items()
] ]

View file

@ -152,7 +152,7 @@ def _condition_checker(evaluator, conditions, uid, display_name, cache):
elif res is True: elif res is True:
continue continue
res = evaluator.matches(cond, uid, display_name, None) res = evaluator.matches(cond, uid, display_name)
if _id: if _id:
cache[_id] = bool(res) cache[_id] = bool(res)

View file

@ -23,12 +23,11 @@ logger = logging.getLogger(__name__)
class HttpPusher(Pusher): class HttpPusher(Pusher):
def __init__(self, _hs, profile_tag, user_id, app_id, def __init__(self, _hs, user_id, app_id,
app_display_name, device_display_name, pushkey, pushkey_ts, app_display_name, device_display_name, pushkey, pushkey_ts,
data, last_token, last_success, failing_since): data, last_token, last_success, failing_since):
super(HttpPusher, self).__init__( super(HttpPusher, self).__init__(
_hs, _hs,
profile_tag,
user_id, user_id,
app_id, app_id,
app_display_name, app_display_name,

View file

@ -33,7 +33,7 @@ INEQUALITY_EXPR = re.compile("^([=<>]*)([0-9]*)$")
@defer.inlineCallbacks @defer.inlineCallbacks
def evaluator_for_user_id_and_profile_tag(user_id, profile_tag, room_id, store): def evaluator_for_user_id(user_id, room_id, store):
rawrules = yield store.get_push_rules_for_user(user_id) rawrules = yield store.get_push_rules_for_user(user_id)
enabled_map = yield store.get_push_rules_enabled_for_user(user_id) enabled_map = yield store.get_push_rules_enabled_for_user(user_id)
our_member_event = yield store.get_current_state( our_member_event = yield store.get_current_state(
@ -43,7 +43,7 @@ def evaluator_for_user_id_and_profile_tag(user_id, profile_tag, room_id, store):
) )
defer.returnValue(PushRuleEvaluator( defer.returnValue(PushRuleEvaluator(
user_id, profile_tag, rawrules, enabled_map, user_id, rawrules, enabled_map,
room_id, our_member_event, store room_id, our_member_event, store
)) ))
@ -77,10 +77,9 @@ def _room_member_count(ev, condition, room_member_count):
class PushRuleEvaluator: class PushRuleEvaluator:
DEFAULT_ACTIONS = [] DEFAULT_ACTIONS = []
def __init__(self, user_id, profile_tag, raw_rules, enabled_map, room_id, def __init__(self, user_id, raw_rules, enabled_map, room_id,
our_member_event, store): our_member_event, store):
self.user_id = user_id self.user_id = user_id
self.profile_tag = profile_tag
self.room_id = room_id self.room_id = room_id
self.our_member_event = our_member_event self.our_member_event = our_member_event
self.store = store self.store = store
@ -152,7 +151,7 @@ class PushRuleEvaluator:
matches = True matches = True
for c in conditions: for c in conditions:
matches = evaluator.matches( matches = evaluator.matches(
c, self.user_id, my_display_name, self.profile_tag c, self.user_id, my_display_name
) )
if not matches: if not matches:
break break
@ -189,13 +188,9 @@ class PushRuleEvaluatorForEvent(object):
# Maps strings of e.g. 'content.body' -> event["content"]["body"] # Maps strings of e.g. 'content.body' -> event["content"]["body"]
self._value_cache = _flatten_dict(event) self._value_cache = _flatten_dict(event)
def matches(self, condition, user_id, display_name, profile_tag): def matches(self, condition, user_id, display_name):
if condition['kind'] == 'event_match': if condition['kind'] == 'event_match':
return self._event_match(condition, user_id) return self._event_match(condition, user_id)
elif condition['kind'] == 'device':
if 'profile_tag' not in condition:
return True
return condition['profile_tag'] == profile_tag
elif condition['kind'] == 'contains_display_name': elif condition['kind'] == 'contains_display_name':
return self._contains_display_name(display_name) return self._contains_display_name(display_name)
elif condition['kind'] == 'room_member_count': elif condition['kind'] == 'room_member_count':

View file

@ -29,6 +29,7 @@ class PusherPool:
def __init__(self, _hs): def __init__(self, _hs):
self.hs = _hs self.hs = _hs
self.store = self.hs.get_datastore() self.store = self.hs.get_datastore()
self.clock = self.hs.get_clock()
self.pushers = {} self.pushers = {}
self.last_pusher_started = -1 self.last_pusher_started = -1
@ -38,8 +39,11 @@ class PusherPool:
self._start_pushers(pushers) self._start_pushers(pushers)
@defer.inlineCallbacks @defer.inlineCallbacks
def add_pusher(self, user_id, access_token, profile_tag, kind, app_id, def add_pusher(self, user_id, access_token, kind, app_id,
app_display_name, device_display_name, pushkey, lang, data): app_display_name, device_display_name, pushkey, lang, data,
profile_tag=""):
time_now_msec = self.clock.time_msec()
# we try to create the pusher just to validate the config: it # we try to create the pusher just to validate the config: it
# will then get pulled out of the database, # will then get pulled out of the database,
# recreated, added and started: this means we have only one # recreated, added and started: this means we have only one
@ -47,23 +51,31 @@ class PusherPool:
self._create_pusher({ self._create_pusher({
"user_name": user_id, "user_name": user_id,
"kind": kind, "kind": kind,
"profile_tag": profile_tag,
"app_id": app_id, "app_id": app_id,
"app_display_name": app_display_name, "app_display_name": app_display_name,
"device_display_name": device_display_name, "device_display_name": device_display_name,
"pushkey": pushkey, "pushkey": pushkey,
"ts": self.hs.get_clock().time_msec(), "ts": time_now_msec,
"lang": lang, "lang": lang,
"data": data, "data": data,
"last_token": None, "last_token": None,
"last_success": None, "last_success": None,
"failing_since": None "failing_since": None
}) })
yield self._add_pusher_to_store( yield self.store.add_pusher(
user_id, access_token, profile_tag, kind, app_id, user_id=user_id,
app_display_name, device_display_name, access_token=access_token,
pushkey, lang, data kind=kind,
app_id=app_id,
app_display_name=app_display_name,
device_display_name=device_display_name,
pushkey=pushkey,
pushkey_ts=time_now_msec,
lang=lang,
data=data,
profile_tag=profile_tag,
) )
yield self._refresh_pusher(app_id, pushkey, user_id)
@defer.inlineCallbacks @defer.inlineCallbacks
def remove_pushers_by_app_id_and_pushkey_not_user(self, app_id, pushkey, def remove_pushers_by_app_id_and_pushkey_not_user(self, app_id, pushkey,
@ -94,30 +106,10 @@ class PusherPool:
) )
yield self.remove_pusher(p['app_id'], p['pushkey'], p['user_name']) yield self.remove_pusher(p['app_id'], p['pushkey'], p['user_name'])
@defer.inlineCallbacks
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_id=user_id,
access_token=access_token,
profile_tag=profile_tag,
kind=kind,
app_id=app_id,
app_display_name=app_display_name,
device_display_name=device_display_name,
pushkey=pushkey,
pushkey_ts=self.hs.get_clock().time_msec(),
lang=lang,
data=data,
)
yield self._refresh_pusher(app_id, pushkey, user_id)
def _create_pusher(self, pusherdict): def _create_pusher(self, pusherdict):
if pusherdict['kind'] == 'http': if pusherdict['kind'] == 'http':
return HttpPusher( return HttpPusher(
self.hs, self.hs,
profile_tag=pusherdict['profile_tag'],
user_id=pusherdict['user_name'], user_id=pusherdict['user_name'],
app_id=pusherdict['app_id'], app_id=pusherdict['app_id'],
app_display_name=pusherdict['app_display_name'], app_display_name=pusherdict['app_display_name'],

View file

@ -60,7 +60,6 @@ class PushRuleRestServlet(ClientV1RestServlet):
spec['template'], spec['template'],
spec['rule_id'], spec['rule_id'],
content, content,
device=spec['device'] if 'device' in spec else None
) )
except InvalidRuleException as e: except InvalidRuleException as e:
raise SynapseError(400, e.message) raise SynapseError(400, e.message)
@ -153,23 +152,7 @@ class PushRuleRestServlet(ClientV1RestServlet):
elif pattern_type == "user_localpart": elif pattern_type == "user_localpart":
c["pattern"] = user.localpart c["pattern"] = user.localpart
if r['priority_class'] > PRIORITY_CLASS_MAP['override']: rulearray = rules['global'][template_name]
# per-device rule
profile_tag = _profile_tag_from_conditions(r["conditions"])
r = _strip_device_condition(r)
if not profile_tag:
continue
if profile_tag not in rules['device']:
rules['device'][profile_tag] = {}
rules['device'][profile_tag] = (
_add_empty_priority_class_arrays(
rules['device'][profile_tag]
)
)
rulearray = rules['device'][profile_tag][template_name]
else:
rulearray = rules['global'][template_name]
template_rule = _rule_to_template(r) template_rule = _rule_to_template(r)
if template_rule: if template_rule:
@ -195,24 +178,6 @@ class PushRuleRestServlet(ClientV1RestServlet):
path = path[1:] path = path[1:]
result = _filter_ruleset_with_path(rules['global'], path) result = _filter_ruleset_with_path(rules['global'], path)
defer.returnValue((200, result)) defer.returnValue((200, result))
elif path[0] == 'device':
path = path[1:]
if path == []:
raise UnrecognizedRequestError(
PushRuleRestServlet.SLIGHTLY_PEDANTIC_TRAILING_SLASH_ERROR
)
if path[0] == '':
defer.returnValue((200, rules['device']))
profile_tag = path[0]
path = path[1:]
if profile_tag not in rules['device']:
ret = {}
ret = _add_empty_priority_class_arrays(ret)
defer.returnValue((200, ret))
ruleset = rules['device'][profile_tag]
result = _filter_ruleset_with_path(ruleset, path)
defer.returnValue((200, result))
else: else:
raise UnrecognizedRequestError() raise UnrecognizedRequestError()
@ -252,16 +217,9 @@ def _rule_spec_from_path(path):
scope = path[1] scope = path[1]
path = path[2:] path = path[2:]
if scope not in ['global', 'device']: if scope != 'global':
raise UnrecognizedRequestError() raise UnrecognizedRequestError()
device = None
if scope == 'device':
if len(path) == 0:
raise UnrecognizedRequestError()
device = path[0]
path = path[1:]
if len(path) == 0: if len(path) == 0:
raise UnrecognizedRequestError() raise UnrecognizedRequestError()
@ -278,8 +236,6 @@ def _rule_spec_from_path(path):
'template': template, 'template': template,
'rule_id': rule_id 'rule_id': rule_id
} }
if device:
spec['profile_tag'] = device
path = path[1:] path = path[1:]
@ -289,7 +245,7 @@ def _rule_spec_from_path(path):
return spec return spec
def _rule_tuple_from_request_object(rule_template, rule_id, req_obj, device=None): def _rule_tuple_from_request_object(rule_template, rule_id, req_obj):
if rule_template in ['override', 'underride']: if rule_template in ['override', 'underride']:
if 'conditions' not in req_obj: if 'conditions' not in req_obj:
raise InvalidRuleException("Missing 'conditions'") raise InvalidRuleException("Missing 'conditions'")
@ -322,12 +278,6 @@ def _rule_tuple_from_request_object(rule_template, rule_id, req_obj, device=None
else: else:
raise InvalidRuleException("Unknown rule template: %s" % (rule_template,)) raise InvalidRuleException("Unknown rule template: %s" % (rule_template,))
if device:
conditions.append({
'kind': 'device',
'profile_tag': device
})
if 'actions' not in req_obj: if 'actions' not in req_obj:
raise InvalidRuleException("No actions found") raise InvalidRuleException("No actions found")
actions = req_obj['actions'] actions = req_obj['actions']
@ -349,17 +299,6 @@ def _add_empty_priority_class_arrays(d):
return d return d
def _profile_tag_from_conditions(conditions):
"""
Given a list of conditions, return the profile tag of the
device rule if there is one
"""
for c in conditions:
if c['kind'] == 'device':
return c['profile_tag']
return None
def _filter_ruleset_with_path(ruleset, path): def _filter_ruleset_with_path(ruleset, path):
if path == []: if path == []:
raise UnrecognizedRequestError( raise UnrecognizedRequestError(
@ -403,19 +342,11 @@ def _priority_class_from_spec(spec):
raise InvalidRuleException("Unknown template: %s" % (spec['template'])) raise InvalidRuleException("Unknown template: %s" % (spec['template']))
pc = PRIORITY_CLASS_MAP[spec['template']] pc = PRIORITY_CLASS_MAP[spec['template']]
if spec['scope'] == 'device':
pc += len(PRIORITY_CLASS_MAP)
return pc return pc
def _priority_class_to_template_name(pc): def _priority_class_to_template_name(pc):
if pc > PRIORITY_CLASS_MAP['override']: return PRIORITY_CLASS_INVERSE_MAP[pc]
# per-device
prio_class_index = pc - len(PRIORITY_CLASS_MAP)
return PRIORITY_CLASS_INVERSE_MAP[prio_class_index]
else:
return PRIORITY_CLASS_INVERSE_MAP[pc]
def _rule_to_template(rule): def _rule_to_template(rule):
@ -445,23 +376,12 @@ def _rule_to_template(rule):
return templaterule return templaterule
def _strip_device_condition(rule):
for i, c in enumerate(rule['conditions']):
if c['kind'] == 'device':
del rule['conditions'][i]
return rule
def _namespaced_rule_id_from_spec(spec): def _namespaced_rule_id_from_spec(spec):
return _namespaced_rule_id(spec, spec['rule_id']) return _namespaced_rule_id(spec, spec['rule_id'])
def _namespaced_rule_id(spec, rule_id): def _namespaced_rule_id(spec, rule_id):
if spec['scope'] == 'global': return "global/%s/%s" % (spec['template'], rule_id)
scope = 'global'
else:
scope = 'device/%s' % (spec['profile_tag'])
return "%s/%s/%s" % (scope, spec['template'], rule_id)
def _rule_id_from_namespaced(in_rule_id): def _rule_id_from_namespaced(in_rule_id):

View file

@ -45,7 +45,7 @@ class PusherRestServlet(ClientV1RestServlet):
) )
defer.returnValue((200, {})) defer.returnValue((200, {}))
reqd = ['profile_tag', 'kind', 'app_id', 'app_display_name', reqd = ['kind', 'app_id', 'app_display_name',
'device_display_name', 'pushkey', 'lang', 'data'] 'device_display_name', 'pushkey', 'lang', 'data']
missing = [] missing = []
for i in reqd: for i in reqd:
@ -73,14 +73,14 @@ class PusherRestServlet(ClientV1RestServlet):
yield pusher_pool.add_pusher( yield pusher_pool.add_pusher(
user_id=user.to_string(), user_id=user.to_string(),
access_token=requester.access_token_id, access_token=requester.access_token_id,
profile_tag=content['profile_tag'],
kind=content['kind'], kind=content['kind'],
app_id=content['app_id'], app_id=content['app_id'],
app_display_name=content['app_display_name'], app_display_name=content['app_display_name'],
device_display_name=content['device_display_name'], device_display_name=content['device_display_name'],
pushkey=content['pushkey'], pushkey=content['pushkey'],
lang=content['lang'], lang=content['lang'],
data=content['data'] data=content['data'],
profile_tag=content.get('profile_tag', ""),
) )
except PusherConfigException as pce: except PusherConfigException as pce:
raise SynapseError(400, "Config Error: " + pce.message, raise SynapseError(400, "Config Error: " + pce.message,

View file

@ -27,15 +27,14 @@ class EventPushActionsStore(SQLBaseStore):
def _set_push_actions_for_event_and_users_txn(self, txn, event, tuples): def _set_push_actions_for_event_and_users_txn(self, txn, event, tuples):
""" """
:param event: the event set actions for :param event: the event set actions for
:param tuples: list of tuples of (user_id, profile_tag, actions) :param tuples: list of tuples of (user_id, actions)
""" """
values = [] values = []
for uid, profile_tag, actions in tuples: for uid, actions in tuples:
values.append({ values.append({
'room_id': event.room_id, 'room_id': event.room_id,
'event_id': event.event_id, 'event_id': event.event_id,
'user_id': uid, 'user_id': uid,
'profile_tag': profile_tag,
'actions': json.dumps(actions), 'actions': json.dumps(actions),
'stream_ordering': event.internal_metadata.stream_ordering, 'stream_ordering': event.internal_metadata.stream_ordering,
'topological_ordering': event.depth, 'topological_ordering': event.depth,
@ -43,7 +42,7 @@ class EventPushActionsStore(SQLBaseStore):
'highlight': 1 if _action_has_highlight(actions) else 0, 'highlight': 1 if _action_has_highlight(actions) else 0,
}) })
for uid, _, __ in tuples: for uid, __ in tuples:
txn.call_after( txn.call_after(
self.get_unread_event_push_actions_by_room_for_user.invalidate_many, self.get_unread_event_push_actions_by_room_for_user.invalidate_many,
(event.room_id, uid) (event.room_id, uid)

View file

@ -80,9 +80,9 @@ class PusherStore(SQLBaseStore):
defer.returnValue(rows) defer.returnValue(rows)
@defer.inlineCallbacks @defer.inlineCallbacks
def add_pusher(self, user_id, access_token, profile_tag, kind, app_id, def add_pusher(self, user_id, access_token, kind, app_id,
app_display_name, device_display_name, app_display_name, device_display_name,
pushkey, pushkey_ts, lang, data): pushkey, pushkey_ts, lang, data, profile_tag=""):
try: try:
next_id = yield self._pushers_id_gen.get_next() next_id = yield self._pushers_id_gen.get_next()
yield self._simple_upsert( yield self._simple_upsert(
@ -95,12 +95,12 @@ class PusherStore(SQLBaseStore):
dict( dict(
access_token=access_token, access_token=access_token,
kind=kind, kind=kind,
profile_tag=profile_tag,
app_display_name=app_display_name, app_display_name=app_display_name,
device_display_name=device_display_name, device_display_name=device_display_name,
ts=pushkey_ts, ts=pushkey_ts,
lang=lang, lang=lang,
data=encode_canonical_json(data), data=encode_canonical_json(data),
profile_tag=profile_tag,
), ),
insertion_values=dict( insertion_values=dict(
id=next_id, id=next_id,