Update sync method to leave out unneeded bytes

changelog.d/8336.bugfix

@@ -0,0 +1 @@
+Remove optional keys from `/sync` response according to spec.

synapse/rest/client/v2_alpha/sync.py

@@ -15,6 +15,7 @@
 
 import itertools
 import logging
+from typing import Dict
 
 from synapse.api.constants import PresenceState
 from synapse.api.errors import Codes, StoreError, SynapseError
@@ -72,6 +73,15 @@ class SyncRestServlet(RestServlet):
 
     PATTERNS = client_patterns("/sync$")
     ALLOWED_PRESENCE = {"online", "offline", "unavailable"}
+    SYNC_RESPONSE_PRUNE_KEYS = {
+        "account_data": True,
+        "to_device": True,
+        "device_lists": True,
+        "presence": True,
+        "rooms": True,
+        "groups": True,  # todo verify, not in spec
+        "device_one_time_keys_count": False,
+    }
 
     def __init__(self, hs):
         super(SyncRestServlet, self).__init__()
@@ -221,23 +231,54 @@ 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": {"join": joined, "invite": invited, "leave": archived},
-            "groups": {
-                "join": sync_result.groups.join,
-                "invite": sync_result.groups.invite,
-                "leave": sync_result.groups.leave,
+        return self.prune_response(
+            {
+                "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": {"join": joined, "invite": invited, "leave": archived},
+                "groups": {
+                    "join": sync_result.groups.join,
+                    "invite": sync_result.groups.invite,
+                    "leave": sync_result.groups.leave,
+                },
+                "device_one_time_keys_count": sync_result.device_one_time_keys_count,
+                "next_batch": sync_result.next_batch.to_string(),
             },
-            "device_one_time_keys_count": sync_result.device_one_time_keys_count,
-            "next_batch": sync_result.next_batch.to_string(),
-        }
+            self.SYNC_RESPONSE_PRUNE_KEYS,
+        )
+
+    @staticmethod
+    def prune_response(
+        obj: dict, keys_with_subkey_prune_allowed: Dict[str, bool]
+    ) -> dict:
+        def prunable(o: object) -> bool:
+            return isinstance(o, (list, dict, set))
+
+        for key, subkey_prune_allowed in keys_with_subkey_prune_allowed.items():
+            if key in obj:
+
+                if prunable(obj[key]):
+
+                    if not obj[key]:
+                        del obj[key]
+
+                    elif subkey_prune_allowed and isinstance(obj[key], dict):
+
+                        for subkey in list(obj[key].keys()):
+                            if prunable(obj[key][subkey]) and not obj[key][subkey]:
+                                del obj[key][subkey]
+
+                        if not obj[key]:
+                            del obj[key]
+
+        return obj
 
     @staticmethod
     def encode_presence(events, time_now):

tests/rest/client/v2_alpha/test_sync.py

@@ -14,6 +14,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import json
+import logging
 
 import synapse.rest.admin
 from synapse.api.constants import EventContentFields, EventTypes, RelationTypes
@@ -23,6 +24,8 @@
 from tests import unittest
 from tests.server import TimedOutException
 
+logger = logging.getLogger(__name__)
+
 
 class FilterTestCase(unittest.HomeserverTestCase):
 
@@ -38,18 +41,18 @@ def test_sync_argless(self):
         request, channel = self.make_request("GET", "/sync")
         self.render(request)
 
+        logger.info("test_sync_argless: json_body type: %s", type(channel.json_body))
+        logger.info("test_sync_argless: json_body: %s", repr(channel.json_body))
+        logger.info("test_sync_argless: json_body test: {}")
+
         self.assertEqual(channel.code, 200)
-        self.assertTrue(
-            {
-                "next_batch",
-                "rooms",
-                "presence",
-                "account_data",
-                "to_device",
-                "device_lists",
-            }.issubset(set(channel.json_body.keys()))
-        )
+        self.assertTrue({"next_batch"}.issubset(set(channel.json_body.keys())))
 
+    # fixme: the above test already checks for the minimal presence of "next_batch"
+    # in a /sync test, the spec allows /sync to be a JSON object with only next_batch
+    # as a key, should this following test exist? if not, how should it be rewritten?
+    # Explicit non-existence of "presence" key in /sync batch? Should the homeserver
+    # be poked with a presence-inducing event to be sure it does not propagate it?
     def test_sync_presence_disabled(self):
         """
         When presence is disabled, the key does not appear in /sync.
@@ -60,15 +63,7 @@ def test_sync_presence_disabled(self):
         self.render(request)
 
         self.assertEqual(channel.code, 200)
-        self.assertTrue(
-            {
-                "next_batch",
-                "rooms",
-                "account_data",
-                "to_device",
-                "device_lists",
-            }.issubset(set(channel.json_body.keys()))
-        )
+        self.assertTrue({"next_batch"}.issubset(set(channel.json_body.keys())))
 
 
 class SyncFilterTestCase(unittest.HomeserverTestCase):

tests/server_notices/test_consent.py

@@ -76,8 +76,11 @@ def test_get_sync_message(self):
         self.render(request)
         self.assertEqual(channel.code, 200)
 
+        invite_keys = list(channel.json_body.get("rooms", {}).get("invite", {}).keys())
+        self.assertTrue(len(invite_keys) > 0, channel.json_body)
+
         # Get the Room ID to join
-        room_id = list(channel.json_body["rooms"]["invite"].keys())[0]
+        room_id = invite_keys[0]
 
         # Join the room
         request, channel = self.make_request(

tests/server_notices/test_resource_limits_server_notices.py

@@ -308,7 +308,7 @@ def test_no_invite_without_notice(self):
         request, channel = self.make_request("GET", "/sync?timeout=0", access_token=tok)
         self.render(request)
 
-        invites = channel.json_body["rooms"]["invite"]
+        invites = channel.json_body.get("rooms", {}).get("invite", [])
         self.assertEqual(len(invites), 0, invites)
 
     def test_invite_with_notice(self):
@@ -366,7 +366,7 @@ 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"]
+            invites = channel.json_body.get("rooms", {}).get("invite", [])
 
         # Make sure we have an invite to process.
         self.assertEqual(len(invites), 1, invites)