Merge branch 'develop' into application-services

5 files changed, 222 insertions, 163 deletions

diff --git a/synapse/push/__init__.py b/synapse/push/__init__.py
index 8c6f0a6571..7293715293 100644
--- a/synapse/push/__init__.py
+++ b/synapse/push/__init__.py
@@ -77,15 +77,15 @@ class Pusher(object):
             if ev['state_key'] != self.user_name:
                 defer.returnValue(['dont_notify'])
 
-        rules = yield self.store.get_push_rules_for_user_name(self.user_name)
+        rawrules = yield self.store.get_push_rules_for_user_name(self.user_name)
 
-        for r in rules:
+        for r in rawrules:
             r['conditions'] = json.loads(r['conditions'])
             r['actions'] = json.loads(r['actions'])
 
-        user_name_localpart = UserID.from_string(self.user_name).localpart
+        user = UserID.from_string(self.user_name)
 
-        rules.extend(baserules.make_base_rules(user_name_localpart))
+        rules = baserules.list_with_base_rules(rawrules, user)
 
         # get *our* member event for display name matching
         member_events_for_room = yield self.store.get_current_state(
diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py
index 376d1d4d33..8d4b806da6 100644
--- a/synapse/push/baserules.py
+++ b/synapse/push/baserules.py
@@ -1,20 +1,69 @@
-def make_base_rules(user_name):
-    rules = [
+from synapse.push.rulekinds import PRIORITY_CLASS_MAP, PRIORITY_CLASS_INVERSE_MAP
+
+def list_with_base_rules(rawrules, user_name):
+    ruleslist = []
+
+    # shove the server default rules for each kind onto the end of each
+    current_prio_class = 1
+    for r in rawrules:
+        if r['priority_class'] > current_prio_class:
+            while current_prio_class < r['priority_class']:
+                ruleslist.extend(make_base_rules(
+                        user_name,
+                        PRIORITY_CLASS_INVERSE_MAP[current_prio_class])
+                    )
+                current_prio_class += 1
+
+        ruleslist.append(r)
+
+    while current_prio_class <= PRIORITY_CLASS_INVERSE_MAP.keys()[-1]:
+        ruleslist.extend(make_base_rules(
+            user_name,
+            PRIORITY_CLASS_INVERSE_MAP[current_prio_class])
+        )
+        current_prio_class += 1
+
+    return ruleslist
+
+
+def make_base_rules(user, kind):
+    rules = []
+
+    if kind == 'override':
+        rules = make_base_override_rules()
+    elif kind == 'content':
+        rules = make_base_content_rules(user)
+
+    for r in rules:
+        r['priority_class'] = PRIORITY_CLASS_MAP[kind]
+        r['default'] = True
+
+    return rules
+
+
+def make_base_content_rules(user):
+    return [
         {
             'conditions': [
                 {
                     'kind': 'event_match',
                     'key': 'content.body',
-                    'pattern': '*%s*' % (user_name,), # Matrix ID match
+                    'pattern': user.localpart,  # Matrix ID match
                 }
             ],
             'actions': [
                 'notify',
                 {
-                    'set_sound': 'default'
+                    'set_tweak': 'sound',
+                    'value': 'default',
                 }
             ]
         },
+    ]
+
+
+def make_base_override_rules():
+    return [
         {
             'conditions': [
                 {
@@ -24,7 +73,8 @@ def make_base_rules(user_name):
             'actions': [
                 'notify',
                 {
-                    'set_sound': 'default'
+                    'set_tweak': 'sound',
+                    'value': 'default'
                 }
             ]
         },
@@ -44,6 +94,3 @@ def make_base_rules(user_name):
             ]
         }
     ]
-    for r in rules:
-        r['priority_class'] = 0
-    return rules
\ No newline at end of file
diff --git a/synapse/push/rulekinds.py b/synapse/push/rulekinds.py
new file mode 100644
index 0000000000..763bdee58e
--- /dev/null
+++ b/synapse/push/rulekinds.py
@@ -0,0 +1,8 @@
+PRIORITY_CLASS_MAP = {
+        'underride': 1,
+        'sender': 2,
+        'room': 3,
+        'content': 4,
+        'override': 5,
+    }
+PRIORITY_CLASS_INVERSE_MAP = {v: k for k, v in PRIORITY_CLASS_MAP.items()}
diff --git a/synapse/rest/client/v1/push_rule.py b/synapse/rest/client/v1/push_rule.py
index 5582f33c8e..80f116b1ed 100644
--- a/synapse/rest/client/v1/push_rule.py
+++ b/synapse/rest/client/v1/push_rule.py
@@ -20,118 +20,19 @@ from synapse.api.errors import SynapseError, Codes, UnrecognizedRequestError, No
 from .base import ClientV1RestServlet, client_path_pattern
 from synapse.storage.push_rule import InconsistentRuleException, RuleNotFoundException
 import synapse.push.baserules as baserules
+from synapse.push.rulekinds import PRIORITY_CLASS_MAP, PRIORITY_CLASS_INVERSE_MAP
 
 import json
 
 
 class PushRuleRestServlet(ClientV1RestServlet):
     PATTERN = client_path_pattern("/pushrules/.*$")
-    PRIORITY_CLASS_MAP = {
-        'default': 0,
-        'underride': 1,
-        'sender': 2,
-        'room': 3,
-        'content': 4,
-        'override': 5,
-    }
-    PRIORITY_CLASS_INVERSE_MAP = {v: k for k, v in PRIORITY_CLASS_MAP.items()}
     SLIGHTLY_PEDANTIC_TRAILING_SLASH_ERROR = (
         "Unrecognised request: You probably wanted a trailing slash")
 
-    def rule_spec_from_path(self, path):
-        if len(path) < 2:
-            raise UnrecognizedRequestError()
-        if path[0] != 'pushrules':
-            raise UnrecognizedRequestError()
-
-        scope = path[1]
-        path = path[2:]
-        if scope not in ['global', 'device']:
-            raise UnrecognizedRequestError()
-
-        device = None
-        if scope == 'device':
-            if len(path) == 0:
-                raise UnrecognizedRequestError()
-            device = path[0]
-            path = path[1:]
-
-        if len(path) == 0:
-            raise UnrecognizedRequestError()
-
-        template = path[0]
-        path = path[1:]
-
-        if len(path) == 0:
-            raise UnrecognizedRequestError()
-
-        rule_id = path[0]
-
-        spec = {
-            'scope': scope,
-            'template': template,
-            'rule_id': rule_id
-        }
-        if device:
-            spec['profile_tag'] = device
-        return spec
-
-    def rule_tuple_from_request_object(self, rule_template, rule_id, req_obj, device=None):
-        if rule_template in ['override', 'underride']:
-            if 'conditions' not in req_obj:
-                raise InvalidRuleException("Missing 'conditions'")
-            conditions = req_obj['conditions']
-            for c in conditions:
-                if 'kind' not in c:
-                    raise InvalidRuleException("Condition without 'kind'")
-        elif rule_template == 'room':
-            conditions = [{
-                'kind': 'event_match',
-                'key': 'room_id',
-                'pattern': rule_id
-            }]
-        elif rule_template == 'sender':
-            conditions = [{
-                'kind': 'event_match',
-                'key': 'user_id',
-                'pattern': rule_id
-            }]
-        elif rule_template == 'content':
-            if 'pattern' not in req_obj:
-                raise InvalidRuleException("Content rule missing 'pattern'")
-            pat = req_obj['pattern']
-
-            conditions = [{
-                'kind': 'event_match',
-                'key': 'content.body',
-                'pattern': pat
-            }]
-        else:
-            raise InvalidRuleException("Unknown rule template: %s" % (rule_template,))
-
-        if device:
-            conditions.append({
-                'kind': 'device',
-                'profile_tag': device
-            })
-
-        if 'actions' not in req_obj:
-            raise InvalidRuleException("No actions found")
-        actions = req_obj['actions']
-
-        for a in actions:
-            if a in ['notify', 'dont_notify', 'coalesce']:
-                pass
-            elif isinstance(a, dict) and 'set_sound' in a:
-                pass
-            else:
-                raise InvalidRuleException("Unrecognised action")
-
-        return conditions, actions
-
     @defer.inlineCallbacks
     def on_PUT(self, request):
-        spec = self.rule_spec_from_path(request.postpath)
+        spec = _rule_spec_from_path(request.postpath)
         try:
             priority_class = _priority_class_from_spec(spec)
         except InvalidRuleException as e:
@@ -139,13 +40,13 @@ class PushRuleRestServlet(ClientV1RestServlet):
 
         user, _ = yield self.auth.get_user_by_req(request)
 
-        if spec['template'] == 'default':
-            raise SynapseError(403, "The default rules are immutable.")
+        if '/' in spec['rule_id'] or '\\' in spec['rule_id']:
+            raise SynapseError(400, "rule_id may not contain slashes")
 
         content = _parse_json(request)
 
         try:
-            (conditions, actions) = self.rule_tuple_from_request_object(
+            (conditions, actions) = _rule_tuple_from_request_object(
                 spec['template'],
                 spec['rule_id'],
                 content,
@@ -164,7 +65,7 @@ class PushRuleRestServlet(ClientV1RestServlet):
         try:
             yield self.hs.get_datastore().add_push_rule(
                 user_name=user.to_string(),
-                rule_id=spec['rule_id'],
+                rule_id=_namespaced_rule_id_from_spec(spec),
                 priority_class=priority_class,
                 conditions=conditions,
                 actions=actions,
@@ -180,7 +81,7 @@ class PushRuleRestServlet(ClientV1RestServlet):
 
     @defer.inlineCallbacks
     def on_DELETE(self, request):
-        spec = self.rule_spec_from_path(request.postpath)
+        spec = _rule_spec_from_path(request.postpath)
         try:
             priority_class = _priority_class_from_spec(spec)
         except InvalidRuleException as e:
@@ -188,32 +89,18 @@ class PushRuleRestServlet(ClientV1RestServlet):
 
         user, _ = yield self.auth.get_user_by_req(request)
 
-        if 'profile_tag' in spec:
-            rules = yield self.hs.get_datastore().get_push_rules_for_user_name(
-                user.to_string()
-            )
+        namespaced_rule_id = _namespaced_rule_id_from_spec(spec)
 
-            for r in rules:
-                conditions = json.loads(r['conditions'])
-                pt = _profile_tag_from_conditions(conditions)
-                if pt == spec['profile_tag'] and r['priority_class'] == priority_class:
-                    yield self.hs.get_datastore().delete_push_rule(
-                        user.to_string(), spec['rule_id']
-                    )
-                    defer.returnValue((200, {}))
-            raise NotFoundError()
-        else:
-            try:
-                yield self.hs.get_datastore().delete_push_rule(
-                    user.to_string(), spec['rule_id'],
-                    priority_class=priority_class
-                )
-                defer.returnValue((200, {}))
-            except StoreError as e:
-                if e.code == 404:
-                    raise NotFoundError()
-                else:
-                    raise
+        try:
+            yield self.hs.get_datastore().delete_push_rule(
+                user.to_string(), namespaced_rule_id
+            )
+            defer.returnValue((200, {}))
+        except StoreError as e:
+            if e.code == 404:
+                raise NotFoundError()
+            else:
+                raise
 
     @defer.inlineCallbacks
     def on_GET(self, request):
@@ -223,21 +110,23 @@ class PushRuleRestServlet(ClientV1RestServlet):
         # to send which means doing unnecessary work sometimes but is
         # is probably not going to make a whole lot of difference
         rawrules = yield self.hs.get_datastore().get_push_rules_for_user_name(user.to_string())
+
         for r in rawrules:
             r["conditions"] = json.loads(r["conditions"])
             r["actions"] = json.loads(r["actions"])
-        rawrules.extend(baserules.make_base_rules(user.to_string()))
+
+        ruleslist = baserules.list_with_base_rules(rawrules, user)
 
         rules = {'global': {}, 'device': {}}
 
         rules['global'] = _add_empty_priority_class_arrays(rules['global'])
 
-        for r in rawrules:
+        for r in ruleslist:
             rulearray = None
 
             template_name = _priority_class_to_template_name(r['priority_class'])
 
-            if r['priority_class'] > PushRuleRestServlet.PRIORITY_CLASS_MAP['override']:
+            if r['priority_class'] > PRIORITY_CLASS_MAP['override']:
                 # per-device rule
                 profile_tag = _profile_tag_from_conditions(r["conditions"])
                 r = _strip_device_condition(r)
@@ -298,8 +187,101 @@ class PushRuleRestServlet(ClientV1RestServlet):
         return 200, {}
 
 
+def _rule_spec_from_path(path):
+    if len(path) < 2:
+        raise UnrecognizedRequestError()
+    if path[0] != 'pushrules':
+        raise UnrecognizedRequestError()
+
+    scope = path[1]
+    path = path[2:]
+    if scope not in ['global', 'device']:
+        raise UnrecognizedRequestError()
+
+    device = None
+    if scope == 'device':
+        if len(path) == 0:
+            raise UnrecognizedRequestError()
+        device = path[0]
+        path = path[1:]
+
+    if len(path) == 0:
+        raise UnrecognizedRequestError()
+
+    template = path[0]
+    path = path[1:]
+
+    if len(path) == 0:
+        raise UnrecognizedRequestError()
+
+    rule_id = path[0]
+
+    spec = {
+        'scope': scope,
+        'template': template,
+        'rule_id': rule_id
+    }
+    if device:
+        spec['profile_tag'] = device
+    return spec
+
+
+def _rule_tuple_from_request_object(rule_template, rule_id, req_obj, device=None):
+    if rule_template in ['override', 'underride']:
+        if 'conditions' not in req_obj:
+            raise InvalidRuleException("Missing 'conditions'")
+        conditions = req_obj['conditions']
+        for c in conditions:
+            if 'kind' not in c:
+                raise InvalidRuleException("Condition without 'kind'")
+    elif rule_template == 'room':
+        conditions = [{
+            'kind': 'event_match',
+            'key': 'room_id',
+            'pattern': rule_id
+        }]
+    elif rule_template == 'sender':
+        conditions = [{
+            'kind': 'event_match',
+            'key': 'user_id',
+            'pattern': rule_id
+        }]
+    elif rule_template == 'content':
+        if 'pattern' not in req_obj:
+            raise InvalidRuleException("Content rule missing 'pattern'")
+        pat = req_obj['pattern']
+
+        conditions = [{
+            'kind': 'event_match',
+            'key': 'content.body',
+            'pattern': pat
+        }]
+    else:
+        raise InvalidRuleException("Unknown rule template: %s" % (rule_template,))
+
+    if device:
+        conditions.append({
+            'kind': 'device',
+            'profile_tag': device
+        })
+
+    if 'actions' not in req_obj:
+        raise InvalidRuleException("No actions found")
+    actions = req_obj['actions']
+
+    for a in actions:
+        if a in ['notify', 'dont_notify', 'coalesce']:
+            pass
+        elif isinstance(a, dict) and 'set_sound' in a:
+            pass
+        else:
+            raise InvalidRuleException("Unrecognised action")
+
+    return conditions, actions
+
+
 def _add_empty_priority_class_arrays(d):
-    for pc in PushRuleRestServlet.PRIORITY_CLASS_MAP.keys():
+    for pc in PRIORITY_CLASS_MAP.keys():
         d[pc] = []
     return d
 
@@ -341,42 +323,50 @@ def _filter_ruleset_with_path(ruleset, path):
 
 
 def _priority_class_from_spec(spec):
-    if spec['template'] not in PushRuleRestServlet.PRIORITY_CLASS_MAP.keys():
+    if spec['template'] not in PRIORITY_CLASS_MAP.keys():
         raise InvalidRuleException("Unknown template: %s" % (spec['kind']))
-    pc = PushRuleRestServlet.PRIORITY_CLASS_MAP[spec['template']]
+    pc = PRIORITY_CLASS_MAP[spec['template']]
 
     if spec['scope'] == 'device':
-        pc += len(PushRuleRestServlet.PRIORITY_CLASS_MAP)
+        pc += len(PRIORITY_CLASS_MAP)
 
     return pc
 
 
 def _priority_class_to_template_name(pc):
-    if pc > PushRuleRestServlet.PRIORITY_CLASS_MAP['override']:
+    if pc > PRIORITY_CLASS_MAP['override']:
         # per-device
         prio_class_index = pc - len(PushRuleRestServlet.PRIORITY_CLASS_MAP)
-        return PushRuleRestServlet.PRIORITY_CLASS_INVERSE_MAP[prio_class_index]
+        return PRIORITY_CLASS_INVERSE_MAP[prio_class_index]
     else:
-        return PushRuleRestServlet.PRIORITY_CLASS_INVERSE_MAP[pc]
+        return PRIORITY_CLASS_INVERSE_MAP[pc]
 
 
 def _rule_to_template(rule):
+    unscoped_rule_id = None
+    if 'rule_id' in rule:
+        unscoped_rule_id = _rule_id_from_namespaced(rule['rule_id'])
+
     template_name = _priority_class_to_template_name(rule['priority_class'])
-    if template_name in ['default']:
-        return {k: rule[k] for k in ["conditions", "actions"]}
-    elif template_name in ['override', 'underride']:
-        return {k: rule[k] for k in ["rule_id", "conditions", "actions"]}
+    if template_name in ['override', 'underride']:
+        templaterule = {k: rule[k] for k in ["conditions", "actions"]}
     elif template_name in ["sender", "room"]:
-        return {k: rule[k] for k in ["rule_id", "actions"]}
+        templaterule = {'actions': rule['actions']}
+        unscoped_rule_id = rule['conditions'][0]['pattern']
     elif template_name == 'content':
         if len(rule["conditions"]) != 1:
             return None
         thecond = rule["conditions"][0]
         if "pattern" not in thecond:
             return None
-        ret = {k: rule[k] for k in ["rule_id", "actions"]}
-        ret["pattern"] = thecond["pattern"]
-        return ret
+        templaterule = {'actions': rule['actions']}
+        templaterule["pattern"] = thecond["pattern"]
+
+    if unscoped_rule_id:
+            templaterule['rule_id'] = unscoped_rule_id
+    if 'default' in rule:
+        templaterule['default'] = rule['default']
+    return templaterule
 
 
 def _strip_device_condition(rule):
@@ -386,6 +376,17 @@ def _strip_device_condition(rule):
     return rule
 
 
+def _namespaced_rule_id_from_spec(spec):
+    if spec['scope'] == 'global':
+        scope = 'global'
+    else:
+        scope = 'device/%s' % (spec['profile_tag'])
+    return "%s/%s/%s" % (scope, spec['template'], spec['rule_id'])
+
+
+def _rule_id_from_namespaced(in_rule_id):
+    return in_rule_id.split('/')[-1]
+
 class InvalidRuleException(Exception):
     pass
 
diff --git a/synapse/storage/push_rule.py b/synapse/storage/push_rule.py
index 27502d2399..30e23445d9 100644
--- a/synapse/storage/push_rule.py
+++ b/synapse/storage/push_rule.py
@@ -176,7 +176,7 @@ class PushRuleStore(SQLBaseStore):
         txn.execute(sql, new_rule.values())
 
     @defer.inlineCallbacks
-    def delete_push_rule(self, user_name, rule_id, **kwargs):
+    def delete_push_rule(self, user_name, rule_id):
         """
         Delete a push rule. Args specify the row to be deleted and can be
         any of the columns in the push_rule table, but below are the
@@ -186,7 +186,10 @@ class PushRuleStore(SQLBaseStore):
             user_name (str): The matrix ID of the push rule owner
             rule_id (str): The rule_id of the rule to be deleted
         """
-        yield self._simple_delete_one(PushRuleTable.table_name, kwargs)
+        yield self._simple_delete_one(
+            PushRuleTable.table_name,
+            {'user_name': user_name, 'rule_id': rule_id}
+        )
 
 
 class RuleNotFoundException(Exception):