diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/mailman/commands/cli_import.py | 27 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 4 | ||||
| -rw-r--r-- | src/mailman/model/listmanager.py | 4 | ||||
| -rw-r--r-- | src/mailman/model/mailinglist.py | 3 | ||||
| -rw-r--r-- | src/mailman/model/tests/test_mailinglist.py | 24 | ||||
| -rw-r--r-- | src/mailman/model/tests/test_user.py | 32 | ||||
| -rw-r--r-- | src/mailman/model/user.py | 4 | ||||
| -rw-r--r-- | src/mailman/model/usermanager.py | 1 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_lists.py | 16 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_users.py | 21 | ||||
| -rw-r--r-- | src/mailman/rest/users.py | 2 | ||||
| -rw-r--r-- | src/mailman/utilities/importer.py | 23 | ||||
| -rw-r--r-- | src/mailman/utilities/tests/test_import.py | 43 |
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) |
