From c90597fe70247bc70448147b8566a6095b10659c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 19 Jan 2023 12:08:12 +0000 Subject: [PATCH 1/3] Remove non-existent kwarg passed to function _remove_user_data_client _remove_user_data_client does not take a `content` argument. --- synapse/handlers/account_data.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/handlers/account_data.py b/synapse/handlers/account_data.py index 797de46dbc3b..ed663c2773ea 100644 --- a/synapse/handlers/account_data.py +++ b/synapse/handlers/account_data.py @@ -248,7 +248,6 @@ async def remove_account_data_for_user( instance_name=random.choice(self._account_data_writers), user_id=user_id, account_data_type=account_data_type, - content={}, ) return response["max_stream_id"] From e0ae9cfc68c93ccffb712fe3378a8c3c3239a4f3 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 2 Mar 2023 17:36:53 +0000 Subject: [PATCH 2/3] Always return max stream ID from remove account data repl cmds ...instead of returning None when there was not actually any account data to delete. This means that in the case of a replication call to remove account data that didn't exist, the replication response body is now {max_stream_id: current_stream_id} instead of {max_stream_id: None}. --- synapse/handlers/account_data.py | 6 ---- .../storage/databases/main/account_data.py | 34 ++++++++----------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/synapse/handlers/account_data.py b/synapse/handlers/account_data.py index ed663c2773ea..7e01c18c6c90 100644 --- a/synapse/handlers/account_data.py +++ b/synapse/handlers/account_data.py @@ -155,9 +155,6 @@ async def remove_account_data_for_room( max_stream_id = await self._store.remove_account_data_for_room( user_id, room_id, account_data_type ) - if max_stream_id is None: - # The referenced account data did not exist, so no delete occurred. - return None self._notifier.on_new_event( StreamKeyType.ACCOUNT_DATA, max_stream_id, users=[user_id] @@ -230,9 +227,6 @@ async def remove_account_data_for_user( max_stream_id = await self._store.remove_account_data_for_user( user_id, account_data_type ) - if max_stream_id is None: - # The referenced account data did not exist, so no delete occurred. - return None self._notifier.on_new_event( StreamKeyType.ACCOUNT_DATA, max_stream_id, users=[user_id] diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 308d19440f63..22fb8cc71177 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -585,7 +585,7 @@ async def add_account_data_to_room( async def remove_account_data_for_room( self, user_id: str, room_id: str, account_data_type: str - ) -> Optional[int]: + ) -> int: """Delete the room account data for the user of a given type. Args: @@ -637,15 +637,13 @@ def _remove_account_data_for_room_txn( next_id, ) - if not row_updated: - return None - - self._account_data_stream_cache.entity_has_changed(user_id, next_id) - self.get_room_account_data_for_user.invalidate((user_id,)) - self.get_account_data_for_room.invalidate((user_id, room_id)) - self.get_account_data_for_room_and_type.prefill( - (user_id, room_id, account_data_type), {} - ) + if row_updated: + self._account_data_stream_cache.entity_has_changed(user_id, next_id) + self.get_room_account_data_for_user.invalidate((user_id,)) + self.get_account_data_for_room.invalidate((user_id, room_id)) + self.get_account_data_for_room_and_type.prefill( + (user_id, room_id, account_data_type), {} + ) return self._account_data_id_gen.get_current_token() @@ -753,7 +751,7 @@ async def remove_account_data_for_user( self, user_id: str, account_data_type: str, - ) -> Optional[int]: + ) -> int: """ Delete a single piece of user account data by type. @@ -840,14 +838,12 @@ def _remove_account_data_for_user_txn( next_id, ) - if not row_updated: - return None - - self._account_data_stream_cache.entity_has_changed(user_id, next_id) - self.get_global_account_data_for_user.invalidate((user_id,)) - self.get_global_account_data_by_type_for_user.prefill( - (user_id, account_data_type), {} - ) + if row_updated: + self._account_data_stream_cache.entity_has_changed(user_id, next_id) + self.get_global_account_data_for_user.invalidate((user_id,)) + self.get_global_account_data_by_type_for_user.prefill( + (user_id, account_data_type), {} + ) return self._account_data_id_gen.get_current_token() From 74b6981beb6df312e6e7709e27c801b85f2bd7c8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 2 Mar 2023 18:23:40 +0000 Subject: [PATCH 3/3] changelog --- changelog.d/14869.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14869.bugfix diff --git a/changelog.d/14869.bugfix b/changelog.d/14869.bugfix new file mode 100644 index 000000000000..865b597741bb --- /dev/null +++ b/changelog.d/14869.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.75.0rc1 that caused experimental support for deleting account data to raise an internal server error while using an account data writer worker. \ No newline at end of file