From bbd10e3e7b9425d4fd299c4d96e6f2a30def9b1d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 21 Jan 2022 13:43:02 -0500 Subject: [PATCH 1/5] Remove an unused variable. --- synapse/handlers/search.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py index 02bb5ae72f51..461cad0bae4e 100644 --- a/synapse/handlers/search.py +++ b/synapse/handlers/search.py @@ -238,8 +238,6 @@ async def search( results = search_result["results"] - results_map = {r["event"].event_id: r for r in results} - rank_map.update({r["event"].event_id: r["rank"] for r in results}) filtered_events = await search_filter.filter([r["event"] for r in results]) From 4b3aebefd673c3aa7440886a202a855ae03046dd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 21 Jan 2022 14:04:06 -0500 Subject: [PATCH 2/5] Include aggregations in results. --- synapse/config/experimental.py | 2 ++ synapse/handlers/search.py | 27 +++++++++++++++++--- tests/rest/client/test_relations.py | 39 ++++++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 4 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index dbaeb1091861..6b15f0f563b8 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -28,6 +28,8 @@ def read_config(self, config: JsonDict, **kwargs): self.msc1849_enabled = config.get("experimental_msc1849_support_enabled", True) # MSC3440 (thread relation) self.msc3440_enabled: bool = experimental.get("msc3440_enabled", False) + # MSC3666: including bundled relations in /search. + self.msc3666_enabled: bool = experimental.get("msc3666_enabled", False) # MSC3026 (busy presence state) self.msc3026_enabled: bool = experimental.get("msc3026_enabled", False) diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py index 461cad0bae4e..41cb80907893 100644 --- a/synapse/handlers/search.py +++ b/synapse/handlers/search.py @@ -43,6 +43,8 @@ def __init__(self, hs: "HomeServer"): self.state_store = self.storage.state self.auth = hs.get_auth() + self._msc3666_enabled = hs.config.experimental.msc3666_enabled + async def get_old_rooms_from_upgraded_room(self, room_id: str) -> Iterable[str]: """Retrieves room IDs of old rooms in the history of an upgraded room. @@ -418,12 +420,29 @@ async def search( time_now = self.clock.time_msec() + aggregations = None + if self._msc3666_enabled: + aggregations = await self.store.get_bundled_aggregations( + # Generate an iterable of EventBase for all the events that will be + # returned, including contextual events. + itertools.chain( + # The events_before and events_after for each context. + itertools.chain.from_iterable( + itertools.chain(context["events_before"], context["events_after"]) # type: ignore[arg-type] + for context in contexts.values() + ), + # The returned events. + allowed_events, + ), + user.to_string(), + ) + for context in contexts.values(): context["events_before"] = self._event_serializer.serialize_events( - context["events_before"], time_now # type: ignore[arg-type] + context["events_before"], time_now, bundle_aggregations=aggregations # type: ignore[arg-type] ) context["events_after"] = self._event_serializer.serialize_events( - context["events_after"], time_now # type: ignore[arg-type] + context["events_after"], time_now, bundle_aggregations=aggregations # type: ignore[arg-type] ) state_results = {} @@ -440,7 +459,9 @@ async def search( results.append( { "rank": rank_map[e.event_id], - "result": self._event_serializer.serialize_event(e, time_now), + "result": self._event_serializer.serialize_event( + e, time_now, bundle_aggregations=aggregations + ), "context": contexts.get(e.event_id, {}), } ) diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index 96ae7790bb15..06721e67c9c0 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -453,7 +453,9 @@ def test_aggregation_must_be_annotation(self): ) self.assertEquals(400, channel.code, channel.json_body) - @unittest.override_config({"experimental_features": {"msc3440_enabled": True}}) + @unittest.override_config( + {"experimental_features": {"msc3440_enabled": True, "msc3666_enabled": True}} + ) def test_bundled_aggregations(self): """ Test that annotations, references, and threads get correctly bundled. @@ -579,6 +581,23 @@ def assert_bundle(event_json: JsonDict) -> None: self.assertTrue(room_timeline["limited"]) assert_bundle(self._find_event_in_chunk(room_timeline["events"])) + # Request search. + channel = self.make_request( + "POST", + "/search", + # Search term matches the parent message. + content={"search_categories": {"room_events": {"search_term": "Hi"}}}, + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + chunk = [ + result["result"] + for result in channel.json_body["search_categories"]["room_events"][ + "results" + ] + ] + assert_bundle(self._find_event_in_chunk(chunk)) + def test_aggregation_get_event_for_annotation(self): """Test that annotations do not get bundled aggregations included when directly requested. @@ -759,6 +778,7 @@ def test_ignore_invalid_room(self): self.assertEquals(200, channel.code, channel.json_body) self.assertNotIn("m.relations", channel.json_body["unsigned"]) + @unittest.override_config({"experimental_features": {"msc3666_enabled": True}}) def test_edit(self): """Test that a simple edit works.""" @@ -825,6 +845,23 @@ def assert_bundle(event_json: JsonDict) -> None: self.assertTrue(room_timeline["limited"]) assert_bundle(self._find_event_in_chunk(room_timeline["events"])) + # Request search. + channel = self.make_request( + "POST", + "/search", + # Search term matches the parent message. + content={"search_categories": {"room_events": {"search_term": "Hi"}}}, + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + chunk = [ + result["result"] + for result in channel.json_body["search_categories"]["room_events"][ + "results" + ] + ] + assert_bundle(self._find_event_in_chunk(chunk)) + def test_multi_edit(self): """Test that multiple edits, including attempts by people who shouldn't be allowed, are correctly handled. From 1de1e58a2291dcd929423262039208d7de7b36d3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 26 Jan 2022 13:51:57 -0500 Subject: [PATCH 3/5] Newsfragment --- changelog.d/11837.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11837.feature diff --git a/changelog.d/11837.feature b/changelog.d/11837.feature new file mode 100644 index 000000000000..62ef707123db --- /dev/null +++ b/changelog.d/11837.feature @@ -0,0 +1 @@ +Experimental support for [MSC3666](https://github.com/matrix-org/matrix-doc/pull/3666): including bundled aggregations in server side search results. From adfdc646120666772f2e5176694eb2b4e488e1fd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 7 Feb 2022 15:26:06 -0500 Subject: [PATCH 4/5] Only request bundled aggregations for a single event once. --- synapse/storage/databases/main/relations.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/synapse/storage/databases/main/relations.py b/synapse/storage/databases/main/relations.py index 37468a518381..69289bceabf9 100644 --- a/synapse/storage/databases/main/relations.py +++ b/synapse/storage/databases/main/relations.py @@ -682,10 +682,20 @@ async def get_bundled_aggregations( A map of event ID to the bundled aggregation for the event. Not all events may have bundled aggregations in the results. """ + # The already processed event IDs. Tracked separately from the result + # since the result omits events which do not have bundled aggregations. + seen_events = set() # TODO Parallelize. results = {} for event in events: + # De-duplicate events by ID to handle the same event requested multiple + # times. The caches that _get_bundled_aggregation_for_event use should + # capture this, but best to reduce work. + if event.event_id in seen_events: + continue + seen_events.add(event.event_id) + event_result = await self._get_bundled_aggregation_for_event(event, user_id) if event_result: results[event.event_id] = event_result From 3c2336ba80c2a54aa353490bc60336dc1c6b0426 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 8 Feb 2022 07:57:13 -0500 Subject: [PATCH 5/5] Fix some inefficiencies due to a merge. --- synapse/storage/databases/main/relations.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/synapse/storage/databases/main/relations.py b/synapse/storage/databases/main/relations.py index 44f98dcec615..7718acbf1cb8 100644 --- a/synapse/storage/databases/main/relations.py +++ b/synapse/storage/databases/main/relations.py @@ -717,7 +717,7 @@ async def get_bundled_aggregations( """ # The already processed event IDs. Tracked separately from the result # since the result omits events which do not have bundled aggregations. - seen_events = set() + seen_event_ids = set() # State events and redacted events do not get bundled aggregations. events = [ @@ -734,17 +734,16 @@ async def get_bundled_aggregations( # De-duplicate events by ID to handle the same event requested multiple # times. The caches that _get_bundled_aggregation_for_event use should # capture this, but best to reduce work. - if event.event_id in seen_events: + if event.event_id in seen_event_ids: continue - seen_events.add(event.event_id) + seen_event_ids.add(event.event_id) event_result = await self._get_bundled_aggregation_for_event(event, user_id) if event_result: results[event.event_id] = event_result # Fetch any edits. - event_ids = [event.event_id for event in events] - edits = await self._get_applicable_edits(event_ids) + edits = await self._get_applicable_edits(seen_event_ids) for event_id, edit in edits.items(): results.setdefault(event_id, BundledAggregations()).replace = edit