From f97511a1f3197c6011b5ef7a363885dde9939d6b Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 21 Nov 2016 17:42:16 +0000 Subject: Move event_fields filtering to serialize_event Also make it an inclusive not exclusive filter, as the spec demands. --- tests/events/test_utils.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'tests/events') diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index fb0953c4ec..b9f55d174d 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -114,3 +114,24 @@ class PruneEventTestCase(unittest.TestCase): 'unsigned': {}, } ) + + +class SerializeEventTestCase(unittest.TestCase): + + def test_event_fields_works_with_keys(self): + pass + + def test_event_fields_works_with_nested_keys(self): + pass + + def test_event_fields_works_with_dot_keys(self): + pass + + def test_event_fields_works_with_nested_dot_keys(self): + pass + + def test_event_fields_nops_with_unknown_keys(self): + pass + + def test_event_fields_nops_with_non_dict_keys(self): + pass -- cgit 1.4.1 From 70a2157b6458369b374cceeb0e5c8b0d985c6946 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 21 Nov 2016 17:52:45 +0000 Subject: Start adding some tests --- synapse/events/utils.py | 4 +++- tests/events/test_utils.py | 40 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) (limited to 'tests/events') diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 4febd98f43..a14d9bd0ca 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -16,6 +16,8 @@ from synapse.api.constants import EventTypes from . import EventBase +from frozendict import frozendict + import re # Split strings on "." but not "\." This uses a negative lookbehind assertion for '\' @@ -130,7 +132,7 @@ def _copy_field(src, dst, field): key_to_move = field.pop(-1) sub_dict = src for sub_field in field: # e.g. sub_field => "content" - if sub_field in sub_dict and type(sub_dict[sub_field]) == dict: + if sub_field in sub_dict and type(sub_dict[sub_field]) == frozendict: sub_dict = sub_dict[sub_field] else: return diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index b9f55d174d..7136cca7c2 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -17,7 +17,11 @@ from .. import unittest from synapse.events import FrozenEvent -from synapse.events.utils import prune_event +from synapse.events.utils import prune_event, serialize_event + + +def MockEvent(**kwargs): + return FrozenEvent(kwargs) class PruneEventTestCase(unittest.TestCase): @@ -118,11 +122,41 @@ class PruneEventTestCase(unittest.TestCase): class SerializeEventTestCase(unittest.TestCase): + def serialize(self, ev, fields): + return serialize_event(ev, 1924354, event_fields=fields) + def test_event_fields_works_with_keys(self): - pass + self.assertEquals( + self.serialize( + MockEvent( + sender="@alice:localhost", + room_id="!foo:bar" + ), + ["room_id"] + ), + { + "room_id": "!foo:bar", + } + ) def test_event_fields_works_with_nested_keys(self): - pass + self.assertEquals( + self.serialize( + MockEvent( + sender="@alice:localhost", + room_id="!foo:bar", + content={ + "body": "A message", + }, + ), + ["content.body"] + ), + { + "content": { + "body": "A message", + } + } + ) def test_event_fields_works_with_dot_keys(self): pass -- cgit 1.4.1 From 53b27bbf06f3fc23343d510da70ef1edc7f182d8 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 21 Nov 2016 17:58:22 +0000 Subject: Add remaining tests --- tests/events/test_utils.py | 74 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 4 deletions(-) (limited to 'tests/events') diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index 7136cca7c2..d415e4cb3b 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -159,13 +159,79 @@ class SerializeEventTestCase(unittest.TestCase): ) def test_event_fields_works_with_dot_keys(self): - pass + self.assertEquals( + self.serialize( + MockEvent( + sender="@alice:localhost", + room_id="!foo:bar", + content={ + "key.with.dots": {}, + }, + ), + ["content.key\.with\.dots"] + ), + { + "content": { + "key.with.dots": {}, + } + } + ) def test_event_fields_works_with_nested_dot_keys(self): - pass + self.assertEquals( + self.serialize( + MockEvent( + sender="@alice:localhost", + room_id="!foo:bar", + content={ + "not_me": 1, + "nested.dot.key": { + "leaf.key": 42, + "not_me_either": 1, + }, + }, + ), + ["content.nested\.dot\.key.leaf\.key"] + ), + { + "content": { + "nested.dot.key": { + "leaf.key": 42, + }, + } + } + ) def test_event_fields_nops_with_unknown_keys(self): - pass + self.assertEquals( + self.serialize( + MockEvent( + sender="@alice:localhost", + room_id="!foo:bar", + content={ + "foo": "bar", + }, + ), + ["content.foo", "content.notexists"] + ), + { + "content": { + "foo": "bar", + } + } + ) def test_event_fields_nops_with_non_dict_keys(self): - pass + self.assertEquals( + self.serialize( + MockEvent( + sender="@alice:localhost", + room_id="!foo:bar", + content={ + "foo": ["I", "am", "an", "array"], + }, + ), + ["content.foo.am"] + ), + {} + ) -- cgit 1.4.1 From 0a8b0eeca17442a839d9f3a8624e331604b74711 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 22 Nov 2016 09:59:27 +0000 Subject: More tests --- synapse/events/utils.py | 7 +++--- tests/events/test_utils.py | 57 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 4 deletions(-) (limited to 'tests/events') diff --git a/synapse/events/utils.py b/synapse/events/utils.py index a14d9bd0ca..9a700d39bb 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -229,7 +229,7 @@ def format_event_for_client_v2_without_room_id(d): def serialize_event(e, time_now_ms, as_client_event=True, event_format=format_event_for_client_v1, - token_id=None, event_fields=None): + token_id=None, only_event_fields=None): # FIXME(erikj): To handle the case of presence events and the like if not isinstance(e, EventBase): return e @@ -258,7 +258,8 @@ def serialize_event(e, time_now_ms, as_client_event=True, if as_client_event: d = event_format(d) - if isinstance(event_fields, list): - d = only_fields(d, event_fields) + if (isinstance(only_event_fields, list) and + all(isinstance(f, basestring) for f in only_event_fields)): + d = only_fields(d, only_event_fields) return d diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index d415e4cb3b..5b3326ce8d 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -123,7 +123,7 @@ class PruneEventTestCase(unittest.TestCase): class SerializeEventTestCase(unittest.TestCase): def serialize(self, ev, fields): - return serialize_event(ev, 1924354, event_fields=fields) + return serialize_event(ev, 1479807801915, only_event_fields=fields) def test_event_fields_works_with_keys(self): self.assertEquals( @@ -235,3 +235,58 @@ class SerializeEventTestCase(unittest.TestCase): ), {} ) + + def test_event_fields_nops_with_array_keys(self): + self.assertEquals( + self.serialize( + MockEvent( + sender="@alice:localhost", + room_id="!foo:bar", + content={ + "foo": ["I", "am", "an", "array"], + }, + ), + ["content.foo.1"] + ), + {} + ) + + def test_event_fields_all_fields_if_empty(self): + self.assertEquals( + self.serialize( + MockEvent( + room_id="!foo:bar", + content={ + "foo": "bar", + }, + ), + [] + ), + { + "room_id": "!foo:bar", + "content": { + "foo": "bar", + }, + "unsigned": {} + } + ) + + def test_event_fields_fail_if_fields_not_str(self): + self.assertEquals( + self.serialize( + MockEvent( + room_id="!foo:bar", + content={ + "foo": "bar", + }, + ), + ["room_id", 4] + ), + { + "room_id": "!foo:bar", + "content": { + "foo": "bar", + }, + "unsigned": {} + } + ) -- cgit 1.4.1 From c3d963ac2405d601fff86421156dc0ba543499b6 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 22 Nov 2016 13:42:11 +0000 Subject: Review comments --- synapse/events/utils.py | 20 +++++++++----------- tests/events/test_utils.py | 12 ++---------- 2 files changed, 11 insertions(+), 21 deletions(-) (limited to 'tests/events') diff --git a/synapse/events/utils.py b/synapse/events/utils.py index f4b21ca517..5bbaef8187 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -145,9 +145,7 @@ def _copy_field(src, dst, field): # the empty objects if the key didn't exist. sub_out_dict = dst for sub_field in field: - if sub_field not in sub_out_dict: - sub_out_dict[sub_field] = {} - sub_out_dict = sub_out_dict[sub_field] + sub_out_dict = sub_out_dict.setdefault(sub_field, {}) sub_out_dict[key_to_move] = sub_dict[key_to_move] @@ -176,12 +174,10 @@ def only_fields(dictionary, fields): split_fields = [SPLIT_FIELD_REGEX.split(f) for f in fields] # for each element of the output array of arrays: - # remove escaping so we can use the right key names. This purposefully avoids - # using list comprehensions to avoid needless allocations as this may be called - # on a lot of events. - for field_array in split_fields: - for i, field in enumerate(field_array): - field_array[i] = field.replace(r'\.', r'.') + # remove escaping so we can use the right key names. + split_fields[:] = [ + [f.replace(r'\.', r'.') for f in field_array] for field_array in split_fields + ] output = {} for field_array in split_fields: @@ -258,8 +254,10 @@ def serialize_event(e, time_now_ms, as_client_event=True, if as_client_event: d = event_format(d) - if (only_event_fields and isinstance(only_event_fields, list) and - all(isinstance(f, basestring) for f in only_event_fields)): + if only_event_fields: + if (not isinstance(only_event_fields, list) or + not all(isinstance(f, basestring) for f in only_event_fields)): + raise TypeError("only_event_fields must be a list of strings") d = only_fields(d, only_event_fields) return d diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index 5b3326ce8d..29f068d1f1 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -272,7 +272,7 @@ class SerializeEventTestCase(unittest.TestCase): ) def test_event_fields_fail_if_fields_not_str(self): - self.assertEquals( + with self.assertRaises(TypeError): self.serialize( MockEvent( room_id="!foo:bar", @@ -281,12 +281,4 @@ class SerializeEventTestCase(unittest.TestCase): }, ), ["room_id", 4] - ), - { - "room_id": "!foo:bar", - "content": { - "foo": "bar", - }, - "unsigned": {} - } - ) + ) -- cgit 1.4.1