[prev in list] [next in list] [prev in thread] [next in thread]
List: mailman-cvs
Subject: [Mailman-checkins] [Git][mailman/mailman][master] When deleting an Address, dependencies must be del
From: Barry Warsaw <gitlab () mg ! gitlab ! com>
Date: 2015-11-22 4:17:25
Message-ID: 565141d5d482_7261065f1c6851c () worker2 ! cluster ! gitlab ! com ! mail
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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('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'))
=====================================
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
[Attachment #5 (text/html)]
<html lang='en'>
<head>
<meta content='text/html; charset=utf-8' http-equiv='Content-Type'>
<title>
GitLab
</title>
</meta>
</head>
<style>
img {
max-width: 100%;
height: auto;
}
p.details {
font-style:italic;
color:#777
}
.footer p {
font-size:small;
color:#777
}
pre.commit-message {
white-space: pre-wrap;
}
.file-stats a {
text-decoration: none;
}
.file-stats .new-file {
color: #090;
}
.file-stats .deleted-file {
color: #B00;
}
</style>
<body>
<div class='content'>
<h3>Barry Warsaw pushed to branch master at <a \
href="https://gitlab.com/mailman/mailman">mailman / Mailman</a></h3> <h4>
Commits:
</h4>
<ul>
<li>
<strong><a href="https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542">8040aeab</a></strong>
<div>
<span>by Aurélien Bompard</span>
<i>at 2015-11-21T23:16:59Z</i>
</div>
<pre class='commit-message'>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.</pre>
</li>
</ul>
<h4>4 changed files:</h4>
<ul>
<li class='file-stats'>
<a href='#diff-0'>
src/mailman/docs/NEWS.rst
</a>
</li>
<li class='file-stats'>
<a href='#diff-1'>
src/mailman/model/tests/test_usermanager.py
</a>
</li>
<li class='file-stats'>
<a href='#diff-2'>
src/mailman/model/usermanager.py
</a>
</li>
<li class='file-stats'>
<a href='#diff-3'>
src/mailman/rest/users.py
</a>
</li>
</ul>
<h4>Changes:</h4>
<li id='diff-0'>
<a href='https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542#diff-0'>
<strong>
src/mailman/docs/NEWS.rst
</strong>
</a>
<hr>
<pre class="highlight"><code><span style="color: #000000;background-color: \
#ffdddd">--- a/src/mailman/docs/NEWS.rst </span><span style="color: \
#000000;background-color: #ddffdd">+++ b/src/mailman/docs/NEWS.rst </span><span \
style="color: #aaaaaa">@@ -46,6 +46,8 @@ Bugs </span> email to the mailing list \
moderators. (Closes: #144)
* Fix traceback in approved handler when the moderator password is None.
Given by Aurélien Bompard.
<span style="color: #000000;background-color: #ddffdd">+ * Fix IntegrityErrors raised \
under PostreSQL when deleting users and + addresses. Given by Aurélien Bompard.
</span>
Configuration
-------------
</code></pre>
<br>
</li>
<li id='diff-1'>
<a href='https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542#diff-1'>
<strong>
src/mailman/model/tests/test_usermanager.py
</strong>
</a>
<hr>
<pre class="highlight"><code><span style="color: #000000;background-color: \
#ffdddd">--- a/src/mailman/model/tests/test_usermanager.py </span><span style="color: \
#000000;background-color: #ddffdd">+++ b/src/mailman/model/tests/test_usermanager.py \
</span><span style="color: #aaaaaa">@@ -24,9 +24,14 @@ __all__ = [ </span>
import unittest
<span style="color: #000000;background-color: #ddffdd">+from mailman.app.lifecycle \
import create_list +from mailman.config import config
</span> from mailman.interfaces.address import ExistingAddressError
<span style="color: #000000;background-color: #ddffdd">+from \
mailman.interfaces.autorespond import IAutoResponseSet, Response +from \
mailman.interfaces.member import DeliveryMode </span> from \
mailman.interfaces.usermanager import IUserManager from mailman.testing.layers \
import ConfigLayer <span style="color: #000000;background-color: #ddffdd">+from \
mailman.utilities.datetime import now </span> from zope.component import getUtility
<span style="color: #aaaaaa">@@ -86,3 +91,39 @@ class \
TestUserManager(unittest.TestCase): </span> original = \
self._usermanager.make_user('anne@example.com') copy = \
self._usermanager.get_user_by_id(original.user_id) self.assertEqual(original, copy)
<span style="color: #000000;background-color: #ddffdd">+
+ 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'))
</span></code></pre>
<br>
</li>
<li id='diff-2'>
<a href='https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542#diff-2'>
<strong>
src/mailman/model/usermanager.py
</strong>
</a>
<hr>
<pre class="highlight"><code><span style="color: #000000;background-color: \
#ffdddd">--- a/src/mailman/model/usermanager.py </span><span style="color: \
#000000;background-color: #ddffdd">+++ b/src/mailman/model/usermanager.py \
</span><span style="color: #aaaaaa">@@ -26,6 +26,8 @@ from \
mailman.database.transaction import dbconnection </span> from \
mailman.interfaces.address import ExistingAddressError from \
mailman.interfaces.usermanager import IUserManager from mailman.model.address import \
Address <span style="color: #000000;background-color: #ddffdd">+from \
mailman.model.autorespond import AutoResponseRecord +from mailman.model.digests \
import OneLastDigest </span> from mailman.model.member import Member
from mailman.model.preferences import Preferences
from mailman.model.user import User
<span style="color: #aaaaaa">@@ -69,6 +71,15 @@ class UserManager:
</span> @dbconnection
def delete_user(self, store, user):
"""See `IUserManager`."""
<span style="color: #000000;background-color: #ddffdd">+ # 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()
</span> store.delete(user.preferences)
store.delete(user)
<span style="color: #aaaaaa">@@ -119,6 +130,14 @@ class UserManager:
</span> # unlinked before the address can be deleted.
if address.user:
address.user.unlink(address)
<span style="color: #000000;background-color: #ddffdd">+ # 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.
</span> store.delete(address)
@dbconnection
</code></pre>
<br>
</li>
<li id='diff-3'>
<a href='https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542#diff-3'>
<strong>
src/mailman/rest/users.py
</strong>
</a>
<hr>
<pre class="highlight"><code><span style="color: #000000;background-color: \
#ffdddd">--- a/src/mailman/rest/users.py </span><span style="color: \
#000000;background-color: #ddffdd">+++ b/src/mailman/rest/users.py </span><span \
style="color: #aaaaaa">@@ -222,12 +222,6 @@ class AUser(_UserBase): </span> \
for member in self._user.memberships.members: member.unsubscribe()
user_manager = getUtility(IUserManager)
<span style="color: #000000;background-color: #ffdddd">- # 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)
</span> user_manager.delete_user(self._user)
no_content(response)
</code></pre>
<br>
</li>
</div>
<div class='footer' style='margin-top: 10px;'>
<p>
—
<br>
<a href="https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542">View \
it on GitLab</a>. <br>
You're receiving this email because of your account on gitlab.com.
If you'd like to receive fewer emails, you can adjust your notification settings.
<script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View \
Commit","url":"https://gitlab.com/mailman/mailman/commit/8040aeab55f2f2def7129893756e7a97b0745542"}}</script>
</p>
</div>
</body>
</html>
_______________________________________________
Mailman-checkins mailing list
Mailman-checkins@python.org
Unsubscribe: https://mail.python.org/mailman/options/mailman-checkins/mailman-cvs%40progressive-comp.com
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic