diff options
86 files changed, 1521 insertions, 1021 deletions
@@ -24,8 +24,8 @@ import sys from setuptools import setup, find_packages from string import Template -if sys.hexversion < 0x20600f0: - print 'Mailman requires at least Python 2.6' +if sys.hexversion < 0x20700f0: + print 'Mailman requires at least Python 2.7' sys.exit(1) diff --git a/src/mailman/app/docs/bans.rst b/src/mailman/app/docs/bans.rst index bb6d5902e..5cb56e409 100644 --- a/src/mailman/app/docs/bans.rst +++ b/src/mailman/app/docs/bans.rst @@ -6,20 +6,26 @@ Email addresses can be banned from ever subscribing, either to a specific mailing list or globally within the Mailman system. Both explicit email addresses and email address patterns can be banned. -Bans are managed through the `Ban Manager`. +Bans are managed through the `Ban Manager`. There are ban managers for +specific lists, and there is a global ban manager. To get access to the +global ban manager, adapt ``None``. - >>> from zope.component import getUtility >>> from mailman.interfaces.bans import IBanManager - >>> ban_manager = getUtility(IBanManager) + >>> global_bans = IBanManager(None) -At first, no email addresses are banned, either globally... +At first, no email addresses are banned globally. - >>> ban_manager.is_banned('anne@example.com') + >>> global_bans.is_banned('anne@example.com') False -...or for a specific mailing list. +To get a list-specific ban manager, adapt the mailing list object. - >>> ban_manager.is_banned('bart@example.com', 'test@example.com') + >>> mlist = create_list('test@example.com') + >>> test_bans = IBanManager(mlist) + +There are no bans for this particular list. + + >>> test_bans.is_banned('bart@example.com') False @@ -27,17 +33,17 @@ Specific bans ============= An email address can be banned from a specific mailing list by adding a ban to -the ban manager. +the list's ban manager. - >>> ban_manager.ban('cris@example.com', 'test@example.com') - >>> ban_manager.is_banned('cris@example.com', 'test@example.com') + >>> test_bans.ban('cris@example.com') + >>> test_bans.is_banned('cris@example.com') True - >>> ban_manager.is_banned('bart@example.com', 'test@example.com') + >>> test_bans.is_banned('bart@example.com') False However, this is not a global ban. - >>> ban_manager.is_banned('cris@example.com') + >>> global_bans.is_banned('cris@example.com') False @@ -47,48 +53,53 @@ Global bans An email address can be banned globally, so that it cannot be subscribed to any mailing list. - >>> ban_manager.ban('dave@example.com') + >>> global_bans.ban('dave@example.com') -Dave is banned from the test mailing list... +Because there is a global ban, Dave is also banned from the mailing list. - >>> ban_manager.is_banned('dave@example.com', 'test@example.com') + >>> test_bans.is_banned('dave@example.com') True -...and the sample mailing list. +Even when a new mailing list is created, Dave is still banned from this list +because of his global ban. - >>> ban_manager.is_banned('dave@example.com', 'sample@example.com') + >>> sample = create_list('sample@example.com') + >>> sample_bans = IBanManager(sample) + >>> sample_bans.is_banned('dave@example.com') True -Dave is also banned globally. +Dave is of course banned globally. - >>> ban_manager.is_banned('dave@example.com') + >>> global_bans.is_banned('dave@example.com') True Cris however is not banned globally. - >>> ban_manager.is_banned('cris@example.com') + >>> global_bans.is_banned('cris@example.com') False Even though Cris is not banned globally, we can add a global ban for her. - >>> ban_manager.ban('cris@example.com') - >>> ban_manager.is_banned('cris@example.com') + >>> global_bans.ban('cris@example.com') + >>> global_bans.is_banned('cris@example.com') True -Cris is obviously still banned from specific mailing lists. +Cris is now banned from all mailing lists. - >>> ban_manager.is_banned('cris@example.com', 'test@example.com') + >>> test_bans.is_banned('cris@example.com') True - >>> ban_manager.is_banned('cris@example.com', 'sample@example.com') + >>> sample_bans.is_banned('cris@example.com') True -We can remove the global ban to once again just ban her address from the test -list. +We can remove the global ban to once again just ban her address from just the +test list. - >>> ban_manager.unban('cris@example.com') - >>> ban_manager.is_banned('cris@example.com', 'test@example.com') + >>> global_bans.unban('cris@example.com') + >>> global_bans.is_banned('cris@example.com') + False + >>> test_bans.is_banned('cris@example.com') True - >>> ban_manager.is_banned('cris@example.com', 'sample@example.com') + >>> sample_bans.is_banned('cris@example.com') False @@ -100,52 +111,56 @@ and globally, just as specific addresses can be banned. Use this for example, when an entire domain is a spam faucet. When using a pattern, the email address must start with a caret (^). - >>> ban_manager.ban('^.*@example.org', 'test@example.com') + >>> test_bans.ban('^.*@example.org') -Now, no one from example.org can subscribe to the test list. +Now, no one from example.org can subscribe to the test mailing list. - >>> ban_manager.is_banned('elle@example.org', 'test@example.com') + >>> test_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('eperson@example.org', 'test@example.com') + >>> test_bans.is_banned('eperson@example.org') True - >>> ban_manager.is_banned('elle@example.com', 'test@example.com') + +example.com addresses are not banned. + + >>> test_bans.is_banned('elle@example.com') False -They are not, however banned globally. +example.org addresses are not banned globally, nor for any other mailing +list. - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> sample_bans.is_banned('elle@example.org') False - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') False Of course, we can ban everyone from example.org globally too. - >>> ban_manager.ban('^.*@example.org') - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> global_bans.ban('^.*@example.org') + >>> sample_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') True We can remove the mailing list ban on the pattern, though the global ban will still be in place. - >>> ban_manager.unban('^.*@example.org', 'test@example.com') - >>> ban_manager.is_banned('elle@example.org', 'test@example.com') + >>> test_bans.unban('^.*@example.org') + >>> test_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> sample_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') True But once the global ban is removed, everyone from example.org can subscribe to the mailing lists. - >>> ban_manager.unban('^.*@example.org') - >>> ban_manager.is_banned('elle@example.org', 'test@example.com') + >>> global_bans.unban('^.*@example.org') + >>> test_bans.is_banned('elle@example.org') False - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> sample_bans.is_banned('elle@example.org') False - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') False @@ -154,14 +169,14 @@ Adding and removing bans It is not an error to add a ban more than once. These are just ignored. - >>> ban_manager.ban('fred@example.com', 'test@example.com') - >>> ban_manager.ban('fred@example.com', 'test@example.com') - >>> ban_manager.is_banned('fred@example.com', 'test@example.com') + >>> test_bans.ban('fred@example.com') + >>> test_bans.ban('fred@example.com') + >>> test_bans.is_banned('fred@example.com') True Nor is it an error to remove a ban more than once. - >>> ban_manager.unban('fred@example.com', 'test@example.com') - >>> ban_manager.unban('fred@example.com', 'test@example.com') - >>> ban_manager.is_banned('fred@example.com', 'test@example.com') + >>> test_bans.unban('fred@example.com') + >>> test_bans.unban('fred@example.com') + >>> test_bans.is_banned('fred@example.com') False diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py index c73735b35..3f012e5a5 100644 --- a/src/mailman/app/membership.py +++ b/src/mailman/app/membership.py @@ -73,7 +73,7 @@ def add_member(mlist, email, display_name, password, delivery_mode, language, # Let's be extra cautious. getUtility(IEmailValidator).validate(email) # Check to see if the email address is banned. - if getUtility(IBanManager).is_banned(email, mlist.fqdn_listname): + if IBanManager(mlist).is_banned(email): raise MembershipIsBannedError(mlist, email) # See if there's already a user linked with the given address. user_manager = getUtility(IUserManager) diff --git a/src/mailman/app/tests/test_bounces.py b/src/mailman/app/tests/test_bounces.py index 4be272c34..fae30724e 100644 --- a/src/mailman/app/tests/test_bounces.py +++ b/src/mailman/app/tests/test_bounces.py @@ -320,9 +320,8 @@ $address $optionsurl $owneraddr """, file=fp) - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. + # Let assertMultiLineEqual work without bounds. self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) def tearDown(self): config.pop('xx template dir') @@ -343,7 +342,7 @@ $owneraddr send_probe(self._member, self._msg) message = get_queue_messages('virgin')[0].msg notice = message.get_payload(0).get_payload() - self.eq(notice, """\ + self.assertMultiLineEqual(notice, """\ blah blah blah test@example.com anne@example.com http://example.com/anne@example.com test-owner@example.com""") diff --git a/src/mailman/app/tests/test_inject.py b/src/mailman/app/tests/test_inject.py index 4e08ff0a7..7a47ed74a 100644 --- a/src/mailman/app/tests/test_inject.py +++ b/src/mailman/app/tests/test_inject.py @@ -55,16 +55,15 @@ Date: Tue, 14 Jun 2011 21:12:00 -0400 Nothing. """) - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. - self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) + # Let assertMultiLineEqual work without bounds. def test_inject_message(self): # Test basic inject_message() call. inject_message(self.mlist, self.msg) items = get_queue_messages('in') self.assertEqual(len(items), 1) - self.eq(items[0].msg.as_string(), self.msg.as_string()) + self.assertMultiLineEqual(items[0].msg.as_string(), + self.msg.as_string()) self.assertEqual(items[0].msgdata['listname'], 'test@example.com') self.assertEqual(items[0].msgdata['original_size'], len(self.msg.as_string())) @@ -83,7 +82,8 @@ Nothing. self.assertEqual(len(items), 0) items = get_queue_messages('virgin') self.assertEqual(len(items), 1) - self.eq(items[0].msg.as_string(), self.msg.as_string()) + self.assertMultiLineEqual(items[0].msg.as_string(), + self.msg.as_string()) self.assertEqual(items[0].msgdata['listname'], 'test@example.com') self.assertEqual(items[0].msgdata['original_size'], len(self.msg.as_string())) @@ -155,7 +155,6 @@ Nothing. """ # Python 2.7 has a better equality tester for message texts. self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) def _remove_line(self, header): return NL.join(line for line in self.text.splitlines() @@ -171,7 +170,7 @@ Nothing. 'GUXXQKNCHBFQAHGBFMGCME6HKZCUUH3K') # Delete that header because it is not in the original text. del items[0].msg['x-message-id-hash'] - self.eq(items[0].msg.as_string(), self.text) + self.assertMultiLineEqual(items[0].msg.as_string(), self.text) self.assertEqual(items[0].msgdata['listname'], 'test@example.com') self.assertEqual(items[0].msgdata['original_size'], # Add back the X-Message-ID-Header which was in the @@ -196,7 +195,7 @@ Nothing. self.assertEqual(len(items), 1) # Remove the X-Message-ID-Hash header which isn't in the original text. del items[0].msg['x-message-id-hash'] - self.eq(items[0].msg.as_string(), self.text) + self.assertMultiLineEqual(items[0].msg.as_string(), self.text) self.assertEqual(items[0].msgdata['listname'], 'test@example.com') self.assertEqual(items[0].msgdata['original_size'], # Add back the X-Message-ID-Header which was in the diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py index 00c279910..a22a239b7 100644 --- a/src/mailman/app/tests/test_membership.py +++ b/src/mailman/app/tests/test_membership.py @@ -69,7 +69,7 @@ class AddMemberTest(unittest.TestCase): def test_add_member_banned(self): # Test that members who are banned by specific address cannot # subscribe to the mailing list. - getUtility(IBanManager).ban('anne@example.com', 'test@example.com') + IBanManager(self._mlist).ban('anne@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', @@ -78,43 +78,43 @@ class AddMemberTest(unittest.TestCase): def test_add_member_globally_banned(self): # Test that members who are banned by specific address cannot # subscribe to the mailing list. - getUtility(IBanManager).ban('anne@example.com') + IBanManager(None).ban('anne@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) def test_add_member_banned_from_different_list(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('anne@example.com', 'sample@example.com') + # Test that members who are banned by on a different list can still be + # subscribed to other mlists. + sample_list = create_list('sample@example.com') + IBanManager(sample_list).ban('anne@example.com') member = add_member(self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) self.assertEqual(member.address.email, 'anne@example.com') def test_add_member_banned_by_pattern(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('^.*@example.com', 'test@example.com') + # Addresses matching regexp ban patterns cannot subscribe. + IBanManager(self._mlist).ban('^.*@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) def test_add_member_globally_banned_by_pattern(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('^.*@example.com') + # Addresses matching global regexp ban patterns cannot subscribe. + IBanManager(None).ban('^.*@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) def test_add_member_banned_from_different_list_by_pattern(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('^.*@example.com', 'sample@example.com') + # Addresses matching regexp ban patterns on one list can still + # subscribe to other mailing lists. + sample_list = create_list('sample@example.com') + IBanManager(sample_list).ban('^.*@example.com') member = add_member(self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) @@ -137,17 +137,14 @@ class AddMemberTest(unittest.TestCase): 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language, MemberRole.member) - try: + with self.assertRaises(AlreadySubscribedError) as cm: add_member(self._mlist, 'aperson@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language, MemberRole.member) - except AlreadySubscribedError as exc: - self.assertEqual(exc.fqdn_listname, 'test@example.com') - self.assertEqual(exc.email, 'aperson@example.com') - self.assertEqual(exc.role, MemberRole.member) - else: - raise AssertionError('AlreadySubscribedError expected') + self.assertEqual(cm.exception.fqdn_listname, 'test@example.com') + self.assertEqual(cm.exception.email, 'aperson@example.com') + self.assertEqual(cm.exception.role, MemberRole.member) def test_add_member_with_different_roles(self): # Adding a member twice with different roles is okay. diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py index ef6adf5ed..dc1217d67 100644 --- a/src/mailman/app/tests/test_moderation.py +++ b/src/mailman/app/tests/test_moderation.py @@ -58,9 +58,6 @@ Message-ID: <alpha> self._in = make_testable_runner(IncomingRunner, 'in') self._pipeline = make_testable_runner(PipelineRunner, 'pipeline') self._out = make_testable_runner(OutgoingRunner, 'out') - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. - self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) def test_accepted_message_gets_posted(self): # A message that is accepted by the moderator should get posted to the diff --git a/src/mailman/app/tests/test_notifications.py b/src/mailman/app/tests/test_notifications.py index 8cce1be6f..303fa020a 100644 --- a/src/mailman/app/tests/test_notifications.py +++ b/src/mailman/app/tests/test_notifications.py @@ -74,9 +74,8 @@ Welcome to the $list_name mailing list. os.makedirs(path) with open(os.path.join(path, 'welcome.txt'), 'w') as fp: print('You just joined the $list_name mailing list!', file=fp) - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. + # Let assertMultiLineEqual work without bounds. self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) def tearDown(self): config.pop('template config') @@ -94,7 +93,7 @@ Welcome to the $list_name mailing list. message = messages[0].msg self.assertEqual(str(message['subject']), 'Welcome to the "Test List" mailing list') - self.eq(message.get_payload(), """\ + self.assertMultiLineEqual(message.get_payload(), """\ Welcome to the Test List mailing list. Posting address: test@example.com diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py index 1c37d4cb9..c4c8f2795 100644 --- a/src/mailman/app/tests/test_subscriptions.py +++ b/src/mailman/app/tests/test_subscriptions.py @@ -51,19 +51,13 @@ class TestJoin(unittest.TestCase): def test_join_user_with_bogus_id(self): # When `subscriber` is a missing user id, an exception is raised. - try: + with self.assertRaises(MissingUserError) as cm: self._service.join('test.example.com', uuid.UUID(int=99)) - except MissingUserError as exc: - self.assertEqual(exc.user_id, uuid.UUID(int=99)) - else: - raise AssertionError('MissingUserError expected') + self.assertEqual(cm.exception.user_id, uuid.UUID(int=99)) def test_join_user_with_invalid_email_address(self): # When `subscriber` is a string that is not an email address, an # exception is raised. - try: + with self.assertRaises(InvalidEmailAddressError) as cm: self._service.join('test.example.com', 'bogus') - except InvalidEmailAddressError as exc: - self.assertEqual(exc.email, 'bogus') - else: - raise AssertionError('InvalidEmailAddressError expected') + self.assertEqual(cm.exception.email, 'bogus') diff --git a/src/mailman/app/tests/test_templates.py b/src/mailman/app/tests/test_templates.py index 6dfbd7109..788412a57 100644 --- a/src/mailman/app/tests/test_templates.py +++ b/src/mailman/app/tests/test_templates.py @@ -98,49 +98,31 @@ class TestTemplateLoader(unittest.TestCase): self.assertEqual(content, 'Test content') def test_uri_not_found(self): - try: + with self.assertRaises(urllib2.URLError) as cm: self._loader.get('mailman:///missing.txt') - except urllib2.URLError as error: - self.assertEqual(error.reason, 'No such file') - else: - raise AssertionError('Exception expected') + self.assertEqual(cm.exception.reason, 'No such file') def test_shorter_url_error(self): - try: + with self.assertRaises(urllib2.URLError) as cm: self._loader.get('mailman:///') - except urllib2.URLError as error: - self.assertEqual(error.reason, 'No template specified') - else: - raise AssertionError('Exception expected') + self.assertEqual(cm.exception.reason, 'No template specified') def test_short_url_error(self): - try: + with self.assertRaises(urllib2.URLError) as cm: self._loader.get('mailman://') - except urllib2.URLError as error: - self.assertEqual(error.reason, 'No template specified') - else: - raise AssertionError('Exception expected') + self.assertEqual(cm.exception.reason, 'No template specified') def test_bad_language(self): - try: + with self.assertRaises(urllib2.URLError) as cm: self._loader.get('mailman:///xx/demo.txt') - except urllib2.URLError as error: - self.assertEqual(error.reason, 'Bad language or list name') - else: - raise AssertionError('Exception expected') + self.assertEqual(cm.exception.reason, 'Bad language or list name') def test_bad_mailing_list(self): - try: + with self.assertRaises(urllib2.URLError) as cm: self._loader.get('mailman:///missing@example.com/demo.txt') - except urllib2.URLError as error: - self.assertEqual(error.reason, 'Bad language or list name') - else: - raise AssertionError('Exception expected') + self.assertEqual(cm.exception.reason, 'Bad language or list name') def test_too_many_path_components(self): - try: + with self.assertRaises(urllib2.URLError) as cm: self._loader.get('mailman:///missing@example.com/en/foo/demo.txt') - except urllib2.URLError as error: - self.assertEqual(error.reason, 'No such file') - else: - raise AssertionError('Exception expected') + self.assertEqual(cm.exception.reason, 'No such file') diff --git a/src/mailman/archiving/mailarchive.py b/src/mailman/archiving/mailarchive.py index f1427dd61..bbd3d5ce0 100644 --- a/src/mailman/archiving/mailarchive.py +++ b/src/mailman/archiving/mailarchive.py @@ -30,6 +30,7 @@ from urlparse import urljoin from zope.interface import implementer from mailman.config import config +from mailman.config.config import external_configuration from mailman.interfaces.archiver import ArchivePolicy, IArchiver @@ -45,13 +46,15 @@ class MailArchive: def __init__(self): # Read our specific configuration file - self.config = config.archiver_config("mail_archive") + archiver_config = external_configuration( + config.archiver.mail_archive.configuration) + self.base_url = archiver_config.get('general', 'base_url') + self.recipient = archiver_config.get('general', 'recipient') def list_url(self, mlist): """See `IArchiver`.""" if mlist.archive_policy is ArchivePolicy.public: - return urljoin(self.config.get("general", "base_url"), - quote(mlist.posting_address)) + return urljoin(self.base_url, quote(mlist.posting_address)) return None def permalink(self, mlist, msg): @@ -64,7 +67,7 @@ class MailArchive: message_id_hash = msg.get('x-message-id-hash') if message_id_hash is None: return None - return urljoin(self.config.get("general", "base_url"), message_id_hash) + return urljoin(self.base_url, message_id_hash) def archive_message(self, mlist, msg): """See `IArchiver`.""" @@ -72,4 +75,4 @@ class MailArchive: config.switchboards['out'].enqueue( msg, listname=mlist.fqdn_listname, - recipients=[self.config.get("general", "recipient")]) + recipients=[self.recipient]) diff --git a/src/mailman/archiving/mhonarc.py b/src/mailman/archiving/mhonarc.py index 14dcc8300..06119ae35 100644 --- a/src/mailman/archiving/mhonarc.py +++ b/src/mailman/archiving/mhonarc.py @@ -32,6 +32,7 @@ from urlparse import urljoin from zope.interface import implementer from mailman.config import config +from mailman.config.config import external_configuration from mailman.interfaces.archiver import IArchiver from mailman.utilities.string import expand @@ -48,12 +49,15 @@ class MHonArc: def __init__(self): # Read our specific configuration file - self.config = config.archiver_config("mhonarc") + archiver_config = external_configuration( + config.archiver.mhonarc.configuration) + self.base_url = archiver_config.get('general', 'base_url') + self.command = archiver_config.get('general', 'command') def list_url(self, mlist): """See `IArchiver`.""" # XXX What about private MHonArc archives? - return expand(self.config.get("general", "base_url"), + return expand(self.base_url, dict(listname=mlist.fqdn_listname, hostname=mlist.domain.url_host, fqdn_listname=mlist.fqdn_listname, @@ -74,7 +78,7 @@ class MHonArc: """See `IArchiver`.""" substitutions = config.__dict__.copy() substitutions['listname'] = mlist.fqdn_listname - command = expand(self.config.get("general", "command"), substitutions) + command = expand(self.command, substitutions) proc = subprocess.Popen( command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True) diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py index 65a23d891..e8856031c 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -44,8 +44,6 @@ class TestHeaderChain(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') - # Python 2.6 does not have assertListEqual(). - self._leq = getattr(self, 'assertListEqual', self.assertEqual) @configuration('antispam', header_checks=""" Foo: a+ @@ -71,7 +69,7 @@ class TestHeaderChain(unittest.TestCase): self.assertEqual(link.rule.name[:13], 'header-match-') self.assertEqual(link.action, LinkAction.defer) post_checks.append((link.rule.header, link.rule.pattern)) - self._leq(post_checks, [ + self.assertListEqual(post_checks, [ ('Foo', 'a+'), ('Bar', 'bb?'), ]) @@ -103,7 +101,7 @@ class TestHeaderChain(unittest.TestCase): self.assertEqual(link.rule.name[:13], 'header-match-') self.assertEqual(link.action, LinkAction.defer) post_checks.append((link.rule.header, link.rule.pattern)) - self._leq(post_checks, [ + self.assertListEqual(post_checks, [ ('Foo', 'foo'), ('Bar', 'bar'), ]) diff --git a/src/mailman/chains/tests/test_hold.py b/src/mailman/chains/tests/test_hold.py index 515894505..66f43fa60 100644 --- a/src/mailman/chains/tests/test_hold.py +++ b/src/mailman/chains/tests/test_hold.py @@ -44,9 +44,8 @@ class TestAutorespond(unittest.TestCase): def setUp(self): self._mlist = create_list('test@example.com') - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. + # Let assertMultiLineEqual work without bounds. self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) @configuration('mta', max_autoresponses_per_day=1) def test_max_autoresponses_per_day(self): @@ -71,7 +70,7 @@ class TestAutorespond(unittest.TestCase): del message['message-id'] self.assertTrue('date' in message) del message['date'] - self.eq(messages[0].msg.as_string(), """\ + self.assertMultiLineEqual(messages[0].msg.as_string(), """\ MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit diff --git a/src/mailman/commands/cli_aliases.py b/src/mailman/commands/cli_aliases.py index d692ba356..d26e5a754 100644 --- a/src/mailman/commands/cli_aliases.py +++ b/src/mailman/commands/cli_aliases.py @@ -25,18 +25,11 @@ __all__ = [ ] -import sys - -from operator import attrgetter -from zope.component import getUtility from zope.interface import implementer from mailman.config import config from mailman.core.i18n import _ from mailman.interfaces.command import ICLISubCommand -from mailman.interfaces.listmanager import IListManager -from mailman.interfaces.mta import ( - IMailTransportAgentAliases, IMailTransportAgentLifecycle) from mailman.utilities.modules import call_name @@ -51,84 +44,11 @@ class Aliases: """See `ICLISubCommand`.""" self.parser = parser command_parser.add_argument( - '-o', '--output', - action='store', help=_("""\ - File to send the output to. If not given, a file in $VAR/data is - used. The argument can be '-' to use standard output..""")) - command_parser.add_argument( - '-f', '--format', + '-d', '--directory', action='store', help=_("""\ - Alternative output format to use. This is the Python object path - to an implementation of the `IMailTransportAgentLifecycle` - interface.""")) - command_parser.add_argument( - '-s', '--simple', - action='store_true', default=False, help=_("""\ - Simply output the list of aliases. - """)) + An alternative directory to output the various MTA files to.""")) def process(self, args): """See `ICLISubCommand`.""" - if args.format is not None and args.simple: - self.parser.error(_('Cannot use both -s and -f')) - # Does not return. - output = None - if args.output == '-': - output = sys.stdout - elif args.output is None: - output = None - else: - output = args.output - if args.simple: - Dummy().regenerate(output) - else: - format_arg = (config.mta.incoming - if args.format is None - else args.format) - # Call the MTA-specific regeneration method. - call_name(format_arg).regenerate(output) - - - -@implementer(IMailTransportAgentLifecycle) -class Dummy: - """Dummy aliases implementation for simpler output format.""" - - def create(self, mlist): - """See `IMailTransportAgentLifecycle`.""" - raise NotImplementedError - - def delete(self, mlist): - """See `IMailTransportAgentLifecycle`.""" - raise NotImplementedError - - def regenerate(self, output=None): - """See `IMailTransportAgentLifecycle`.""" - fp = None - close = False - try: - if output is None: - # There's really no place to print the output. - return - elif isinstance(output, basestring): - fp = open(output, 'w') - close = True - else: - fp = output - self._do_write_file(fp) - finally: - if fp is not None and close: - fp.close() - - def _do_write_file(self, fp): - # First, sort mailing lists by domain. - by_domain = {} - for mlist in getUtility(IListManager).mailing_lists: - by_domain.setdefault(mlist.mail_host, []).append(mlist) - sort_key = attrgetter('list_name') - for domain in sorted(by_domain): - for mlist in sorted(by_domain[domain], key=sort_key): - utility = getUtility(IMailTransportAgentAliases) - for alias in utility.aliases(mlist): - print(alias, file=fp) - print(file=fp) + # Call the MTA-specific regeneration method. + call_name(config.mta.incoming).regenerate(args.directory) diff --git a/src/mailman/commands/cli_control.py b/src/mailman/commands/cli_control.py index 8349feb60..b5a79b820 100644 --- a/src/mailman/commands/cli_control.py +++ b/src/mailman/commands/cli_control.py @@ -121,7 +121,11 @@ class Start: os.setsid() # Instead of cd'ing to root, cd to the Mailman runtime directory. # However, before we do that, set an environment variable used by the - # subprocesses to calculate their path to the $VAR_DIR. + # subprocesses to calculate their path to the $VAR_DIR. Before we + # chdir() though, calculate the absolute path to the configuration + # file. + config_path = (os.path.abspath(args.config) + if args.config else None) os.environ['MAILMAN_VAR_DIR'] = config.VAR_DIR os.chdir(config.VAR_DIR) # Exec the master watcher. @@ -131,8 +135,8 @@ class Start: ] if args.force: execl_args.append('--force') - if args.config: - execl_args.extend(['-C', args.config]) + if config_path: + execl_args.extend(['-C', config_path]) qlog.debug('starting: %s', execl_args) os.execl(*execl_args) # We should never get here. diff --git a/src/mailman/commands/docs/aliases.rst b/src/mailman/commands/docs/aliases.rst index c0d4c10c9..713064b0f 100644 --- a/src/mailman/commands/docs/aliases.rst +++ b/src/mailman/commands/docs/aliases.rst @@ -2,16 +2,14 @@ Generating aliases ================== -For some mail servers, Mailman must generate a data file that is used to hook +For some mail servers, Mailman must generate data files that are used to hook Mailman up to the mail server. The details of this differ for each mail server. Generally these files are automatically kept up-to-date when mailing lists are created or removed, but you might occasionally need to manually regenerate the file. The ``bin/mailman aliases`` command does this. >>> class FakeArgs: - ... output = None - ... format = None - ... simple = None + ... directory = None >>> from mailman.commands.cli_aliases import Aliases >>> command = Aliases() @@ -28,16 +26,32 @@ generation. ... incoming: mailman.mta.postfix.LMTP ... lmtp_host: lmtp.example.com ... lmtp_port: 24 - ... postfix_map_cmd: true ... """) Let's create a mailing list and then display the transport map for it. We'll -send the output to stdout. +write the appropriate files to a temporary directory. :: - >>> FakeArgs.output = '-' + >>> import os, shutil, tempfile + >>> output_directory = tempfile.mkdtemp() + >>> cleanups.append((shutil.rmtree, output_directory)) + + >>> FakeArgs.directory = output_directory >>> mlist = create_list('test@example.com') >>> command.process(FakeArgs) + +For Postfix, there are two files in the output directory. + + >>> files = sorted(os.listdir(output_directory)) + >>> for file in files: + ... print file + postfix_domains + postfix_lmtp + +The transport map file contains all the aliases for the mailing list. + + >>> with open(os.path.join(output_directory, 'postfix_lmtp')) as fp: + ... print fp.read() # AUTOMATICALLY GENERATED BY MAILMAN ON ... ... test@example.com lmtp:[lmtp.example.com]:24 @@ -51,61 +65,14 @@ send the output to stdout. test-unsubscribe@example.com lmtp:[lmtp.example.com]:24 <BLANKLINE> - >>> config.pop('postfix') - +The relay domains file contains a list of all the domains. -Alternative output -================== - -By using a command line switch, we can select a different output format. The -option must point to an alternative implementation of the -``IMailTransportAgentAliases`` interface. - -Mailman comes with an alternative implementation that just prints the aliases, -with no adornment. - - >>> FakeArgs.format = 'mailman.commands.cli_aliases.Dummy' - >>> command.process(FakeArgs) - test@example.com - test-bounces@example.com - test-confirm@example.com - test-join@example.com - test-leave@example.com - test-owner@example.com - test-request@example.com - test-subscribe@example.com - test-unsubscribe@example.com - <BLANKLINE> - -A simpler way of getting the same output is with the ``--simple`` flag. - - >>> FakeArgs.format = None - >>> FakeArgs.simple = True - >>> command.process(FakeArgs) - test@example.com - test-bounces@example.com - test-confirm@example.com - test-join@example.com - test-leave@example.com - test-owner@example.com - test-request@example.com - test-subscribe@example.com - test-unsubscribe@example.com - <BLANKLINE> - - -Mutually exclusive arguments -============================ - -You cannot use both ``--simple`` and ``--format``. - - >>> FakeArgs.format = 'mailman.commands.cli_aliases.Dummy' - >>> FakeArgs.simple = True - >>> class Parser: - ... def error(self, message): - ... raise RuntimeError(message) - >>> command.parser = Parser() - >>> command.process(FakeArgs) - Traceback (most recent call last): + >>> with open(os.path.join(output_directory, 'postfix_domains')) as fp: + ... print fp.read() + # AUTOMATICALLY GENERATED BY MAILMAN ON ... ... - RuntimeError: Cannot use both -s and -f + example.com example.com + +.. + Clean up. + >>> config.pop('postfix') diff --git a/src/mailman/config/config.py b/src/mailman/config/config.py index 3b662553e..0293dacd4 100644 --- a/src/mailman/config/config.py +++ b/src/mailman/config/config.py @@ -22,15 +22,17 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ 'Configuration', + 'external_configuration', + 'load_external' ] import os import sys -from ConfigParser import SafeConfigParser +from ConfigParser import SafeConfigParser from lazr.config import ConfigSchema, as_boolean -from pkg_resources import resource_stream, resource_exists +from pkg_resources import resource_filename, resource_stream, resource_string from string import Template from zope.component import getUtility from zope.event import notify @@ -40,7 +42,7 @@ import mailman.templates from mailman import version from mailman.interfaces.configuration import ( - ConfigurationUpdatedEvent, IConfiguration) + ConfigurationUpdatedEvent, IConfiguration, MissingConfigurationFileError) from mailman.interfaces.languages import ILanguageManager from mailman.utilities.filesystem import makedirs from mailman.utilities.modules import call_name @@ -237,30 +239,54 @@ class Configuration: for section in self._config.getByCategory('language', []): yield section - def get_additional_config(self, name, keyname): - """Return an external ini-file as specified by the keyname.""" - add_config = SafeConfigParser() - if resource_exists('mailman.config', '%s.cfg' % name): - included_config_file = resource_stream('mailman.config', - '%s.cfg' % name) - add_config.readfp(included_config_file) - # Resolve the path value from the key name - path = self._config - for key in keyname.split("."): - path = getattr(path, key) - # Load the file - configured_path = os.path.expanduser(path) # TODO: allow URLs - if not configured_path.startswith("/"): - # Consider it relative to the mailman.cfg file - for overlay in self.overlays: - if os.sep in overlay.filename: - configured_path = os.path.join( - os.path.dirname(overlay.filename), - configured_path) - break - r = add_config.read([configured_path]) - return add_config - def archiver_config(self, name): - """A shortcut to self.get_additional_config() for achivers.""" - return self.get_additional_config(name, "archiver.%s.configure" % name) + +def load_external(path, encoding=None): + """Load the configuration file named by path. + + :param path: A string naming the location of the external configuration + file. This is either an absolute file system path or a special + ``python:`` path. When path begins with ``python:``, the rest of the + value must name a ``.cfg`` file located within Python's import path, + however the trailing ``.cfg`` suffix is implied (don't provide it + here). + :param encoding: The encoding to apply to the data read from path. If + None, then bytes will be returned. + :return: A unicode string or bytes, depending on ``encoding``. + """ + # Is the context coming from a file system or Python path? + if path.startswith('python:'): + resource_path = path[7:] + package, dot, resource = resource_path.rpartition('.') + config_string = resource_string(package, resource + '.cfg') + else: + with open(path, 'rb') as fp: + config_string = fp.read() + if encoding is None: + return config_string + return config_string.decode(encoding) + + +def external_configuration(path): + """Parse the configuration file named by path. + + :param path: A string naming the location of the external configuration + file. This is either an absolute file system path or a special + ``python:`` path. When path begins with ``python:``, the rest of the + value must name a ``.cfg`` file located within Python's import path, + however the trailing ``.cfg`` suffix is implied (don't provide it + here). + :return: A `ConfigParser` instance. + """ + # Is the context coming from a file system or Python path? + if path.startswith('python:'): + resource_path = path[7:] + package, dot, resource = resource_path.rpartition('.') + cfg_path = resource_filename(package, resource + '.cfg') + else: + cfg_path = path + parser = SafeConfigParser() + files = parser.read(cfg_path) + if files != [cfg_path]: + raise MissingConfigurationFileError(path) + return parser diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml index ed85ae1a6..efb449538 100644 --- a/src/mailman/config/configure.zcml +++ b/src/mailman/config/configure.zcml @@ -6,6 +6,18 @@ <adapter for="mailman.interfaces.mailinglist.IMailingList" + provides="mailman.interfaces.bans.IBanManager" + factory="mailman.model.bans.BanManager" + /> + + <adapter + for="None" + provides="mailman.interfaces.bans.IBanManager" + factory="mailman.model.bans.BanManager" + /> + + <adapter + for="mailman.interfaces.mailinglist.IMailingList" provides="mailman.interfaces.autorespond.IAutoResponseSet" factory="mailman.model.autorespond.AutoResponseSet" /> @@ -37,11 +49,6 @@ /> <utility - provides="mailman.interfaces.bans.IBanManager" - factory="mailman.model.bans.BanManager" - /> - - <utility provides="mailman.interfaces.bounce.IBounceProcessor" factory="mailman.model.bounce.BounceProcessor" /> diff --git a/src/mailman/config/mail_archive.cfg b/src/mailman/config/mail_archive.cfg index 29e716dae..ee8430317 100644 --- a/src/mailman/config/mail_archive.cfg +++ b/src/mailman/config/mail_archive.cfg @@ -24,5 +24,6 @@ base_url: http://www.mail-archive.com/ # If the archiver works by getting a copy of the message, this is the address # to send the copy to. +# # See: http://www.mail-archive.com/faq.html#newlist recipient: archive@mail-archive.com diff --git a/src/mailman/config/postfix.cfg b/src/mailman/config/postfix.cfg new file mode 100644 index 000000000..9bdba221e --- /dev/null +++ b/src/mailman/config/postfix.cfg @@ -0,0 +1,8 @@ +[postfix] +# Additional configuration variables for the postfix MTA. + +# This variable describe the program to use for regenerating the transport map +# db file, from the associated plain text files. The file being updated will +# be appended to this string (with a separating space), so it must be +# appropriate for os.system(). +postmap_command: /usr/sbin/postmap diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg index ee42e51b4..37322418a 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -159,7 +159,7 @@ wait: 10s # system paths must be absolute since no guarantees are made about the current # working directory. Python paths should not include the trailing .cfg, which # the file must end with. -path: python:mailman.config.passlib +configuration: python:mailman.config.passlib # When Mailman generates them, this is the default length of passwords. password_length: 8 @@ -513,11 +513,13 @@ max_autoresponses_per_day: 10 # may wish to remove these headers by setting this to 'yes'. remove_dkim_headers: no -# This variable describe the program to use for regenerating the transport map -# db file, from the associated plain text files. The file being updated will -# be appended to this string (with a separating space), so it must be -# appropriate for os.system(). -postfix_map_cmd: /usr/sbin/postmap +# Where can we find the mail server specific configuration file? The path can +# be either a file system path or a Python import path. If the value starts +# with python: then it is a Python import path, otherwise it is a file system +# path. File system paths must be absolute since no guarantees are made about +# the current working directory. Python paths should not include the trailing +# .cfg, which the file must end with. +configuration: python:mailman.config.postfix [bounces] @@ -535,8 +537,13 @@ class: mailman.archiving.prototype.Prototype # Set this to 'yes' to enable the archiver. enable: no -# Additional configuration for the archiver. -configure: /path/to/file.cfg +# Additional configuration for the archiver. The path can be either a file +# system path or a Python import path. If the value starts with python: then +# it is a Python import path, otherwise it is a file system path. File system +# paths must be absolute since no guarantees are made about the current +# working directory. Python paths should not include the trailing .cfg, which +# the file must end with. +configuration: changeme # When sending the message to the archiver, you have the option of # "clobbering" the Date: header, specifically to make it more sane. Some @@ -555,20 +562,15 @@ configure: /path/to/file.cfg clobber_date: maybe clobber_skew: 1d - [archiver.mhonarc] # This is the stock MHonArc archiver. class: mailman.archiving.mhonarc.MHonArc - -configure: $ext_dir/mhonarc.cfg - +configuration: python:mailman.config.mhonarc [archiver.mail_archive] # This is the stock mail-archive.com archiver. class: mailman.archiving.mailarchive.MailArchive - -configure: $ext_dir/mail_archive.cfg - +configuration: python:mailman.config.mail_archive [archiver.prototype] # This is a prototypical sample archiver. diff --git a/src/mailman/config/tests/test_configuration.py b/src/mailman/config/tests/test_configuration.py index 88e00cbb9..08f27c094 100644 --- a/src/mailman/config/tests/test_configuration.py +++ b/src/mailman/config/tests/test_configuration.py @@ -22,12 +22,17 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ 'TestConfiguration', + 'TestExternal', ] import unittest -from mailman.interfaces.configuration import ConfigurationUpdatedEvent +from pkg_resources import resource_filename + +from mailman.config.config import external_configuration, load_external +from mailman.interfaces.configuration import ( + ConfigurationUpdatedEvent, MissingConfigurationFileError) from mailman.testing.helpers import configuration, event_subscribers from mailman.testing.layers import ConfigLayer @@ -51,3 +56,46 @@ class TestConfiguration(unittest.TestCase): # for the push leaving 'my test' on the top of the stack, and one for # the pop, leaving the ConfigLayer's 'test config' on top. self.assertEqual(events, ['my test', 'test config']) + + + +class TestExternal(unittest.TestCase): + """Test external configuration file loading APIs.""" + + def test_load_external_by_filename_as_bytes(self): + filename = resource_filename('mailman.config', 'postfix.cfg') + contents = load_external(filename) + self.assertIsInstance(contents, bytes) + self.assertEqual(contents[:9], b'[postfix]') + + def test_load_external_by_path_as_bytes(self): + contents = load_external('python:mailman.config.postfix') + self.assertIsInstance(contents, bytes) + self.assertEqual(contents[:9], b'[postfix]') + + def test_load_external_by_filename_as_string(self): + filename = resource_filename('mailman.config', 'postfix.cfg') + contents = load_external(filename, encoding='utf-8') + self.assertIsInstance(contents, unicode) + self.assertEqual(contents[:9], '[postfix]') + + def test_load_external_by_path_as_string(self): + contents = load_external('python:mailman.config.postfix', 'utf-8') + self.assertIsInstance(contents, unicode) + self.assertEqual(contents[:9], '[postfix]') + + def test_external_configuration_by_filename(self): + filename = resource_filename('mailman.config', 'postfix.cfg') + parser = external_configuration(filename) + self.assertEqual(parser.get('postfix', 'postmap_command'), + '/usr/sbin/postmap') + + def test_external_configuration_by_path(self): + parser = external_configuration('python:mailman.config.postfix') + self.assertEqual(parser.get('postfix', 'postmap_command'), + '/usr/sbin/postmap') + + def test_missing_configuration_file(self): + with self.assertRaises(MissingConfigurationFileError) as cm: + external_configuration('path:mailman.config.missing') + self.assertEqual(cm.exception.path, 'path:mailman.config.missing') diff --git a/src/mailman/core/errors.py b/src/mailman/core/errors.py index 529ac86fe..42b536d46 100644 --- a/src/mailman/core/errors.py +++ b/src/mailman/core/errors.py @@ -43,7 +43,10 @@ __all__ = [ 'MemberError', 'MustDigestError', 'PasswordError', + 'RESTError', + 'ReadOnlyPATCHRequestError', 'RejectMessage', + 'UnknownPATCHRequestError', ] @@ -87,6 +90,12 @@ def _(s): class HandlerError(MailmanError): """Base class for all handler errors.""" + def __init__(self, message=None): + self.message = message + + def __str__(self): + return self.message + class HoldMessage(HandlerError): """Base class for all message-being-held short circuits.""" @@ -126,3 +135,22 @@ class BadPasswordSchemeError(PasswordError): def __str__(self): return 'A bad password scheme was given: %s' % self.scheme_name + + + +class RESTError(MailmanError): + """Base class for REST API errors.""" + + +class UnknownPATCHRequestError(RESTError): + """A PATCH request contained an unknown attribute.""" + + def __init__(self, attribute): + self.attribute = attribute + + +class ReadOnlyPATCHRequestError(RESTError): + """A PATCH request contained a read-only attribute.""" + + def __init__(self, attribute): + self.attribute = attribute diff --git a/src/mailman/core/logging.py b/src/mailman/core/logging.py index af04b58d1..2442d4913 100644 --- a/src/mailman/core/logging.py +++ b/src/mailman/core/logging.py @@ -40,8 +40,8 @@ _handlers = {} -# XXX I would love to simplify things and use Python 2.6's WatchedFileHandler, -# but there are two problems. First, it's more difficult to handle the test +# XXX I would love to simplify things and use Python's WatchedFileHandler, but +# there are two problems. First, it's more difficult to handle the test # suite's need to reopen the file handler to a different path. Does # zope.testing's logger support fix this? # diff --git a/src/mailman/core/tests/test_runner.py b/src/mailman/core/tests/test_runner.py index ad2548adc..80a503f5d 100644 --- a/src/mailman/core/tests/test_runner.py +++ b/src/mailman/core/tests/test_runner.py @@ -81,7 +81,7 @@ Message-ID: <ant> self.assertEqual(event.message['message-id'], '<ant>') self.assertEqual(event.metadata['listname'], 'test@example.com') self.assertTrue(isinstance(event.error, RuntimeError)) - self.assertEqual(event.error.message, 'borked') + self.assertEqual(str(event.error), 'borked') self.assertTrue(isinstance(event.runner, CrashingRunner)) # The message should also have ended up in the shunt queue. shunted = get_queue_messages('shunt') diff --git a/src/mailman/database/docs/migration.rst b/src/mailman/database/docs/migration.rst index 6216f9bbc..2988579d3 100644 --- a/src/mailman/database/docs/migration.rst +++ b/src/mailman/database/docs/migration.rst @@ -30,12 +30,13 @@ already applied. >>> from mailman.model.version import Version >>> results = config.db.store.find(Version, component='schema') >>> results.count() - 2 + 3 >>> versions = sorted(result.version for result in results) >>> for version in versions: ... print version 00000000000000 20120407000000 + 20121015000000 Migrations @@ -96,6 +97,7 @@ This will load the new migration, since it hasn't been loaded before. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 >>> test = config.db.store.find(Version, component='test').one() >>> print test.version @@ -126,6 +128,7 @@ The first time we load this new migration, we'll get the 801 marker. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 20129999000001 >>> test = config.db.store.find(Version, component='test') @@ -142,6 +145,7 @@ We do not get an 802 marker because the migration has already been loaded. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 20129999000001 >>> test = config.db.store.find(Version, component='test') @@ -176,6 +180,7 @@ You'll notice that the ...04 version is not present. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 20129999000001 20129999000002 diff --git a/src/mailman/database/schema/mm_20120407000000.py b/src/mailman/database/schema/mm_20120407000000.py index d6e647c1a..3910e9438 100644 --- a/src/mailman/database/schema/mm_20120407000000.py +++ b/src/mailman/database/schema/mm_20120407000000.py @@ -149,7 +149,7 @@ def upgrade_sqlite(database, store, version, module_path): def upgrade_postgres(database, store, version, module_path): # Get the old values from the mailinglist table. results = store.execute(""" - SELECT id, archive, archive_private, list_name, mail_host + SELECT id, archive, archive_private, list_name, mail_host FROM mailinglist; """) # Do the simple renames first. diff --git a/src/mailman/database/schema/mm_20121015000000.py b/src/mailman/database/schema/mm_20121015000000.py new file mode 100644 index 000000000..51e0602e7 --- /dev/null +++ b/src/mailman/database/schema/mm_20121015000000.py @@ -0,0 +1,88 @@ +# Copyright (C) 2012 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. + +"""3.0b2 -> 3.0b3 schema migrations. + +* bans.mailing_list -> bans.list_id +""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ + 'upgrade', + ] + + +VERSION = '20121015000000' + + + +def upgrade(database, store, version, module_path): + if database.TAG == 'sqlite': + upgrade_sqlite(database, store, version, module_path) + else: + upgrade_postgres(database, store, version, module_path) + + + +def _make_listid(fqdn_listname): + list_name, at, mail_host = fqdn_listname.partition('@') + if at == '': + # If there is no @ sign in the value, assume it already contains the + # list-id. + return fqdn_listname + return '{0}.{1}'.format(list_name, mail_host) + + + +def upgrade_sqlite(database, store, version, module_path): + database.load_schema( + store, version, 'sqlite_{0}_01.sql'.format(version), module_path) + results = store.execute(""" + SELECT id, mailing_list + FROM ban; + """) + for id, mailing_list in results: + # Skip global bans since there's nothing to update. + if mailing_list is None: + continue + store.execute(""" + UPDATE ban_backup SET list_id = '{0}' + WHERE id = {1}; + """.format(_make_listid(mailing_list), id)) + # Pivot the backup table to the real thing. + store.execute('DROP TABLE ban;') + store.execute('ALTER TABLE ban_backup RENAME TO ban;') + + + +def upgrade_postgres(database, store, version, module_path): + # Get the old values from the ban table. + results = store.execute('SELECT id, mailing_list FROM ban;') + store.execute('ALTER TABLE ban ADD COLUMN list_id TEXT;') + for id, mailing_list in results: + # Skip global bans since there's nothing to update. + if mailing_list is None: + continue + store.execute(""" + UPDATE ban SET list_id = '{0}' + WHERE id = {1}; + """.format(_make_listid(mailing_list), id)) + store.execute('ALTER TABLE ban DROP COLUMN mailing_list;') + # Record the migration in the version table. + database.load_schema(store, version, None, module_path) diff --git a/src/mailman/database/schema/sqlite_20120407000000_01.sql b/src/mailman/database/schema/sqlite_20120407000000_01.sql index b93e214c4..53bab70dd 100644 --- a/src/mailman/database/schema/sqlite_20120407000000_01.sql +++ b/src/mailman/database/schema/sqlite_20120407000000_01.sql @@ -269,7 +269,7 @@ INSERT INTO mem_backup SELECT preferences_id, user_id FROM member; - + -- Add the new columns. They'll get inserted at the Python layer. ALTER TABLE ml_backup ADD COLUMN archive_policy INTEGER; diff --git a/src/mailman/database/schema/sqlite_20121015000000_01.sql b/src/mailman/database/schema/sqlite_20121015000000_01.sql new file mode 100644 index 000000000..c0df75111 --- /dev/null +++ b/src/mailman/database/schema/sqlite_20121015000000_01.sql @@ -0,0 +1,22 @@ +-- THIS FILE CONTAINS THE SQLITE3 SCHEMA MIGRATION FROM +-- 3.0b2 TO 3.0b3 +-- +-- AFTER 3.0b3 IS RELEASED YOU MAY NOT EDIT THIS FILE. + +-- REMOVALS from the ban table. +-- REM mailing_list + +-- ADDS to the ban table. +-- ADD list_id + +CREATE TABLE ban_backup ( + id INTEGER NOT NULL, + email TEXT, + PRIMARY KEY (id) + ); + +INSERT INTO ban_backup SELECT + id, email + FROM ban; + +ALTER TABLE ban_backup ADD COLUMN list_id TEXT; diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index f50ce35d3..4401b030f 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -29,6 +29,7 @@ __all__ = [ import unittest +from operator import attrgetter from pkg_resources import resource_string from storm.exceptions import DatabaseError from zope.component import getUtility @@ -40,30 +41,14 @@ from mailman.interfaces.listmanager import IListManager from mailman.interfaces.mailinglist import IAcceptableAliasSet from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.subscriptions import ISubscriptionService +from mailman.model.bans import Ban from mailman.testing.helpers import temporary_db from mailman.testing.layers import ConfigLayer class MigrationTestBase(unittest.TestCase): - """Test the dated migration (LP: #971013) - - Circa: 3.0b1 -> 3.0b2 - - table mailinglist: - * news_moderation -> newsgroup_moderation - * news_prefix_subject_too -> nntp_prefix_subject_too - * include_list_post_header -> allow_list_posts - * ADD archive_policy - * ADD list_id - * REMOVE archive - * REMOVE archive_private - * REMOVE archive_volume_frequency - * REMOVE nntp_host - - table member: - * mailing_list -> list_id - """ + """Test database migrations.""" layer = ConfigLayer @@ -73,6 +58,30 @@ class MigrationTestBase(unittest.TestCase): def tearDown(self): self._database._cleanup() + def _missing_present(self, table, migrations, missing, present): + """The appropriate migrations leave columns missing and present. + + :param table: The table to test columns from. + :param migrations: Sequence of migrations to load. + :param missing: Set of columns which should be missing after the + migrations are loaded. + :param present: Set of columns which should be present after the + migrations are loaded. + """ + for migration in migrations: + self._database.load_migrations(migration) + self._database.store.commit() + for column in missing: + self.assertRaises(DatabaseError, + self._database.store.execute, + 'select {0} from {1};'.format(column, table)) + self._database.store.rollback() + for column in present: + # This should not produce an exception. Is there some better test + # that we can perform? + self._database.store.execute( + 'select {0} from {1};'.format(column, table)) + class TestMigration20120407Schema(MigrationTestBase): @@ -81,70 +90,52 @@ class TestMigration20120407Schema(MigrationTestBase): def test_pre_upgrade_columns_migration(self): # Test that before the migration, the old table columns are present # and the new database columns are not. - # - # Load all the migrations to just before the one we're testing. - self._database.load_migrations('20120406999999') - self._database.store.commit() - # Verify that the database has not yet been migrated. - for missing in ('allow_list_posts', - 'archive_policy', - 'list_id', - 'nntp_prefix_subject_too'): - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select {0} from mailinglist;'.format(missing)) - self._database.store.rollback() - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select list_id from member;') - self._database.store.rollback() - for present in ('archive', - 'archive_private', - 'archive_volume_frequency', - 'generic_nonmember_action', - 'include_list_post_header', - 'news_moderation', - 'news_prefix_subject_too', - 'nntp_host'): - # This should not produce an exception. Is there some better test - # that we can perform? - self._database.store.execute( - 'select {0} from mailinglist;'.format(present)) - # Again, this should not produce an exception. - self._database.store.execute('select mailing_list from member;') + self._missing_present('mailinglist', + ['20120406999999'], + # New columns are missing. + ('allow_list_posts', + 'archive_policy', + 'list_id', + 'nntp_prefix_subject_too'), + # Old columns are present. + ('archive', + 'archive_private', + 'archive_volume_frequency', + 'generic_nonmember_action', + 'include_list_post_header', + 'news_moderation', + 'news_prefix_subject_too', + 'nntp_host')) + self._missing_present('member', + ['20120406999999'], + ('list_id',), + ('mailing_list',)) def test_post_upgrade_columns_migration(self): # Test that after the migration, the old table columns are missing # and the new database columns are present. - # - # Load all the migrations up to and including the one we're testing. - self._database.load_migrations('20120406999999') - self._database.load_migrations('20120407000000') - # Verify that the database has been migrated. - for present in ('allow_list_posts', - 'archive_policy', - 'list_id', - 'nntp_prefix_subject_too'): - # This should not produce an exception. Is there some better test - # that we can perform? - self._database.store.execute( - 'select {0} from mailinglist;'.format(present)) - self._database.store.execute('select list_id from member;') - for missing in ('archive', - 'archive_private', - 'archive_volume_frequency', - 'generic_nonmember_action', - 'include_list_post_header', - 'news_moderation', - 'news_prefix_subject_too', - 'nntp_host'): - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select {0} from mailinglist;'.format(missing)) - self._database.store.rollback() - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select mailing_list from member;') + self._missing_present('mailinglist', + ['20120406999999', + '20120407000000'], + # The old columns are missing. + ('archive', + 'archive_private', + 'archive_volume_frequency', + 'generic_nonmember_action', + 'include_list_post_header', + 'news_moderation', + 'news_prefix_subject_too', + 'nntp_host'), + # The new columns are present. + ('allow_list_posts', + 'archive_policy', + 'list_id', + 'nntp_prefix_subject_too')) + self._missing_present('member', + ['20120406999999', + '20120407000000'], + ('mailing_list',), + ('list_id',)) @@ -367,3 +358,48 @@ class TestMigration20120407MigratedData(MigrationTestBase): with temporary_db(self._database): mlist = getUtility(IListManager).get('test@example.com') self.assertTrue(mlist.allow_list_posts) + + + +class TestMigration20121015Schema(MigrationTestBase): + """Test column migrations.""" + + def test_pre_upgrade_column_migrations(self): + self._missing_present('ban', + ['20121014999999'], + ('list_id',), + ('mailing_list',)) + + def test_post_upgrade_column_migrations(self): + self._missing_present('ban', + ['20121014999999', + '20121015000000'], + ('mailing_list',), + ('list_id',)) + + +class TestMigration20121015MigratedData(MigrationTestBase): + """Test non-migrated data.""" + + def test_migration_bans(self): + # Load all the migrations to just before the one we're testing. + self._database.load_migrations('20121014999999') + # Insert a list-specific ban. + self._database.store.execute(""" + INSERT INTO ban VALUES ( + 1, 'anne@example.com', 'test@example.com'); + """) + # Insert a global ban. + self._database.store.execute(""" + INSERT INTO ban VALUES ( + 2, 'bart@example.com', NULL); + """) + # Update to the current migration we're testing. + self._database.load_migrations('20121015000000') + # Now both the local and global bans should still be present. + bans = sorted(self._database.store.find(Ban), + key=attrgetter('email')) + self.assertEqual(bans[0].email, 'anne@example.com') + self.assertEqual(bans[0].list_id, 'test.example.com') + self.assertEqual(bans[1].email, 'bart@example.com') + self.assertEqual(bans[1].list_id, None) diff --git a/src/mailman/docs/ACKNOWLEDGMENTS.rst b/src/mailman/docs/ACKNOWLEDGMENTS.rst index 10d4bcd93..c0ef13a22 100644 --- a/src/mailman/docs/ACKNOWLEDGMENTS.rst +++ b/src/mailman/docs/ACKNOWLEDGMENTS.rst @@ -101,6 +101,7 @@ left off the list! * Stuart Bishop * David Blomquist * Bojan +* Aurélien Bompard * Søren Bondrup * Grant Bowman * Alessio Bragadini diff --git a/src/mailman/docs/INTRODUCTION.rst b/src/mailman/docs/INTRODUCTION.rst index 11d13c239..8e334c08a 100644 --- a/src/mailman/docs/INTRODUCTION.rst +++ b/src/mailman/docs/INTRODUCTION.rst @@ -82,7 +82,7 @@ lists and archives, etc., are available at: Requirements ============ -Mailman 3.0 requires `Python 2.6`_ or newer. +Mailman 3.0 requires `Python 2.7`_ or newer. .. _`GNU Mailman`: http://www.list.org @@ -90,4 +90,4 @@ Mailman 3.0 requires `Python 2.6`_ or newer. .. _`Getting Started`: START.html .. _Python: http://www.python.org .. _FAQ: http://wiki.list.org/display/DOC/Frequently+Asked+Questions -.. _`Python 2.6`: http://www.python.org/download/releases/2.6.6/ +.. _`Python 2.7`: http://www.python.org/download/releases/2.7.3/ diff --git a/src/mailman/docs/MTA.rst b/src/mailman/docs/MTA.rst index c6d2230c4..ccfd18c0e 100644 --- a/src/mailman/docs/MTA.rst +++ b/src/mailman/docs/MTA.rst @@ -93,7 +93,7 @@ Transport maps By default, Mailman works well with Postfix transport maps as a way to deliver incoming messages to Mailman's LMTP server. Mailman will automatically write -the correct transport map when its `bin/mailman genaliases` command is run, or +the correct transport map when its `bin/mailman aliases` command is run, or whenever a mailing list is created or removed via other commands. To connect Postfix to Mailman's LMTP server, add the following to Postfix's `main.cf` file:: @@ -102,17 +102,33 @@ file:: hash:/path-to-mailman/var/data/postfix_lmtp local_recipient_maps = hash:/path-to-mailman/var/data/postfix_lmtp + relay_domains = + hash:/path-to-mailman/var/data/postfix_domains where `path-to-mailman` is replaced with the actual path that you're running Mailman from. Setting `local_recipient_maps` as well as `transport_maps` allows Postfix to properly reject all messages destined for non-existent local -users. +users. Setting `relay_domains`_ means Postfix will start to accept mail for +newly added domains even if they are not part of `mydestination`_. +Note that if you are not using virtual domains, then `relay_domains`_ isn't +strictly needed (but it is harmless). All you need to do in this scenario is +to make sure that Postfix accepts mail for your one domain, normally by +including it in `mydestination`. -Virtual domains ---------------- -TBD: figure out how virtual domains interact with the transport maps. +Postfix documentation +--------------------- + +For more information regarding how to configure Postfix, please see +the Postfix documentation at: + +.. _`The official Postfix documentation`: + http://www.postfix.org/documentation.html +.. _`The reference page for all Postfix configuration parameters`: + http://www.postfix.org/postconf.5.html +.. _`relay_domains`: http://www.postfix.org/postconf.5.html#relay_domains +.. _`mydestination`: http://www.postfix.org/postconf.5.html#mydestination Sendmail diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 329264e31..3c7580126 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -12,6 +12,61 @@ Here is a history of user visible changes to Mailman. ========================== (2012-XX-XX) +Compatibility +------------- + * Python 2.7 is not required. Python 2.6 is no longer officially supported. + The code base is now also `python2.7 -3` clean, although there are still + some warnings in 3rd party dependencies. LP: #1073506 + +REST +---- + * Add list_id to JSON representation for a mailing list (given by Jimmy + Bergman). + * The canonical resource for a mailing list (and thus its self_link) is now + the URL with the list-id. To reference a mailing list, the list-id url is + preferred, but for backward compatibility, the posting address is still + accepted. + * You can now PUT and PATCH on user resources to change the user's display + name or password. For passwords, you pass in the clear text password and + Mailman will hash it before storing. + * You can now verify and unverify an email address through the REST API. + POST to .../addresses/<email>/verify and .../addresses/<email>/unverify + respectively. The POST data is ignored. It is not an error to verify or + unverify an address more than once, but verifying an already verified + address does not change its `.verified_on` date. (LP: #1054730) + +Configuration +------------- + * `[passlib]path` configuration variable renamed to `[passlib]configuration`. + * Postfix-specific configurations in the `[mta]` section are moved to a + separate file, named by the `[mta]configuration` variable. + * In the new `postfix.cfg` file, `postfix_map_cmd` is renamed to + `postmap_command`. + +Database +-------- + * The `ban` table now uses list-ids to cross-reference the mailing list, + since these cannot change even if the mailing list is moved or renamed. + +Interfaces +---------- + * The `IBanManager` is no longer a global utility. Instead, you adapt an + `IMailingList` to an `IBanManager` to manage the bans for a specific + mailing list. To manage the global bans, adapt ``None``. + +Integration +----------- + * Added support for Postfix `relay_domains` setting for better virtual domain + support. Contributed by Jimmy Bergman. + +Commands +-------- + * `bin/mailman aliases` loses the `--output`, `--format`, and `--simple` + arguments, and adds a `--directory` argument. This is necessary to support + the Postfix `relay_domains` support. + * `bin/mailman start` was passing the wrong relative path to its runner + subprocesses when -C was given. LP: #982551 + 3.0 beta 2 -- "Freeze" ====================== diff --git a/src/mailman/docs/START.rst b/src/mailman/docs/START.rst index da76feae0..dbf4966c4 100644 --- a/src/mailman/docs/START.rst +++ b/src/mailman/docs/START.rst @@ -32,14 +32,13 @@ mailman-developers@python.org mailing list. Requirements ============ -Python 2.6 or 2.7 is required. It can either be the default 'python' on your -$PATH or it can be accessible via the ``python2.6`` or ``python2.7`` binary. +Python 2.7 is required. It can either be the default 'python' on your +$PATH or it can be accessible via the ``python2.7`` binary. If your operating system does not include Python, see http://www.python.org downloading and installing it from source. Python 3 is not yet supported. In this documentation, a bare ``python`` refers to the Python executable used -to invoke ``bootstrap.py``, which might be ``python2.6`` or ``python2.7``, as -well as the system ``python`` or an absolute path. +to invoke ``bootstrap.py``. Mailman 3 is now based on the `zc.buildout`_ infrastructure, which greatly simplifies building and testing Mailman. diff --git a/src/mailman/docs/STYLEGUIDE.rst b/src/mailman/docs/STYLEGUIDE.rst index b744c6557..29661701b 100644 --- a/src/mailman/docs/STYLEGUIDE.rst +++ b/src/mailman/docs/STYLEGUIDE.rst @@ -15,7 +15,7 @@ http://barry.warsaw.us/software/STYLEGUIDE.txt This document contains a style guide for Python programming, as used in GNU Mailman. `PEP 8`_ is the basis for this style guide so it's recommendations should be followed except for the differences outlined here. This document -assumes the use of Python 2.6 or 2.7, but not (yet) Python 3. +assumes the use of Python 2.7, but not (yet) Python 3. * After file comments (e.g. license block), add a ``__metaclass__`` definition so that all classes will be new-style. Following that, add an ``__all__`` diff --git a/src/mailman/docs/WebUIin5.rst b/src/mailman/docs/WebUIin5.rst index 56f2df9fb..6419e4752 100644 --- a/src/mailman/docs/WebUIin5.rst +++ b/src/mailman/docs/WebUIin5.rst @@ -7,8 +7,8 @@ Mailman 3's web UI, called Postorius. If all goes as planned, you should be done within 5 minutes. This has been tested on Ubuntu 11.04. In order to download the components necessary you need to have the `Bazaar`_ -version control system installed on your system. Mailman and mailman.client -need at least Python version 2.6. +version control system installed on your system. Mailman requires Python 2.7, +while mailman.client needs at least Python version 2.6. It's probably a good idea to set up a virtual Python environment using `virtualenv`_. `Here is a brief HOWTO`_. diff --git a/src/mailman/email/message.py b/src/mailman/email/message.py index dcea82425..601d4f839 100644 --- a/src/mailman/email/message.py +++ b/src/mailman/email/message.py @@ -219,11 +219,7 @@ class UserNotification(Message): if mlist is not None: enqueue_kws['listname'] = mlist.fqdn_listname enqueue_kws.update(_kws) - # Keywords must be strings in Python 2.6. - str_keywords = dict() - for key, val in enqueue_kws.items(): - str_keywords[str(key)] = val - virginq.enqueue(self, **str_keywords) + virginq.enqueue(self, **enqueue_kws) diff --git a/src/mailman/handlers/tests/test_mimedel.py b/src/mailman/handlers/tests/test_mimedel.py index 6ca34b17b..1f74db6ef 100644 --- a/src/mailman/handlers/tests/test_mimedel.py +++ b/src/mailman/handlers/tests/test_mimedel.py @@ -37,8 +37,7 @@ from mailman.interfaces.action import FilterAction from mailman.interfaces.member import MemberRole from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( - LogFileMark, - get_queue_messages, + LogFileMark, configuration, get_queue_messages, specialized_message_from_string as mfs) from mailman.testing.layers import ConfigLayer @@ -58,38 +57,29 @@ Subject: A disposable message Message-ID: <ant> """) - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. - self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) config.push('dispose', """ [mailman] site_owner: noreply@example.com """) + # Let assertMultiLineEqual work without bounds. + self.maxDiff = None def tearDown(self): config.pop('dispose') def test_dispose_discard(self): self._mlist.filter_action = FilterAction.discard - try: + with self.assertRaises(errors.DiscardMessage) as cm: mime_delete.dispose(self._mlist, self._msg, {}, 'discarding') - except errors.DiscardMessage as error: - pass - else: - raise AssertionError('DiscardMessage exception expected') - self.assertEqual(error.message, 'discarding') + self.assertEqual(cm.exception.message, 'discarding') # There should be no messages in the 'bad' queue. self.assertEqual(len(get_queue_messages('bad')), 0) def test_dispose_bounce(self): self._mlist.filter_action = FilterAction.reject - try: + with self.assertRaises(errors.RejectMessage) as cm: mime_delete.dispose(self._mlist, self._msg, {}, 'rejecting') - except errors.RejectMessage as error: - pass - else: - raise AssertionError('RejectMessage exception expected') - self.assertEqual(error.message, 'rejecting') + self.assertEqual(cm.exception.message, 'rejecting') # There should be no messages in the 'bad' queue. self.assertEqual(len(get_queue_messages('bad')), 0) @@ -103,13 +93,9 @@ Message-ID: <ant> self._mlist.subscribe(bart, MemberRole.moderator) # Now set the filter action and dispose the message. self._mlist.filter_action = FilterAction.forward - try: + with self.assertRaises(errors.DiscardMessage) as cm: mime_delete.dispose(self._mlist, self._msg, {}, 'forwarding') - except errors.DiscardMessage as error: - pass - else: - raise AssertionError('DiscardMessage exception expected') - self.assertEqual(error.message, 'forwarding') + self.assertEqual(cm.exception.message, 'forwarding') # There should now be a multipart message in the virgin queue destined # for the mailing list owners. messages = get_queue_messages('virgin') @@ -128,7 +114,7 @@ Message-ID: <ant> # The body of the first part provides the moderators some details. part0 = message.get_payload(0) self.assertEqual(part0.get_content_type(), 'text/plain') - self.eq(part0.get_payload(), """\ + self.assertMultiLineEqual(part0.get_payload(), """\ The attached message matched the Test mailing list's content filtering rules and was prevented from being forwarded on to the list membership. You are receiving the only remaining copy of the discarded @@ -143,46 +129,28 @@ message. self.assertEqual(original['subject'], 'A disposable message') self.assertEqual(original['message-id'], '<ant>') + @configuration('mailman', filtered_messages_are_preservable='no') def test_dispose_non_preservable(self): # Two actions can happen here, depending on a site-wide setting. If # the site owner has indicated that filtered messages cannot be # preserved, then this is the same as discarding them. self._mlist.filter_action = FilterAction.preserve - config.push('non-preservable', """ - [mailman] - filtered_messages_are_preservable: no - """) - try: + with self.assertRaises(errors.DiscardMessage) as cm: mime_delete.dispose(self._mlist, self._msg, {}, 'not preserved') - except errors.DiscardMessage as error: - pass - else: - raise AssertionError('DiscardMessage exception expected') - finally: - config.pop('non-preservable') - self.assertEqual(error.message, 'not preserved') + self.assertEqual(cm.exception.message, 'not preserved') # There should be no messages in the 'bad' queue. self.assertEqual(len(get_queue_messages('bad')), 0) + @configuration('mailman', filtered_messages_are_preservable='yes') def test_dispose_preservable(self): # Two actions can happen here, depending on a site-wide setting. If # the site owner has indicated that filtered messages can be # preserved, then this is similar to discarding the message except # that a copy is preserved in the 'bad' queue. self._mlist.filter_action = FilterAction.preserve - config.push('preservable', """ - [mailman] - filtered_messages_are_preservable: yes - """) - try: + with self.assertRaises(errors.DiscardMessage) as cm: mime_delete.dispose(self._mlist, self._msg, {}, 'preserved') - except errors.DiscardMessage as error: - pass - else: - raise AssertionError('DiscardMessage exception expected') - finally: - config.pop('preservable') - self.assertEqual(error.message, 'preserved') + self.assertEqual(cm.exception.message, 'preserved') # There should be no messages in the 'bad' queue. messages = get_queue_messages('bad') self.assertEqual(len(messages), 1) @@ -200,13 +168,9 @@ message. FilterAction.defer): self._mlist.filter_action = action mark = LogFileMark('mailman.error') - try: + with self.assertRaises(errors.DiscardMessage) as cm: mime_delete.dispose(self._mlist, self._msg, {}, 'bad action') - except errors.DiscardMessage as error: - pass - else: - raise AssertionError('DiscardMessage exception expected') - self.assertEqual(error.message, 'bad action') + self.assertEqual(cm.exception.message, 'bad action') line = mark.readline()[:-1] self.assertTrue(line.endswith( '{0} invalid FilterAction: test@example.com. ' diff --git a/src/mailman/interfaces/address.py b/src/mailman/interfaces/address.py index 54bf6b283..7df15b91f 100644 --- a/src/mailman/interfaces/address.py +++ b/src/mailman/interfaces/address.py @@ -47,11 +47,7 @@ class EmailError(MailmanError): self.email = email def __str__(self): - # This is a workaround for Python 2.6 support. When self.email - # contains non-ascii characters, this will cause unprintable output in - # doctests. Python 2.7 can handle it but we haven't dropped support - # for 2.6 yet. - return self.email.encode('us-ascii', 'backslashreplace') + return self.email class AddressError(MailmanError): diff --git a/src/mailman/interfaces/bans.py b/src/mailman/interfaces/bans.py index 233a374b1..e09d48575 100644 --- a/src/mailman/interfaces/bans.py +++ b/src/mailman/interfaces/bans.py @@ -17,7 +17,7 @@ """Manager of email address bans.""" -from __future__ import absolute_import, unicode_literals +from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ @@ -33,49 +33,48 @@ from zope.interface import Attribute, Interface class IBan(Interface): """A specific ban. - In general, this interface isn't publicly useful. + In general, this interface isn't used publicly. Instead, bans are managed + through the `IBanManager` interface. """ email = Attribute('The banned email address, or pattern.') - mailing_list = Attribute( - """The fqdn name of the mailing list the ban applies to. + list_id = Attribute( + """The list-id of the mailing list the ban applies to. - Use None if this is a global ban. + Use ``None`` if this is a global ban. """) class IBanManager(Interface): - """The global manager of email address bans.""" + """The global manager of email address bans. - def ban(email, mailing_list=None): + To manage bans for a specific mailing list, adapt that `IMailingList` + to an `IBanManager`. To manage global bans, adapt ``None``. + """ + + def ban(email): """Ban an email address from subscribing to a mailing list. When an email address is banned, it will not be allowed to subscribe - to a the named mailing list. This does not affect any email address - already subscribed to the mailing list. With the default arguments, - an email address can be banned globally from subscribing to any - mailing list on the system. + to the mailing list. This does not affect any email address that may + already be subscribed to a mailing list. It is also possible to add a 'ban pattern' whereby all email addresses matching a Python regular expression can be banned. This is accomplished by using a `^` as the first character in `email`. - When an email address is already banned for the given mailing list (or - globally), then this method does nothing. However, it is possible to + When an email address is already banned. However, it is possible to extend a ban for a specific mailing list into a global ban; both bans would be in place and they can be removed individually. :param email: The text email address being banned or, if the string starts with a caret (^), the email address pattern to ban. :type email: str - :param mailing_list: The fqdn name of the mailing list to which the - ban applies. If None, then the ban is global. - :type mailing_list: string """ - def unban(email, mailing_list=None): + def unban(email): """Remove an email address ban. This removes a specific or global email address ban, which would have @@ -85,12 +84,9 @@ class IBanManager(Interface): :param email: The text email address being unbanned or, if the string starts with a caret (^), the email address pattern to unban. :type email: str - :param mailing_list: The fqdn name of the mailing list to which the - unban applies. If None, then the unban is global. - :type mailing_list: string """ - def is_banned(email, mailing_list=None): + def is_banned(email): """Check whether a specific email address is banned. `email` must be a text email address; it cannot be a pattern. The @@ -100,10 +96,6 @@ class IBanManager(Interface): :param email: The text email address being checked. :type email: str - :param mailing_list: The fqdn name of the mailing list being checked. - Note that if not None, both specific and global bans will be - checked. If None, then only global bans will be checked. - :type mailing_list: string :return: A flag indicating whether the given email address is banned or not. :rtype: bool diff --git a/src/mailman/interfaces/configuration.py b/src/mailman/interfaces/configuration.py index 8c4fb52a6..901706f91 100644 --- a/src/mailman/interfaces/configuration.py +++ b/src/mailman/interfaces/configuration.py @@ -23,11 +23,22 @@ __metaclass__ = type __all__ = [ 'ConfigurationUpdatedEvent', 'IConfiguration', + 'MissingConfigurationFileError', ] from zope.interface import Interface +from mailman.core.errors import MailmanError + + + +class MissingConfigurationFileError(MailmanError): + """A named configuration file was not found.""" + + def __init__(self, path): + self.path = path + class IConfiguration(Interface): diff --git a/src/mailman/interfaces/mta.py b/src/mailman/interfaces/mta.py index 34c210edd..303a8e42a 100644 --- a/src/mailman/interfaces/mta.py +++ b/src/mailman/interfaces/mta.py @@ -84,12 +84,12 @@ class IMailTransportAgentLifecycle(Interface): def delete(mlist): """Tell the MTA that the mailing list was deleted.""" - def regenerate(output=None): + def regenerate(directory=None): """Regenerate the full aliases file. - :param output: The file name or file object to send the output to. If - not given or None, and MTA specific file is used. - :type output: string, file object, None + :param directory: The directory to write the MTA specific support + files to. Defaults to $DATA_DIR. + :type directory: string """ diff --git a/src/mailman/model/bans.py b/src/mailman/model/bans.py index b6de9336f..8687cd993 100644 --- a/src/mailman/model/bans.py +++ b/src/mailman/model/bans.py @@ -42,12 +42,12 @@ class Ban(Model): id = Int(primary=True) email = Unicode() - mailing_list = Unicode() + list_id = Unicode() - def __init__(self, email, mailing_list): + def __init__(self, email, list_id): super(Ban, self).__init__() self.email = email - self.mailing_list = mailing_list + self.list_id = list_id @@ -55,55 +55,58 @@ class Ban(Model): class BanManager: """See `IBanManager`.""" + def __init__(self, mailing_list=None): + self._list_id = (None if mailing_list is None + else mailing_list.list_id) + @dbconnection - def ban(self, store, email, mailing_list=None): + def ban(self, store, email): """See `IBanManager`.""" - bans = store.find(Ban, email=email, mailing_list=mailing_list) + bans = store.find(Ban, email=email, list_id=self._list_id) if bans.count() == 0: - ban = Ban(email, mailing_list) + ban = Ban(email, self._list_id) store.add(ban) @dbconnection - def unban(self, store, email, mailing_list=None): + def unban(self, store, email): """See `IBanManager`.""" - ban = store.find(Ban, email=email, mailing_list=mailing_list).one() + ban = store.find(Ban, email=email, list_id=self._list_id).one() if ban is not None: store.remove(ban) @dbconnection - def is_banned(self, store, email, mailing_list=None): + def is_banned(self, store, email): """See `IBanManager`.""" - # A specific mailing list ban is being checked, however the email - # address could be banned specifically, or globally. - if mailing_list is not None: - # Try specific bans first. - bans = store.find(Ban, email=email, mailing_list=mailing_list) + list_id = self._list_id + if list_id is None: + # The client is asking for global bans. Look up bans on the + # specific email address first. + bans = store.find(Ban, email=email, list_id=None) + if bans.count() > 0: + return True + # And now look for global pattern bans. + bans = store.find(Ban, list_id=None) + for ban in bans: + if (ban.email.startswith('^') and + re.match(ban.email, email, re.IGNORECASE) is not None): + return True + else: + # This is a list-specific ban. + bans = store.find(Ban, email=email, list_id=list_id) if bans.count() > 0: return True # Try global bans next. - bans = store.find(Ban, email=email, mailing_list=None) + bans = store.find(Ban, email=email, list_id=None) if bans.count() > 0: return True # Now try specific mailing list bans, but with a pattern. - bans = store.find(Ban, mailing_list=mailing_list) + bans = store.find(Ban, list_id=list_id) for ban in bans: if (ban.email.startswith('^') and re.match(ban.email, email, re.IGNORECASE) is not None): return True # And now try global pattern bans. - bans = store.find(Ban, mailing_list=None) - for ban in bans: - if (ban.email.startswith('^') and - re.match(ban.email, email, re.IGNORECASE) is not None): - return True - else: - # The client is asking for global bans. Look up bans on the - # specific email address first. - bans = store.find(Ban, email=email, mailing_list=None) - if bans.count() > 0: - return True - # And now look for global pattern bans. - bans = store.find(Ban, mailing_list=None) + bans = store.find(Ban, list_id=None) for ban in bans: if (ban.email.startswith('^') and re.match(ban.email, email, re.IGNORECASE) is not None): diff --git a/src/mailman/model/docs/listmanager.rst b/src/mailman/model/docs/listmanager.rst index 380fe7704..41450b15d 100644 --- a/src/mailman/model/docs/listmanager.rst +++ b/src/mailman/model/docs/listmanager.rst @@ -126,8 +126,7 @@ address components. test_3@example.com test_4@example.com - >>> for list_name, mail_host in sorted(list_manager.name_components, - ... key=lambda (name, host): name): + >>> for list_name, mail_host in sorted(list_manager.name_components): ... print list_name, '@', mail_host test @ example.com test_3 @ example.com diff --git a/src/mailman/mta/postfix.py b/src/mailman/mta/postfix.py index c04e38f02..ecd71db32 100644 --- a/src/mailman/mta/postfix.py +++ b/src/mailman/mta/postfix.py @@ -34,6 +34,7 @@ from zope.component import getUtility from zope.interface import implementer from mailman.config import config +from mailman.config.config import external_configuration from mailman.interfaces.listmanager import IListManager from mailman.interfaces.mta import ( IMailTransportAgentAliases, IMailTransportAgentLifecycle) @@ -42,6 +43,7 @@ from mailman.utilities.datetime import now log = logging.getLogger('mailman.error') ALIASTMPL = '{0:{2}}lmtp:[{1.mta.lmtp_host}]:{1.mta.lmtp_port}' +NL = '\n' @@ -59,6 +61,11 @@ class _FakeList: class LMTP: """Connect Mailman to Postfix via LMTP.""" + def __init__(self): + # Locate and read the Postfix specific configuration file. + mta_config = external_configuration(config.mta.configuration) + self.postmap_command = mta_config.get('postfix', 'postmap_command') + def create(self, mlist): """See `IMailTransportAgentLifecycle`.""" # We can ignore the mlist argument because for LMTP delivery, we just @@ -67,52 +74,49 @@ class LMTP: delete = create - def regenerate(self, output=None): - """See `IMailTransportAgentLifecycle`. - - The format for Postfix's LMTP transport map is defined here: - http://www.postfix.org/transport.5.html - """ + def regenerate(self, directory=None): + """See `IMailTransportAgentLifecycle`.""" # Acquire a lock file to prevent other processes from racing us here. + if directory is None: + directory = config.DATA_DIR lock_file = os.path.join(config.LOCK_DIR, 'mta') with Lock(lock_file): - # If output is a filename, open up a backing file and write the - # output there, then do the atomic rename dance. First though, if - # it's None, we use a calculated path. - if output is None: - path = os.path.join(config.DATA_DIR, 'postfix_lmtp') - path_new = path + '.new' - elif isinstance(output, basestring): - path = output - path_new = output + '.new' - else: - path = path_new = None - if path_new is None: - self._do_write_file(output) - # There's nothing to rename, and we can't generate the .db - # file, so we're done. - return - # Write the file. - with open(path_new, 'w') as fp: - self._do_write_file(fp) + lmtp_path = os.path.join(directory, 'postfix_lmtp') + lmtp_path_new = lmtp_path + '.new' + with open(lmtp_path_new, 'w') as fp: + self._generate_lmtp_file(fp) + # Atomically rename to the intended path. + os.rename(lmtp_path_new, lmtp_path) + domains_path = os.path.join(directory, 'postfix_domains') + domains_path_new = domains_path + '.new' + with open(domains_path_new, 'w') as fp: + self._generate_domains_file(fp) # Atomically rename to the intended path. - os.rename(path + '.new', path) - # Now that the new file is in place, we must tell Postfix to - # generate a new .db file. - command = config.mta.postfix_map_cmd + ' ' + path - status = (os.system(command) >> 8) & 0xff - if status: - msg = 'command failure: %s, %s, %s' - errstr = os.strerror(status) - log.error(msg, command, status, errstr) - raise RuntimeError(msg % (command, status, errstr)) + os.rename(domains_path_new, domains_path) + # Now, run the postmap command on both newly generated files. If + # one files, still try the other one. + errors = [] + for path in (lmtp_path, domains_path): + command = self.postmap_command + ' ' + path + status = (os.system(command) >> 8) & 0xff + if status: + msg = 'command failure: %s, %s, %s' + errstr = os.strerror(status) + log.error(msg, command, status, errstr) + errors.append(msg % (command, status, errstr)) + if errors: + raise RuntimeError(NL.join(errors)) - def _do_write_file(self, fp): - """Do the actual file writes for list creation.""" - # Sort all existing mailing list names first by domain, then by local - # part. For postfix we need a dummy entry for the domain. + def _generate_lmtp_file(self, fp): + # The format for Postfix's LMTP transport map is defined here: + # http://www.postfix.org/transport.5.html + # + # Sort all existing mailing list names first by domain, then by + # local part. For Postfix we need a dummy entry for the domain. list_manager = getUtility(IListManager) + utility = getUtility(IMailTransportAgentAliases) by_domain = {} + sort_key = attrgetter('list_name') for list_name, mail_host in list_manager.name_components: mlist = _FakeList(list_name, mail_host) by_domain.setdefault(mlist.mail_host, []).append(mlist) @@ -123,17 +127,32 @@ class LMTP: # file. YOU SHOULD NOT MANUALLY EDIT THIS FILE unless you know what you're # doing, and can keep the two files properly in sync. If you screw it up, # you're on your own. -""".format(now().replace(microsecond=0)), file=fp) - sort_key = attrgetter('list_name') + """.format(now().replace(microsecond=0)), file=fp) for domain in sorted(by_domain): print("""\ # Aliases which are visible only in the @{0} domain.""".format(domain), - file=fp) + file=fp) for mlist in sorted(by_domain[domain], key=sort_key): - utility = getUtility(IMailTransportAgentAliases) aliases = list(utility.aliases(mlist)) width = max(len(alias) for alias in aliases) + 3 print(ALIASTMPL.format(aliases.pop(0), config, width), file=fp) for alias in aliases: print(ALIASTMPL.format(alias, config, width), file=fp) print(file=fp) + + def _generate_domains_file(self, fp): + # Uniquify the domains, then sort them alphabetically. + domains = set() + for list_name, mail_host in getUtility(IListManager).name_components: + domains.add(mail_host) + print("""\ +# AUTOMATICALLY GENERATED BY MAILMAN ON {0} +# +# This file is generated by Mailman, and is kept in sync with the binary hash +# file. YOU SHOULD NOT MANUALLY EDIT THIS FILE unless you know what you're +# doing, and can keep the two files properly in sync. If you screw it up, +# you're on your own. +""".format(now().replace(microsecond=0)), file=fp) + for domain in sorted(domains): + print('{0} {0}'.format(domain), file=fp) + print(file=fp) diff --git a/src/mailman/mta/tests/test_aliases.py b/src/mailman/mta/tests/test_aliases.py index b1a60bc95..e22385dcf 100644 --- a/src/mailman/mta/tests/test_aliases.py +++ b/src/mailman/mta/tests/test_aliases.py @@ -15,28 +15,39 @@ # You should have received a copy of the GNU General Public License along with # GNU Mailman. If not, see <http://www.gnu.org/licenses/>. -"""Test the template generating utility.""" +"""Test the MTA file generating utility.""" from __future__ import absolute_import, unicode_literals __metaclass__ = type __all__ = [ + 'TestAliases', + 'TestPostfix', ] +import os +import shutil +import tempfile import unittest -from cStringIO import StringIO from zope.component import getUtility from mailman.app.lifecycle import create_list +from mailman.interfaces.domain import IDomainManager from mailman.interfaces.mta import IMailTransportAgentAliases from mailman.mta.postfix import LMTP from mailman.testing.layers import ConfigLayer + NL = '\n' +def _strip_header(contents): + lines = contents.splitlines() + return NL.join(lines[7:]) + + class TestAliases(unittest.TestCase): @@ -129,21 +140,33 @@ class TestPostfix(unittest.TestCase): layer = ConfigLayer def setUp(self): + self.tempdir = tempfile.mkdtemp() self.utility = getUtility(IMailTransportAgentAliases) self.mlist = create_list('test@example.com') - self.output = StringIO() self.postfix = LMTP() - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. + # Let assertMultiLineEqual work without bounds. self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) + + def tearDown(self): + shutil.rmtree(self.tempdir) def test_aliases(self): # Test the format of the Postfix alias generator. - self.postfix.regenerate(self.output) - # Strip out the variable and unimportant bits of the output. - lines = self.output.getvalue().splitlines() - output = NL.join(lines[7:]) - self.eq(output, """\ + self.postfix.regenerate(self.tempdir) + # There are two files in this directory. + self.assertEqual(sorted(os.listdir(self.tempdir)), + ['postfix_domains', 'postfix_lmtp']) + # The domains file, just contains the example.com domain. We have to + # ignore the file header. + with open(os.path.join(self.tempdir, 'postfix_domains')) as fp: + contents = _strip_header(fp.read()) + self.assertMultiLineEqual(contents, """\ +example.com example.com +""") + # The lmtp file contains transport mappings to the lmtp server. + with open(os.path.join(self.tempdir, 'postfix_lmtp')) as fp: + contents = _strip_header(fp.read()) + self.assertMultiLineEqual(contents, """\ # Aliases which are visible only in the @example.com domain. test@example.com lmtp:[127.0.0.1]:9024 test-bounces@example.com lmtp:[127.0.0.1]:9024 @@ -160,11 +183,21 @@ test-unsubscribe@example.com lmtp:[127.0.0.1]:9024 # Both lists need to show up in the aliases file. LP: #874929. # Create a second list. create_list('other@example.com') - self.postfix.regenerate(self.output) - # Strip out the variable and unimportant bits of the output. - lines = self.output.getvalue().splitlines() - output = NL.join(lines[7:]) - self.eq(output, """\ + self.postfix.regenerate(self.tempdir) + # There are two files in this directory. + self.assertEqual(sorted(os.listdir(self.tempdir)), + ['postfix_domains', 'postfix_lmtp']) + # Because both lists are in the same domain, there should be only one + # entry in the relays file. + with open(os.path.join(self.tempdir, 'postfix_domains')) as fp: + contents = _strip_header(fp.read()) + self.assertMultiLineEqual(contents, """\ +example.com example.com +""") + # The transport file contains entries for both lists. + with open(os.path.join(self.tempdir, 'postfix_lmtp')) as fp: + contents = _strip_header(fp.read()) + self.assertMultiLineEqual(contents, """\ # Aliases which are visible only in the @example.com domain. other@example.com lmtp:[127.0.0.1]:9024 other-bounces@example.com lmtp:[127.0.0.1]:9024 @@ -186,3 +219,48 @@ test-request@example.com lmtp:[127.0.0.1]:9024 test-subscribe@example.com lmtp:[127.0.0.1]:9024 test-unsubscribe@example.com lmtp:[127.0.0.1]:9024 """) + + def test_two_lists_two_domains(self): + # Now we have two lists in two different domains. Both lists will + # show up in the postfix_lmtp file, and both domains will show up in + # the postfix_domains file. + getUtility(IDomainManager).add('example.net') + create_list('other@example.net') + self.postfix.regenerate(self.tempdir) + # There are two files in this directory. + self.assertEqual(sorted(os.listdir(self.tempdir)), + ['postfix_domains', 'postfix_lmtp']) + # Because both lists are in the same domain, there should be only one + # entry in the relays file. + with open(os.path.join(self.tempdir, 'postfix_domains')) as fp: + contents = _strip_header(fp.read()) + self.assertMultiLineEqual(contents, """\ +example.com example.com +example.net example.net +""") + # The transport file contains entries for both lists. + with open(os.path.join(self.tempdir, 'postfix_lmtp')) as fp: + contents = _strip_header(fp.read()) + self.assertMultiLineEqual(contents, """\ +# Aliases which are visible only in the @example.com domain. +test@example.com lmtp:[127.0.0.1]:9024 +test-bounces@example.com lmtp:[127.0.0.1]:9024 +test-confirm@example.com lmtp:[127.0.0.1]:9024 +test-join@example.com lmtp:[127.0.0.1]:9024 +test-leave@example.com lmtp:[127.0.0.1]:9024 +test-owner@example.com lmtp:[127.0.0.1]:9024 +test-request@example.com lmtp:[127.0.0.1]:9024 +test-subscribe@example.com lmtp:[127.0.0.1]:9024 +test-unsubscribe@example.com lmtp:[127.0.0.1]:9024 + +# Aliases which are visible only in the @example.net domain. +other@example.net lmtp:[127.0.0.1]:9024 +other-bounces@example.net lmtp:[127.0.0.1]:9024 +other-confirm@example.net lmtp:[127.0.0.1]:9024 +other-join@example.net lmtp:[127.0.0.1]:9024 +other-leave@example.net lmtp:[127.0.0.1]:9024 +other-owner@example.net lmtp:[127.0.0.1]:9024 +other-request@example.net lmtp:[127.0.0.1]:9024 +other-subscribe@example.net lmtp:[127.0.0.1]:9024 +other-unsubscribe@example.net lmtp:[127.0.0.1]:9024 +""") diff --git a/src/mailman/mta/tests/test_delivery.py b/src/mailman/mta/tests/test_delivery.py index fbac0d121..c56058832 100644 --- a/src/mailman/mta/tests/test_delivery.py +++ b/src/mailman/mta/tests/test_delivery.py @@ -95,9 +95,8 @@ options : $user_optionsurl template_dir: {0} """.format(self._template_dir)) self._mlist.footer_uri = 'mailman:///member-footer.txt' - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. + # Let assertMultiLineEqual work without bounds. self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) def tearDown(self): # Free references. @@ -124,7 +123,7 @@ options : $user_optionsurl self.assertEqual(len(refused), 0) self.assertEqual(len(_deliveries), 1) _mlist, _msg, _msgdata, _recipients = _deliveries[0] - self.eq(_msg.as_string(), """\ + self.assertMultiLineEqual(_msg.as_string(), """\ From: anne@example.org To: test@example.com Subject: test diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py index 2e81cb030..1e043c2c7 100644 --- a/src/mailman/rest/addresses.py +++ b/src/mailman/rest/addresses.py @@ -31,10 +31,11 @@ from operator import attrgetter from restish import http, resource from zope.component import getUtility -from mailman.rest.helpers import CollectionMixin, etag, path_to +from mailman.rest.helpers import CollectionMixin, etag, no_content, path_to from mailman.rest.members import MemberCollection from mailman.rest.preferences import Preferences from mailman.interfaces.usermanager import IUserManager +from mailman.utilities.datetime import now @@ -76,6 +77,24 @@ class AllAddresses(_AddressBase): +class _VerifyResource(resource.Resource): + """A helper resource for verify/unverify POSTS.""" + + def __init__(self, address, action): + self._address = address + self._action = action + assert action in ('verify', 'unverify') + + @resource.POST() + def verify(self, request): + # We don't care about the POST data, just do the action. + if self._action == 'verify' and self._address.verified_on is None: + self._address.verified_on = now() + elif self._action == 'unverify': + self._address.verified_on = None + return no_content() + + class AnAddress(_AddressBase): """An address.""" @@ -115,6 +134,26 @@ class AnAddress(_AddressBase): 'addresses/{0}'.format(self._address.email)) return child, [] + @resource.child() + def verify(self, request, segments): + """/addresses/<email>/verify""" + if len(segments) != 0: + return http.bad_request() + if self._address is None: + return http.not_found() + child = _VerifyResource(self._address, 'verify') + return child, [] + + @resource.child() + def unverify(self, request, segments): + """/addresses/<email>/verify""" + if len(segments) != 0: + return http.bad_request() + if self._address is None: + return http.not_found() + child = _VerifyResource(self._address, 'unverify') + return child, [] + class UserAddresses(_AddressBase): diff --git a/src/mailman/rest/configuration.py b/src/mailman/rest/configuration.py index 83d4c74f6..8db23136a 100644 --- a/src/mailman/rest/configuration.py +++ b/src/mailman/rest/configuration.py @@ -29,78 +29,17 @@ from lazr.config import as_boolean, as_timedelta from restish import http, resource from mailman.config import config +from mailman.core.errors import ( + ReadOnlyPATCHRequestError, UnknownPATCHRequestError) from mailman.interfaces.action import Action from mailman.interfaces.archiver import ArchivePolicy from mailman.interfaces.autorespond import ResponseAction from mailman.interfaces.mailinglist import IAcceptableAliasSet, ReplyToMunging -from mailman.rest.helpers import PATCH, etag, no_content -from mailman.rest.validator import Validator, enum_validator +from mailman.rest.helpers import GetterSetter, PATCH, etag, no_content +from mailman.rest.validator import PatchValidator, Validator, enum_validator -class GetterSetter: - """Get and set attributes on mailing lists. - - Most attributes are fairly simple - a getattr() or setattr() on the - mailing list does the trick, with the appropriate encoding or decoding on - the way in and out. Encoding doesn't happen here though; the standard - JSON library handles most types, but see ExtendedEncoder in - mailman.rest.helpers for additional support. - - Others are more complicated since they aren't kept in the model as direct - columns in the database. These will use subclasses of this base class. - Read-only attributes will have a decoder which always raises ValueError. - """ - - def __init__(self, decoder=None): - """Create a getter/setter for a specific list attribute. - - :param decoder: The callable for decoding a web request value string - into the specific data type needed by the `IMailingList` - attribute. Use None to indicate a read-only attribute. The - callable should raise ValueError when the web request value cannot - be converted. - :type decoder: callable - """ - self.decoder = decoder - - def get(self, mlist, attribute): - """Return the named mailing list attribute value. - - :param mlist: The mailing list. - :type mlist: `IMailingList` - :param attribute: The attribute name. - :type attribute: string - :return: The attribute value, ready for JSON encoding. - :rtype: object - """ - return getattr(mlist, attribute) - - def put(self, mlist, attribute, value): - """Set the named mailing list attribute value. - - :param mlist: The mailing list. - :type mlist: `IMailingList` - :param attribute: The attribute name. - :type attribute: string - :param value: The new value for the attribute. - :type request_value: object - """ - setattr(mlist, attribute, value) - - def __call__(self, value): - """Convert the value to its internal format. - - :param value: The web request value to convert. - :type value: string - :return: The converted value. - :rtype: object - """ - if self.decoder is None: - return value - return self.decoder(value) - - class AcceptableAliases(GetterSetter): """Resource for the acceptable aliases of a mailing list.""" @@ -239,19 +178,6 @@ class ListConfiguration(resource.Resource): resource[attribute] = value return http.ok([], etag(resource)) - # XXX 2010-09-01 barry: Refactor {put,patch}_configuration() for common - # code paths. - - def _set_writable_attributes(self, validator, request): - """Common code for setting all attributes given in the request. - - Returns an HTTP 400 when a request tries to write to a read-only - attribute. - """ - converted = validator(request) - for key, value in converted.items(): - ATTRIBUTES[key].put(self._mlist, key, value) - @resource.PUT() def put_configuration(self, request): """Set a mailing list configuration.""" @@ -259,7 +185,7 @@ class ListConfiguration(resource.Resource): if attribute is None: validator = Validator(**VALIDATORS) try: - self._set_writable_attributes(validator, request) + validator.update(self._mlist, request) except ValueError as error: return http.bad_request([], str(error)) elif attribute not in ATTRIBUTES: @@ -271,7 +197,7 @@ class ListConfiguration(resource.Resource): else: validator = Validator(**{attribute: VALIDATORS[attribute]}) try: - self._set_writable_attributes(validator, request) + validator.update(self._mlist, request) except ValueError as error: return http.bad_request([], str(error)) return no_content() @@ -279,20 +205,16 @@ class ListConfiguration(resource.Resource): @PATCH() def patch_configuration(self, request): """Patch the configuration (i.e. partial update).""" - # Validate only the partial subset of attributes given in the request. - validationators = {} - for attribute in request.PATCH: - if attribute not in ATTRIBUTES: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(attribute)) - elif ATTRIBUTES[attribute].decoder is None: - return http.bad_request( - [], b'Read-only attribute: {0}'.format(attribute)) - else: - validationators[attribute] = VALIDATORS[attribute] - validator = Validator(**validationators) try: - self._set_writable_attributes(validator, request) + validator = PatchValidator(request, ATTRIBUTES) + except UnknownPATCHRequestError as error: + return http.bad_request( + [], b'Unknown attribute: {0}'.format(error.attribute)) + except ReadOnlyPATCHRequestError as error: + return http.bad_request( + [], b'Read-only attribute: {0}'.format(error.attribute)) + try: + validator.update(self._mlist, request) except ValueError as error: return http.bad_request([], str(error)) return no_content() diff --git a/src/mailman/rest/docs/addresses.rst b/src/mailman/rest/docs/addresses.rst index cb9242d2b..f05b6b9b2 100644 --- a/src/mailman/rest/docs/addresses.rst +++ b/src/mailman/rest/docs/addresses.rst @@ -90,7 +90,6 @@ Verifying When the address gets verified, this attribute is available in the REST representation. -:: >>> from mailman.utilities.datetime import now >>> anne.verified_on = now() @@ -103,6 +102,47 @@ representation. self_link: http://localhost:9001/3.0/addresses/anne@example.com verified_on: 2005-08-01T07:49:23 +Addresses can also be verified through the REST API, by POSTing to the +'verify' sub-resource. The POST data is ignored. + + >>> dump_json('http://localhost:9001/3.0/addresses/' + ... 'cris@example.com/verify', {}) + content-length: 0 + date: ... + server: ... + status: 204 + +Now Cris's address is verified. + + >>> dump_json('http://localhost:9001/3.0/addresses/cris@example.com') + display_name: Cris Person + email: cris@example.com + http_etag: "..." + original_email: cris@example.com + registered_on: 2005-08-01T07:49:23 + self_link: http://localhost:9001/3.0/addresses/cris@example.com + verified_on: 2005-08-01T07:49:23 + +If you should ever need to 'unverify' an address, POST to the 'unverify' +sub-resource. Again, the POST data is ignored. + + >>> dump_json('http://localhost:9001/3.0/addresses/' + ... 'cris@example.com/unverify', {}) + content-length: 0 + date: ... + server: ... + status: 204 + +Now Cris's address is unverified. + + >>> dump_json('http://localhost:9001/3.0/addresses/cris@example.com') + display_name: Cris Person + email: cris@example.com + http_etag: "..." + original_email: cris@example.com + registered_on: 2005-08-01T07:49:23 + self_link: http://localhost:9001/3.0/addresses/cris@example.com + User addresses ============== diff --git a/src/mailman/rest/docs/domains.rst b/src/mailman/rest/docs/domains.rst index c890af7fa..92c73ffbf 100644 --- a/src/mailman/rest/docs/domains.rst +++ b/src/mailman/rest/docs/domains.rst @@ -131,7 +131,7 @@ example.com domain does not contain any mailing lists. ... }) content-length: 0 date: ... - location: http://localhost:9001/3.0/lists/test-domains@example.com + location: http://localhost:9001/3.0/lists/test-domains.example.com ... >>> dump_json('http://localhost:9001/3.0/domains/example.com/lists') @@ -141,7 +141,7 @@ example.com domain does not contain any mailing lists. http_etag: "..." ... member_count: 0 - self_link: http://localhost:9001/3.0/lists/test-domains@example.com + self_link: http://localhost:9001/3.0/lists/test-domains.example.com volume: 1 http_etag: "..." start: 0 diff --git a/src/mailman/rest/docs/lists.rst b/src/mailman/rest/docs/lists.rst index 610244968..0c4bbc419 100644 --- a/src/mailman/rest/docs/lists.rst +++ b/src/mailman/rest/docs/lists.rst @@ -23,10 +23,11 @@ Create a mailing list in a domain and it's accessible via the API. display_name: Test-one fqdn_listname: test-one@example.com http_etag: "..." + list_id: test-one.example.com list_name: test-one mail_host: example.com member_count: 0 - self_link: http://localhost:9001/3.0/lists/test-one@example.com + self_link: http://localhost:9001/3.0/lists/test-one.example.com volume: 1 http_etag: "..." start: 0 @@ -40,10 +41,11 @@ You can also query for lists from a particular domain. display_name: Test-one fqdn_listname: test-one@example.com http_etag: "..." + list_id: test-one.example.com list_name: test-one mail_host: example.com member_count: 0 - self_link: http://localhost:9001/3.0/lists/test-one@example.com + self_link: http://localhost:9001/3.0/lists/test-one.example.com volume: 1 http_etag: "..." start: 0 @@ -66,7 +68,7 @@ New mailing lists can also be created through the API, by posting to the ... }) content-length: 0 date: ... - location: http://localhost:9001/3.0/lists/test-two@example.com + location: http://localhost:9001/3.0/lists/test-two.example.com ... The mailing list exists in the database. @@ -85,14 +87,31 @@ The mailing list exists in the database. It is also available via the location given in the response. + >>> dump_json('http://localhost:9001/3.0/lists/test-two.example.com') + display_name: Test-two + fqdn_listname: test-two@example.com + http_etag: "..." + list_id: test-two.example.com + list_name: test-two + mail_host: example.com + member_count: 0 + self_link: http://localhost:9001/3.0/lists/test-two.example.com + volume: 1 + +Normally, you access the list via its RFC 2369 list-id as shown above, but for +backward compatibility purposes, you can also access it via the list's posting +address, if that has never been changed (since the list-id is immutable, but +the posting address is not). + >>> dump_json('http://localhost:9001/3.0/lists/test-two@example.com') display_name: Test-two fqdn_listname: test-two@example.com http_etag: "..." + list_id: test-two.example.com list_name: test-two mail_host: example.com member_count: 0 - self_link: http://localhost:9001/3.0/lists/test-two@example.com + self_link: http://localhost:9001/3.0/lists/test-two.example.com volume: 1 However, you are not allowed to create a mailing list in a domain that does @@ -122,34 +141,48 @@ Existing mailing lists can be deleted through the API, by doing an HTTP ``DELETE`` on the mailing list URL. :: - >>> dump_json('http://localhost:9001/3.0/lists/test-two@example.com', + >>> dump_json('http://localhost:9001/3.0/lists/test-two.example.com', ... method='DELETE') content-length: 0 date: ... server: ... status: 204 - # The above starts a Storm transaction, which will lock the database - # unless we abort it. - >>> transaction.abort() - The mailing list does not exist. >>> print list_manager.get('test-two@example.com') None + # Unlock the database. + >>> transaction.abort() + You cannot delete a mailing list that does not exist or has already been deleted. :: - >>> dump_json('http://localhost:9001/3.0/lists/test-two@example.com', + >>> dump_json('http://localhost:9001/3.0/lists/test-two.example.com', ... method='DELETE') Traceback (most recent call last): ... HTTPError: HTTP Error 404: 404 Not Found - >>> dump_json('http://localhost:9001/3.0/lists/test-ten@example.com', + >>> dump_json('http://localhost:9001/3.0/lists/test-ten.example.com', ... method='DELETE') Traceback (most recent call last): ... HTTPError: HTTP Error 404: 404 Not Found + +For backward compatibility purposes, you can delete a list via its posting +address as well. + + >>> dump_json('http://localhost:9001/3.0/lists/test-one@example.com', + ... method='DELETE') + content-length: 0 + date: ... + server: ... + status: 204 + +The mailing list does not exist. + + >>> print list_manager.get('test-one@example.com') + None diff --git a/src/mailman/rest/docs/users.rst b/src/mailman/rest/docs/users.rst index cdede10ee..888bc43fd 100644 --- a/src/mailman/rest/docs/users.rst +++ b/src/mailman/rest/docs/users.rst @@ -40,7 +40,7 @@ The user ids match. >>> json['entries'][0]['user_id'] == anne.user_id.int True -A user might not have a real name, in which case, the attribute will not be +A user might not have a display name, in which case, the attribute will not be returned in the REST API. >>> dave = user_manager.create_user('dave@example.com') @@ -66,7 +66,8 @@ Creating users via the API ========================== New users can be created through the REST API. To do so requires the initial -email address for the user, a password, and optionally the user's full name. +email address for the user, a password, and optionally the user's display +name. :: >>> transaction.abort() @@ -134,6 +135,75 @@ therefore cannot be retrieved. It can be reset though. user_id: 4 +Updating users +============== + +Users have a password and a display name. The display name can be changed +through the REST API. + + >>> dump_json('http://localhost:9001/3.0/users/4', { + ... 'display_name': 'Chrissy Person', + ... }, method='PATCH') + content-length: 0 + date: ... + server: ... + status: 204 + +Cris's display name has been updated. + + >>> dump_json('http://localhost:9001/3.0/users/4') + created_on: 2005-08-01T07:49:23 + display_name: Chrissy Person + http_etag: "..." + password: {plaintext}... + self_link: http://localhost:9001/3.0/users/4 + user_id: 4 + +You can also change the user's password by passing in the new clear text +password. Mailman will hash this before it is stored internally. + + >>> dump_json('http://localhost:9001/3.0/users/4', { + ... 'cleartext_password': 'clockwork angels', + ... }, method='PATCH') + content-length: 0 + date: ... + server: ... + status: 204 + +Even though you see *{plaintext}clockwork angels* below, it has still been +hashed before storage. The default hashing algorithm for the test suite is a +plain text hash, but you can see that it works by the addition of the +algorithm prefix. + + >>> dump_json('http://localhost:9001/3.0/users/4') + created_on: 2005-08-01T07:49:23 + display_name: Chrissy Person + http_etag: "..." + password: {plaintext}clockwork angels + self_link: http://localhost:9001/3.0/users/4 + user_id: 4 + +You can change both the display name and the password by PUTing the full +resource. + + >>> dump_json('http://localhost:9001/3.0/users/4', { + ... 'display_name': 'Christopherson Person', + ... 'cleartext_password': 'the garden', + ... }, method='PUT') + content-length: 0 + date: ... + server: ... + status: 204 + + >>> dump_json('http://localhost:9001/3.0/users/4') + created_on: 2005-08-01T07:49:23 + display_name: Christopherson Person + http_etag: "..." + password: {plaintext}the garden + self_link: http://localhost:9001/3.0/users/4 + user_id: 4 + + Deleting users via the API ========================== @@ -145,11 +215,21 @@ Users can also be deleted via the API. date: ... server: ... status: 204 + +Cris's resource cannot be retrieved either by email address... + >>> dump_json('http://localhost:9001/3.0/users/cris@example.com') Traceback (most recent call last): ... HTTPError: HTTP Error 404: 404 Not Found +...or user id. + + >>> dump_json('http://localhost:9001/3.0/users/4') + Traceback (most recent call last): + ... + HTTPError: HTTP Error 404: 404 Not Found + Missing users ============= @@ -171,6 +251,23 @@ user id... ... HTTPError: HTTP Error 404: 404 Not Found +You also can't update a missing user. + + >>> dump_json('http://localhost:9001/3.0/users/zed@example.org', { + ... 'display_name': 'Is Dead', + ... }, method='PATCH') + Traceback (most recent call last): + ... + HTTPError: HTTP Error 404: 404 Not Found + + >>> dump_json('http://localhost:9001/3.0/users/zed@example.org', { + ... 'display_name': 'Is Dead', + ... 'cleartext_password': 'vroom', + ... }, method='PUT') + Traceback (most recent call last): + ... + HTTPError: HTTP Error 404: 404 Not Found + User addresses ============== diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py index 2824a894e..72040c848 100644 --- a/src/mailman/rest/helpers.py +++ b/src/mailman/rest/helpers.py @@ -21,6 +21,7 @@ from __future__ import absolute_import, unicode_literals __metaclass__ = type __all__ = [ + 'GetterSetter', 'PATCH', 'etag', 'no_content', @@ -205,3 +206,63 @@ class PATCH(MethodDecorator): def __call__(self, func): really_wrapped_func = PATCHWrapper(func) return super(PATCH, self).__call__(really_wrapped_func) + + +class GetterSetter: + """Get and set attributes on an object. + + Most attributes are fairly simple - a getattr() or setattr() on the object + does the trick, with the appropriate encoding or decoding on the way in + and out. Encoding doesn't happen here though; the standard JSON library + handles most types, but see ExtendedEncoder for additional support. + + Others are more complicated since they aren't kept in the model as direct + columns in the database. These will use subclasses of this base class. + Read-only attributes will have a decoder which always raises ValueError. + """ + + def __init__(self, decoder=None): + """Create a getter/setter for a specific attribute. + + :param decoder: The callable for decoding a web request value string + into the specific data type needed by the object's attribute. Use + None to indicate a read-only attribute. The callable should raise + ValueError when the web request value cannot be converted. + :type decoder: callable + """ + self.decoder = decoder + + def get(self, obj, attribute): + """Return the named object attribute value. + + :param obj: The object to access. + :type obj: object + :param attribute: The attribute name. + :type attribute: string + :return: The attribute value, ready for JSON encoding. + :rtype: object + """ + return getattr(obj, attribute) + + def put(self, obj, attribute, value): + """Set the named object attribute value. + + :param obj: The object to change. + :type obj: object + :param attribute: The attribute name. + :type attribute: string + :param value: The new value for the attribute. + """ + setattr(obj, attribute, value) + + def __call__(self, value): + """Convert the value to its internal format. + + :param value: The web request value to convert. + :type value: string + :return: The converted value. + :rtype: object + """ + if self.decoder is None: + return value + return self.decoder(value) diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index f25133211..4e9de6905 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -107,11 +107,12 @@ class _ListBase(resource.Resource, CollectionMixin): return dict( display_name=mlist.display_name, fqdn_listname=mlist.fqdn_listname, + list_id=mlist.list_id, list_name=mlist.list_name, mail_host=mlist.mail_host, member_count=mlist.members.member_count, volume=mlist.volume, - self_link=path_to('lists/{0}'.format(mlist.fqdn_listname)), + self_link=path_to('lists/{0}'.format(mlist.list_id)), ) def _get_collection(self, request): @@ -122,8 +123,15 @@ class _ListBase(resource.Resource, CollectionMixin): class AList(_ListBase): """A mailing list.""" - def __init__(self, list_name): - self._mlist = getUtility(IListManager).get(list_name) + def __init__(self, list_identifier): + # list-id is preferred, but for backward compatibility, fqdn_listname + # is also accepted. If the string contains '@', treat it as the + # latter. + manager = getUtility(IListManager) + if '@' in list_identifier: + self._mlist = manager.get(list_identifier) + else: + self._mlist = manager.get_by_list_id(list_identifier) @resource.GET() def mailing_list(self, request): @@ -192,7 +200,7 @@ class AllLists(_ListBase): except ValueError as error: return http.bad_request([], str(error)) # wsgiref wants headers to be bytes, not unicodes. - location = path_to('lists/{0}'.format(mlist.fqdn_listname)) + location = path_to('lists/{0}'.format(mlist.list_id)) # Include no extra headers or body. return http.created(location, [], None) diff --git a/src/mailman/rest/preferences.py b/src/mailman/rest/preferences.py index be598458e..1961c8b84 100644 --- a/src/mailman/rest/preferences.py +++ b/src/mailman/rest/preferences.py @@ -30,7 +30,8 @@ from lazr.config import as_boolean from restish import http, resource from mailman.interfaces.member import DeliveryMode, DeliveryStatus -from mailman.rest.helpers import PATCH, etag, no_content, path_to +from mailman.rest.helpers import ( + GetterSetter, PATCH, etag, no_content, path_to) from mailman.rest.validator import ( Validator, enum_validator, language_validator) @@ -72,7 +73,7 @@ class ReadOnlyPreferences(resource.Resource): resource['self_link'] = path_to( '{0}/preferences'.format(self._base_url)) return http.ok([], etag(resource)) - + class Preferences(ReadOnlyPreferences): @@ -82,22 +83,20 @@ class Preferences(ReadOnlyPreferences): if self._parent is None: return http.not_found() kws = dict( - acknowledge_posts=as_boolean, - delivery_mode=enum_validator(DeliveryMode), - delivery_status=enum_validator(DeliveryStatus), - preferred_language=language_validator, - receive_list_copy=as_boolean, - receive_own_postings=as_boolean, + acknowledge_posts=GetterSetter(as_boolean), + delivery_mode=GetterSetter(enum_validator(DeliveryMode)), + delivery_status=GetterSetter(enum_validator(DeliveryStatus)), + preferred_language=GetterSetter(language_validator), + receive_list_copy=GetterSetter(as_boolean), + receive_own_postings=GetterSetter(as_boolean), ) if is_optional: # For a PUT, all attributes are optional. kws['_optional'] = kws.keys() try: - values = Validator(**kws)(request) + Validator(**kws).update(self._parent, request) except ValueError as error: return http.bad_request([], str(error)) - for key, value in values.items(): - setattr(self._parent, key, value) return no_content() @PATCH() diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py index 8c1a31cf2..ea1650e75 100644 --- a/src/mailman/rest/root.py +++ b/src/mailman/rest/root.py @@ -123,8 +123,10 @@ class TopLevel(resource.Resource): if len(segments) == 0: return AllLists() else: - list_name = segments.pop(0) - return AList(list_name), segments + # list-id is preferred, but for backward compatibility, + # fqdn_listname is also accepted. + list_identifier = segments.pop(0) + return AList(list_identifier), segments @resource.child() def members(self, request, segments): diff --git a/src/mailman/rest/tests/test_addresses.py b/src/mailman/rest/tests/test_addresses.py index 385b83912..35a19f77f 100644 --- a/src/mailman/rest/tests/test_addresses.py +++ b/src/mailman/rest/tests/test_addresses.py @@ -28,11 +28,14 @@ __all__ = [ import unittest from urllib2 import HTTPError +from zope.component import getUtility from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction +from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import call_api from mailman.testing.layers import RESTLayer +from mailman.utilities.datetime import now @@ -45,11 +48,64 @@ class TestAddresses(unittest.TestCase): def test_membership_of_missing_address(self): # Try to get the memberships of a missing address. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/addresses/' 'nobody@example.com/memberships') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) + + def test_verify_a_missing_address(self): + # POSTing to the 'verify' sub-resource returns a 404. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/addresses/' + 'nobody@example.com/verify', {}) + self.assertEqual(cm.exception.code, 404) + + def test_unverify_a_missing_address(self): + # POSTing to the 'unverify' sub-resource returns a 404. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/addresses/' + 'nobody@example.com/unverify', {}) + self.assertEqual(cm.exception.code, 404) + + def test_verify_already_verified(self): + # It's okay to verify an already verified; it just doesn't change the + # value. + verified_on = now() + with transaction(): + anne = getUtility(IUserManager).create_address('anne@example.com') + anne.verified_on = verified_on + response, content = call_api( + 'http://localhost:9001/3.0/addresses/anne@example.com/verify', {}) + self.assertEqual(content['status'], '204') + self.assertEqual(anne.verified_on, verified_on) + + def test_unverify_already_unverified(self): + # It's okay to unverify an already unverified; it just doesn't change + # the value. + with transaction(): + anne = getUtility(IUserManager).create_address('anne@example.com') + self.assertEqual(anne.verified_on, None) + response, content = call_api( + 'http://localhost:9001/3.0/addresses/anne@example.com/unverify', {}) + self.assertEqual(content['status'], '204') + self.assertEqual(anne.verified_on, None) + + def test_verify_bad_request(self): + # Too many segments after /verify. + with transaction(): + anne = getUtility(IUserManager).create_address('anne@example.com') + self.assertEqual(anne.verified_on, None) + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/addresses/' + 'anne@example.com/verify/foo', {}) + self.assertEqual(cm.exception.code, 400) + + def test_unverify_bad_request(self): + # Too many segments after /verify. + with transaction(): + anne = getUtility(IUserManager).create_address('anne@example.com') + self.assertEqual(anne.verified_on, None) + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/addresses/' + 'anne@example.com/unverify/foo', {}) + self.assertEqual(cm.exception.code, 400) diff --git a/src/mailman/rest/tests/test_domains.py b/src/mailman/rest/tests/test_domains.py index a86768481..dea6a0aa6 100644 --- a/src/mailman/rest/tests/test_domains.py +++ b/src/mailman/rest/tests/test_domains.py @@ -47,24 +47,16 @@ class TestDomains(unittest.TestCase): def test_bogus_endpoint_extension(self): # /domains/<domain>/lists/<anything> is not a valid endpoint. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/domains/example.com' '/lists/wrong') - except HTTPError as exc: - self.assertEqual(exc.code, 400) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) def test_bogus_endpoint(self): # /domains/<domain>/<!lists> does not exist. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/domains/example.com/wrong') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) def test_lists_are_deleted_when_domain_is_deleted(self): # /domains/<domain> DELETE removes all associated mailing lists. diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py index cd0ebaf8e..9686ce6a8 100644 --- a/src/mailman/rest/tests/test_lists.py +++ b/src/mailman/rest/tests/test_lists.py @@ -46,47 +46,31 @@ class TestListsMissing(unittest.TestCase): def test_missing_list_roster_member_404(self): # /lists/<missing>/roster/member gives 404 - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/lists/missing@example.com' '/roster/member') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) def test_missing_list_roster_owner_404(self): # /lists/<missing>/roster/owner gives 404 - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/lists/missing@example.com' '/roster/owner') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) def test_missing_list_roster_moderator_404(self): # /lists/<missing>/roster/member gives 404 - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/lists/missing@example.com' '/roster/moderator') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) def test_missing_list_configuration_404(self): # /lists/<missing>/config gives 404 - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api( 'http://localhost:9001/3.0/lists/missing@example.com/config') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index 18469e537..3bbe821ac 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -50,41 +50,29 @@ class TestMembership(unittest.TestCase): def test_try_to_join_missing_list(self): # A user tries to join a non-existent list. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members', { 'list_id': 'missing.example.com', 'subscriber': 'nobody@example.com', }) - except HTTPError as exc: - self.assertEqual(exc.code, 400) - self.assertEqual(exc.msg, 'No such list') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.msg, 'No such list') def test_try_to_leave_missing_list(self): # A user tries to leave a non-existent list. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/lists/missing@example.com' '/member/nobody@example.com', method='DELETE') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - self.assertEqual(exc.msg, '404 Not Found') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) + self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_leave_list_with_bogus_address(self): # Try to leave a mailing list using an invalid membership address. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members/1', method='DELETE') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - self.assertEqual(exc.msg, '404 Not Found') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) + self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_leave_a_list_twice(self): with transaction(): @@ -96,45 +84,34 @@ class TestMembership(unittest.TestCase): # content. self.assertEqual(content, None) self.assertEqual(response.status, 204) - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api(url, method='DELETE') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - self.assertEqual(exc.msg, '404 Not Found') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) + self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_join_a_list_twice(self): with transaction(): anne = self._usermanager.create_address('anne@example.com') self._mlist.subscribe(anne) - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members', { 'list_id': 'test.example.com', 'subscriber': 'anne@example.com', }) - except HTTPError as exc: - self.assertEqual(exc.code, 409) - self.assertEqual(exc.msg, 'Member already subscribed') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 409) + self.assertEqual(cm.exception.msg, 'Member already subscribed') def test_join_with_invalid_delivery_mode(self): - try: + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members', { 'list_id': 'test.example.com', 'subscriber': 'anne@example.com', 'display_name': 'Anne Person', 'delivery_mode': 'invalid-mode', }) - except HTTPError as exc: - self.assertEqual(exc.code, 400) - self.assertEqual(exc.msg, - 'Cannot convert parameters: delivery_mode') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.msg, + 'Cannot convert parameters: delivery_mode') def test_join_email_contains_slash(self): content, response = call_api('http://localhost:9001/3.0/members', { @@ -196,47 +173,31 @@ class TestMembership(unittest.TestCase): def test_get_nonexistent_member(self): # /members/<bogus> returns 404 - try: - # For Python 2.6 + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members/bogus') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) def test_patch_nonexistent_member(self): # /members/<missing> PATCH returns 404 - try: - # For Python 2.6 + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members/801', method='PATCH') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) def test_patch_member_bogus_attribute(self): # /members/<id> PATCH 'bogus' returns 400 with transaction(): anne = self._usermanager.create_address('anne@example.com') self._mlist.subscribe(anne) - try: - # For Python 2.6 + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members/1', { 'powers': 'super', }, method='PATCH') - except HTTPError as exc: - self.assertEqual(exc.code, 400) - self.assertEqual(exc.msg, 'Unexpected parameters: powers') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.msg, 'Unexpected parameters: powers') def test_member_all_without_preferences(self): # /members/<id>/all should return a 404 when it isn't trailed by # `preferences` - try: - # For Python 2.6 + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members/1/all') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py index dfcedef05..2ee796c87 100644 --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -56,24 +56,16 @@ Something else. def test_not_found(self): # When a bogus mailing list is given, 404 should result. - try: - # For Python 2.6 + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/lists/bee@example.com/held') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) def test_bad_request_id(self): # Bad request when request_id is not an integer. - try: - # For Python 2.6 + with self.assertRaises(HTTPError) as cm: call_api( 'http://localhost:9001/3.0/lists/ant@example.com/held/bogus') - except HTTPError as exc: - self.assertEqual(exc.code, 400) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) def test_subscription_request_as_held_message(self): # Provide the request id of a subscription request using the held @@ -85,12 +77,9 @@ Something else. DeliveryMode.regular, 'en') config.db.store.commit() url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{0}' - try: + with self.assertRaises(HTTPError) as cm: call_api(url.format(subscribe_id)) - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) # But using the held_id returns a valid response. response, content = call_api(url.format(held_id)) self.assertEqual(response['key'], '<alpha>') @@ -99,10 +88,7 @@ Something else. # POSTing to a held message with a bad action. held_id = hold_message(self._mlist, self._msg) url = 'http://localhost:9001/3.0/lists/ant@example.com/held/{0}' - try: + with self.assertRaises(HTTPError) as cm: call_api(url.format(held_id), {'action': 'bogus'}) - except HTTPError as exc: - self.assertEqual(exc.code, 400) - self.assertEqual(exc.msg, 'Cannot convert parameters: action') - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.msg, 'Cannot convert parameters: action') diff --git a/src/mailman/rest/tests/test_root.py b/src/mailman/rest/tests/test_root.py index 90d30bd80..4a9ba0dd1 100644 --- a/src/mailman/rest/tests/test_root.py +++ b/src/mailman/rest/tests/test_root.py @@ -38,38 +38,25 @@ class TestSystem(unittest.TestCase): def test_system_url_too_long(self): # /system/foo/bar is not allowed. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/system/foo/bar') - except HTTPError as exc: - self.assertEqual(exc.code, 400) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) def test_system_url_not_preferences(self): # /system/foo where `foo` is not `preferences`. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/system/foo') - except HTTPError as exc: - self.assertEqual(exc.code, 400) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 400) def test_system_preferences_are_read_only(self): # /system/preferences are read-only. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/system/preferences', { 'acknowledge_posts': True, }, method='PATCH') - except HTTPError as exc: - self.assertEqual(exc.code, 405) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 405) # /system/preferences are read-only. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/system/preferences', { 'acknowledge_posts': False, 'delivery_mode': 'regular', @@ -79,7 +66,4 @@ class TestSystem(unittest.TestCase): 'receive_list_copy': True, 'receive_own_postings': True, }, method='PUT') - except HTTPError as exc: - self.assertEqual(exc.code, 405) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 405) diff --git a/src/mailman/rest/tests/test_users.py b/src/mailman/rest/tests/test_users.py index 301027885..4595c69d8 100644 --- a/src/mailman/rest/tests/test_users.py +++ b/src/mailman/rest/tests/test_users.py @@ -45,10 +45,6 @@ class TestUsers(unittest.TestCase): def test_delete_bogus_user(self): # Try to delete a user that does not exist. - try: - # For Python 2.6. + with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/users/99', method='DELETE') - except HTTPError as exc: - self.assertEqual(exc.code, 404) - else: - raise AssertionError('Expected HTTPError') + self.assertEqual(cm.exception.code, 404) diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index bce541ae5..94817da49 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -32,12 +32,34 @@ from uuid import UUID from zope.component import getUtility from mailman.config import config +from mailman.core.errors import ( + ReadOnlyPATCHRequestError, UnknownPATCHRequestError) from mailman.interfaces.address import ExistingAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.addresses import UserAddresses -from mailman.rest.helpers import CollectionMixin, etag, no_content, path_to +from mailman.rest.helpers import ( + CollectionMixin, GetterSetter, PATCH, etag, no_content, path_to) from mailman.rest.preferences import Preferences -from mailman.rest.validator import Validator +from mailman.rest.validator import PatchValidator, Validator + + +# Attributes of a user which can be changed via the REST API. +class PasswordEncrypterGetterSetter(GetterSetter): + def __init__(self): + super(PasswordEncrypterGetterSetter, self).__init__( + config.password_context.encrypt) + def get(self, obj, attribute): + assert attribute == 'cleartext_password' + super(PasswordEncrypterGetterSetter, self).get(obj, 'password') + def put(self, obj, attribute, value): + assert attribute == 'cleartext_password' + super(PasswordEncrypterGetterSetter, self).put(obj, 'password', value) + + +ATTRIBUTES = dict( + display_name=GetterSetter(unicode), + cleartext_password=PasswordEncrypterGetterSetter(), + ) @@ -165,3 +187,37 @@ class AUser(_UserBase): self._user.preferences, 'users/{0}'.format(self._user.user_id.int)) return child, [] + + @PATCH() + def patch_update(self, request): + """Patch the user's configuration (i.e. partial update).""" + if self._user is None: + return http.not_found() + try: + validator = PatchValidator(request, ATTRIBUTES) + except UnknownPATCHRequestError as error: + return http.bad_request( + [], b'Unknown attribute: {0}'.format(error.attribute)) + except ReadOnlyPATCHRequestError as error: + return http.bad_request( + [], b'Read-only attribute: {0}'.format(error.attribute)) + validator.update(self._user, request) + return no_content() + + @resource.PUT() + def put_update(self, request): + """Put the user's configuration (i.e. full update).""" + if self._user is None: + return http.not_found() + validator = Validator(**ATTRIBUTES) + try: + validator.update(self._user, request) + except UnknownPATCHRequestError as error: + return http.bad_request( + [], b'Unknown attribute: {0}'.format(error.attribute)) + except ReadOnlyPATCHRequestError as error: + return http.bad_request( + [], b'Read-only attribute: {0}'.format(error.attribute)) + except ValueError as error: + return http.bad_request([], str(error)) + return no_content() diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py index 7484aa260..f6f0cd7e6 100644 --- a/src/mailman/rest/validator.py +++ b/src/mailman/rest/validator.py @@ -21,6 +21,7 @@ from __future__ import absolute_import, unicode_literals __metaclass__ = type __all__ = [ + 'PatchValidator', 'Validator', 'enum_validator', 'language_validator', @@ -31,6 +32,8 @@ __all__ = [ from uuid import UUID from zope.component import getUtility +from mailman.core.errors import ( + ReadOnlyPATCHRequestError, UnknownPATCHRequestError) from mailman.interfaces.languages import ILanguageManager @@ -119,3 +122,50 @@ class Validator: missing = COMMASPACE.join(sorted(required_keys - value_keys)) raise ValueError('Missing parameters: {0}'.format(missing)) return values + + def update(self, obj, request): + """Update the object with the values in the request. + + This first validates and converts the attributes in the request, then + updates the given object with the newly converted values. + + :param obj: The object to update. + :type obj: object + :param request: The HTTP request. + :raises ValueError: if conversion failed for some attribute. + """ + for key, value in self.__call__(request).items(): + self._converters[key].put(obj, key, value) + + + +class PatchValidator(Validator): + """Create a special validator for PATCH requests. + + PATCH is different than PUT because with the latter, you're changing the + entire resource, so all expected attributes must exist. With the former, + you're only changing a subset of the attributes, so you only validate the + ones that exist in the request. + """ + def __init__(self, request, converters): + """Create a validator for the PATCH request. + + :param request: The request object, which must have a .PATCH + attribute. + :param converters: A mapping of attribute names to the converter for + that attribute's type. Generally, this will be a GetterSetter + instance, but it might be something more specific for custom data + types (e.g. non-basic types like unicodes). + :raises UnknownPATCHRequestError: if the request contains an unknown + attribute, i.e. one that is not in the `attributes` mapping. + :raises ReadOnlyPATCHRequest: if the requests contains an attribute + that is defined as read-only. + """ + validationators = {} + for attribute in request.PATCH: + if attribute not in converters: + raise UnknownPATCHRequestError(attribute) + if converters[attribute].decoder is None: + raise ReadOnlyPATCHRequestError(attribute) + validationators[attribute] = converters[attribute] + super(PatchValidator, self).__init__(**validationators) diff --git a/src/mailman/rules/administrivia.py b/src/mailman/rules/administrivia.py index 4c49e4ff2..6f5f41cd3 100644 --- a/src/mailman/rules/administrivia.py +++ b/src/mailman/rules/administrivia.py @@ -84,7 +84,7 @@ class Administrivia: if len(line) == 0: continue lineno += 1 - if lineno > config.mailman.email_commands_max_lines: + if lineno > int(config.mailman.email_commands_max_lines): break lines_to_check.append(line) # Only look at the first text/plain part. diff --git a/src/mailman/rules/tests/test_approved.py b/src/mailman/rules/tests/test_approved.py index a1b8f99ac..19fa2bb06 100644 --- a/src/mailman/rules/tests/test_approved.py +++ b/src/mailman/rules/tests/test_approved.py @@ -462,7 +462,7 @@ schemes = roundup_plaintext, plaintext default = plaintext deprecated = roundup_plaintext """, file=fp) - with configuration('passwords', path=config_file): + with configuration('passwords', configuration=config_file): self._msg['Approved'] = 'super secret' result = self._rule.check(self._mlist, self._msg, {}) self.assertTrue(result) @@ -485,7 +485,7 @@ schemes = roundup_plaintext, plaintext default = plaintext deprecated = roundup_plaintext """, file=fp) - with configuration('passwords', path=config_file): + with configuration('passwords', configuration=config_file): self._msg['Approved'] = 'not the password' result = self._rule.check(self._mlist, self._msg, {}) self.assertFalse(result) diff --git a/src/mailman/runners/digest.py b/src/mailman/runners/digest.py index 99710dff5..a2054f412 100644 --- a/src/mailman/runners/digest.py +++ b/src/mailman/runners/digest.py @@ -30,7 +30,6 @@ import logging # cStringIO doesn't support unicode. from StringIO import StringIO -from contextlib import nested from copy import deepcopy from email.header import Header from email.message import Message @@ -318,9 +317,9 @@ class DigestRunner(Runner): """See `IRunner`.""" volume = msgdata['volume'] digest_number = msgdata['digest_number'] - with nested(Mailbox(msgdata['digest_path']), - _.using(mlist.preferred_language.code)) as (mailbox, - language_code): + # Backslashes make me cry. + code = mlist.preferred_language.code + with Mailbox(msgdata['digest_path']) as mailbox, _.using(code): # Create the digesters. mime_digest = MIMEDigester(mlist, volume, digest_number) rfc1153_digest = RFC1153Digester(mlist, volume, digest_number) @@ -354,7 +353,7 @@ class DigestRunner(Runner): # receive. digest_members = set(mlist.digest_members.members) for member in digest_members: - if member.delivery_status <> DeliveryStatus.enabled: + if member.delivery_status is not DeliveryStatus.enabled: continue # Send the digest to the case-preserved address of the digest # members. diff --git a/src/mailman/runners/tests/test_confirm.py b/src/mailman/runners/tests/test_confirm.py index 62171979c..78f6a382c 100644 --- a/src/mailman/runners/tests/test_confirm.py +++ b/src/mailman/runners/tests/test_confirm.py @@ -28,6 +28,7 @@ __all__ = [ import unittest from datetime import datetime +from email.iterators import body_line_iterator from zope.component import getUtility from mailman.app.lifecycle import create_list @@ -37,7 +38,6 @@ from mailman.interfaces.registrar import IRegistrar from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner from mailman.testing.helpers import ( - body_line_iterator, get_queue_messages, make_testable_runner, specialized_message_from_string as mfs) diff --git a/src/mailman/runners/tests/test_join.py b/src/mailman/runners/tests/test_join.py index a584fd2c2..41adcc450 100644 --- a/src/mailman/runners/tests/test_join.py +++ b/src/mailman/runners/tests/test_join.py @@ -28,6 +28,7 @@ __all__ = [ import unittest +from email.iterators import body_line_iterator from zope.component import getUtility from mailman.app.lifecycle import create_list @@ -38,10 +39,7 @@ from mailman.interfaces.subscriptions import ISubscriptionService from mailman.interfaces.usermanager import IUserManager from mailman.runners.command import CommandRunner from mailman.testing.helpers import ( - body_line_iterator, - get_queue_messages, - make_testable_runner, - reset_the_world, + get_queue_messages, make_testable_runner, reset_the_world, specialized_message_from_string as mfs) from mailman.testing.layers import ConfigLayer diff --git a/src/mailman/runners/tests/test_lmtp.py b/src/mailman/runners/tests/test_lmtp.py index 46d4ed986..a502c317d 100644 --- a/src/mailman/runners/tests/test_lmtp.py +++ b/src/mailman/runners/tests/test_lmtp.py @@ -53,22 +53,19 @@ class TestLMTP(unittest.TestCase): def test_message_id_required(self): # The message is rejected if it does not have a Message-ID header. - try: + with self.assertRaises(smtplib.SMTPDataError) as cm: self._lmtp.sendmail('anne@example.com', ['test@example.com'], """\ From: anne@example.com To: test@example.com Subject: This has no Message-ID header """) - except smtplib.SMTPDataError as error: - pass - else: - raise AssertionError('SMTPDataError expected') # LMTP returns a 550: Requested action not taken: mailbox unavailable # (e.g., mailbox not found, no access, or command rejected for policy # reasons) - self.assertEqual(error.smtp_code, 550) - self.assertEqual(error.smtp_error, 'No Message-ID header provided') + self.assertEqual(cm.exception.smtp_code, 550) + self.assertEqual(cm.exception.smtp_error, + 'No Message-ID header provided') def test_message_id_hash_is_added(self): self._lmtp.sendmail('anne@example.com', ['test@example.com'], """\ diff --git a/src/mailman/runners/tests/test_owner.py b/src/mailman/runners/tests/test_owner.py index 4ea5771be..3118ab9f4 100644 --- a/src/mailman/runners/tests/test_owner.py +++ b/src/mailman/runners/tests/test_owner.py @@ -74,9 +74,6 @@ class TestEmailToOwner(unittest.TestCase): self._inq = make_testable_runner(IncomingRunner, 'in') self._pipelineq = make_testable_runner(PipelineRunner, 'pipeline') self._outq = make_testable_runner(OutgoingRunner, 'out') - # Python 2.7 has assertMultiLineEqual. Let this work without bounds. - self.maxDiff = None - self.eq = getattr(self, 'assertMultiLineEqual', self.assertEqual) def test_owners_get_email(self): # XXX 2012-03-23 BAW: We can't use a layer here because we need both diff --git a/src/mailman/styles/tests/test_styles.py b/src/mailman/styles/tests/test_styles.py index 990ce541f..a2ffc931f 100644 --- a/src/mailman/styles/tests/test_styles.py +++ b/src/mailman/styles/tests/test_styles.py @@ -63,28 +63,16 @@ class TestStyle(unittest.TestCase): # Registering a style with the same name as a previous style raises an # exception. self.manager.register(DummyStyle()) - try: - self.manager.register(DummyStyle()) - except DuplicateStyleError: - pass - else: - raise AssertionError('DuplicateStyleError exception expected') + self.assertRaises(DuplicateStyleError, + self.manager.register, DummyStyle()) def test_register_a_non_style(self): # You can't register something that doesn't implement the IStyle # interface. - try: - self.manager.register(object()) - except DoesNotImplement: - pass - else: - raise AssertionError('DoesNotImplement exception expected') + self.assertRaises(DoesNotImplement, + self.manager.register, object()) def test_unregister_a_non_registered_style(self): # You cannot unregister a style that hasn't yet been registered. - try: - self.manager.unregister(DummyStyle()) - except KeyError: - pass - else: - raise AssertionError('KeyError expected') + self.assertRaises(KeyError, + self.manager.unregister, DummyStyle()) diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py index 054dd4ff7..99f4b8961 100644 --- a/src/mailman/testing/helpers.py +++ b/src/mailman/testing/helpers.py @@ -249,8 +249,8 @@ def get_lmtp_client(quiet=False): if not quiet: print(response) return lmtp - except socket.error as error: - if error[0] == errno.ECONNREFUSED: + except IOError as error: + if error.errno == errno.ECONNREFUSED: time.sleep(0.1) else: raise @@ -284,8 +284,8 @@ def wait_for_webservice(): try: socket.socket().connect((config.webservice.hostname, int(config.webservice.port))) - except socket.error as error: - if error[0] == errno.ECONNREFUSED: + except IOError as error: + if error.errno == errno.ECONNREFUSED: time.sleep(0.1) else: raise @@ -515,19 +515,3 @@ class LogFileMark: with open(self._filename) as fp: fp.seek(self._filepos) return fp.readline() - - - -# In Python 2.6, body_line_iterator() uses a cStringIO.StringIO() which cannot -# handle unicode. In Python 2.7 this works fine. I hate version checks but -# this is the easiest way to handle it. OTOH, we could just use the manual -# way for all Python versions instead. -import sys -if sys.hexversion >= 0x2070000: - from email.iterators import body_line_iterator -else: - def body_line_iterator(msg, decode=False): - payload = msg.get_payload(decode=decode) - bytes_payload = payload.encode('utf-8') - for line in bytes_payload.splitlines(): - yield line diff --git a/src/mailman/testing/layers.py b/src/mailman/testing/layers.py index 3a3e1f684..99450d143 100644 --- a/src/mailman/testing/layers.py +++ b/src/mailman/testing/layers.py @@ -114,14 +114,25 @@ class ConfigLayer(MockAndMonkeyLayer): # runners) we'll need a file that we can specify to the with the -C # option. Craft the full test configuration string here, push it, and # also write it out to a temp file for -C. + # + # Create a dummy postfix.cfg file so that the test suite doesn't try + # to run the actual postmap command, which may not exist anyway. + postfix_cfg = os.path.join(cls.var_dir, 'postfix.cfg') + with open(postfix_cfg, 'w') as fp: + print(dedent(""" + [postfix] + postmap_command: true + """), file=fp) test_config = dedent(""" [mailman] layout: testing [paths.testing] - var_dir: %s + var_dir: {0} [devmode] testing: yes - """ % cls.var_dir) + [mta] + configuration: {1} + """.format(cls.var_dir, postfix_cfg)) # Read the testing config and push it. test_config += resource_string('mailman.testing', 'testing.cfg') config.create_paths = True diff --git a/src/mailman/testing/mail_archive.cfg b/src/mailman/testing/mail_archive.cfg new file mode 100644 index 000000000..0011781ba --- /dev/null +++ b/src/mailman/testing/mail_archive.cfg @@ -0,0 +1,4 @@ +[general] +base_url: http://go.mail-archive.dev/ + +recipient: archive@mail-archive.dev diff --git a/src/mailman/testing/mhonarc.cfg b/src/mailman/testing/mhonarc.cfg new file mode 100644 index 000000000..11a15eef3 --- /dev/null +++ b/src/mailman/testing/mhonarc.cfg @@ -0,0 +1,4 @@ +[general] +base_url: http://$hostname/archives/$fqdn_listname + +command: /bin/echo "/usr/bin/mhonarc -add -dbfile $PRIVATE_ARCHIVE_FILE_DIR/${listname}.mbox/mhonarc.db -outdir $VAR_DIR/mhonarc/${listname} -stderr $LOG_DIR/mhonarc -stdout $LOG_DIR/mhonarc -spammode -umask 022" diff --git a/src/mailman/testing/testing.cfg b/src/mailman/testing/testing.cfg index 141d74a8f..85b284cfe 100644 --- a/src/mailman/testing/testing.cfg +++ b/src/mailman/testing/testing.cfg @@ -31,7 +31,7 @@ lmtp_port: 9024 incoming: mailman.testing.mta.FakeMTA [passwords] -path: python:mailman.testing.passlib +configuration: python:mailman.testing.passlib [webservice] port: 9001 @@ -74,12 +74,11 @@ enable: yes [archiver.mail_archive] enable: yes -base_url: http://go.mail-archive.dev/ -recipient: archive@mail-archive.dev +configuration: python:mailman.testing.mail_archive [archiver.mhonarc] enable: yes -command: /bin/echo "/usr/bin/mhonarc -add -dbfile $PRIVATE_ARCHIVE_FILE_DIR/${listname}.mbox/mhonarc.db -outdir $VAR_DIR/mhonarc/${listname} -stderr $LOG_DIR/mhonarc -stdout $LOG_DIR/mhonarc -spammode -umask 022" +configuration: python:mailman.testing.mhonarc [language.ja] description: Japanese diff --git a/src/mailman/utilities/passwords.py b/src/mailman/utilities/passwords.py index 95c85c47a..cf08260fa 100644 --- a/src/mailman/utilities/passwords.py +++ b/src/mailman/utilities/passwords.py @@ -27,22 +27,15 @@ __all__ = [ from passlib.context import CryptContext -from pkg_resources import resource_string +from mailman.config.config import load_external from mailman.interfaces.configuration import ConfigurationUpdatedEvent class PasswordContext: def __init__(self, config): - # Is the context coming from a file system or Python path? - if config.passwords.path.startswith('python:'): - resource_path = config.passwords.path[7:] - package, dot, resource = resource_path.rpartition('.') - config_string = resource_string(package, resource + '.cfg') - else: - with open(config.passwords.path, 'rb') as fp: - config_string = fp.read() + config_string = load_external(config.passwords.configuration) self._context = CryptContext.from_string(config_string) def encrypt(self, secret): diff --git a/src/mailman/utilities/string.py b/src/mailman/utilities/string.py index 7470bd476..cd3adc536 100644 --- a/src/mailman/utilities/string.py +++ b/src/mailman/utilities/string.py @@ -60,19 +60,7 @@ def expand(template, substitutions, template_class=Template): :return: The substituted string. :rtype: string """ - # Python 2.6 requires ** dictionaries to have str, not unicode keys, so - # convert as necessary. Note that string.Template uses **. For our - # purposes, keys should always be ascii. Values though can be anything. - cooked = substitutions.__class__() - for key in substitutions: - if isinstance(key, unicode): - key = key.encode('ascii') - cooked[key] = substitutions[key] - try: - return template_class(template).safe_substitute(cooked) - except (TypeError, ValueError): - # The template is really screwed up. - log.exception('broken template: %s', template) + return template_class(template).safe_substitute(substitutions) diff --git a/src/mailman/utilities/tests/test_passwords.py b/src/mailman/utilities/tests/test_passwords.py index 7b2931855..22ef22757 100644 --- a/src/mailman/utilities/tests/test_passwords.py +++ b/src/mailman/utilities/tests/test_passwords.py @@ -55,6 +55,6 @@ class TestPasswords(unittest.TestCase): [passlib] schemes = plaintext """, file=fp) - with configuration('passwords', path=config_file): + with configuration('passwords', configuration=config_file): self.assertEqual(config.password_context.encrypt('my password'), 'my password') diff --git a/src/mailman/utilities/tests/test_templates.py b/src/mailman/utilities/tests/test_templates.py index d205eef34..8e335f69b 100644 --- a/src/mailman/utilities/tests/test_templates.py +++ b/src/mailman/utilities/tests/test_templates.py @@ -224,13 +224,9 @@ class TestFind(unittest.TestCase): self.assertEqual(self.fp.read(), 'List template') def test_template_not_found(self): - # Python 2.6 compatibility. - try: + with self.assertRaises(TemplateNotFoundError) as cm: find('missing.txt', self.mlist) - except TemplateNotFoundError as error: - self.assertEqual(error.template_file, 'missing.txt') - else: - raise AssertionError('TemplateNotFoundError expected') + self.assertEqual(cm.exception.template_file, 'missing.txt') |