Do not try to serialize raw aggregations dict. (#11791)
							parent
							
								
									bfe6d5553a
								
							
						
					
					
						commit
						7a11509d17
					
				|  | @ -0,0 +1 @@ | |||
| Include the bundled aggregations in the `/sync` response, per [MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675). | ||||
|  | @ -1 +0,0 @@ | |||
| Avoid database access in the JSON serialization process. | ||||
|  | @ -0,0 +1 @@ | |||
| Include the bundled aggregations in the `/sync` response, per [MSC2675](https://github.com/matrix-org/matrix-doc/pull/2675). | ||||
|  | @ -402,7 +402,7 @@ class EventClientSerializer: | |||
|         if bundle_aggregations: | ||||
|             event_aggregations = bundle_aggregations.get(event.event_id) | ||||
|             if event_aggregations: | ||||
|                 self._injected_bundled_aggregations( | ||||
|                 self._inject_bundled_aggregations( | ||||
|                     event, | ||||
|                     time_now, | ||||
|                     bundle_aggregations[event.event_id], | ||||
|  | @ -411,7 +411,7 @@ class EventClientSerializer: | |||
| 
 | ||||
|         return serialized_event | ||||
| 
 | ||||
|     def _injected_bundled_aggregations( | ||||
|     def _inject_bundled_aggregations( | ||||
|         self, | ||||
|         event: EventBase, | ||||
|         time_now: int, | ||||
|  |  | |||
|  | @ -744,20 +744,15 @@ class RoomEventContextServlet(RestServlet): | |||
|             ) | ||||
| 
 | ||||
|         time_now = self.clock.time_msec() | ||||
|         aggregations = results.pop("aggregations", None) | ||||
|         results["events_before"] = self._event_serializer.serialize_events( | ||||
|             results["events_before"], | ||||
|             time_now, | ||||
|             bundle_aggregations=results["aggregations"], | ||||
|             results["events_before"], time_now, bundle_aggregations=aggregations | ||||
|         ) | ||||
|         results["event"] = self._event_serializer.serialize_event( | ||||
|             results["event"], | ||||
|             time_now, | ||||
|             bundle_aggregations=results["aggregations"], | ||||
|             results["event"], time_now, bundle_aggregations=aggregations | ||||
|         ) | ||||
|         results["events_after"] = self._event_serializer.serialize_events( | ||||
|             results["events_after"], | ||||
|             time_now, | ||||
|             bundle_aggregations=results["aggregations"], | ||||
|             results["events_after"], time_now, bundle_aggregations=aggregations | ||||
|         ) | ||||
|         results["state"] = self._event_serializer.serialize_events( | ||||
|             results["state"], time_now | ||||
|  |  | |||
|  | @ -714,18 +714,15 @@ class RoomEventContextServlet(RestServlet): | |||
|             raise SynapseError(404, "Event not found.", errcode=Codes.NOT_FOUND) | ||||
| 
 | ||||
|         time_now = self.clock.time_msec() | ||||
|         aggregations = results.pop("aggregations", None) | ||||
|         results["events_before"] = self._event_serializer.serialize_events( | ||||
|             results["events_before"], | ||||
|             time_now, | ||||
|             bundle_aggregations=results["aggregations"], | ||||
|             results["events_before"], time_now, bundle_aggregations=aggregations | ||||
|         ) | ||||
|         results["event"] = self._event_serializer.serialize_event( | ||||
|             results["event"], time_now, bundle_aggregations=results["aggregations"] | ||||
|             results["event"], time_now, bundle_aggregations=aggregations | ||||
|         ) | ||||
|         results["events_after"] = self._event_serializer.serialize_events( | ||||
|             results["events_after"], | ||||
|             time_now, | ||||
|             bundle_aggregations=results["aggregations"], | ||||
|             results["events_after"], time_now, bundle_aggregations=aggregations | ||||
|         ) | ||||
|         results["state"] = self._event_serializer.serialize_events( | ||||
|             results["state"], time_now | ||||
|  |  | |||
|  | @ -21,6 +21,7 @@ from unittest.mock import patch | |||
| from synapse.api.constants import EventTypes, RelationTypes | ||||
| from synapse.rest import admin | ||||
| from synapse.rest.client import login, register, relations, room, sync | ||||
| from synapse.types import JsonDict | ||||
| 
 | ||||
| from tests import unittest | ||||
| from tests.server import FakeChannel | ||||
|  | @ -454,7 +455,14 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
| 
 | ||||
|     @unittest.override_config({"experimental_features": {"msc3440_enabled": True}}) | ||||
|     def test_bundled_aggregations(self): | ||||
|         """Test that annotations, references, and threads get correctly bundled.""" | ||||
|         """ | ||||
|         Test that annotations, references, and threads get correctly bundled. | ||||
| 
 | ||||
|         Note that this doesn't test against /relations since only thread relations | ||||
|         get bundled via that API. See test_aggregation_get_event_for_thread. | ||||
| 
 | ||||
|         See test_edit for a similar test for edits. | ||||
|         """ | ||||
|         # Setup by sending a variety of relations. | ||||
|         channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", "a") | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|  | @ -482,12 +490,13 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         thread_2 = channel.json_body["event_id"] | ||||
| 
 | ||||
|         def assert_bundle(actual): | ||||
|         def assert_bundle(event_json: JsonDict) -> None: | ||||
|             """Assert the expected values of the bundled aggregations.""" | ||||
|             relations_dict = event_json["unsigned"].get("m.relations") | ||||
| 
 | ||||
|             # Ensure the fields are as expected. | ||||
|             self.assertCountEqual( | ||||
|                 actual.keys(), | ||||
|                 relations_dict.keys(), | ||||
|                 ( | ||||
|                     RelationTypes.ANNOTATION, | ||||
|                     RelationTypes.REFERENCE, | ||||
|  | @ -503,20 +512,20 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
|                         {"type": "m.reaction", "key": "b", "count": 1}, | ||||
|                     ] | ||||
|                 }, | ||||
|                 actual[RelationTypes.ANNOTATION], | ||||
|                 relations_dict[RelationTypes.ANNOTATION], | ||||
|             ) | ||||
| 
 | ||||
|             self.assertEquals( | ||||
|                 {"chunk": [{"event_id": reply_1}, {"event_id": reply_2}]}, | ||||
|                 actual[RelationTypes.REFERENCE], | ||||
|                 relations_dict[RelationTypes.REFERENCE], | ||||
|             ) | ||||
| 
 | ||||
|             self.assertEquals( | ||||
|                 2, | ||||
|                 actual[RelationTypes.THREAD].get("count"), | ||||
|                 relations_dict[RelationTypes.THREAD].get("count"), | ||||
|             ) | ||||
|             self.assertTrue( | ||||
|                 actual[RelationTypes.THREAD].get("current_user_participated") | ||||
|                 relations_dict[RelationTypes.THREAD].get("current_user_participated") | ||||
|             ) | ||||
|             # The latest thread event has some fields that don't matter. | ||||
|             self.assert_dict( | ||||
|  | @ -533,20 +542,9 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
|                     "type": "m.room.test", | ||||
|                     "user_id": self.user_id, | ||||
|                 }, | ||||
|                 actual[RelationTypes.THREAD].get("latest_event"), | ||||
|                 relations_dict[RelationTypes.THREAD].get("latest_event"), | ||||
|             ) | ||||
| 
 | ||||
|         def _find_and_assert_event(events): | ||||
|             """ | ||||
|             Find the parent event in a chunk of events and assert that it has the proper bundled aggregations. | ||||
|             """ | ||||
|             for event in events: | ||||
|                 if event["event_id"] == self.parent_id: | ||||
|                     break | ||||
|             else: | ||||
|                 raise AssertionError(f"Event {self.parent_id} not found in chunk") | ||||
|             assert_bundle(event["unsigned"].get("m.relations")) | ||||
| 
 | ||||
|         # Request the event directly. | ||||
|         channel = self.make_request( | ||||
|             "GET", | ||||
|  | @ -554,7 +552,7 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
|             access_token=self.user_token, | ||||
|         ) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         assert_bundle(channel.json_body["unsigned"].get("m.relations")) | ||||
|         assert_bundle(channel.json_body) | ||||
| 
 | ||||
|         # Request the room messages. | ||||
|         channel = self.make_request( | ||||
|  | @ -563,7 +561,7 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
|             access_token=self.user_token, | ||||
|         ) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         _find_and_assert_event(channel.json_body["chunk"]) | ||||
|         assert_bundle(self._find_event_in_chunk(channel.json_body["chunk"])) | ||||
| 
 | ||||
|         # Request the room context. | ||||
|         channel = self.make_request( | ||||
|  | @ -572,17 +570,14 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
|             access_token=self.user_token, | ||||
|         ) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         assert_bundle(channel.json_body["event"]["unsigned"].get("m.relations")) | ||||
|         assert_bundle(channel.json_body["event"]) | ||||
| 
 | ||||
|         # Request sync. | ||||
|         channel = self.make_request("GET", "/sync", access_token=self.user_token) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         room_timeline = channel.json_body["rooms"]["join"][self.room]["timeline"] | ||||
|         self.assertTrue(room_timeline["limited"]) | ||||
|         _find_and_assert_event(room_timeline["events"]) | ||||
| 
 | ||||
|         # Note that /relations is tested separately in test_aggregation_get_event_for_thread | ||||
|         # since it needs different data configured. | ||||
|         self._find_event_in_chunk(room_timeline["events"]) | ||||
| 
 | ||||
|     def test_aggregation_get_event_for_annotation(self): | ||||
|         """Test that annotations do not get bundled aggregations included | ||||
|  | @ -777,25 +772,58 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
| 
 | ||||
|         edit_event_id = channel.json_body["event_id"] | ||||
| 
 | ||||
|         def assert_bundle(event_json: JsonDict) -> None: | ||||
|             """Assert the expected values of the bundled aggregations.""" | ||||
|             relations_dict = event_json["unsigned"].get("m.relations") | ||||
|             self.assertIn(RelationTypes.REPLACE, relations_dict) | ||||
| 
 | ||||
|             m_replace_dict = relations_dict[RelationTypes.REPLACE] | ||||
|             for key in ["event_id", "sender", "origin_server_ts"]: | ||||
|                 self.assertIn(key, m_replace_dict) | ||||
| 
 | ||||
|             self.assert_dict( | ||||
|                 {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict | ||||
|             ) | ||||
| 
 | ||||
|         channel = self.make_request( | ||||
|             "GET", | ||||
|             "/rooms/%s/event/%s" % (self.room, self.parent_id), | ||||
|             f"/rooms/{self.room}/event/{self.parent_id}", | ||||
|             access_token=self.user_token, | ||||
|         ) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
| 
 | ||||
|         self.assertEquals(channel.json_body["content"], new_body) | ||||
|         assert_bundle(channel.json_body) | ||||
| 
 | ||||
|         relations_dict = channel.json_body["unsigned"].get("m.relations") | ||||
|         self.assertIn(RelationTypes.REPLACE, relations_dict) | ||||
| 
 | ||||
|         m_replace_dict = relations_dict[RelationTypes.REPLACE] | ||||
|         for key in ["event_id", "sender", "origin_server_ts"]: | ||||
|             self.assertIn(key, m_replace_dict) | ||||
| 
 | ||||
|         self.assert_dict( | ||||
|             {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict | ||||
|         # Request the room messages. | ||||
|         channel = self.make_request( | ||||
|             "GET", | ||||
|             f"/rooms/{self.room}/messages?dir=b", | ||||
|             access_token=self.user_token, | ||||
|         ) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         assert_bundle(self._find_event_in_chunk(channel.json_body["chunk"])) | ||||
| 
 | ||||
|         # Request the room context. | ||||
|         channel = self.make_request( | ||||
|             "GET", | ||||
|             f"/rooms/{self.room}/context/{self.parent_id}", | ||||
|             access_token=self.user_token, | ||||
|         ) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         assert_bundle(channel.json_body["event"]) | ||||
| 
 | ||||
|         # Request sync, but limit the timeline so it becomes limited (and includes | ||||
|         # bundled aggregations). | ||||
|         filter = urllib.parse.quote_plus( | ||||
|             '{"room": {"timeline": {"limit": 2}}}'.encode() | ||||
|         ) | ||||
|         channel = self.make_request( | ||||
|             "GET", f"/sync?filter={filter}", access_token=self.user_token | ||||
|         ) | ||||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         room_timeline = channel.json_body["rooms"]["join"][self.room]["timeline"] | ||||
|         self.assertTrue(room_timeline["limited"]) | ||||
|         assert_bundle(self._find_event_in_chunk(room_timeline["events"])) | ||||
| 
 | ||||
|     def test_multi_edit(self): | ||||
|         """Test that multiple edits, including attempts by people who | ||||
|  | @ -1102,6 +1130,16 @@ class RelationsTestCase(unittest.HomeserverTestCase): | |||
|         self.assertEquals(200, channel.code, channel.json_body) | ||||
|         self.assertEquals(channel.json_body["chunk"], []) | ||||
| 
 | ||||
|     def _find_event_in_chunk(self, events: List[JsonDict]) -> JsonDict: | ||||
|         """ | ||||
|         Find the parent event in a chunk of events and assert that it has the proper bundled aggregations. | ||||
|         """ | ||||
|         for event in events: | ||||
|             if event["event_id"] == self.parent_id: | ||||
|                 return event | ||||
| 
 | ||||
|         raise AssertionError(f"Event {self.parent_id} not found in chunk") | ||||
| 
 | ||||
|     def _send_relation( | ||||
|         self, | ||||
|         relation_type: str, | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 Patrick Cloke
						Patrick Cloke