Commit 8040aeab authored by Aurélien Bompard's avatar Aurélien Bompard Committed by Barry Warsaw

When deleting an Address, dependencies must be deleted first

SQLite doesn't not enforce foreign key constraints, but PostgreSQL does,
and without this fix, IntegrityErrors get raised.
parent 6c002fbd
Pipeline #273861 passed with stage
......@@ -46,6 +46,8 @@ Bugs
email to the mailing list moderators. (Closes: #144)
* Fix traceback in approved handler when the moderator password is None.
Given by Aurélien Bompard.
* Fix IntegrityErrors raised under PostreSQL when deleting users and
addresses. Given by Aurélien Bompard.
Configuration
-------------
......
......@@ -24,9 +24,14 @@ __all__ = [
import unittest
from mailman.app.lifecycle import create_list
from mailman.config import config
from mailman.interfaces.address import ExistingAddressError
from mailman.interfaces.autorespond import IAutoResponseSet, Response
from mailman.interfaces.member import DeliveryMode
from mailman.interfaces.usermanager import IUserManager
from mailman.testing.layers import ConfigLayer
from mailman.utilities.datetime import now
from zope.component import getUtility
......@@ -86,3 +91,39 @@ class TestUserManager(unittest.TestCase):
original = self._usermanager.make_user('[email protected]')
copy = self._usermanager.get_user_by_id(original.user_id)
self.assertEqual(original, copy)
def test_delete_user(self):
user = self._usermanager.make_user('[email protected]', 'Anne Person')
address = self._usermanager.create_address('[email protected]')
address.verified_on = now()
user.preferred_address = address
# Subscribe the user and the address to a list.
mlist = create_list('[email protected]')
mlist.subscribe(user)
mlist.subscribe(address)
# Now delete the user.
self._usermanager.delete_user(user)
# Flush the database to provoke an integrity error on PostgreSQL
# without the fix.
config.db.store.flush()
self.assertIsNone(self._usermanager.get_user('[email protected]'))
self.assertIsNone(
self._usermanager.get_address('[email protected]'))
def test_delete_address(self):
address = self._usermanager.create_address('[email protected]')
address.verified_on = now()
# Subscribe the address to a list.
mlist = create_list('[email protected]')
mlist.subscribe(address)
# Set an autorespond record.
response_set = IAutoResponseSet(mlist)
response_set.response_sent(address, Response.hold)
# And add a digest record.
mlist.send_one_last_digest_to(address, DeliveryMode.plaintext_digests)
# Now delete the address.
self._usermanager.delete_address(address)
# Flush the database to provoke an integrity error on PostgreSQL
# without the fix.
config.db.store.flush()
self.assertIsNone(self._usermanager.get_address('[email protected]'))
......@@ -26,6 +26,8 @@ from mailman.database.transaction import dbconnection
from mailman.interfaces.address import ExistingAddressError
from mailman.interfaces.usermanager import IUserManager
from mailman.model.address import Address
from mailman.model.autorespond import AutoResponseRecord
from mailman.model.digests import OneLastDigest
from mailman.model.member import Member
from mailman.model.preferences import Preferences
from mailman.model.user import User
......@@ -69,6 +71,15 @@ class UserManager:
@dbconnection
def delete_user(self, store, user):
"""See `IUserManager`."""
# SQLAlchemy is susceptable to delete-elements-while-iterating bugs so
# first figure out all the addresses we want to delete, then in a
# separate pass, delete those addresses. (See LP: #1419519)
to_delete = list(user.addresses)
for address in to_delete:
self.delete_address(address)
# Remove memberships.
for membership in store.query(Member).filter_by(user_id=user.id):
membership.unsubscribe()
store.delete(user.preferences)
store.delete(user)
......@@ -119,6 +130,14 @@ class UserManager:
# unlinked before the address can be deleted.
if address.user:
address.user.unlink(address)
# Remove memberships.
for membership in store.query(Member).filter_by(address_id=address.id):
membership.unsubscribe()
# Remove auto-response records.
store.query(AutoResponseRecord).filter_by(address=address).delete()
# Remove last digest record.
store.query(OneLastDigest).filter_by(address=address).delete()
# Now delete the address.
store.delete(address)
@dbconnection
......
......@@ -222,12 +222,6 @@ class AUser(_UserBase):
for member in self._user.memberships.members:
member.unsubscribe()
user_manager = getUtility(IUserManager)
# SQLAlchemy is susceptable to delete-elements-while-iterating bugs so
# first figure out all the addresses we want to delete, then in a
# separate pass, delete those addresses. (See LP: #1419519)
delete = list(self._user.addresses)
for address in delete:
user_manager.delete_address(address)
user_manager.delete_user(self._user)
no_content(response)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment