From 9d2497ac412c86348d6a6a52f686eb4c7bcd5f12 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 18 Nov 2021 09:55:35 -0500 Subject: [PATCH 01/12] Store any relations type when persisting events. --- synapse/storage/databases/main/events.py | 27 +++++++------- tests/rest/client/test_relations.py | 47 ++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 120e4807d161..8560aaff9407 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1,6 +1,6 @@ # Copyright 2014-2016 OpenMarket Ltd # Copyright 2018-2019 New Vector Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. +# Copyright 2019-2021 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -1696,34 +1696,33 @@ def non_null_str_or_none(val: Any) -> Optional[str]: }, ) - def _handle_event_relations(self, txn, event): - """Handles inserting relation data during peristence of events + def _handle_event_relations( + self, txn: LoggingTransaction, event: EventBase + ) -> None: + """Handles inserting relation data during persistence of events Args: - txn - event (EventBase) + txn: The current database transaction. + event: The event which might have relations. """ relation = event.content.get("m.relates_to") if not relation: # No relations return + # Relations must have a type and parent event ID. rel_type = relation.get("rel_type") - if rel_type not in ( - RelationTypes.ANNOTATION, - RelationTypes.REFERENCE, - RelationTypes.REPLACE, - RelationTypes.THREAD, - ): - # Unknown relation type + if not rel_type: return parent_id = relation.get("event_id") if not parent_id: - # Invalid relation return - aggregation_key = relation.get("key") + # Annotations have a key field. + aggregation_key = None + if rel_type == RelationTypes.ANNOTATION: + aggregation_key = relation.get("key") self.db_pool.simple_insert_txn( txn, diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index b8a1b92a896f..d5a3fdfe51ff 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -1,4 +1,5 @@ # Copyright 2019 New Vector Ltd +# Copyright 2021 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -765,6 +766,52 @@ def test_aggregations_redaction_prevents_access_to_aggregations(self): self.assertIn("chunk", channel.json_body) self.assertEquals(channel.json_body["chunk"], []) + def test_unknown_relations(self): + """Unknown relations should be accepted.""" + channel = self._send_relation("m.relation.test", "m.room.test") + self.assertEquals(200, channel.code, channel.json_body) + event_id = channel.json_body["event_id"] + + channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/relations/%s?limit=1" + % (self.room, self.parent_id), + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + + # We expect to get back a single pagination result, which is the full + # relation event we sent above. + self.assertEquals(len(channel.json_body["chunk"]), 1, channel.json_body) + self.assert_dict( + {"event_id": event_id, "sender": self.user_id, "type": "m.room.test"}, + channel.json_body["chunk"][0], + ) + + # We also expect to get the original event (the id of which is self.parent_id) + self.assertEquals( + channel.json_body["original_event"]["event_id"], self.parent_id + ) + + # When bundling the unknown relation is not included. + channel = self.make_request( + "GET", + "/rooms/%s/event/%s" % (self.room, self.parent_id), + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + self.assertNotIn("m.relations", channel.json_body["unsigned"]) + + # But unknown relations can be directly queried. + channel = self.make_request( + "GET", + "/_matrix/client/unstable/rooms/%s/aggregations/%s?limit=1" + % (self.room, self.parent_id), + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + self.assertEquals(channel.json_body["chunk"], []) + def _send_relation( self, relation_type: str, From f9952d663f764f7c9943355d1c94b17e4d74f5ef Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 18 Nov 2021 11:24:07 -0500 Subject: [PATCH 02/12] Replace the event_thread_relation with a no-op. --- .../databases/main/events_bg_updates.py | 4 +--- .../main/delta/65/02_thread_relations.sql | 18 ------------------ 2 files changed, 1 insertion(+), 21 deletions(-) delete mode 100644 synapse/storage/schema/main/delta/65/02_thread_relations.sql diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index ae3a8a63e42f..672180132435 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -171,9 +171,7 @@ def __init__(self, database: DatabasePool, db_conn, hs: "HomeServer"): self._purged_chain_cover_index, ) - self.db_pool.updates.register_background_update_handler( - "event_thread_relation", self._event_thread_relation - ) + self.db_pool.updates.register_noop_background_update("event_thread_relation") ################################################################################ diff --git a/synapse/storage/schema/main/delta/65/02_thread_relations.sql b/synapse/storage/schema/main/delta/65/02_thread_relations.sql deleted file mode 100644 index d60517f7b4ca..000000000000 --- a/synapse/storage/schema/main/delta/65/02_thread_relations.sql +++ /dev/null @@ -1,18 +0,0 @@ -/* Copyright 2021 The Matrix.org Foundation C.I.C - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - --- Check old events for thread relations. -INSERT INTO background_updates (ordering, update_name, progress_json) VALUES - (6502, 'event_thread_relation', '{}'); From 5333205ce1b7aa4dcebb162411c463ea17a5a2dd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 18 Nov 2021 11:41:35 -0500 Subject: [PATCH 03/12] Rework the background update to include any relation type. --- .../databases/main/events_bg_updates.py | 44 +++++++++++++------ .../main/delta/65/07_arbitrary_relations.sql | 18 ++++++++ 2 files changed, 48 insertions(+), 14 deletions(-) create mode 100644 synapse/storage/schema/main/delta/65/07_arbitrary_relations.sql diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 672180132435..24823122e990 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1,4 +1,4 @@ -# Copyright 2019 The Matrix.org Foundation C.I.C. +# Copyright 2019-2021 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -17,7 +17,7 @@ import attr -from synapse.api.constants import EventContentFields, RelationTypes +from synapse.api.constants import EventContentFields from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.events import make_event_from_dict from synapse.storage._base import SQLBaseStore, db_to_json, make_in_list_sql_clause @@ -173,6 +173,11 @@ def __init__(self, database: DatabasePool, db_conn, hs: "HomeServer"): self.db_pool.updates.register_noop_background_update("event_thread_relation") + self.db_pool.updates.register_background_update_handler( + "event_arbitrary_relations", + self._event_arbitrary_relations, + ) + ################################################################################ # bg updates for replacing stream_ordering with a BIGINT @@ -1097,11 +1102,16 @@ def purged_chain_cover_txn(txn) -> int: return result - async def _event_thread_relation(self, progress: JsonDict, batch_size: int) -> int: - """Background update handler which will store thread relations for existing events.""" + async def _event_arbitrary_relations( + self, progress: JsonDict, batch_size: int + ) -> int: + """Background update handler which will store previously unknown relations for existing events.""" last_event_id = progress.get("last_event_id", "") - def _event_thread_relation_txn(txn: LoggingTransaction) -> int: + def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: + # Iterate over events which do not appear in the event_relations + # table -- they might have a relation that was not previously stored + # due to an unknown relation type. txn.execute( """ SELECT event_id, json FROM event_json @@ -1113,7 +1123,7 @@ def _event_thread_relation_txn(txn: LoggingTransaction) -> int: ) results = list(txn) - missing_thread_relations = [] + missing_relations = [] for (event_id, event_json_raw) in results: try: event_json = db_to_json(event_json_raw) @@ -1125,11 +1135,15 @@ def _event_thread_relation_txn(txn: LoggingTransaction) -> int: ) continue - # If there's no relation (or it is not a thread), skip! + # If there's no relation, skip! relates_to = event_json["content"].get("m.relates_to") if not relates_to or not isinstance(relates_to, dict): continue - if relates_to.get("rel_type") != RelationTypes.THREAD: + + # The only expected relation type would be from threads, but + # there could be other unknown ones. + rel_type = relates_to.get("rel_type") + if not isinstance(rel_type, str): continue # Get the parent ID. @@ -1137,7 +1151,7 @@ def _event_thread_relation_txn(txn: LoggingTransaction) -> int: if not isinstance(parent_id, str): continue - missing_thread_relations.append((event_id, parent_id)) + missing_relations.append((event_id, parent_id, rel_type)) # Insert the missing data. self.db_pool.simple_insert_many_txn( @@ -1147,26 +1161,28 @@ def _event_thread_relation_txn(txn: LoggingTransaction) -> int: { "event_id": event_id, "relates_to_Id": parent_id, - "relation_type": RelationTypes.THREAD, + "relation_type": rel_type, } - for event_id, parent_id in missing_thread_relations + for event_id, parent_id, rel_type in missing_relations ], ) if results: latest_event_id = results[-1][0] self.db_pool.updates._background_update_progress_txn( - txn, "event_thread_relation", {"last_event_id": latest_event_id} + txn, "event_arbitrary_relations", {"last_event_id": latest_event_id} ) return len(results) num_rows = await self.db_pool.runInteraction( - desc="event_thread_relation", func=_event_thread_relation_txn + desc="event_arbitrary_relations", func=_event_arbitrary_relations_txn ) if not num_rows: - await self.db_pool.updates._end_background_update("event_thread_relation") + await self.db_pool.updates._end_background_update( + "event_arbitrary_relations" + ) return num_rows diff --git a/synapse/storage/schema/main/delta/65/07_arbitrary_relations.sql b/synapse/storage/schema/main/delta/65/07_arbitrary_relations.sql new file mode 100644 index 000000000000..267b2cb53963 --- /dev/null +++ b/synapse/storage/schema/main/delta/65/07_arbitrary_relations.sql @@ -0,0 +1,18 @@ +/* Copyright 2021 The Matrix.org Foundation C.I.C + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- Check old events for thread relations. +INSERT INTO background_updates (ordering, update_name, progress_json) VALUES + (6507, 'event_arbitrary_relations', '{}'); From 08a2048b3bbb4f60d161ae73fa18d445f779dc8a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 18 Nov 2021 12:24:27 -0500 Subject: [PATCH 04/12] Newsfragment --- changelog.d/11391.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11391.feature diff --git a/changelog.d/11391.feature b/changelog.d/11391.feature new file mode 100644 index 000000000000..4f696285a778 --- /dev/null +++ b/changelog.d/11391.feature @@ -0,0 +1 @@ +Store and allow querying of arbitrary event relations. From 6f5fd123c62001a5cb2734c1ff781c51616a4ef4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 08:18:44 -0500 Subject: [PATCH 05/12] Better error checking of relation type / parent ID. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/databases/main/events.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 8560aaff9407..06832221adc2 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1712,11 +1712,11 @@ def _handle_event_relations( # Relations must have a type and parent event ID. rel_type = relation.get("rel_type") - if not rel_type: + if not isinstance(rel_type, str): return parent_id = relation.get("event_id") - if not parent_id: + if not isinstance(parent_id, str): return # Annotations have a key field. From c8b866a7cc2914fafe628c749b8995a0bb77e7ff Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 08:19:02 -0500 Subject: [PATCH 06/12] Add a missing type hint. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/databases/main/events_bg_updates.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 24823122e990..264434ce9005 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1123,7 +1123,8 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: ) results = list(txn) - missing_relations = [] + # (event_id, parent_id, rel_type) for each relation + missing_relations: List[Tuple[str, str, str]] = [] for (event_id, event_json_raw) in results: try: event_json = db_to_json(event_json_raw) From cf90b40bc3a287f7df1f9197933c13087ca03bc2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 08:19:14 -0500 Subject: [PATCH 07/12] Fix typo. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/databases/main/events_bg_updates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 264434ce9005..a5abcbd393c4 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1161,7 +1161,7 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: values=[ { "event_id": event_id, - "relates_to_Id": parent_id, + "relates_to_id": parent_id, "relation_type": rel_type, } for event_id, parent_id, rel_type in missing_relations From 999ab1cd16f26335fd1984264f4b4f0f79ca7288 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 08:31:42 -0500 Subject: [PATCH 08/12] Add explanatory comment. --- synapse/storage/databases/main/events_bg_updates.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index a5abcbd393c4..9f8ab66f757b 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -171,6 +171,9 @@ def __init__(self, database: DatabasePool, db_conn, hs: "HomeServer"): self._purged_chain_cover_index, ) + # The event_thread_relation background update was replaced with the + # event_arbitrary_relations one, which handles any relation to avoid + # needed to potentially crawl the entire events table in the future. self.db_pool.updates.register_noop_background_update("event_thread_relation") self.db_pool.updates.register_background_update_handler( From 3df00a35c39cd8ef958ce71e38fa4b928700585a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 10:38:43 -0500 Subject: [PATCH 09/12] Rework background update for performance. --- .../databases/main/events_bg_updates.py | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 9f8ab66f757b..dc1ec10dffa2 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -17,7 +17,7 @@ import attr -from synapse.api.constants import EventContentFields +from synapse.api.constants import EventContentFields, RelationTypes from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.events import make_event_from_dict from synapse.storage._base import SQLBaseStore, db_to_json, make_in_list_sql_clause @@ -1112,14 +1112,12 @@ async def _event_arbitrary_relations( last_event_id = progress.get("last_event_id", "") def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: - # Iterate over events which do not appear in the event_relations - # table -- they might have a relation that was not previously stored - # due to an unknown relation type. + # Fetch events and then filter based on whether the event has a + # relation or not. txn.execute( """ SELECT event_id, json FROM event_json - LEFT JOIN event_relations USING (event_id) - WHERE event_id > ? AND event_relations.event_id IS NULL + WHERE event_id > ? ORDER BY event_id LIMIT ? """, (last_event_id, batch_size), @@ -1144,32 +1142,36 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: if not relates_to or not isinstance(relates_to, dict): continue - # The only expected relation type would be from threads, but - # there could be other unknown ones. + # If the relation type or parent event ID is not a string a + # string, skip it. + # + # Do not consider relation types that have existed for a long time, + # only include the new thread relation and any unknown relations. rel_type = relates_to.get("rel_type") - if not isinstance(rel_type, str): + if not isinstance(rel_type, str) or rel_type in ( + RelationTypes.ANNOTATION, + RelationTypes.REFERENCE, + RelationTypes.REPLACE, + ): continue - # Get the parent ID. parent_id = relates_to.get("event_id") if not isinstance(parent_id, str): continue missing_relations.append((event_id, parent_id, rel_type)) - # Insert the missing data. - self.db_pool.simple_insert_many_txn( - txn=txn, - table="event_relations", - values=[ - { - "event_id": event_id, - "relates_to_id": parent_id, - "relation_type": rel_type, - } - for event_id, parent_id, rel_type in missing_relations - ], - ) + # Insert the missing data, note that we upsert here in-case the event + # has already been processed. + if missing_relations: + self.db_pool.simple_upsert_many_txn( + txn=txn, + table="event_relations", + key_names=("event_id",), + key_values=[(r[0],) for r in missing_relations], + value_names=("relates_to_id", "relation_type"), + value_values=[r[1:] for r in missing_relations], + ) if results: latest_event_id = results[-1][0] From 1e65f1488153a9a5cc5501d50ea5893e6a8dd0ec Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 11:05:07 -0500 Subject: [PATCH 10/12] Add a test for the background update and clear caches. --- .../databases/main/events_bg_updates.py | 13 ++++ tests/rest/client/test_relations.py | 64 +++++++++++++++++++ tests/unittest.py | 7 +- 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index dc1ec10dffa2..b3bdee2045c4 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1173,6 +1173,19 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: value_values=[r[1:] for r in missing_relations], ) + # Iterate the parent IDs and invalidate caches. + for parent_id in {r[1] for r in missing_relations}: + cache_tuple = (parent_id,) + self._invalidate_cache_and_stream( + txn, self.get_relations_for_event, cache_tuple + ) + self._invalidate_cache_and_stream( + txn, self.get_aggregation_groups_for_event, cache_tuple + ) + self._invalidate_cache_and_stream( + txn, self.get_thread_summary, cache_tuple + ) + if results: latest_event_id = results[-1][0] self.db_pool.updates._background_update_progress_txn( diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index d5a3fdfe51ff..eb10d4321793 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -47,6 +47,8 @@ def default_config(self) -> dict: return config def prepare(self, reactor, clock, hs): + self.store = hs.get_datastore() + self.user_id, self.user_token = self._create_user("alice") self.user2_id, self.user2_token = self._create_user("bob") @@ -858,3 +860,65 @@ def _create_user(self, localpart: str) -> Tuple[str, str]: access_token = self.login(localpart, "abc123") return user_id, access_token + + def test_background_update(self): + """Test the event_arbitrary_relations background update.""" + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="👍") + self.assertEquals(200, channel.code, channel.json_body) + annotation_event_id_good = channel.json_body["event_id"] + + channel = self._send_relation(RelationTypes.ANNOTATION, "m.reaction", key="A") + self.assertEquals(200, channel.code, channel.json_body) + annotation_event_id_bad = channel.json_body["event_id"] + + channel = self._send_relation(RelationTypes.THREAD, "m.room.test") + self.assertEquals(200, channel.code, channel.json_body) + thread_event_id = channel.json_body["event_id"] + + # Clean-up the table as if the inserts did not happen during event creation. + self.get_success( + self.store.db_pool.simple_delete_many( + table="event_relations", + column="event_id", + iterable=(annotation_event_id_bad, thread_event_id), + keyvalues={}, + desc="RelationsTestCase.test_background_update", + ) + ) + + # Only the "good" annotation should be found. + channel = self.make_request( + "GET", + f"/_matrix/client/unstable/rooms/{self.room}/relations/{self.parent_id}?limit=10", + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + self.assertEquals( + [ev["event_id"] for ev in channel.json_body["chunk"]], + [annotation_event_id_good], + ) + + # Insert and run the background update. + self.get_success( + self.store.db_pool.simple_insert( + "background_updates", + {"update_name": "event_arbitrary_relations", "progress_json": "{}"}, + ) + ) + + # Ugh, have to reset this flag + self.store.db_pool.updates._all_done = False + self.wait_for_background_updates() + + # The "good" annotation and the thread should be found, but not the "bad" + # annotation. + channel = self.make_request( + "GET", + f"/_matrix/client/unstable/rooms/{self.room}/relations/{self.parent_id}?limit=10", + access_token=self.user_token, + ) + self.assertEquals(200, channel.code, channel.json_body) + self.assertCountEqual( + [ev["event_id"] for ev in channel.json_body["chunk"]], + [annotation_event_id_good, thread_event_id], + ) diff --git a/tests/unittest.py b/tests/unittest.py index c9a08a3420ea..165aafc57453 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -331,7 +331,12 @@ def wait_on_thread(self, deferred, timeout=10): time.sleep(0.01) def wait_for_background_updates(self) -> None: - """Block until all background database updates have completed.""" + """ + Block until all background database updates have completed. + + Note that callers must ensure that's a store property created on the + testcase. + """ while not self.get_success( self.store.db_pool.updates.has_completed_background_updates() ): From 8699b4a363f9d9188e84cae4489107054bbccdc5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 11:28:15 -0500 Subject: [PATCH 11/12] Comment fixes. Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/databases/main/events_bg_updates.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index b3bdee2045c4..2a28599d358b 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1142,11 +1142,10 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: if not relates_to or not isinstance(relates_to, dict): continue - # If the relation type or parent event ID is not a string a - # string, skip it. + # If the relation type or parent event ID is not a string, skip it. # # Do not consider relation types that have existed for a long time, - # only include the new thread relation and any unknown relations. + # since they will already be listed in the `event_relations` table. rel_type = relates_to.get("rel_type") if not isinstance(rel_type, str) or rel_type in ( RelationTypes.ANNOTATION, @@ -1161,7 +1160,7 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: missing_relations.append((event_id, parent_id, rel_type)) - # Insert the missing data, note that we upsert here in-case the event + # Insert the missing data, note that we upsert here in case the event # has already been processed. if missing_relations: self.db_pool.simple_upsert_many_txn( From d918d20c3b18d164452b7d0e33d41296c90779cc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 22 Nov 2021 11:28:54 -0500 Subject: [PATCH 12/12] Rename variable. --- synapse/storage/databases/main/events_bg_updates.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 2a28599d358b..c88fd35e7f3a 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1125,7 +1125,7 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: results = list(txn) # (event_id, parent_id, rel_type) for each relation - missing_relations: List[Tuple[str, str, str]] = [] + relations_to_insert: List[Tuple[str, str, str]] = [] for (event_id, event_json_raw) in results: try: event_json = db_to_json(event_json_raw) @@ -1158,22 +1158,22 @@ def _event_arbitrary_relations_txn(txn: LoggingTransaction) -> int: if not isinstance(parent_id, str): continue - missing_relations.append((event_id, parent_id, rel_type)) + relations_to_insert.append((event_id, parent_id, rel_type)) # Insert the missing data, note that we upsert here in case the event # has already been processed. - if missing_relations: + if relations_to_insert: self.db_pool.simple_upsert_many_txn( txn=txn, table="event_relations", key_names=("event_id",), - key_values=[(r[0],) for r in missing_relations], + key_values=[(r[0],) for r in relations_to_insert], value_names=("relates_to_id", "relation_type"), - value_values=[r[1:] for r in missing_relations], + value_values=[r[1:] for r in relations_to_insert], ) # Iterate the parent IDs and invalidate caches. - for parent_id in {r[1] for r in missing_relations}: + for parent_id in {r[1] for r in relations_to_insert}: cache_tuple = (parent_id,) self._invalidate_cache_and_stream( txn, self.get_relations_for_event, cache_tuple