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. (#15163)

This commit is contained in:
reivilibre 2023-02-28 17:11:26 +00:00 committed by GitHub
parent 682d31c702
commit d62cd940cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 77 additions and 2 deletions

1
changelog.d/15163.bugfix Normal file
View File

@ -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.

View File

@ -16,7 +16,7 @@ import logging
from collections import defaultdict from collections import defaultdict
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union 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.errors import Codes, StoreError, SynapseError
from synapse.api.filtering import FilterCollection from synapse.api.filtering import FilterCollection
from synapse.api.presence import UserPresenceState from synapse.api.presence import UserPresenceState
@ -139,7 +139,28 @@ class SyncRestServlet(RestServlet):
device_id, device_id,
) )
request_key = (user, timeout, since, filter_id, full_state, device_id) # 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.
# (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: if filter_id is None:
filter_collection = self.filtering.DEFAULT_FILTER_COLLECTION filter_collection = self.filtering.DEFAULT_FILTER_COLLECTION

View File

@ -237,6 +237,37 @@ class AccountDataWorkerStore(PushRulesWorkerStore, CacheInvalidationWorkerStore)
else: else:
return None 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) @cached(num_args=2, tree=True)
async def get_account_data_for_room( async def get_account_data_for_room(
self, user_id: str, room_id: str self, user_id: str, room_id: str

View File

@ -140,3 +140,25 @@ class IgnoredUsersTestCase(unittest.HomeserverTestCase):
# No one ignores the user now. # No one ignores the user now.
self.assert_ignored(self.user, set()) self.assert_ignored(self.user, set())
self.assert_ignorers("@other:test", 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)