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

Fix iteration in _remove_deleted_email_pushers background job. #10734

Merged
merged 4 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from all 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/10734.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove pushers when deleting a 3pid from an account. Pushers for old unlinked emails will also be deleted.
3 changes: 2 additions & 1 deletion synapse/storage/databases/main/pusher.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,10 +430,11 @@ def _delete_pushers(txn) -> int:
"""

txn.execute(sql, (last_pusher, batch_size))
rows = txn.fetchall()

last = None
num_deleted = 0
for row in txn:
for row in rows:
last = row[0]
num_deleted += 1
self.db_pool.simple_delete_txn(
Expand Down
44 changes: 44 additions & 0 deletions tests/push/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,50 @@ def test_no_email_sent_after_removed(self):
pushers = list(pushers)
self.assertEqual(len(pushers), 0)

def test_remove_unlinked_pushers_background_job(self):
"""Checks that all existing pushers associated with unlinked email addresses are removed
upon running the remove_deleted_email_pushers background update.
"""
# disassociate the user's email address manually (without deleting the pusher).
# This resembles the old behaviour, which the background update below is intended
# to clean up.
self.get_success(
self.hs.get_datastore().user_delete_threepid(
self.user_id, "email", "[email protected]"
)
)

# Run the "remove_deleted_email_pushers" background job
self.get_success(
self.hs.get_datastore().db_pool.simple_insert(
table="background_updates",
values={
"update_name": "remove_deleted_email_pushers",
"progress_json": "{}",
"depends_on": None,
},
)
)

# ... and tell the DataStore that it hasn't finished all updates yet
self.hs.get_datastore().db_pool.updates._all_done = False

# Now let's actually drive the updates to completion
while not self.get_success(
self.hs.get_datastore().db_pool.updates.has_completed_background_updates()
):
self.get_success(
self.hs.get_datastore().db_pool.updates.do_next_background_update(100),
by=0.1,
)
Comment on lines +360 to +382
Copy link
Member

Choose a reason for hiding this comment

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

We should probably extract all this to a utility function. Maybe next time.


# Check that all pushers with unlinked addresses were deleted
pushers = self.get_success(
self.hs.get_datastore().get_pushers_by({"user_name": self.user_id})
)
pushers = list(pushers)
self.assertEqual(len(pushers), 0)

def _check_for_mail(self):
"""Check that the user receives an email notification"""

Expand Down