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.

4 files changed, 62 insertions, 6 deletions

diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index a26e18c8a..b92b17773 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -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
 -------------
diff --git a/src/mailman/model/tests/test_usermanager.py b/src/mailman/model/tests/test_usermanager.py
index f4643f031..9ecaca4e3 100644
--- a/src/mailman/model/tests/test_usermanager.py
+++ b/src/mailman/model/tests/test_usermanager.py
@@ -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('anne@example.com')
         copy = self._usermanager.get_user_by_id(original.user_id)
         self.assertEqual(original, copy)
+
+    def test_delete_user(self):
+        user = self._usermanager.make_user('anne@example.com', 'Anne Person')
+        address = self._usermanager.create_address('anne.address@example.com')
+        address.verified_on = now()
+        user.preferred_address = address
+        # Subscribe the user and the address to a list.
+        mlist = create_list('ant@example.com')
+        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('anne@example.com'))
+        self.assertIsNone(
+            self._usermanager.get_address('anne.address@example.com'))
+
+    def test_delete_address(self):
+        address = self._usermanager.create_address('anne@example.com')
+        address.verified_on = now()
+        # Subscribe the address to a list.
+        mlist = create_list('ant@example.com')
+        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('anne@example.com'))
diff --git a/src/mailman/model/usermanager.py b/src/mailman/model/usermanager.py
index 5d63a2146..490214149 100644
--- a/src/mailman/model/usermanager.py
+++ b/src/mailman/model/usermanager.py
@@ -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
diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py
index 2398f5f7e..2242c3ab7 100644
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -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)