From fe18fa8c76f92e32c301e232b8b396f9d2c46f47 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 27 Feb 2023 12:51:31 +0000 Subject: [PATCH 1/4] Invalidate initial sync if ignored user set changes --- synapse/rest/client/sync.py | 25 +++++++++++++-- .../storage/databases/main/account_data.py | 31 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index f2013faeb206..e424bd849984 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -16,7 +16,7 @@ from collections import defaultdict from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union -from synapse.api.constants import EduTypes, Membership, PresenceState +from synapse.api.constants import AccountDataTypes, EduTypes, Membership, PresenceState from synapse.api.errors import Codes, StoreError, SynapseError from synapse.api.filtering import FilterCollection from synapse.api.presence import UserPresenceState @@ -139,7 +139,28 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: device_id, ) - request_key = (user, timeout, since, filter_id, full_state, device_id) + # ID of the last ignored users account data event for this user, + # if we're initial syncing. + # We include this in the request key to invalidate an initial sync + # in the response cache once the set of ignored users has changed. + # (We filter out ignored users from timeline events, so our sync response + # is invalid once the set of ignored users changes.) + last_ignore_accdata_streampos: Optional[int] = None + if not since: + # No `since`, so this is an initial sync. + last_ignore_accdata_streampos = await self.store.get_latest_stream_id_for_global_account_data_by_type_for_user( + user.to_string(), AccountDataTypes.IGNORED_USER_LIST + ) + + request_key = ( + user, + timeout, + since, + filter_id, + full_state, + device_id, + last_ignore_accdata_streampos, + ) if filter_id is None: filter_collection = self.filtering.DEFAULT_FILTER_COLLECTION diff --git a/synapse/storage/databases/main/account_data.py b/synapse/storage/databases/main/account_data.py index 95567826f27a..308d19440f63 100644 --- a/synapse/storage/databases/main/account_data.py +++ b/synapse/storage/databases/main/account_data.py @@ -237,6 +237,37 @@ async def get_global_account_data_by_type_for_user( else: return None + async def get_latest_stream_id_for_global_account_data_by_type_for_user( + self, user_id: str, data_type: str + ) -> Optional[int]: + """ + Returns: + The stream ID of the account data, + or None if there is no such account data. + """ + + def get_latest_stream_id_for_global_account_data_by_type_for_user_txn( + txn: LoggingTransaction, + ) -> Optional[int]: + sql = """ + SELECT stream_id FROM account_data + WHERE user_id = ? AND account_data_type = ? + ORDER BY stream_id DESC + LIMIT 1 + """ + txn.execute(sql, (user_id, data_type)) + + row = txn.fetchone() + if row: + return row[0] + else: + return None + + return await self.db_pool.runInteraction( + "get_latest_stream_id_for_global_account_data_by_type_for_user", + get_latest_stream_id_for_global_account_data_by_type_for_user_txn, + ) + @cached(num_args=2, tree=True) async def get_account_data_for_room( self, user_id: str, room_id: str From e478dd74e3360f4aba6f4186c6db1e79ae395cde Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 27 Feb 2023 12:52:40 +0000 Subject: [PATCH 2/4] Newsfile Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/15163.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15163.bugfix diff --git a/changelog.d/15163.bugfix b/changelog.d/15163.bugfix new file mode 100644 index 000000000000..7ff1cd4463cd --- /dev/null +++ b/changelog.d/15163.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where an initial sync would not respond to changes to the list of ignored users if there was an initial sync cached. \ No newline at end of file From 29e0f1f795f342fed5a82958f86124a54a0f058c Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Feb 2023 11:35:44 +0000 Subject: [PATCH 3/4] Add a test --- tests/storage/test_account_data.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/storage/test_account_data.py b/tests/storage/test_account_data.py index 1bfd11ceae4f..b12691a9d354 100644 --- a/tests/storage/test_account_data.py +++ b/tests/storage/test_account_data.py @@ -140,3 +140,25 @@ def test_invalid_data(self) -> None: # No one ignores the user now. self.assert_ignored(self.user, set()) self.assert_ignorers("@other:test", set()) + + def test_ignoring_users_with_latest_stream_ids(self) -> None: + """Test that ignoring users updates the latest stream ID for the ignored + user list account data.""" + + def get_latest_ignore_streampos(user_id: str) -> Optional[int]: + return self.get_success( + self.store.get_latest_stream_id_for_global_account_data_by_type_for_user( + user_id, AccountDataTypes.IGNORED_USER_LIST + ) + ) + + self.assertIsNone(get_latest_ignore_streampos("@user:test")) + + self._update_ignore_list("@other:test", "@another:remote") + + self.assertEqual(get_latest_ignore_streampos("@user:test"), 2) + + # Add one user, remove one user, and leave one user. + self._update_ignore_list("@foo:test", "@another:remote") + + self.assertEqual(get_latest_ignore_streampos("@user:test"), 3) From 3e047905f9606ceeddf6c491caf18399b26987bf Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 28 Feb 2023 16:29:40 +0000 Subject: [PATCH 4/4] Wording tweak --- synapse/rest/client/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index e424bd849984..8fcb8ac3d9d4 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -139,7 +139,7 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: device_id, ) - # ID of the last ignored users account data event for this user, + # Stream position of the last ignored users account data event for this user, # if we're initial syncing. # We include this in the request key to invalidate an initial sync # in the response cache once the set of ignored users has changed.