From 38036b2e793f7df26e7954e095f7fde3cd503364 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 12 Sep 2024 17:59:07 -0500 Subject: [PATCH 01/11] Return room tags in account data extension Fix https://github.com/element-hq/synapse/issues/17694 --- synapse/handlers/sliding_sync/extensions.py | 34 +++++++++++++++++-- .../storage/databases/main/account_data.py | 10 +++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 6f37cc3462f..329e7275973 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -396,13 +396,16 @@ async def get_account_data_extension_response( ) global_account_data_map = dict(all_global_account_data) + # TODO: This should take into account the `to_token` global_account_data_map[ AccountDataTypes.PUSH_RULES ] = await self.push_rules_handler.push_rules_for_user(sync_config.user) # Fetch room account data - account_data_by_room_map: Mapping[str, Mapping[str, JsonMapping]] = {} + # + # Mapping from room_id to mapping of `type` to `content` of room account data events. + account_data_by_room_map: Dict[str, Dict[str, JsonMapping]] = {} relevant_room_ids = self.find_relevant_room_ids_for_extension( requested_lists=account_data_request.lists, requested_room_ids=account_data_request.rooms, @@ -417,11 +420,38 @@ async def get_account_data_extension_response( user_id, from_token.stream_token.account_data_key ) ) + + # Add room tags + # + # TODO: This should take into account the `from_token` and `to_token` + tags_by_room = await self.store.get_updated_tags( + user_id, from_token.stream_token.account_data_key + ) + for room_id, tags in tags_by_room.items(): + account_data_by_room_map.setdefault(room_id, {})[ + AccountDataTypes.TAG + ] = {"tags": tags} else: # TODO: This should take into account the `to_token` - account_data_by_room_map = ( + immutable_account_data_by_room_map = ( await self.store.get_room_account_data_for_user(user_id) ) + # We have to make a copy of the immutable data from the cache as we will + # be modifying it. + for ( + room_id, + room_account_data_map, + ) in immutable_account_data_by_room_map.items(): + account_data_by_room_map[room_id] = dict(room_account_data_map) + + # Add room tags + # + # TODO: This should take into account the `to_token` + tags_by_room = await self.store.get_tags_for_user(user_id) + for room_id, tags in tags_by_room.items(): + account_data_by_room_map.setdefault(room_id, {})[ + AccountDataTypes.TAG + ] = {"tags": tags} # Filter down to the relevant rooms account_data_by_room_map = { diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 966393869b5..174271163e5 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -177,7 +177,7 @@ async def get_room_account_data_for_user( def get_room_account_data_for_user_txn( txn: LoggingTransaction, - ) -> Dict[str, Dict[str, JsonDict]]: + ) -> Dict[str, Dict[str, JsonMapping]]: # The 'content != '{}' condition below prevents us from using # `simple_select_list_txn` here, as it doesn't support conditions # other than 'equals'. @@ -194,7 +194,7 @@ def get_room_account_data_for_user_txn( txn.execute(sql, (user_id,)) - by_room: Dict[str, Dict[str, JsonDict]] = {} + by_room: Dict[str, Dict[str, JsonMapping]] = {} for room_id, account_data_type, content in txn: room_data = by_room.setdefault(room_id, {}) @@ -429,7 +429,7 @@ def get_updated_global_account_data_for_user( async def get_updated_room_account_data_for_user( self, user_id: str, stream_id: int - ) -> Dict[str, Dict[str, JsonDict]]: + ) -> Dict[str, Dict[str, JsonMapping]]: """Get all the room account_data that's changed for a user. Args: @@ -442,14 +442,14 @@ async def get_updated_room_account_data_for_user( def get_updated_room_account_data_for_user_txn( txn: LoggingTransaction, - ) -> Dict[str, Dict[str, JsonDict]]: + ) -> Dict[str, Dict[str, JsonMapping]]: sql = """ SELECT room_id, account_data_type, content FROM room_account_data WHERE user_id = ? AND stream_id > ? """ txn.execute(sql, (user_id, stream_id)) - account_data_by_room: Dict[str, Dict[str, JsonDict]] = {} + account_data_by_room: Dict[str, Dict[str, JsonMapping]] = {} for row in txn: room_account_data = account_data_by_room.setdefault(row[0], {}) room_account_data[row[1]] = db_to_json(row[2]) From 9659f0a8ee1c00051ad17938b09d11bbd94a1349 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 12 Sep 2024 18:48:13 -0500 Subject: [PATCH 02/11] Make it a little more clear that we're making copies --- synapse/handlers/sliding_sync/extensions.py | 26 ++++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 329e7275973..0ea0914ac0c 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -14,7 +14,15 @@ import itertools import logging -from typing import TYPE_CHECKING, AbstractSet, Dict, Mapping, Optional, Sequence, Set +from typing import ( + TYPE_CHECKING, + AbstractSet, + Dict, + Mapping, + Optional, + Sequence, + Set, +) from typing_extensions import assert_never @@ -391,13 +399,17 @@ async def get_account_data_extension_response( ] = await self.push_rules_handler.push_rules_for_user(sync_config.user) else: # TODO: This should take into account the `to_token` - all_global_account_data = await self.store.get_global_account_data_for_user( - user_id + immutable_global_account_data_map = ( + await self.store.get_global_account_data_for_user(user_id) ) - global_account_data_map = dict(all_global_account_data) + # We have to make a copy of the immutable data from the cache as we will + # be mutating it below. + # + # FIXME: It would be good to avoid this big copy + global_account_data_map = dict(immutable_global_account_data_map) - # TODO: This should take into account the `to_token` + # TODO: This should take into account the `to_token` global_account_data_map[ AccountDataTypes.PUSH_RULES ] = await self.push_rules_handler.push_rules_for_user(sync_config.user) @@ -437,7 +449,9 @@ async def get_account_data_extension_response( await self.store.get_room_account_data_for_user(user_id) ) # We have to make a copy of the immutable data from the cache as we will - # be modifying it. + # be mutating it below. + # + # FIXME: It would be good to avoid this big copy for ( room_id, room_account_data_map, From c6238c6acac0a82ca64422c10fd5a525de3331f2 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 12 Sep 2024 18:58:12 -0500 Subject: [PATCH 03/11] Add tests for room tags in the account data --- .../test_extension_account_data.py | 57 ++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_extension_account_data.py b/tests/rest/client/sliding_sync/test_extension_account_data.py index 6cc883a4bee..d20647a7ff1 100644 --- a/tests/rest/client/sliding_sync/test_extension_account_data.py +++ b/tests/rest/client/sliding_sync/test_extension_account_data.py @@ -255,6 +255,15 @@ def test_room_account_data_initial_sync(self) -> None: content={"roo": "rar"}, ) ) + # Add a room tag to mark the room as a favourite + self.get_success( + self.account_data_handler.add_tag_to_room( + user_id=user1_id, + room_id=room_id1, + tag="m.favourite", + content={}, + ) + ) # Create another room with some room account data room_id2 = self.helper.create_room_as(user1_id, tok=user1_tok) @@ -266,6 +275,15 @@ def test_room_account_data_initial_sync(self) -> None: content={"roo": "rar"}, ) ) + # Add a room tag to mark the room as a favourite + self.get_success( + self.account_data_handler.add_tag_to_room( + user_id=user1_id, + room_id=room_id2, + tag="m.favourite", + content={}, + ) + ) # Make an initial Sliding Sync request with the account_data extension enabled sync_body = { @@ -301,7 +319,7 @@ def test_room_account_data_initial_sync(self) -> None: .get("rooms") .get(room_id1) }, - {"org.matrix.roorarraz"}, + {"org.matrix.roorarraz", AccountDataTypes.TAG}, exact=True, ) @@ -323,6 +341,15 @@ def test_room_account_data_incremental_sync(self) -> None: content={"roo": "rar"}, ) ) + # Add a room tag to mark the room as a favourite + self.get_success( + self.account_data_handler.add_tag_to_room( + user_id=user1_id, + room_id=room_id1, + tag="m.favourite", + content={}, + ) + ) # Create another room with some room account data room_id2 = self.helper.create_room_as(user1_id, tok=user1_tok) @@ -334,6 +361,15 @@ def test_room_account_data_incremental_sync(self) -> None: content={"roo": "rar"}, ) ) + # Add a room tag to mark the room as a favourite + self.get_success( + self.account_data_handler.add_tag_to_room( + user_id=user1_id, + room_id=room_id2, + tag="m.favourite", + content={}, + ) + ) sync_body = { "lists": {}, @@ -369,6 +405,23 @@ def test_room_account_data_incremental_sync(self) -> None: content={"roo": "rar"}, ) ) + # Add another room tag + self.get_success( + self.account_data_handler.add_tag_to_room( + user_id=user1_id, + room_id=room_id1, + tag="m.server_notice", + content={}, + ) + ) + self.get_success( + self.account_data_handler.add_tag_to_room( + user_id=user1_id, + room_id=room_id2, + tag="m.server_notice", + content={}, + ) + ) # Make an incremental Sliding Sync request with the account_data extension enabled response_body, _ = self.do_sync(sync_body, since=from_token, tok=user1_tok) @@ -390,7 +443,7 @@ def test_room_account_data_incremental_sync(self) -> None: .get("rooms") .get(room_id1) }, - {"org.matrix.roorarraz2"}, + {"org.matrix.roorarraz2", AccountDataTypes.TAG}, exact=True, ) From 11ad1a43c7771856d68585733d9d3035b2e660e7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 12 Sep 2024 19:16:00 -0500 Subject: [PATCH 04/11] Avoid copy on global account data --- synapse/handlers/sliding_sync/extensions.py | 30 +++++++++++-------- .../storage/databases/main/account_data.py | 4 +-- synapse/types/handlers/sliding_sync.py | 12 +++++--- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 0ea0914ac0c..01e15deb1dd 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -17,11 +17,14 @@ from typing import ( TYPE_CHECKING, AbstractSet, + ChainMap, Dict, Mapping, + MutableMapping, Optional, Sequence, Set, + cast, ) from typing_extensions import assert_never @@ -388,12 +391,11 @@ async def get_account_data_extension_response( ) ) + # TODO: This should take into account the `from_token` and `to_token` have_push_rules_changed = await self.store.have_push_rules_changed_for_user( user_id, from_token.stream_token.push_rules_key ) if have_push_rules_changed: - global_account_data_map = dict(global_account_data_map) - # TODO: This should take into account the `from_token` and `to_token` global_account_data_map[ AccountDataTypes.PUSH_RULES ] = await self.push_rules_handler.push_rules_for_user(sync_config.user) @@ -403,16 +405,20 @@ async def get_account_data_extension_response( await self.store.get_global_account_data_for_user(user_id) ) - # We have to make a copy of the immutable data from the cache as we will - # be mutating it below. - # - # FIXME: It would be good to avoid this big copy - global_account_data_map = dict(immutable_global_account_data_map) - - # TODO: This should take into account the `to_token` - global_account_data_map[ - AccountDataTypes.PUSH_RULES - ] = await self.push_rules_handler.push_rules_for_user(sync_config.user) + # Use a `ChainMap` to avoid copying the immutable data from the cache + global_account_data_map = ChainMap( + { + # TODO: This should take into account the `to_token` + AccountDataTypes.PUSH_RULES: await self.push_rules_handler.push_rules_for_user( + sync_config.user + ) + }, + # Cast is safe because `ChainMap` only mutates the top-most map, + # see https://github.com/python/typeshed/issues/8430 + cast( + MutableMapping[str, JsonMapping], immutable_global_account_data_map + ), + ) # Fetch room account data # diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 174271163e5..b30639b4e6b 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -394,7 +394,7 @@ def get_updated_room_account_data_txn( async def get_updated_global_account_data_for_user( self, user_id: str, stream_id: int - ) -> Mapping[str, JsonMapping]: + ) -> Dict[str, JsonMapping]: """Get all the global account_data that's changed for a user. Args: @@ -407,7 +407,7 @@ async def get_updated_global_account_data_for_user( def get_updated_global_account_data_for_user( txn: LoggingTransaction, - ) -> Dict[str, JsonDict]: + ) -> Dict[str, JsonMapping]: sql = """ SELECT account_data_type, content FROM account_data WHERE user_id = ? AND stream_id > ? diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 48badeacda2..2db923c307d 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -314,14 +314,18 @@ class AccountDataExtension: """The Account Data extension (MSC3959) Attributes: - global_account_data_map: Mapping from `type` to `content` of global account - data events. - account_data_by_room_map: Mapping from room_id to mapping of `type` to - `content` of room account data events. + global_account_data_map: Mapping from `type` to `content` of + global account data events. + account_data_by_room_map: List of -> Mapping from room_id to mapping of + `type` to `content` of room account data events. These are lists so + we can avoid making copies of immutable data and instead just + provide a multiple maps that need to be combined when sending down + the data. """ global_account_data_map: Mapping[str, JsonMapping] account_data_by_room_map: Mapping[str, Mapping[str, JsonMapping]] + # account_data_by_room_maps: Sequence[Mapping[str, Mapping[str, JsonMapping]]] def __bool__(self) -> bool: return bool( From a93ed6e54e636074bab112ae2718f36ff3502fa4 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 12 Sep 2024 19:56:53 -0500 Subject: [PATCH 05/11] Avoid copy for room account data --- synapse/handlers/sliding_sync/extensions.py | 54 +++++++++++++-------- synapse/types/handlers/sliding_sync.py | 12 ++--- 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 01e15deb1dd..fa262faa7c6 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -19,6 +19,7 @@ AbstractSet, ChainMap, Dict, + List, Mapping, MutableMapping, Optional, @@ -422,8 +423,15 @@ async def get_account_data_extension_response( # Fetch room account data # - # Mapping from room_id to mapping of `type` to `content` of room account data events. - account_data_by_room_map: Dict[str, Dict[str, JsonMapping]] = {} + # List of -> Mapping from room_id to mapping of `type` to `content` of room + # account data events. + # + # This is is a list so we can avoid making copies of immutable data and instead + # just provide a multiple maps that need to be combined. Normally, we could + # reach for `ChainMap` in this scenario, but this is a nested map and accessing + # the ChainMap by room_id won't combine the two maps for that room (we would + # need a new `NestedChainMap` type class). + account_data_by_room_maps: List[Mapping[str, Mapping[str, JsonMapping]]] = [] relevant_room_ids = self.find_relevant_room_ids_for_extension( requested_lists=account_data_request.lists, requested_room_ids=account_data_request.rooms, @@ -449,40 +457,44 @@ async def get_account_data_extension_response( account_data_by_room_map.setdefault(room_id, {})[ AccountDataTypes.TAG ] = {"tags": tags} + + account_data_by_room_maps.append(account_data_by_room_map) else: # TODO: This should take into account the `to_token` immutable_account_data_by_room_map = ( await self.store.get_room_account_data_for_user(user_id) ) - # We have to make a copy of the immutable data from the cache as we will - # be mutating it below. - # - # FIXME: It would be good to avoid this big copy - for ( - room_id, - room_account_data_map, - ) in immutable_account_data_by_room_map.items(): - account_data_by_room_map[room_id] = dict(room_account_data_map) + account_data_by_room_maps.append(immutable_account_data_by_room_map) # Add room tags # # TODO: This should take into account the `to_token` tags_by_room = await self.store.get_tags_for_user(user_id) - for room_id, tags in tags_by_room.items(): - account_data_by_room_map.setdefault(room_id, {})[ - AccountDataTypes.TAG - ] = {"tags": tags} + account_data_by_room_maps.append( + { + room_id: {AccountDataTypes.TAG: {"tags": tags}} + for room_id, tags in tags_by_room.items() + } + ) - # Filter down to the relevant rooms - account_data_by_room_map = { - room_id: account_data_map - for room_id, account_data_map in account_data_by_room_map.items() - if room_id in relevant_room_ids + # Filter down to the relevant rooms ... and combine the maps + relevant_account_data_by_room_map: Mapping[str, Mapping[str, JsonMapping]] = { + room_id: ChainMap( + {}, + *( + # Cast is safe because `ChainMap` only mutates the top-most map, + # see https://github.com/python/typeshed/issues/8430 + cast(MutableMapping[str, JsonMapping], room_map[room_id]) + for room_map in account_data_by_room_maps + if room_map.get(room_id) + ), + ) + for room_id in relevant_room_ids } return SlidingSyncResult.Extensions.AccountDataExtension( global_account_data_map=global_account_data_map, - account_data_by_room_map=account_data_by_room_map, + account_data_by_room_map=relevant_account_data_by_room_map, ) @trace diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py index 2db923c307d..149920f8834 100644 --- a/synapse/types/handlers/sliding_sync.py +++ b/synapse/types/handlers/sliding_sync.py @@ -314,18 +314,14 @@ class AccountDataExtension: """The Account Data extension (MSC3959) Attributes: - global_account_data_map: Mapping from `type` to `content` of - global account data events. - account_data_by_room_map: List of -> Mapping from room_id to mapping of - `type` to `content` of room account data events. These are lists so - we can avoid making copies of immutable data and instead just - provide a multiple maps that need to be combined when sending down - the data. + global_account_data_map: Mapping from `type` to `content` of global + account data events. + account_data_by_room_map: Mapping from room_id to mapping of `type` to + `content` of room account data events. """ global_account_data_map: Mapping[str, JsonMapping] account_data_by_room_map: Mapping[str, Mapping[str, JsonMapping]] - # account_data_by_room_maps: Sequence[Mapping[str, Mapping[str, JsonMapping]]] def __bool__(self) -> bool: return bool( From 78c8547da8a9769273ced9892fa4cd93d334f309 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 12 Sep 2024 20:02:56 -0500 Subject: [PATCH 06/11] Avoid adding empty maps for relevant rooms with no account data --- synapse/handlers/sliding_sync/extensions.py | 37 +++++++++++++-------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index fa262faa7c6..9f8ef3c6bde 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -478,19 +478,30 @@ async def get_account_data_extension_response( ) # Filter down to the relevant rooms ... and combine the maps - relevant_account_data_by_room_map: Mapping[str, Mapping[str, JsonMapping]] = { - room_id: ChainMap( - {}, - *( - # Cast is safe because `ChainMap` only mutates the top-most map, - # see https://github.com/python/typeshed/issues/8430 - cast(MutableMapping[str, JsonMapping], room_map[room_id]) - for room_map in account_data_by_room_maps - if room_map.get(room_id) - ), - ) - for room_id in relevant_room_ids - } + relevant_account_data_by_room_map: MutableMapping[ + str, Mapping[str, JsonMapping] + ] = {} + for room_id in relevant_room_ids: + # We want to avoid adding empty maps for relevant rooms that have no room + # account data so do a quick check to see if it's in any of the maps. + is_room_in_maps = False + for room_map in account_data_by_room_maps: + if room_id in room_map: + is_room_in_maps = True + break + + # If we found the room in any of the maps, combine the maps for that room + if is_room_in_maps: + relevant_account_data_by_room_map[room_id] = ChainMap( + {}, + *( + # Cast is safe because `ChainMap` only mutates the top-most map, + # see https://github.com/python/typeshed/issues/8430 + cast(MutableMapping[str, JsonMapping], room_map[room_id]) + for room_map in account_data_by_room_maps + if room_map.get(room_id) + ), + ) return SlidingSyncResult.Extensions.AccountDataExtension( global_account_data_map=global_account_data_map, From ef0d36cad20ed0154fe01043aced3ffec24e8430 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 12 Sep 2024 20:10:38 -0500 Subject: [PATCH 07/11] Add changelog --- changelog.d/17707.feature | 1 + synapse/handlers/sliding_sync/extensions.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/17707.feature diff --git a/changelog.d/17707.feature b/changelog.d/17707.feature new file mode 100644 index 00000000000..98a00ca34b4 --- /dev/null +++ b/changelog.d/17707.feature @@ -0,0 +1 @@ +Return room tags in Sliding Sync account data extension. diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index 9f8ef3c6bde..fb1f5c44d66 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -427,7 +427,7 @@ async def get_account_data_extension_response( # account data events. # # This is is a list so we can avoid making copies of immutable data and instead - # just provide a multiple maps that need to be combined. Normally, we could + # just provide multiple maps that need to be combined. Normally, we could # reach for `ChainMap` in this scenario, but this is a nested map and accessing # the ChainMap by room_id won't combine the two maps for that room (we would # need a new `NestedChainMap` type class). From b36e8a8a9296dcf6a471a17e6c1ace3c3219777c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 16 Sep 2024 10:32:06 -0500 Subject: [PATCH 08/11] Add todo See https://github.com/element-hq/synapse/pull/17707#discussion_r1759310920 --- synapse/handlers/sliding_sync/extensions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/handlers/sliding_sync/extensions.py b/synapse/handlers/sliding_sync/extensions.py index fb1f5c44d66..9833ce7dc1a 100644 --- a/synapse/handlers/sliding_sync/extensions.py +++ b/synapse/handlers/sliding_sync/extensions.py @@ -397,6 +397,7 @@ async def get_account_data_extension_response( user_id, from_token.stream_token.push_rules_key ) if have_push_rules_changed: + # TODO: This should take into account the `from_token` and `to_token` global_account_data_map[ AccountDataTypes.PUSH_RULES ] = await self.push_rules_handler.push_rules_for_user(sync_config.user) From 2231b9845845f66d0ce0740bd018e16f27eda9a4 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 16 Sep 2024 12:26:59 -0500 Subject: [PATCH 09/11] Assert values --- .../test_extension_account_data.py | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_extension_account_data.py b/tests/rest/client/sliding_sync/test_extension_account_data.py index d20647a7ff1..bd0c907c6fb 100644 --- a/tests/rest/client/sliding_sync/test_extension_account_data.py +++ b/tests/rest/client/sliding_sync/test_extension_account_data.py @@ -312,16 +312,21 @@ def test_room_account_data_initial_sync(self) -> None: {room_id1}, exact=True, ) + account_data_map = { + event["type"]: event["content"] + for event in response_body["extensions"]["account_data"] + .get("rooms") + .get(room_id1) + } self.assertIncludes( - { - event["type"] - for event in response_body["extensions"]["account_data"] - .get("rooms") - .get(room_id1) - }, + account_data_map.keys(), {"org.matrix.roorarraz", AccountDataTypes.TAG}, exact=True, ) + self.assertEqual(account_data_map["org.matrix.roorarraz"], {"roo": "rar"}) + self.assertEqual( + account_data_map[AccountDataTypes.TAG], {"tags": {"m.favourite": {}}} + ) def test_room_account_data_incremental_sync(self) -> None: """ @@ -436,16 +441,22 @@ def test_room_account_data_incremental_sync(self) -> None: exact=True, ) # We should only see the new room account data that happened after the `from_token` + account_data_map = { + event["type"]: event["content"] + for event in response_body["extensions"]["account_data"] + .get("rooms") + .get(room_id1) + } self.assertIncludes( - { - event["type"] - for event in response_body["extensions"]["account_data"] - .get("rooms") - .get(room_id1) - }, + account_data_map.keys(), {"org.matrix.roorarraz2", AccountDataTypes.TAG}, exact=True, ) + self.assertEqual(account_data_map["org.matrix.roorarraz2"], {"roo": "rar"}) + self.assertEqual( + account_data_map[AccountDataTypes.TAG], + {"tags": {"m.favourite": {}, "m.server_notice": {}}}, + ) def test_wait_for_new_data(self) -> None: """ From 77b0b0bf5622a83260afd0ec2b24f064515c168a Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 16 Sep 2024 12:36:30 -0500 Subject: [PATCH 10/11] Assert global values --- .../test_extension_account_data.py | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_extension_account_data.py b/tests/rest/client/sliding_sync/test_extension_account_data.py index bd0c907c6fb..b7d159489cb 100644 --- a/tests/rest/client/sliding_sync/test_extension_account_data.py +++ b/tests/rest/client/sliding_sync/test_extension_account_data.py @@ -80,18 +80,27 @@ def test_no_data_initial_sync(self) -> None: } response_body, _ = self.do_sync(sync_body, tok=user1_tok) + global_account_data_map = { + global_event["type"]: global_event["content"] + for global_event in response_body["extensions"]["account_data"].get( + "global" + ) + } self.assertIncludes( - { - global_event["type"] - for global_event in response_body["extensions"]["account_data"].get( - "global" - ) - }, + global_account_data_map.keys(), # Even though we don't have any global account data set, Synapse saves some # default push rules for us. {AccountDataTypes.PUSH_RULES}, exact=True, ) + logger.info( + "global_account_data_map[AccountDataTypes.PUSH_RULES] %s", + global_account_data_map[AccountDataTypes.PUSH_RULES], + ) + # Push rules are a giant chunk of JSON data so we will just assume the value is correct if they key is here. + # global_account_data_map[AccountDataTypes.PUSH_RULES] + + # No room account data for this test self.assertIncludes( response_body["extensions"]["account_data"].get("rooms").keys(), set(), @@ -121,16 +130,19 @@ def test_no_data_incremental_sync(self) -> None: # There has been no account data changes since the `from_token` so we shouldn't # see any account data here. + global_account_data_map = { + global_event["type"]: global_event["content"] + for global_event in response_body["extensions"]["account_data"].get( + "global" + ) + } self.assertIncludes( - { - global_event["type"] - for global_event in response_body["extensions"]["account_data"].get( - "global" - ) - }, + global_account_data_map.keys(), set(), exact=True, ) + + # No room account data for this test self.assertIncludes( response_body["extensions"]["account_data"].get("rooms").keys(), set(), @@ -165,16 +177,24 @@ def test_global_account_data_initial_sync(self) -> None: response_body, _ = self.do_sync(sync_body, tok=user1_tok) # It should show us all of the global account data + global_account_data_map = { + global_event["type"]: global_event["content"] + for global_event in response_body["extensions"]["account_data"].get( + "global" + ) + } self.assertIncludes( - { - global_event["type"] - for global_event in response_body["extensions"]["account_data"].get( - "global" - ) - }, + global_account_data_map.keys(), {AccountDataTypes.PUSH_RULES, "org.matrix.foobarbaz"}, exact=True, ) + # Push rules are a giant chunk of JSON data so we will just assume the value is correct if they key is here. + # global_account_data_map[AccountDataTypes.PUSH_RULES] + self.assertEqual( + global_account_data_map["org.matrix.foobarbaz"], {"foo": "bar"} + ) + + # No room account data for this test self.assertIncludes( response_body["extensions"]["account_data"].get("rooms").keys(), set(), @@ -220,17 +240,23 @@ def test_global_account_data_incremental_sync(self) -> None: # Make an incremental Sliding Sync request with the account_data extension enabled response_body, _ = self.do_sync(sync_body, since=from_token, tok=user1_tok) + global_account_data_map = { + global_event["type"]: global_event["content"] + for global_event in response_body["extensions"]["account_data"].get( + "global" + ) + } self.assertIncludes( - { - global_event["type"] - for global_event in response_body["extensions"]["account_data"].get( - "global" - ) - }, + global_account_data_map.keys(), # We should only see the new global account data that happened after the `from_token` {"org.matrix.doodardaz"}, exact=True, ) + self.assertEqual( + global_account_data_map["org.matrix.doodardaz"], {"doo": "dar"} + ) + + # No room account data for this test self.assertIncludes( response_body["extensions"]["account_data"].get("rooms").keys(), set(), From 5171a45aecc45cdc4e857ef0a42c5750afff26cd Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 16 Sep 2024 12:55:00 -0500 Subject: [PATCH 11/11] Remove debug log --- tests/rest/client/sliding_sync/test_extension_account_data.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/rest/client/sliding_sync/test_extension_account_data.py b/tests/rest/client/sliding_sync/test_extension_account_data.py index b7d159489cb..03b2db39b91 100644 --- a/tests/rest/client/sliding_sync/test_extension_account_data.py +++ b/tests/rest/client/sliding_sync/test_extension_account_data.py @@ -93,10 +93,6 @@ def test_no_data_initial_sync(self) -> None: {AccountDataTypes.PUSH_RULES}, exact=True, ) - logger.info( - "global_account_data_map[AccountDataTypes.PUSH_RULES] %s", - global_account_data_map[AccountDataTypes.PUSH_RULES], - ) # Push rules are a giant chunk of JSON data so we will just assume the value is correct if they key is here. # global_account_data_map[AccountDataTypes.PUSH_RULES]