Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

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

Merged
merged 4 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/15163.bugfix
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 23 additions & 2 deletions synapse/rest/client/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ID -> Stream position, for consistency?

# 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
Expand Down
31 changes: 31 additions & 0 deletions synapse/storage/databases/main/account_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions tests/storage/test_account_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)