Skip to content

Commit

Permalink
Fix bug where push rules would be empty in /sync (#17142)
Browse files Browse the repository at this point in the history
Fixes #16987

Some old accounts seem to have an entry in global account data table for
push rules, which we should ignore
  • Loading branch information
erikjohnston authored May 16, 2024
1 parent d2d48cc commit 5e89267
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 13 deletions.
1 change: 1 addition & 0 deletions changelog.d/17142.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where push rules would be empty in `/sync` for some accounts. Introduced in v1.93.0.
20 changes: 8 additions & 12 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -1971,23 +1971,19 @@ async def _generate_sync_entry_for_account_data(
)

if push_rules_changed:
global_account_data = {
AccountDataTypes.PUSH_RULES: await self._push_rules_handler.push_rules_for_user(
sync_config.user
),
**global_account_data,
}
global_account_data = dict(global_account_data)
global_account_data[AccountDataTypes.PUSH_RULES] = (
await self._push_rules_handler.push_rules_for_user(sync_config.user)
)
else:
all_global_account_data = await self.store.get_global_account_data_for_user(
user_id
)

global_account_data = {
AccountDataTypes.PUSH_RULES: await self._push_rules_handler.push_rules_for_user(
sync_config.user
),
**all_global_account_data,
}
global_account_data = dict(all_global_account_data)
global_account_data[AccountDataTypes.PUSH_RULES] = (
await self._push_rules_handler.push_rules_for_user(sync_config.user)
)

account_data_for_user = (
await sync_config.filter_collection.filter_global_account_data(
Expand Down
29 changes: 28 additions & 1 deletion tests/handlers/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import EventTypes, JoinRules
from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules
from synapse.api.errors import Codes, ResourceLimitError
from synapse.api.filtering import FilterCollection, Filtering
from synapse.api.room_versions import RoomVersion, RoomVersions
Expand Down Expand Up @@ -895,6 +895,33 @@ async def _check_sigs_and_hash_for_pulled_events_and_fetch(

self.assertIn(private_call_event.event_id, priv_event_ids)

def test_push_rules_with_bad_account_data(self) -> None:
"""Some old accounts have managed to set a `m.push_rules` account data,
which we should ignore in /sync response.
"""

user = self.register_user("alice", "password")

# Insert the bad account data.
self.get_success(
self.store.add_account_data_for_user(user, AccountDataTypes.PUSH_RULES, {})
)

sync_result: SyncResult = self.get_success(
self.sync_handler.wait_for_sync_for_user(
create_requester(user), generate_sync_config(user)
)
)

for account_dict in sync_result.account_data:
if account_dict["type"] == AccountDataTypes.PUSH_RULES:
# We should have lots of push rules here, rather than the bad
# empty data.
self.assertNotEqual(account_dict["content"], {})
return

self.fail("No push rules found")


_request_key = 0

Expand Down

0 comments on commit 5e89267

Please sign in to comment.