summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/mailman/commands/cli_import.py25
-rw-r--r--src/mailman/database/tests/test_factory.py11
-rw-r--r--src/mailman/docs/NEWS.rst4
-rw-r--r--src/mailman/model/listmanager.py4
-rw-r--r--src/mailman/model/mailinglist.py3
-rw-r--r--src/mailman/model/tests/test_mailinglist.py24
-rw-r--r--src/mailman/model/tests/test_user.py32
-rw-r--r--src/mailman/model/user.py4
-rw-r--r--src/mailman/model/usermanager.py1
-rw-r--r--src/mailman/rest/tests/test_lists.py16
-rw-r--r--src/mailman/rest/tests/test_users.py21
-rw-r--r--src/mailman/rest/users.py2
-rw-r--r--src/mailman/utilities/importer.py10
-rw-r--r--src/mailman/utilities/tests/test_import.py36
14 files changed, 151 insertions, 42 deletions
diff --git a/src/mailman/commands/cli_import.py b/src/mailman/commands/cli_import.py
index 50d4f1864..344d5baee 100644
--- a/src/mailman/commands/cli_import.py
+++ b/src/mailman/commands/cli_import.py
@@ -25,6 +25,7 @@ __all__ = [
import sys
import pickle
+from contextlib import ExitStack, contextmanager
from mailman.core.i18n import _
from mailman.database.transaction import transactional
from mailman.interfaces.command import ICLISubCommand
@@ -35,13 +36,24 @@ from zope.interface import implementer
-# Mock the Bouncer class from Mailman 2.1, we don't use it but there are
-# instances in the pickled config files
+# A fake Bouncer class from Mailman 2.1, we don't use it but there are
+# instances in the .pck files.
class Bouncer:
class _BounceInfo:
pass
+@contextmanager
+def hacked_sys_modules():
+ assert 'Mailman.Bouncer' not in sys.modules
+ sys.modules['Mailman.Bouncer'] = Bouncer
+ try:
+ yield
+ finally:
+ del sys.modules['Mailman.Bouncer']
+
+
+
@implementer(ICLISubCommand)
class Import21:
@@ -82,11 +94,13 @@ class Import21:
assert len(args.pickle_file) == 1, (
'Unexpected positional arguments: %s' % args.pickle_file)
filename = args.pickle_file[0]
- sys.modules["Mailman.Bouncer"] = Bouncer
- with open(filename, 'rb') as fp:
+ with ExitStack() as resources:
+ fp = resources.enter_context(open(filename, 'rb'))
+ resources.enter_context(hacked_sys_modules())
while True:
try:
- config_dict = pickle.load(fp, encoding="utf-8", errors="ignore")
+ config_dict = pickle.load(
+ fp, encoding='utf-8', errors='ignore')
except EOFError:
break
except pickle.UnpicklingError:
@@ -103,4 +117,3 @@ class Import21:
except Import21Error as error:
print(error, file=sys.stderr)
sys.exit(1)
- del sys.modules["Mailman.Bouncer"]
diff --git a/src/mailman/database/tests/test_factory.py b/src/mailman/database/tests/test_factory.py
index 6a16c74ab..acb7c32a4 100644
--- a/src/mailman/database/tests/test_factory.py
+++ b/src/mailman/database/tests/test_factory.py
@@ -129,17 +129,14 @@ class TestSchemaManager(unittest.TestCase):
md.tables['alembic_version'].select()).scalar()
self.assertEqual(current_rev, head_rev)
- @patch('alembic.command.stamp')
- def test_storm(self, alembic_command_stamp):
+ @patch('alembic.command')
+ def test_storm(self, alembic_command):
# Existing Storm database.
Model.metadata.create_all(config.db.engine)
self._create_storm_database(LAST_STORM_SCHEMA_VERSION)
self.schema_mgr.setup_database()
- self.assertFalse(alembic_command_stamp.called)
- self.assertTrue(
- self._table_exists('mailinglist')
- and self._table_exists('alembic_version')
- and not self._table_exists('version'))
+ self.assertFalse(alembic_command.stamp.called)
+ self.assertTrue(alembic_command.upgrade.called)
@patch('alembic.command')
def test_old_storm(self, alembic_command):
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 0c1f6bc63..77c45375d 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -26,6 +26,10 @@ Bugs
a 409 error instead of a 500 error. Found by Ankush Sharma. (LP: #1425359)
* ``mailman lists --domain`` was not properly handling its arguments. Given
by Manish Gill. (LP: #1166911)
+ * When deleting a user object, make sure their preferences are also deleted.
+ Given by Abhishek. (LP: #1418276)
+ * Be sure a mailing list's acceptable aliases are deleted when the mailing
+ list itself is deleted. (LP: #1432239)
Configuration
-------------
diff --git a/src/mailman/model/listmanager.py b/src/mailman/model/listmanager.py
index be0e153a3..8fc739543 100644
--- a/src/mailman/model/listmanager.py
+++ b/src/mailman/model/listmanager.py
@@ -27,7 +27,7 @@ from mailman.interfaces.address import InvalidEmailAddressError
from mailman.interfaces.listmanager import (
IListManager, ListAlreadyExistsError, ListCreatedEvent, ListCreatingEvent,
ListDeletedEvent, ListDeletingEvent)
-from mailman.model.mailinglist import MailingList
+from mailman.model.mailinglist import IAcceptableAliasSet, MailingList
from mailman.model.mime import ContentFilter
from mailman.utilities.datetime import now
from zope.event import notify
@@ -74,6 +74,8 @@ class ListManager:
"""See `IListManager`."""
fqdn_listname = mlist.fqdn_listname
notify(ListDeletingEvent(mlist))
+ # First delete information associated with the mailing list.
+ IAcceptableAliasSet(mlist).clear()
store.query(ContentFilter).filter_by(mailing_list=mlist).delete()
store.delete(mlist)
notify(ListDeletedEvent(fqdn_listname))
diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py
index a204d54cd..f7ef04314 100644
--- a/src/mailman/model/mailinglist.py
+++ b/src/mailman/model/mailinglist.py
@@ -504,10 +504,11 @@ class AcceptableAlias(Model):
mailing_list_id = Column(
Integer, ForeignKey('mailinglist.id'),
index=True, nullable=False)
- mailing_list = relationship('MailingList', backref='acceptable_alias')
+ mailing_list = relationship('MailingList', backref='acceptablealias')
alias = Column(Unicode, index=True, nullable=False)
def __init__(self, mailing_list, alias):
+ super(AcceptableAlias, self).__init__()
self.mailing_list = mailing_list
self.alias = alias
diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py
index 843918e5e..745096b4b 100644
--- a/src/mailman/model/tests/test_mailinglist.py
+++ b/src/mailman/model/tests/test_mailinglist.py
@@ -18,6 +18,7 @@
"""Test MailingLists and related model objects.."""
__all__ = [
+ 'TestAcceptableAliases',
'TestDisabledListArchiver',
'TestListArchiver',
'TestMailingList',
@@ -28,7 +29,10 @@ import unittest
from mailman.app.lifecycle import create_list
from mailman.config import config
-from mailman.interfaces.mailinglist import IListArchiverSet
+from mailman.database.transaction import transaction
+from mailman.interfaces.listmanager import IListManager
+from mailman.interfaces.mailinglist import (
+ IAcceptableAliasSet, IListArchiverSet)
from mailman.interfaces.member import (
AlreadySubscribedError, MemberRole, MissingPreferredAddressError)
from mailman.interfaces.usermanager import IUserManager
@@ -141,3 +145,21 @@ class TestDisabledListArchiver(unittest.TestCase):
archiver = archiver_set.get('prototype')
self.assertTrue(archiver.is_enabled)
config.pop('enable prototype')
+
+
+
+class TestAcceptableAliases(unittest.TestCase):
+ layer = ConfigLayer
+
+ def setUp(self):
+ self._mlist = create_list('ant@example.com')
+
+ def test_delete_list_with_acceptable_aliases(self):
+ # LP: #1432239 - deleting a mailing list with acceptable aliases
+ # causes a SQLAlchemy error. The aliases must be deleted first.
+ with transaction():
+ alias_set = IAcceptableAliasSet(self._mlist)
+ alias_set.add('bee@example.com')
+ self.assertEqual(['bee@example.com'], list(alias_set.aliases))
+ getUtility(IListManager).delete(self._mlist)
+ self.assertEqual(len(list(alias_set.aliases)), 0)
diff --git a/src/mailman/model/tests/test_user.py b/src/mailman/model/tests/test_user.py
index a05b69644..3cdac106b 100644
--- a/src/mailman/model/tests/test_user.py
+++ b/src/mailman/model/tests/test_user.py
@@ -25,10 +25,13 @@ __all__ = [
import unittest
from mailman.app.lifecycle import create_list
+from mailman.config import config
+from mailman.database.transaction import transaction
from mailman.interfaces.address import (
AddressAlreadyLinkedError, AddressNotLinkedError)
from mailman.interfaces.user import UnverifiedAddressError
from mailman.interfaces.usermanager import IUserManager
+from mailman.model.preferences import Preferences
from mailman.testing.layers import ConfigLayer
from mailman.utilities.datetime import now
from zope.component import getUtility
@@ -41,8 +44,9 @@ class TestUser(unittest.TestCase):
layer = ConfigLayer
def setUp(self):
+ self._manager = getUtility(IUserManager)
self._mlist = create_list('test@example.com')
- self._anne = getUtility(IUserManager).create_user(
+ self._anne = self._manager.create_user(
'anne@example.com', 'Anne Person')
preferred = list(self._anne.addresses)[0]
preferred.verified_on = now()
@@ -79,7 +83,7 @@ class TestUser(unittest.TestCase):
self._anne.user_id = 'foo'
def test_addresses_may_only_be_linked_to_one_user(self):
- user = getUtility(IUserManager).create_user()
+ user = self._manager.create_user()
# Anne's preferred address is already linked to her.
with self.assertRaises(AddressAlreadyLinkedError) as cm:
user.link(self._anne.preferred_address)
@@ -88,23 +92,39 @@ class TestUser(unittest.TestCase):
def test_unlink_from_address_not_linked_to(self):
# You cannot unlink an address from a user if that address is not
# already linked to the user.
- user = getUtility(IUserManager).create_user()
+ user = self._manager.create_user()
with self.assertRaises(AddressNotLinkedError) as cm:
user.unlink(self._anne.preferred_address)
self.assertEqual(cm.exception.address, self._anne.preferred_address)
def test_unlink_address_which_is_not_linked(self):
# You cannot unlink an address which is not linked to any user.
- address = getUtility(IUserManager).create_address('bart@example.com')
- user = getUtility(IUserManager).create_user()
+ address = self._manager.create_address('bart@example.com')
+ user = self._manager.create_user()
with self.assertRaises(AddressNotLinkedError) as cm:
user.unlink(address)
self.assertEqual(cm.exception.address, address)
def test_set_unverified_preferred_address(self):
# A user's preferred address cannot be set to an unverified address.
- new_preferred = getUtility(IUserManager).create_address(
+ new_preferred = self._manager.create_address(
'anne.person@example.com')
with self.assertRaises(UnverifiedAddressError) as cm:
self._anne.preferred_address = new_preferred
self.assertEqual(cm.exception.address, new_preferred)
+
+ def test_preferences_deletion_on_user_deletion(self):
+ # LP: #1418276 - deleting a user did not delete their preferences.
+ with transaction():
+ # This has to happen in a transaction so that both the user and
+ # the preferences objects get valid ids.
+ user = self._manager.create_user()
+ # The user's preference is in the database.
+ preferences = config.db.store.query(Preferences).filter_by(
+ id=user.preferences.id)
+ self.assertEqual(preferences.count(), 1)
+ self._manager.delete_user(user)
+ # The user's preference has been deleted.
+ preferences = config.db.store.query(Preferences).filter_by(
+ id=user.preferences.id)
+ self.assertEqual(preferences.count(), 0)
diff --git a/src/mailman/model/user.py b/src/mailman/model/user.py
index b74ea6d06..66197d72e 100644
--- a/src/mailman/model/user.py
+++ b/src/mailman/model/user.py
@@ -83,7 +83,9 @@ class User(Model):
'Duplicate user id {0}'.format(user_id))
self._user_id = user_id
self.display_name = ('' if display_name is None else display_name)
- self.preferences = preferences
+ if preferences is not None:
+ store.add(preferences)
+ self.preferences = preferences
store.add(self)
def __repr__(self):
diff --git a/src/mailman/model/usermanager.py b/src/mailman/model/usermanager.py
index 11557bc25..aefd955ed 100644
--- a/src/mailman/model/usermanager.py
+++ b/src/mailman/model/usermanager.py
@@ -49,6 +49,7 @@ class UserManager:
@dbconnection
def delete_user(self, store, user):
"""See `IUserManager`."""
+ store.delete(user.preferences)
store.delete(user)
@dbconnection
diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py
index a365db969..8e89f423c 100644
--- a/src/mailman/rest/tests/test_lists.py
+++ b/src/mailman/rest/tests/test_lists.py
@@ -28,8 +28,12 @@ __all__ = [
import unittest
from mailman.app.lifecycle import create_list
+from mailman.config import config
from mailman.database.transaction import transaction
+from mailman.interfaces.listmanager import IListManager
+from mailman.interfaces.mailinglist import IAcceptableAliasSet
from mailman.interfaces.usermanager import IUserManager
+from mailman.model.mailinglist import AcceptableAlias
from mailman.testing.helpers import call_api
from mailman.testing.layers import RESTLayer
from urllib.error import HTTPError
@@ -176,6 +180,18 @@ class TestLists(unittest.TestCase):
self.assertEqual(member['email'], 'bart@example.com')
self.assertEqual(member['role'], 'member')
+ def test_delete_list_with_acceptable_aliases(self):
+ # LP: #1432239 - deleting a mailing list with acceptable aliases
+ # causes a SQLAlchemy error. The aliases must be deleted first.
+ with transaction():
+ alias_set = IAcceptableAliasSet(self._mlist)
+ alias_set.add('bee@example.com')
+ call_api('http://localhost:9001/3.0/lists/test.example.com',
+ method='DELETE')
+ # Neither the mailing list, nor the aliases are present.
+ self.assertIsNone(getUtility(IListManager).get('test@example.com'))
+ self.assertEqual(config.db.store.query(AcceptableAlias).count(), 0)
+
class TestListArchivers(unittest.TestCase):
diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py
index 030e74bcb..af2c9f0d1 100644
--- a/src/mailman/rest/tests/test_users.py
+++ b/src/mailman/rest/tests/test_users.py
@@ -36,6 +36,7 @@ from mailman.testing.helpers import call_api, configuration
from mailman.testing.layers import RESTLayer
from urllib.error import HTTPError
from zope.component import getUtility
+from mailman.model.preferences import Preferences
@@ -211,6 +212,26 @@ class TestUsers(unittest.TestCase):
content, response = call_api('http://localhost:9001/3.0/users')
self.assertEqual(content['total_size'], 1)
+ def test_preferences_deletion_on_user_deletion(self):
+ # LP: #1418276 - deleting a user did not delete their preferences.
+ with transaction():
+ anne = getUtility(IUserManager).create_user(
+ 'anne@example.com', 'Anne Person')
+ # Anne's preference is in the database.
+ preferences = config.db.store.query(Preferences).filter_by(
+ id=anne.preferences.id)
+ self.assertEqual(preferences.count(), 1)
+ # Delete the user via REST.
+ content, response = call_api(
+ 'http://localhost:9001/3.0/users/anne@example.com',
+ method='DELETE')
+ self.assertEqual(response.status, 204)
+ # The user's preference has been deleted.
+ with transaction():
+ preferences = config.db.store.query(Preferences).filter_by(
+ id=anne.preferences.id)
+ self.assertEqual(preferences.count(), 0)
+
class TestLogin(unittest.TestCase):
diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py
index 018c8441d..a912b6129 100644
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -185,7 +185,7 @@ class AUser(_UserBase):
return UserAddresses(self._user)
def on_delete(self, request, response):
- """Delete the named user, all her memberships, and addresses."""
+ """Delete the named user and all associated resources."""
if self._user is None:
not_found(response)
return
diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py
index f4373be79..bb4273b12 100644
--- a/src/mailman/utilities/importer.py
+++ b/src/mailman/utilities/importer.py
@@ -388,7 +388,7 @@ def import_config_pck(mlist, config_dict):
regulars_set = set(config_dict.get('members', {}))
digesters_set = set(config_dict.get('digest_members', {}))
members = regulars_set.union(digesters_set)
- # don't send welcome messages...
+ # Don't send welcome messages when we import the rosters.
send_welcome_message = mlist.send_welcome_message
mlist.send_welcome_message = False
try:
@@ -415,6 +415,7 @@ def import_roster(mlist, config_dict, members, role):
:type role: MemberRole enum
"""
usermanager = getUtility(IUserManager)
+ validator = getUtility(IEmailValidator)
roster = mlist.get_roster(role)
for email in members:
# For owners and members, the emails can have a mixed case, so
@@ -434,12 +435,13 @@ def import_roster(mlist, config_dict, members, role):
merged_members.update(config_dict.get('digest_members', {}))
if merged_members.get(email, 0) != 0:
original_email = bytes_to_str(merged_members[email])
- if not getUtility(IEmailValidator).is_valid(original_email):
+ if not validator.is_valid(original_email):
original_email = email
else:
original_email = email
- if not getUtility(IEmailValidator).is_valid(original_email):
- continue # skip this one entirely
+ if not validator.is_valid(original_email):
+ # Skip this one entirely.
+ continue
address = usermanager.create_address(original_email)
address.verified_on = datetime.datetime.now()
user.link(address)
diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py
index 83ad06108..938ef7d2e 100644
--- a/src/mailman/utilities/tests/test_import.py
+++ b/src/mailman/utilities/tests/test_import.py
@@ -749,38 +749,46 @@ class TestRosterImport(unittest.TestCase):
self.assertTrue(anne.controls('anne@example.com'))
def test_invalid_original_email(self):
- self._pckdict["members"]["anne@example.com"] = b'invalid email address'
+ # When the member has an original email address (i.e. the
+ # case-preserved version) that is invalid, their new address record's
+ # original_email attribute will only be the case insensitive version.
+ self._pckdict['members']['anne@example.com'] = b'invalid email address'
try:
import_config_pck(self._mlist, self._pckdict)
- except InvalidEmailAddressError as e:
- self.fail(e)
+ except InvalidEmailAddressError as error:
+ self.fail(error)
self.assertIn('anne@example.com',
[a.email for a in self._mlist.members.addresses])
anne = self._usermanager.get_address('anne@example.com')
self.assertEqual(anne.original_email, 'anne@example.com')
def test_invalid_email(self):
- self._pckdict["members"] = {
+ # When a member's email address is invalid, that member is skipped
+ # during the import.
+ self._pckdict['members'] = {
'anne@example.com': 0,
'invalid email address': b'invalid email address'
- }
- self._pckdict["digest_members"] = {}
+ }
+ self._pckdict['digest_members'] = {}
try:
import_config_pck(self._mlist, self._pckdict)
- except InvalidEmailAddressError as e:
- self.fail(e)
+ except InvalidEmailAddressError as error:
+ self.fail(error)
self.assertEqual(['anne@example.com'],
[a.email for a in self._mlist.members.addresses])
def test_no_email_sent(self):
- self._pckdict
+ # No welcome message is sent to newly imported members.
+ self.assertTrue(self._mlist.send_welcome_message)
import_config_pck(self._mlist, self._pckdict)
- self.assertIn("anne@example.com",
+ self.assertIn('anne@example.com',
[a.email for a in self._mlist.members.addresses])
- # no email in any queue
- for qname, sb in config.switchboards.items():
- self.assertEqual(len(sb.files), 0,
- "Queue '{}' has {} emails".format(qname, len(sb.files)))
+ # There are no messages in any of the queues.
+ for queue, switchboard in config.switchboards.items():
+ file_count = len(switchboard.files)
+ self.assertEqual(file_count, 0,
+ "Unexpected queue '{}' file count: {}".format(
+ queue, file_count))
self.assertTrue(self._mlist.send_welcome_message)
generated by cgit v1.2.3-70-g09d2 (git 2.51.2) at 2025-11-08 20:12:42 +0000