summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/mailman/commands/cli_import.py27
-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.py23
-rw-r--r--src/mailman/utilities/tests/test_import.py43
13 files changed, 186 insertions, 18 deletions
diff --git a/src/mailman/commands/cli_import.py b/src/mailman/commands/cli_import.py
index 30aeb7894..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,6 +36,25 @@ from zope.interface import implementer
+# 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:
"""Import Mailman 2.1 list data."""
@@ -74,10 +94,13 @@ class Import21:
assert len(args.pickle_file) == 1, (
'Unexpected positional arguments: %s' % args.pickle_file)
filename = args.pickle_file[0]
- 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)
+ config_dict = pickle.load(
+ fp, encoding='utf-8', errors='ignore')
except EOFError:
break
except pickle.UnpicklingError:
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 0607b90e4..f04c534e1 100644
--- a/src/mailman/model/mailinglist.py
+++ b/src/mailman/model/mailinglist.py
@@ -507,10 +507,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 9b82f10c2..ae499452b 100644
--- a/src/mailman/model/usermanager.py
+++ b/src/mailman/model/usermanager.py
@@ -70,6 +70,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 d41179f67..66a23123c 100644
--- a/src/mailman/utilities/importer.py
+++ b/src/mailman/utilities/importer.py
@@ -32,6 +32,7 @@ from mailman.config import config
from mailman.core.errors import MailmanError
from mailman.handlers.decorate import decorate, decorate_template
from mailman.interfaces.action import Action, FilterAction
+from mailman.interfaces.address import IEmailValidator
from mailman.interfaces.archiver import ArchivePolicy
from mailman.interfaces.autorespond import ResponseAction
from mailman.interfaces.bans import IBanManager
@@ -390,11 +391,17 @@ 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)
- import_roster(mlist, config_dict, members, MemberRole.member)
- import_roster(mlist, config_dict, config_dict.get('owner', []),
- MemberRole.owner)
- import_roster(mlist, config_dict, config_dict.get('moderator', []),
- MemberRole.moderator)
+ # Don't send welcome messages when we import the rosters.
+ send_welcome_message = mlist.send_welcome_message
+ mlist.send_welcome_message = False
+ try:
+ import_roster(mlist, config_dict, members, MemberRole.member)
+ import_roster(mlist, config_dict, config_dict.get('owner', []),
+ MemberRole.owner)
+ import_roster(mlist, config_dict, config_dict.get('moderator', []),
+ MemberRole.moderator)
+ finally:
+ mlist.send_welcome_message = send_welcome_message
@@ -411,6 +418,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
@@ -430,8 +438,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 validator.is_valid(original_email):
+ original_email = email
else:
original_email = email
+ 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 e42bb0a7e..59387dfa9 100644
--- a/src/mailman/utilities/tests/test_import.py
+++ b/src/mailman/utilities/tests/test_import.py
@@ -38,6 +38,7 @@ from mailman.app.lifecycle import create_list
from mailman.config import config
from mailman.handlers.decorate import decorate
from mailman.interfaces.action import Action, FilterAction
+from mailman.interfaces.address import InvalidEmailAddressError
from mailman.interfaces.archiver import ArchivePolicy
from mailman.interfaces.autorespond import ResponseAction
from mailman.interfaces.bans import IBanManager
@@ -772,6 +773,48 @@ class TestRosterImport(unittest.TestCase):
anne = self._usermanager.get_user('anne@example.com')
self.assertTrue(anne.controls('anne@example.com'))
+ def test_invalid_original_email(self):
+ # 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 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):
+ # 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'] = {}
+ try:
+ import_config_pck(self._mlist, self._pckdict)
+ 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):
+ # 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',
+ [a.email for a in self._mlist.members.addresses])
+ # 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)