From 4f43cf8f908db1478fa54a09cf86739e9deb3c00 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Wed, 5 May 2021 13:37:56 +0000 Subject: [PATCH 1/6] Leave out optional keys from /sync (#9919) This leaves out all optional keys from /sync. This should be fine for all clients tested against conduit already, but it may break some clients, as such we should check, that at least most of them don't break horribly and maybe back out some of the individual changes. (We can probably always leave out groups for example, while the others may cause more issues.) Signed-off-by: Nicolas Werner --- changelog.d/9919.feature | 1 + synapse/rest/client/v2_alpha/sync.py | 67 ++++++++++++------- tests/rest/client/v2_alpha/test_sync.py | 30 +-------- .../test_resource_limits_server_notices.py | 8 ++- 4 files changed, 51 insertions(+), 55 deletions(-) create mode 100644 changelog.d/9919.feature diff --git a/changelog.d/9919.feature b/changelog.d/9919.feature new file mode 100644 index 000000000000..07747505d243 --- /dev/null +++ b/changelog.d/9919.feature @@ -0,0 +1 @@ +Omit empty fields from the `/sync` response. Contributed by @deepbluev7. diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index 042e1788b649..6baee012d21b 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -13,6 +13,7 @@ # limitations under the License. import itertools import logging +from collections import defaultdict from typing import TYPE_CHECKING, Any, Callable, Dict, List, Tuple from synapse.api.constants import Membership, PresenceState @@ -232,29 +233,49 @@ async def encode_response(self, time_now, sync_result, access_token_id, filter): ) logger.debug("building sync response dict") - return { - "account_data": {"events": sync_result.account_data}, - "to_device": {"events": sync_result.to_device}, - "device_lists": { - "changed": list(sync_result.device_lists.changed), - "left": list(sync_result.device_lists.left), - }, - "presence": SyncRestServlet.encode_presence(sync_result.presence, time_now), - "rooms": { - Membership.JOIN: joined, - Membership.INVITE: invited, - Membership.KNOCK: knocked, - Membership.LEAVE: archived, - }, - "groups": { - Membership.JOIN: sync_result.groups.join, - Membership.INVITE: sync_result.groups.invite, - Membership.LEAVE: sync_result.groups.leave, - }, - "device_one_time_keys_count": sync_result.device_one_time_keys_count, - "org.matrix.msc2732.device_unused_fallback_key_types": sync_result.device_unused_fallback_key_types, - "next_batch": await sync_result.next_batch.to_string(self.store), - } + + response: dict = defaultdict(dict) + response["next_batch"] = await sync_result.next_batch.to_string(self.store) + + if sync_result.account_data: + response["account_data"] = {"events": sync_result.account_data} + if sync_result.presence: + response["presence"] = SyncRestServlet.encode_presence( + sync_result.presence, time_now + ) + + if sync_result.to_device: + response["to_device"] = {"events": sync_result.to_device} + + if sync_result.device_lists.changed: + response["device_lists"]["changed"] = list(sync_result.device_lists.changed) + if sync_result.device_lists.left: + response["device_lists"]["left"] = list(sync_result.device_lists.left) + + if sync_result.device_one_time_keys_count: + response[ + "device_one_time_keys_count" + ] = sync_result.device_one_time_keys_count + if sync_result.device_unused_fallback_key_types: + response[ + "org.matrix.msc2732.device_unused_fallback_key_types" + ] = sync_result.device_unused_fallback_key_types + + if joined: + response["rooms"]["join"] = joined + if invited: + response["rooms"]["invite"] = invited + if archived: + response["rooms"]["leave"] = archived + + if sync_result.groups.join: + response["groups"]["join"] = sync_result.groups.join + if sync_result.groups.invite: + response["groups"]["invite"] = sync_result.groups.invite + if sync_result.groups.leave: + response["groups"]["leave"] = sync_result.groups.leave + + return response @staticmethod def encode_presence(events, time_now): diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py index 012910f136f1..cdca3a3e2300 100644 --- a/tests/rest/client/v2_alpha/test_sync.py +++ b/tests/rest/client/v2_alpha/test_sync.py @@ -41,35 +41,7 @@ def test_sync_argless(self): channel = self.make_request("GET", "/sync") self.assertEqual(channel.code, 200) - self.assertTrue( - { - "next_batch", - "rooms", - "presence", - "account_data", - "to_device", - "device_lists", - }.issubset(set(channel.json_body.keys())) - ) - - def test_sync_presence_disabled(self): - """ - When presence is disabled, the key does not appear in /sync. - """ - self.hs.config.use_presence = False - - channel = self.make_request("GET", "/sync") - - self.assertEqual(channel.code, 200) - self.assertTrue( - { - "next_batch", - "rooms", - "account_data", - "to_device", - "device_lists", - }.issubset(set(channel.json_body.keys())) - ) + self.assertIn("next_batch", channel.json_body) class SyncFilterTestCase(unittest.HomeserverTestCase): diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py index d46521ccdc0f..3245aa91ca6e 100644 --- a/tests/server_notices/test_resource_limits_server_notices.py +++ b/tests/server_notices/test_resource_limits_server_notices.py @@ -306,8 +306,9 @@ def test_no_invite_without_notice(self): channel = self.make_request("GET", "/sync?timeout=0", access_token=tok) - invites = channel.json_body["rooms"]["invite"] - self.assertEqual(len(invites), 0, invites) + self.assertNotIn( + "rooms", channel.json_body, "Got invites without server notice" + ) def test_invite_with_notice(self): """Tests that, if the MAU limit is hit, the server notices user invites each user @@ -364,7 +365,8 @@ def _trigger_notice_and_join(self): # We could also pick another user and sync with it, which would return an # invite to a system notices room, but it doesn't matter which user we're # using so we use the last one because it saves us an extra sync. - invites = channel.json_body["rooms"]["invite"] + if "rooms" in channel.json_body: + invites = channel.json_body["rooms"]["invite"] # Make sure we have an invite to process. self.assertEqual(len(invites), 1, invites) From 391195913bb012aedbee6f89025c3498730f7754 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 18 Jun 2021 18:42:58 +0100 Subject: [PATCH 2/6] Add knock membership to optional sync response sections --- synapse/rest/client/v2_alpha/sync.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index 6baee012d21b..65d5f5efa204 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -265,6 +265,8 @@ async def encode_response(self, time_now, sync_result, access_token_id, filter): response["rooms"]["join"] = joined if invited: response["rooms"]["invite"] = invited + if knocked: + response["rooms"]["knock"] = knocked if archived: response["rooms"]["leave"] = archived From 15f833b2102aff8d8c3c95920607e68b98c93c01 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 18 Jun 2021 18:44:04 +0100 Subject: [PATCH 3/6] Replace str constants with Membership attributes --- synapse/rest/client/v2_alpha/sync.py | 14 +++++++------- tests/rest/client/v2_alpha/test_sync.py | 11 ++++++++--- .../test_resource_limits_server_notices.py | 13 ++++++++++--- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/synapse/rest/client/v2_alpha/sync.py b/synapse/rest/client/v2_alpha/sync.py index 65d5f5efa204..ecbbcf3851be 100644 --- a/synapse/rest/client/v2_alpha/sync.py +++ b/synapse/rest/client/v2_alpha/sync.py @@ -262,20 +262,20 @@ async def encode_response(self, time_now, sync_result, access_token_id, filter): ] = sync_result.device_unused_fallback_key_types if joined: - response["rooms"]["join"] = joined + response["rooms"][Membership.JOIN] = joined if invited: - response["rooms"]["invite"] = invited + response["rooms"][Membership.INVITE] = invited if knocked: - response["rooms"]["knock"] = knocked + response["rooms"][Membership.KNOCK] = knocked if archived: - response["rooms"]["leave"] = archived + response["rooms"][Membership.LEAVE] = archived if sync_result.groups.join: - response["groups"]["join"] = sync_result.groups.join + response["groups"][Membership.JOIN] = sync_result.groups.join if sync_result.groups.invite: - response["groups"]["invite"] = sync_result.groups.invite + response["groups"][Membership.INVITE] = sync_result.groups.invite if sync_result.groups.leave: - response["groups"]["leave"] = sync_result.groups.leave + response["groups"][Membership.LEAVE] = sync_result.groups.leave return response diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py index cdca3a3e2300..b1998f767d05 100644 --- a/tests/rest/client/v2_alpha/test_sync.py +++ b/tests/rest/client/v2_alpha/test_sync.py @@ -15,7 +15,12 @@ import json import synapse.rest.admin -from synapse.api.constants import EventContentFields, EventTypes, RelationTypes +from synapse.api.constants import ( + EventContentFields, + EventTypes, + Membership, + RelationTypes, +) from synapse.rest.client.v1 import login, room from synapse.rest.client.v2_alpha import knock, read_marker, sync @@ -343,7 +348,7 @@ def test_knock_room_state(self): # We expect to see the knock event in the stripped room state later self.expected_room_state[EventTypes.Member] = { - "content": {"membership": "knock", "displayname": "knocker"}, + "content": {"membership": Membership.KNOCK, "displayname": "knocker"}, "state_key": "@knocker:test", } @@ -356,7 +361,7 @@ def test_knock_room_state(self): self.assertEqual(channel.code, 200, channel.json_body) # Extract the stripped room state events from /sync - knock_entry = channel.json_body["rooms"]["knock"] + knock_entry = channel.json_body["rooms"][Membership.KNOCK] room_state_events = knock_entry[self.room_id]["knock_state"]["events"] # Validate that the knock membership event came last diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py index 3245aa91ca6e..3531fe9296dd 100644 --- a/tests/server_notices/test_resource_limits_server_notices.py +++ b/tests/server_notices/test_resource_limits_server_notices.py @@ -16,7 +16,12 @@ from twisted.internet import defer -from synapse.api.constants import EventTypes, LimitBlockingTypes, ServerNoticeMsgType +from synapse.api.constants import ( + EventTypes, + LimitBlockingTypes, + Membership, + ServerNoticeMsgType, +) from synapse.api.errors import ResourceLimitError from synapse.rest import admin from synapse.rest.client.v1 import login, room @@ -322,7 +327,9 @@ def test_invite_with_notice(self): # Scan the events in the room to search for a message from the server notices # user. - events = channel.json_body["rooms"]["join"][room_id]["timeline"]["events"] + events = channel.json_body["rooms"][Membership.JOIN][room_id]["timeline"][ + "events" + ] notice_in_room = False for event in events: if ( @@ -366,7 +373,7 @@ def _trigger_notice_and_join(self): # invite to a system notices room, but it doesn't matter which user we're # using so we use the last one because it saves us an extra sync. if "rooms" in channel.json_body: - invites = channel.json_body["rooms"]["invite"] + invites = channel.json_body["rooms"][Membership.INVITE] # Make sure we have an invite to process. self.assertEqual(len(invites), 1, invites) From 7978990f5139067e1868d100d223e28e4447d2fb Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 18 Jun 2021 19:34:27 +0100 Subject: [PATCH 4/6] Changelog --- changelog.d/10214.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10214.feature diff --git a/changelog.d/10214.feature b/changelog.d/10214.feature new file mode 100644 index 000000000000..a3818c9d25fb --- /dev/null +++ b/changelog.d/10214.feature @@ -0,0 +1 @@ +Omit empty fields from the `/sync` response. Contributed by @deepbluev7. \ No newline at end of file From 4e6c6709ae0f631570ad3c7827d48a8913a0be30 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 23 Jun 2021 15:17:57 +0100 Subject: [PATCH 5/6] Swap membership constants for str literals in tests To ensure that our constants are doing the right thing. --- tests/rest/client/v2_alpha/test_sync.py | 11 +++-------- .../test_resource_limits_server_notices.py | 13 +++---------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/tests/rest/client/v2_alpha/test_sync.py b/tests/rest/client/v2_alpha/test_sync.py index b1998f767d05..cdca3a3e2300 100644 --- a/tests/rest/client/v2_alpha/test_sync.py +++ b/tests/rest/client/v2_alpha/test_sync.py @@ -15,12 +15,7 @@ import json import synapse.rest.admin -from synapse.api.constants import ( - EventContentFields, - EventTypes, - Membership, - RelationTypes, -) +from synapse.api.constants import EventContentFields, EventTypes, RelationTypes from synapse.rest.client.v1 import login, room from synapse.rest.client.v2_alpha import knock, read_marker, sync @@ -348,7 +343,7 @@ def test_knock_room_state(self): # We expect to see the knock event in the stripped room state later self.expected_room_state[EventTypes.Member] = { - "content": {"membership": Membership.KNOCK, "displayname": "knocker"}, + "content": {"membership": "knock", "displayname": "knocker"}, "state_key": "@knocker:test", } @@ -361,7 +356,7 @@ def test_knock_room_state(self): self.assertEqual(channel.code, 200, channel.json_body) # Extract the stripped room state events from /sync - knock_entry = channel.json_body["rooms"][Membership.KNOCK] + knock_entry = channel.json_body["rooms"]["knock"] room_state_events = knock_entry[self.room_id]["knock_state"]["events"] # Validate that the knock membership event came last diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py index 3531fe9296dd..3245aa91ca6e 100644 --- a/tests/server_notices/test_resource_limits_server_notices.py +++ b/tests/server_notices/test_resource_limits_server_notices.py @@ -16,12 +16,7 @@ from twisted.internet import defer -from synapse.api.constants import ( - EventTypes, - LimitBlockingTypes, - Membership, - ServerNoticeMsgType, -) +from synapse.api.constants import EventTypes, LimitBlockingTypes, ServerNoticeMsgType from synapse.api.errors import ResourceLimitError from synapse.rest import admin from synapse.rest.client.v1 import login, room @@ -327,9 +322,7 @@ def test_invite_with_notice(self): # Scan the events in the room to search for a message from the server notices # user. - events = channel.json_body["rooms"][Membership.JOIN][room_id]["timeline"][ - "events" - ] + events = channel.json_body["rooms"]["join"][room_id]["timeline"]["events"] notice_in_room = False for event in events: if ( @@ -373,7 +366,7 @@ def _trigger_notice_and_join(self): # invite to a system notices room, but it doesn't matter which user we're # using so we use the last one because it saves us an extra sync. if "rooms" in channel.json_body: - invites = channel.json_body["rooms"][Membership.INVITE] + invites = channel.json_body["rooms"]["invite"] # Make sure we have an invite to process. self.assertEqual(len(invites), 1, invites) From c5b74a099080f5d151dd1d821a78ced388fab965 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 23 Jun 2021 15:18:41 +0100 Subject: [PATCH 6/6] Remove #9919 changelog --- changelog.d/9919.feature | 1 - 1 file changed, 1 deletion(-) delete mode 100644 changelog.d/9919.feature diff --git a/changelog.d/9919.feature b/changelog.d/9919.feature deleted file mode 100644 index 07747505d243..000000000000 --- a/changelog.d/9919.feature +++ /dev/null @@ -1 +0,0 @@ -Omit empty fields from the `/sync` response. Contributed by @deepbluev7.