From 566641a0b5f04e444383d5e3a5493b22e551ff03 Mon Sep 17 00:00:00 2001
From: pik <alexander.maznev@gmail.com>
Date: Thu, 23 Mar 2017 11:42:41 -0300
Subject: [PATCH] use jsonschema.FormatChecker for RoomID and UserID strings  *
 use a valid filter in rest/client/v2_alpha test

Signed-off-by: pik <alexander.maznev@gmail.com>
---
 synapse/api/filtering.py                  | 45 ++++++++++++++---------
 tests/api/test_filtering.py               | 15 ++++++--
 tests/rest/client/v2_alpha/test_filter.py |  4 +-
 3 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py
index 3d078e622..83206348e 100644
--- a/synapse/api/filtering.py
+++ b/synapse/api/filtering.py
@@ -19,6 +19,7 @@ from twisted.internet import defer
 
 import ujson as json
 import jsonschema
+from jsonschema import FormatChecker
 
 FILTER_SCHEMA = {
     "additionalProperties": False,
@@ -55,16 +56,10 @@ ROOM_FILTER_SCHEMA = {
     "type": "object",
     "properties": {
         "not_rooms": {
-            "type": "array",
-            "items": {
-                "type": "string"
-            }
+            "$ref": "#/definitions/room_id_array"
         },
         "rooms": {
-            "type": "array",
-            "items": {
-                "type": "string"
-            }
+            "$ref": "#/definitions/room_id_array"
         },
         "ephemeral": {
             "$ref": "#/definitions/room_event_filter"
@@ -110,16 +105,10 @@ ROOM_EVENT_FILTER_SCHEMA = {
             }
         },
         "rooms": {
-            "type": "array",
-            "items": {
-                "type": "string"
-            }
+            "$ref": "#/definitions/room_id_array"
         },
         "not_rooms": {
-            "type": "array",
-            "items": {
-                "type": "string"
-            }
+            "$ref": "#/definitions/room_id_array"
         },
         "contains_url": {
             "type": "boolean"
@@ -131,7 +120,15 @@ USER_ID_ARRAY_SCHEMA = {
     "type": "array",
     "items": {
         "type": "string",
-        "pattern": "^@[A-Za-z0-9_]+:[A-Za-z0-9_\-\.]+$"
+        "format": "matrix_user_id"
+    }
+}
+
+ROOM_ID_ARRAY_SCHEMA = {
+    "type": "array",
+    "items": {
+        "type": "string",
+        "format": "matrix_room_id"
     }
 }
 
@@ -140,6 +137,7 @@ USER_FILTER_SCHEMA = {
     "description": "schema for a Sync filter",
     "type": "object",
     "definitions": {
+        "room_id_array": ROOM_ID_ARRAY_SCHEMA,
         "user_id_array": USER_ID_ARRAY_SCHEMA,
         "filter": FILTER_SCHEMA,
         "room_filter": ROOM_FILTER_SCHEMA,
@@ -175,6 +173,16 @@ USER_FILTER_SCHEMA = {
 }
 
 
+@FormatChecker.cls_checks('matrix_room_id')
+def matrix_room_id_validator(room_id_str):
+    return RoomID.from_string(room_id_str)
+
+
+@FormatChecker.cls_checks('matrix_user_id')
+def matrix_user_id_validator(user_id_str):
+    return UserID.from_string(user_id_str)
+
+
 class Filtering(object):
 
     def __init__(self, hs):
@@ -208,7 +216,8 @@ class Filtering(object):
         # individual top-level key e.g. public_user_data. Filters are made of
         # many definitions.
         try:
-            jsonschema.validate(user_filter_json, USER_FILTER_SCHEMA)
+            jsonschema.validate(user_filter_json, USER_FILTER_SCHEMA,
+                                format_checker=FormatChecker())
         except jsonschema.ValidationError as e:
             raise SynapseError(400, e.message)
 
diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py
index 1ce1acb3c..dcceca7f3 100644
--- a/tests/api/test_filtering.py
+++ b/tests/api/test_filtering.py
@@ -25,6 +25,8 @@ from synapse.api.filtering import Filter
 from synapse.events import FrozenEvent
 from synapse.api.errors import SynapseError
 
+import jsonschema
+
 user_localpart = "test_user"
 
 
@@ -61,7 +63,9 @@ class FilteringTestCase(unittest.TestCase):
             {"account_data": "Hello World"},
             {"event_fields": ["\\foo"]},
             {"room": {"timeline": {"limit": 0}, "state": {"not_bars": ["*"]}}},
-            {"event_format": "other"}
+            {"event_format": "other"},
+            {"room": {"not_rooms": ["#foo:pik-test"]}},
+            {"presence": {"senders": ["@bar;pik.test.com"]}}
         ]
         for filter in invalid_filters:
             with self.assertRaises(SynapseError) as check_filter_error:
@@ -76,8 +80,8 @@ class FilteringTestCase(unittest.TestCase):
                     "state": {"not_types": ["m.room.member"]},
                     "ephemeral": {"limit": 0, "not_types": ["*"]},
                     "include_leave": False,
-                    "rooms": ["#dee:pik-test"],
-                    "not_rooms": ["#gee:pik-test"],
+                    "rooms": ["!dee:pik-test"],
+                    "not_rooms": ["!gee:pik-test"],
                     "account_data": {"limit": 0, "types": ["*"]}
                 }
             },
@@ -108,7 +112,10 @@ class FilteringTestCase(unittest.TestCase):
             }
         ]
         for filter in valid_filters:
-            self.filtering.check_valid_filter(filter)
+            try:
+                self.filtering.check_valid_filter(filter)
+            except jsonschema.ValidationError as e:
+                self.fail(e)
 
     def test_limits_are_applied(self):
         # TODO
diff --git a/tests/rest/client/v2_alpha/test_filter.py b/tests/rest/client/v2_alpha/test_filter.py
index 3d27d03cb..76b833e11 100644
--- a/tests/rest/client/v2_alpha/test_filter.py
+++ b/tests/rest/client/v2_alpha/test_filter.py
@@ -33,8 +33,8 @@ PATH_PREFIX = "/_matrix/client/v2_alpha"
 class FilterTestCase(unittest.TestCase):
 
     USER_ID = "@apple:test"
-    EXAMPLE_FILTER = {"type": ["m.*"]}
-    EXAMPLE_FILTER_JSON = '{"type": ["m.*"]}'
+    EXAMPLE_FILTER = {"room": {"timeline": {"types": ["m.room.message"]}}}
+    EXAMPLE_FILTER_JSON = '{"room": {"timeline": {"types": ["m.room.message"]}}}'
     TO_REGISTER = [filter]
 
     @defer.inlineCallbacks