From 36357f35e23e92116bf7dde6c01df70bcad74a2e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 14:00:17 +0000 Subject: [PATCH 01/13] Move sync_token up to the top --- synapse/handlers/sync.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 53d4627147d4..77e711c6502a 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1512,10 +1512,12 @@ async def _generate_sync_entry_for_rooms( - newly_left_rooms - newly_left_users """ + since_token = sync_result_builder.since_token + # Start by fetching all ephemeral events in rooms we've joined (if required). user_id = sync_result_builder.sync_config.user.to_string() block_all_room_ephemeral = ( - sync_result_builder.since_token is None + since_token is None and sync_result_builder.sync_config.filter_collection.blocks_all_room_ephemeral() ) @@ -1531,7 +1533,6 @@ async def _generate_sync_entry_for_rooms( # We check up front if anything has changed, if it hasn't then there is # no point in going further. - since_token = sync_result_builder.since_token if not sync_result_builder.full_state: if since_token and not ephemeral_by_room and not account_data_by_room: have_changed = await self._have_rooms_changed(sync_result_builder) From f9dff2d60560a31b20ed8245d0469b6677550dee Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 16:17:02 +0000 Subject: [PATCH 02/13] Pull out _get_ignored_users --- synapse/handlers/sync.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 77e711c6502a..3d0cd660f6b1 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1545,19 +1545,7 @@ async def _generate_sync_entry_for_rooms( logger.debug("no-oping sync") return set(), set(), set(), set() - ignored_account_data = ( - await self.store.get_global_account_data_by_type_for_user( - AccountDataTypes.IGNORED_USER_LIST, user_id=user_id - ) - ) - - # If there is ignored users account data and it matches the proper type, - # then use it. - ignored_users: FrozenSet[str] = frozenset() - if ignored_account_data: - ignored_users_data = ignored_account_data.get("ignored_users", {}) - if isinstance(ignored_users_data, dict): - ignored_users = frozenset(ignored_users_data.keys()) + ignored_users = await self._get_ignored_users(user_id) if since_token: room_changes = await self._get_rooms_changed( @@ -1630,6 +1618,22 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: newly_left_users, ) + async def _get_ignored_users(self, user_id: str) -> FrozenSet[str]: + ignored_account_data = ( + await self.store.get_global_account_data_by_type_for_user( + AccountDataTypes.IGNORED_USER_LIST, user_id=user_id + ) + ) + + # If there is ignored users account data and it matches the proper type, + # then use it. + ignored_users: FrozenSet[str] = frozenset() + if ignored_account_data: + ignored_users_data = ignored_account_data.get("ignored_users", {}) + if isinstance(ignored_users_data, dict): + ignored_users = frozenset(ignored_users_data.keys()) + return ignored_users + async def _have_rooms_changed( self, sync_result_builder: "SyncResultBuilder" ) -> bool: From 0af0643c7ea66d151947acebe0138308c8f8ef8f Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 16:31:13 +0000 Subject: [PATCH 03/13] Try to signpost the body of `_generate_sync_entry_for_rooms` --- synapse/handlers/sync.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 3d0cd660f6b1..e4d91fd3b94d 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1506,7 +1506,11 @@ async def _generate_sync_entry_for_rooms( account_data_by_room: Dictionary of per room account data Returns: - Returns a 4-tuple whose entries are: + Returns a 4-tuple describing rooms we've joined or left, and users who've + joined or left rooms we're in. This gets used later in + `_generate_sync_entry_for_device_list`. + + Its entries are: - newly_joined_rooms - newly_joined_or_invited_or_knocked_users - newly_left_rooms @@ -1514,7 +1518,7 @@ async def _generate_sync_entry_for_rooms( """ since_token = sync_result_builder.since_token - # Start by fetching all ephemeral events in rooms we've joined (if required). + # 1. Start by fetching all ephemeral events in rooms we've joined (if required). user_id = sync_result_builder.sync_config.user.to_string() block_all_room_ephemeral = ( since_token is None @@ -1531,7 +1535,7 @@ async def _generate_sync_entry_for_rooms( ) sync_result_builder.now_token = now_token - # We check up front if anything has changed, if it hasn't then there is + # 2. We check up front if anything has changed, if it hasn't then there is # no point in going further. if not sync_result_builder.full_state: if since_token and not ephemeral_by_room and not account_data_by_room: @@ -1545,8 +1549,8 @@ async def _generate_sync_entry_for_rooms( logger.debug("no-oping sync") return set(), set(), set(), set() + # 3. Work out which rooms need reporting in the sync response. ignored_users = await self._get_ignored_users(user_id) - if since_token: room_changes = await self._get_rooms_changed( sync_result_builder, ignored_users @@ -1556,7 +1560,6 @@ async def _generate_sync_entry_for_rooms( ) else: room_changes = await self._get_all_rooms(sync_result_builder, ignored_users) - tags_by_room = await self.store.get_tags_for_user(user_id) log_kv({"rooms_changed": len(room_changes.room_entries)}) @@ -1567,6 +1570,8 @@ async def _generate_sync_entry_for_rooms( newly_joined_rooms = room_changes.newly_joined_rooms newly_left_rooms = room_changes.newly_left_rooms + # 4. We need to apply further processing to `room_entries` (rooms considered + # joined or archived). async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: logger.debug("Generating room entry for %s", room_entry.room_id) await self._generate_room_entry( @@ -1585,7 +1590,9 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: sync_result_builder.invited.extend(invited) sync_result_builder.knocked.extend(knocked) - # Now we want to get any newly joined, invited or knocking users + # 5. Work out which users have joined or left rooms we're in. We use this + # to build the device_list part of the sync response in + # `_generate_sync_entry_for_device_list`. newly_joined_or_invited_or_knocked_users = set() newly_left_users = set() if since_token: From 963de4e5cfb250082ff13521449e2a56c333a114 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 16:43:48 +0000 Subject: [PATCH 04/13] Pull out _calculate_user_changes This didn't use any class instance state, so I made this a free function. --- synapse/handlers/sync.py | 56 +++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index e4d91fd3b94d..43798e03c48a 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1593,30 +1593,10 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: # 5. Work out which users have joined or left rooms we're in. We use this # to build the device_list part of the sync response in # `_generate_sync_entry_for_device_list`. - newly_joined_or_invited_or_knocked_users = set() - newly_left_users = set() - if since_token: - for joined_sync in sync_result_builder.joined: - it = itertools.chain( - joined_sync.timeline.events, joined_sync.state.values() - ) - for event in it: - if event.type == EventTypes.Member: - if ( - event.membership == Membership.JOIN - or event.membership == Membership.INVITE - or event.membership == Membership.KNOCK - ): - newly_joined_or_invited_or_knocked_users.add( - event.state_key - ) - else: - prev_content = event.unsigned.get("prev_content", {}) - prev_membership = prev_content.get("membership", None) - if prev_membership == Membership.JOIN: - newly_left_users.add(event.state_key) - - newly_left_users -= newly_joined_or_invited_or_knocked_users + ( + newly_joined_or_invited_or_knocked_users, + newly_left_users, + ) = _calculate_user_changes(sync_result_builder) return ( set(newly_joined_rooms), @@ -2353,6 +2333,34 @@ class SyncResultBuilder: to_device: List[JsonDict] = attr.Factory(list) +def _calculate_user_changes( + sync_result_builder: "SyncResultBuilder", +) -> Tuple[Set[str], Set[str]]: + newly_joined_or_invited_or_knocked_users = set() + newly_left_users = set() + if sync_result_builder.since_token: + for joined_sync in sync_result_builder.joined: + it = itertools.chain( + joined_sync.timeline.events, joined_sync.state.values() + ) + for event in it: + if event.type == EventTypes.Member: + if ( + event.membership == Membership.JOIN + or event.membership == Membership.INVITE + or event.membership == Membership.KNOCK + ): + newly_joined_or_invited_or_knocked_users.add(event.state_key) + else: + prev_content = event.unsigned.get("prev_content", {}) + prev_membership = prev_content.get("membership", None) + if prev_membership == Membership.JOIN: + newly_left_users.add(event.state_key) + + newly_left_users -= newly_joined_or_invited_or_knocked_users + return newly_joined_or_invited_or_knocked_users, newly_left_users + + @attr.s(slots=True, auto_attribs=True) class RoomSyncResultBuilder: """Stores information needed to create either a `JoinedSyncResult` or From bca977cf3551be3c930e6d47c7f12626397fafb8 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 16:45:48 +0000 Subject: [PATCH 05/13] Add docstring for _calculate_user_changes --- synapse/handlers/sync.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 43798e03c48a..15d763195604 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -2336,6 +2336,12 @@ class SyncResultBuilder: def _calculate_user_changes( sync_result_builder: "SyncResultBuilder", ) -> Tuple[Set[str], Set[str]]: + """Work out which other users have joined or left rooms we are joined to. + + Only applies to an incremental sync. + + The sync_result_builder is not modified by this function. + """ newly_joined_or_invited_or_knocked_users = set() newly_left_users = set() if sync_result_builder.since_token: From 0e2c6f6fcd13237514896d64af8b039e2cf4d9f4 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 17:13:18 +0000 Subject: [PATCH 06/13] Introduce membership_changes_summary I'm not sure if this is helpful---I feel like it might just exchange one set of line noise for another. --- synapse/handlers/sync.py | 52 ++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 15d763195604..c60a505ad379 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -183,6 +183,20 @@ def __bool__(self) -> bool: return bool(self.join or self.invite or self.leave) +@attr.s(slots=True, frozen=True, auto_attribs=True) +class MembershipChangesSummary: + """ + A summary of rooms we've joined or left, plus users who we've seen join or leave. + + Used to create the DeviceLists struct below. + """ + + newly_joined_rooms: Collection[str] + newly_joined_or_invited_or_knocked_users: Collection[str] + newly_left_rooms: Collection[str] + newly_left_users: Collection[str] + + @attr.s(slots=True, frozen=True, auto_attribs=True) class DeviceLists: """ @@ -1112,11 +1126,9 @@ async def generate_sync_result( logger.debug("Fetching room data") - res = await self._generate_sync_entry_for_rooms( + membership_changes_summary = await self._generate_sync_entry_for_rooms( sync_result_builder, account_data_by_room ) - newly_joined_rooms, newly_joined_or_invited_or_knocked_users, _, _ = res - _, _, newly_left_rooms, newly_left_users = res block_all_presence_data = ( since_token is None and sync_config.filter_collection.blocks_all_presence() @@ -1125,8 +1137,8 @@ async def generate_sync_result( logger.debug("Fetching presence data") await self._generate_sync_entry_for_presence( sync_result_builder, - newly_joined_rooms, - newly_joined_or_invited_or_knocked_users, + membership_changes_summary.newly_joined_rooms, + membership_changes_summary.newly_joined_or_invited_or_knocked_users, ) logger.debug("Fetching to-device data") @@ -1134,10 +1146,7 @@ async def generate_sync_result( device_lists = await self._generate_sync_entry_for_device_list( sync_result_builder, - newly_joined_rooms=newly_joined_rooms, - newly_joined_or_invited_or_knocked_users=newly_joined_or_invited_or_knocked_users, - newly_left_rooms=newly_left_rooms, - newly_left_users=newly_left_users, + membership_changes_summary, ) logger.debug("Fetching OTK data") @@ -1164,7 +1173,7 @@ async def generate_sync_result( # debug for https://github.com/matrix-org/synapse/issues/4422 for joined_room in sync_result_builder.joined: room_id = joined_room.room_id - if room_id in newly_joined_rooms: + if room_id in membership_changes_summary.newly_joined_rooms: issue4422_logger.debug( "Sync result for newly joined room %s: %r", room_id, joined_room ) @@ -1242,10 +1251,7 @@ async def _generate_sync_entry_for_groups( async def _generate_sync_entry_for_device_list( self, sync_result_builder: "SyncResultBuilder", - newly_joined_rooms: Set[str], - newly_joined_or_invited_or_knocked_users: Set[str], - newly_left_rooms: Set[str], - newly_left_users: Set[str], + mem_changes: MembershipChangesSummary, ) -> DeviceLists: """Generate the DeviceLists section of sync @@ -1266,9 +1272,9 @@ async def _generate_sync_entry_for_device_list( # We're going to mutate these fields, so lets copy them rather than # assume they won't get used later. newly_joined_or_invited_or_knocked_users = set( - newly_joined_or_invited_or_knocked_users + mem_changes.newly_joined_or_invited_or_knocked_users ) - newly_left_users = set(newly_left_users) + newly_left_users = set(mem_changes.newly_left_users) if since_token and since_token.device_list_key: # We want to figure out what user IDs the client should refetch @@ -1304,7 +1310,7 @@ async def _generate_sync_entry_for_device_list( ) # Step 1b, check for newly joined rooms - for room_id in newly_joined_rooms: + for room_id in mem_changes.newly_joined_rooms: joined_users = await self.store.get_users_in_room(room_id) newly_joined_or_invited_or_knocked_users.update(joined_users) @@ -1320,7 +1326,7 @@ async def _generate_sync_entry_for_device_list( users_that_have_changed.update(user_signatures_changed) # Now find users that we no longer track - for room_id in newly_left_rooms: + for room_id in mem_changes.newly_left_rooms: left_users = await self.store.get_users_in_room(room_id) newly_left_users.update(left_users) @@ -1434,8 +1440,8 @@ async def _generate_sync_entry_for_account_data( async def _generate_sync_entry_for_presence( self, sync_result_builder: "SyncResultBuilder", - newly_joined_rooms: Set[str], - newly_joined_or_invited_users: Set[str], + newly_joined_rooms: Collection[str], + newly_joined_or_invited_users: Collection[str], ) -> None: """Generates the presence portion of the sync response. Populates the `sync_result_builder` with the result. @@ -1493,7 +1499,7 @@ async def _generate_sync_entry_for_rooms( self, sync_result_builder: "SyncResultBuilder", account_data_by_room: Dict[str, Dict[str, JsonDict]], - ) -> Tuple[Set[str], Set[str], Set[str], Set[str]]: + ) -> MembershipChangesSummary: """Generates the rooms portion of the sync response. Populates the `sync_result_builder` with the result. @@ -1547,7 +1553,7 @@ async def _generate_sync_entry_for_rooms( ) if not tags_by_room: logger.debug("no-oping sync") - return set(), set(), set(), set() + return MembershipChangesSummary(set(), set(), set(), set()) # 3. Work out which rooms need reporting in the sync response. ignored_users = await self._get_ignored_users(user_id) @@ -1598,7 +1604,7 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: newly_left_users, ) = _calculate_user_changes(sync_result_builder) - return ( + return MembershipChangesSummary( set(newly_joined_rooms), newly_joined_or_invited_or_knocked_users, set(newly_left_rooms), From 463d2fd22dbe2bdb6faa5ad33856399f556f904e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 17:35:01 +0000 Subject: [PATCH 07/13] Changelog --- changelog.d/11494.misc | 2 +- changelog.d/11515.misc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/11515.misc diff --git a/changelog.d/11494.misc b/changelog.d/11494.misc index 7afd7d3a0727..52cf26a4b620 100644 --- a/changelog.d/11494.misc +++ b/changelog.d/11494.misc @@ -1 +1 @@ -Add comments to various parts of the `/sync` handler. \ No newline at end of file +Refactor various parts of the `/sync` handler. \ No newline at end of file diff --git a/changelog.d/11515.misc b/changelog.d/11515.misc new file mode 100644 index 000000000000..7f9d8cd57f38 --- /dev/null +++ b/changelog.d/11515.misc @@ -0,0 +1 @@ +Refactor various parts of the `/sync` handler. From d8aa9d0ca351670c4173300b7fac01464f56aa0c Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 19:52:08 +0000 Subject: [PATCH 08/13] Update synapse/handlers/sync.py Co-authored-by: Patrick Cloke --- synapse/handlers/sync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index c60a505ad379..d761114232d3 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1512,8 +1512,8 @@ async def _generate_sync_entry_for_rooms( account_data_by_room: Dictionary of per room account data Returns: - Returns a 4-tuple describing rooms we've joined or left, and users who've - joined or left rooms we're in. This gets used later in + Returns a 4-tuple describing rooms the user has joined or left, and users who've + joined or left rooms any rooms the user is in. This gets used later in `_generate_sync_entry_for_device_list`. Its entries are: From dba7c941a2fd03fdbac734c2c26923033feaf14e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 19:44:24 +0000 Subject: [PATCH 09/13] docstring for _get_ignored_users --- synapse/handlers/sync.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index d761114232d3..6d495d2ef557 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1612,6 +1612,12 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: ) async def _get_ignored_users(self, user_id: str) -> FrozenSet[str]: + """Retrieve the users ignored by the given user from their global account_data. + + Returns an empty set if + - there is no global account_data entry for ignored_users + - there is such an entry, but it's not a JSON object. + """ ignored_account_data = ( await self.store.get_global_account_data_by_type_for_user( AccountDataTypes.IGNORED_USER_LIST, user_id=user_id From ecccfb405348661b051048373dea946d686ed8ac Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 19:51:41 +0000 Subject: [PATCH 10/13] Note that we could use a DB table --- synapse/handlers/sync.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 6d495d2ef557..3c1d0a4e8bc1 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1618,6 +1618,7 @@ async def _get_ignored_users(self, user_id: str) -> FrozenSet[str]: - there is no global account_data entry for ignored_users - there is such an entry, but it's not a JSON object. """ + # TODO: Can we `SELECT ignored_user_id FROM ignored_users WHERE ignorer_user_id=?;` instead? ignored_account_data = ( await self.store.get_global_account_data_by_type_for_user( AccountDataTypes.IGNORED_USER_LIST, user_id=user_id From c38478998bdea51fd0565dfd67b9beb96b819caf Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 19:55:22 +0000 Subject: [PATCH 11/13] Revert "Introduce membership_changes_summary" This reverts commit 0e2c6f6fcd13237514896d64af8b039e2cf4d9f4. --- synapse/handlers/sync.py | 52 ++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 3c1d0a4e8bc1..f8b3219976d2 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -183,20 +183,6 @@ def __bool__(self) -> bool: return bool(self.join or self.invite or self.leave) -@attr.s(slots=True, frozen=True, auto_attribs=True) -class MembershipChangesSummary: - """ - A summary of rooms we've joined or left, plus users who we've seen join or leave. - - Used to create the DeviceLists struct below. - """ - - newly_joined_rooms: Collection[str] - newly_joined_or_invited_or_knocked_users: Collection[str] - newly_left_rooms: Collection[str] - newly_left_users: Collection[str] - - @attr.s(slots=True, frozen=True, auto_attribs=True) class DeviceLists: """ @@ -1126,9 +1112,11 @@ async def generate_sync_result( logger.debug("Fetching room data") - membership_changes_summary = await self._generate_sync_entry_for_rooms( + res = await self._generate_sync_entry_for_rooms( sync_result_builder, account_data_by_room ) + newly_joined_rooms, newly_joined_or_invited_or_knocked_users, _, _ = res + _, _, newly_left_rooms, newly_left_users = res block_all_presence_data = ( since_token is None and sync_config.filter_collection.blocks_all_presence() @@ -1137,8 +1125,8 @@ async def generate_sync_result( logger.debug("Fetching presence data") await self._generate_sync_entry_for_presence( sync_result_builder, - membership_changes_summary.newly_joined_rooms, - membership_changes_summary.newly_joined_or_invited_or_knocked_users, + newly_joined_rooms, + newly_joined_or_invited_or_knocked_users, ) logger.debug("Fetching to-device data") @@ -1146,7 +1134,10 @@ async def generate_sync_result( device_lists = await self._generate_sync_entry_for_device_list( sync_result_builder, - membership_changes_summary, + newly_joined_rooms=newly_joined_rooms, + newly_joined_or_invited_or_knocked_users=newly_joined_or_invited_or_knocked_users, + newly_left_rooms=newly_left_rooms, + newly_left_users=newly_left_users, ) logger.debug("Fetching OTK data") @@ -1173,7 +1164,7 @@ async def generate_sync_result( # debug for https://github.com/matrix-org/synapse/issues/4422 for joined_room in sync_result_builder.joined: room_id = joined_room.room_id - if room_id in membership_changes_summary.newly_joined_rooms: + if room_id in newly_joined_rooms: issue4422_logger.debug( "Sync result for newly joined room %s: %r", room_id, joined_room ) @@ -1251,7 +1242,10 @@ async def _generate_sync_entry_for_groups( async def _generate_sync_entry_for_device_list( self, sync_result_builder: "SyncResultBuilder", - mem_changes: MembershipChangesSummary, + newly_joined_rooms: Set[str], + newly_joined_or_invited_or_knocked_users: Set[str], + newly_left_rooms: Set[str], + newly_left_users: Set[str], ) -> DeviceLists: """Generate the DeviceLists section of sync @@ -1272,9 +1266,9 @@ async def _generate_sync_entry_for_device_list( # We're going to mutate these fields, so lets copy them rather than # assume they won't get used later. newly_joined_or_invited_or_knocked_users = set( - mem_changes.newly_joined_or_invited_or_knocked_users + newly_joined_or_invited_or_knocked_users ) - newly_left_users = set(mem_changes.newly_left_users) + newly_left_users = set(newly_left_users) if since_token and since_token.device_list_key: # We want to figure out what user IDs the client should refetch @@ -1310,7 +1304,7 @@ async def _generate_sync_entry_for_device_list( ) # Step 1b, check for newly joined rooms - for room_id in mem_changes.newly_joined_rooms: + for room_id in newly_joined_rooms: joined_users = await self.store.get_users_in_room(room_id) newly_joined_or_invited_or_knocked_users.update(joined_users) @@ -1326,7 +1320,7 @@ async def _generate_sync_entry_for_device_list( users_that_have_changed.update(user_signatures_changed) # Now find users that we no longer track - for room_id in mem_changes.newly_left_rooms: + for room_id in newly_left_rooms: left_users = await self.store.get_users_in_room(room_id) newly_left_users.update(left_users) @@ -1440,8 +1434,8 @@ async def _generate_sync_entry_for_account_data( async def _generate_sync_entry_for_presence( self, sync_result_builder: "SyncResultBuilder", - newly_joined_rooms: Collection[str], - newly_joined_or_invited_users: Collection[str], + newly_joined_rooms: Set[str], + newly_joined_or_invited_users: Set[str], ) -> None: """Generates the presence portion of the sync response. Populates the `sync_result_builder` with the result. @@ -1499,7 +1493,7 @@ async def _generate_sync_entry_for_rooms( self, sync_result_builder: "SyncResultBuilder", account_data_by_room: Dict[str, Dict[str, JsonDict]], - ) -> MembershipChangesSummary: + ) -> Tuple[Set[str], Set[str], Set[str], Set[str]]: """Generates the rooms portion of the sync response. Populates the `sync_result_builder` with the result. @@ -1553,7 +1547,7 @@ async def _generate_sync_entry_for_rooms( ) if not tags_by_room: logger.debug("no-oping sync") - return MembershipChangesSummary(set(), set(), set(), set()) + return set(), set(), set(), set() # 3. Work out which rooms need reporting in the sync response. ignored_users = await self._get_ignored_users(user_id) @@ -1604,7 +1598,7 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: newly_left_users, ) = _calculate_user_changes(sync_result_builder) - return MembershipChangesSummary( + return ( set(newly_joined_rooms), newly_joined_or_invited_or_knocked_users, set(newly_left_rooms), From dd39d8276df299f64c55ae5b1d55904f441a4630 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 6 Dec 2021 19:58:39 +0000 Subject: [PATCH 12/13] Remove unnecessary quote marks. --- synapse/handlers/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index f8b3219976d2..c02986c93ee7 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -2341,7 +2341,7 @@ class SyncResultBuilder: def _calculate_user_changes( - sync_result_builder: "SyncResultBuilder", + sync_result_builder: SyncResultBuilder, ) -> Tuple[Set[str], Set[str]]: """Work out which other users have joined or left rooms we are joined to. From d9cf23da0838025a271beda7837a46ff0d7fd234 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 7 Dec 2021 11:33:30 +0000 Subject: [PATCH 13/13] Make `_calculate_user_changes` a method --- synapse/handlers/sync.py | 63 ++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index c02986c93ee7..74d694de41a5 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1596,7 +1596,7 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: ( newly_joined_or_invited_or_knocked_users, newly_left_users, - ) = _calculate_user_changes(sync_result_builder) + ) = sync_result_builder.calculate_user_changes() return ( set(newly_joined_rooms), @@ -2339,39 +2339,38 @@ class SyncResultBuilder: groups: Optional[GroupsSyncResult] = None to_device: List[JsonDict] = attr.Factory(list) + def calculate_user_changes(self) -> Tuple[Set[str], Set[str]]: + """Work out which other users have joined or left rooms we are joined to. -def _calculate_user_changes( - sync_result_builder: SyncResultBuilder, -) -> Tuple[Set[str], Set[str]]: - """Work out which other users have joined or left rooms we are joined to. + This data only is only useful for an incremental sync. - Only applies to an incremental sync. - - The sync_result_builder is not modified by this function. - """ - newly_joined_or_invited_or_knocked_users = set() - newly_left_users = set() - if sync_result_builder.since_token: - for joined_sync in sync_result_builder.joined: - it = itertools.chain( - joined_sync.timeline.events, joined_sync.state.values() - ) - for event in it: - if event.type == EventTypes.Member: - if ( - event.membership == Membership.JOIN - or event.membership == Membership.INVITE - or event.membership == Membership.KNOCK - ): - newly_joined_or_invited_or_knocked_users.add(event.state_key) - else: - prev_content = event.unsigned.get("prev_content", {}) - prev_membership = prev_content.get("membership", None) - if prev_membership == Membership.JOIN: - newly_left_users.add(event.state_key) - - newly_left_users -= newly_joined_or_invited_or_knocked_users - return newly_joined_or_invited_or_knocked_users, newly_left_users + The SyncResultBuilder is not modified by this function. + """ + newly_joined_or_invited_or_knocked_users = set() + newly_left_users = set() + if self.since_token: + for joined_sync in self.joined: + it = itertools.chain( + joined_sync.timeline.events, joined_sync.state.values() + ) + for event in it: + if event.type == EventTypes.Member: + if ( + event.membership == Membership.JOIN + or event.membership == Membership.INVITE + or event.membership == Membership.KNOCK + ): + newly_joined_or_invited_or_knocked_users.add( + event.state_key + ) + else: + prev_content = event.unsigned.get("prev_content", {}) + prev_membership = prev_content.get("membership", None) + if prev_membership == Membership.JOIN: + newly_left_users.add(event.state_key) + + newly_left_users -= newly_joined_or_invited_or_knocked_users + return newly_joined_or_invited_or_knocked_users, newly_left_users @attr.s(slots=True, auto_attribs=True)