Barry Warsaw pushed to branch master at mailman / Mailman
Commits:
8040aeab by Aurélien Bompard at 2015-11-21T23:16:59Z
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 changed files:
- src/mailman/docs/NEWS.rst
- src/mailman/model/tests/test_usermanager.py
- src/mailman/model/usermanager.py
- src/mailman/rest/users.py
Changes:
=====================================
src/mailman/docs/NEWS.rst
=====================================
--- 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
-------------
=====================================
src/mailman/model/tests/test_usermanager.py
=====================================
--- 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('a...@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('a...@example.com', 'Anne Person')
+ address = self._usermanager.create_address('anne.addr...@example.com')
+ address.verified_on = now()
+ user.preferred_address = address
+ # Subscribe the user and the address to a list.
+ mlist = create_list('a...@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('a...@example.com'))
+ self.assertIsNone(
+ self._usermanager.get_address('anne.addr...@example.com'))
+
+ def test_delete_address(self):
+ address = self._usermanager.create_address('a...@example.com')
+ address.verified_on = now()
+ # Subscribe the address to a list.
+ mlist = create_list('a...@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('a...@example.com'))
=====================================
src/mailman/model/usermanager.py
=====================================
--- 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
=====================================
src/mailman/rest/users.py
=====================================
--- 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)
View it on GitLab:
https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe:
https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
