diff options
| -rw-r--r-- | src/mailman/app/bounces.py | 6 | ||||
| -rw-r--r-- | src/mailman/app/docs/bounces.rst | 12 | ||||
| -rw-r--r-- | src/mailman/app/docs/message.rst | 65 | ||||
| -rw-r--r-- | src/mailman/app/tests/test_bounces.py | 19 | ||||
| -rw-r--r-- | src/mailman/archiving/docs/common.rst | 9 | ||||
| -rw-r--r-- | src/mailman/archiving/prototype.py | 59 | ||||
| -rw-r--r-- | src/mailman/archiving/tests/__init__.py | 0 | ||||
| -rw-r--r-- | src/mailman/archiving/tests/test_prototype.py | 174 | ||||
| -rw-r--r-- | src/mailman/commands/docs/info.rst | 1 | ||||
| -rw-r--r-- | src/mailman/config/config.py | 1 | ||||
| -rw-r--r-- | src/mailman/config/schema.cfg | 3 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 6 | ||||
| -rw-r--r-- | src/mailman/email/message.py | 14 |
13 files changed, 315 insertions, 54 deletions
diff --git a/src/mailman/app/bounces.py b/src/mailman/app/bounces.py index 69250ea5a..3816369fa 100644 --- a/src/mailman/app/bounces.py +++ b/src/mailman/app/bounces.py @@ -225,9 +225,9 @@ def send_probe(member, msg): notice = MIMEText(text, _charset=mlist.preferred_language.charset) probe.attach(notice) probe.attach(MIMEMessage(msg)) - # Send without Precedence: bulk. (LP: #808821) - probe.send(mlist, envsender=probe_sender, verp=False, probe_token=token - noprecedence=True) + # Probes should not have the Precedence: bulk header. + probe.send(mlist, envsender=probe_sender, verp=False, probe_token=token, + add_precedence=False) return token diff --git a/src/mailman/app/docs/bounces.rst b/src/mailman/app/docs/bounces.rst index f825064e3..5510f2207 100644 --- a/src/mailman/app/docs/bounces.rst +++ b/src/mailman/app/docs/bounces.rst @@ -12,12 +12,12 @@ Mailman can bounce messages back to the original sender. This is essentially equivalent to rejecting the message with notification. Mailing lists can bounce a message with an optional error message. - >>> mlist = create_list('_xtest@example.com') + >>> mlist = create_list('text@example.com') Any message can be bounced. >>> msg = message_from_string("""\ - ... To: _xtest@example.com + ... To: text@example.com ... From: aperson@example.com ... Subject: Something important ... @@ -36,7 +36,7 @@ to the original message author. 1 >>> print items[0].msg.as_string() Subject: Something important - From: _xtest-owner@example.com + From: text-owner@example.com To: aperson@example.com MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="..." @@ -54,7 +54,7 @@ to the original message author. Content-Type: message/rfc822 MIME-Version: 1.0 <BLANKLINE> - To: _xtest@example.com + To: text@example.com From: aperson@example.com Subject: Something important <BLANKLINE> @@ -74,7 +74,7 @@ passed in as an instance of a ``RejectMessage`` exception. 1 >>> print items[0].msg.as_string() Subject: Something important - From: _xtest-owner@example.com + From: text-owner@example.com To: aperson@example.com MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="..." @@ -92,7 +92,7 @@ passed in as an instance of a ``RejectMessage`` exception. Content-Type: message/rfc822 MIME-Version: 1.0 <BLANKLINE> - To: _xtest@example.com + To: text@example.com From: aperson@example.com Subject: Something important <BLANKLINE> diff --git a/src/mailman/app/docs/message.rst b/src/mailman/app/docs/message.rst index 11ab32b22..3c3fd8ea8 100644 --- a/src/mailman/app/docs/message.rst +++ b/src/mailman/app/docs/message.rst @@ -2,7 +2,7 @@ Messages ======== -Mailman has its own Message classes, derived from the standard +Mailman has its own `Message` classes, derived from the standard ``email.message.Message`` class, but providing additional useful methods. @@ -13,7 +13,7 @@ When Mailman needs to send a message to a user, it creates a ``UserNotification`` instance, and then calls the ``.send()`` method on this object. This method requires a mailing list instance. - >>> mlist = create_list('_xtest@example.com') + >>> mlist = create_list('test@example.com') The ``UserNotification`` constructor takes the recipient address, the sender address, an optional subject, optional body text, and optional language. @@ -21,25 +21,23 @@ address, an optional subject, optional body text, and optional language. >>> from mailman.email.message import UserNotification >>> msg = UserNotification( ... 'aperson@example.com', - ... '_xtest@example.com', + ... 'test@example.com', ... 'Something you need to know', ... 'I needed to tell you this.') >>> msg.send(mlist) The message will end up in the `virgin` queue. - >>> switchboard = config.switchboards['virgin'] - >>> len(switchboard.files) + >>> from mailman.testing.helpers import get_queue_messages + >>> messages = get_queue_messages('virgin') + >>> len(messages) 1 - >>> filebase = switchboard.files[0] - >>> qmsg, qmsgdata = switchboard.dequeue(filebase) - >>> switchboard.finish(filebase) - >>> print qmsg.as_string() + >>> print messages[0].msg.as_string() MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Subject: Something you need to know - From: _xtest@example.com + From: test@example.com To: aperson@example.com Message-ID: ... Date: ... @@ -47,40 +45,45 @@ The message will end up in the `virgin` queue. <BLANKLINE> I needed to tell you this. -The message above got a Precedence: bulk header added by default. If the -message we're sending already has a Precedence: header, it shouldn't be +The message above got a `Precedence: bulk` header added by default. If the +message we're sending already has a `Precedence:` header, it shouldn't be changed. >>> del msg['precedence'] >>> msg['Precedence'] = 'list' >>> msg.send(mlist) -Again, the message will end up in the `virgin` queue but with our Precedence: -header. +Again, the message will end up in the `virgin` queue but with the original +`Precedence:` header. - >>> switchboard = config.switchboards['virgin'] - >>> len(switchboard.files) + >>> messages = get_queue_messages('virgin') + >>> len(messages) 1 - >>> filebase = switchboard.files[0] - >>> qmsg, qmsgdata = switchboard.dequeue(filebase) - >>> switchboard.finish(filebase) - >>> qmsg['precedence'] == 'list' - True + >>> print messages[0].msg['precedence'] + list -Sometimes we want to send the message without a Precedence: header such as +Sometimes we want to send the message without a `Precedence:` header such as when we send a probe message. >>> del msg['precedence'] - >>> msg.send(mlist, noprecedence=True) + >>> msg.send(mlist, add_precedence=False) Again, the message will end up in the `virgin` queue but without the -Precedence: header. +`Precedence:` header. - >>> switchboard = config.switchboards['virgin'] - >>> len(switchboard.files) + >>> messages = get_queue_messages('virgin') + >>> len(messages) 1 - >>> filebase = switchboard.files[0] - >>> qmsg, qmsgdata = switchboard.dequeue(filebase) - >>> switchboard.finish(filebase) - >>> 'precedence' in qmsg - False + >>> print messages[0].msg['precedence'] + None + +However, if the message already has a `Precedence:` header, setting the +`precedence=False` argument will have no effect. + + >>> msg['Precedence'] = 'junk' + >>> msg.send(mlist, add_precedence=False) + >>> messages = get_queue_messages('virgin') + >>> len(messages) + 1 + >>> print messages[0].msg['precedence'] + junk diff --git a/src/mailman/app/tests/test_bounces.py b/src/mailman/app/tests/test_bounces.py index be2c5cb78..d0d94df5e 100644 --- a/src/mailman/app/tests/test_bounces.py +++ b/src/mailman/app/tests/test_bounces.py @@ -21,6 +21,11 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ + 'TestMaybeForward', + 'TestProbe', + 'TestSendProbe', + 'TestSendProbeNonEnglish', + 'TestVERP', ] @@ -212,7 +217,7 @@ Message-ID: <first> self.assertEqual(set(pendable.keys()), set(['member_id', 'message_id'])) # member_ids are pended as unicodes. - self.assertEqual(uuid.UUID(hex=pendable['member_id']), + self.assertEqual(uuid.UUID(hex=pendable['member_id']), self._member.member_id) self.assertEqual(pendable['message_id'], '<first>') @@ -264,10 +269,16 @@ Message-ID: <first> # Check the headers of the outer message. token = send_probe(self._member, self._msg) message = get_queue_messages('virgin')[0].msg - self.assertEqual(message['From'], + self.assertEqual(message['from'], 'test-bounces+{0}@example.com'.format(token)) - self.assertEqual(message['To'], 'anne@example.com') - self.assertEqual(message['Subject'], 'Test mailing list probe message') + self.assertEqual(message['to'], 'anne@example.com') + self.assertEqual(message['subject'], 'Test mailing list probe message') + + def test_no_precedence_header(self): + # Probe messages should not have a Precedence header (LP: #808821). + send_probe(self._member, self._msg) + message = get_queue_messages('virgin')[0].msg + self.assertEqual(message['precedence'], None) diff --git a/src/mailman/archiving/docs/common.rst b/src/mailman/archiving/docs/common.rst index 45ec8f194..5f7cfe42b 100644 --- a/src/mailman/archiving/docs/common.rst +++ b/src/mailman/archiving/docs/common.rst @@ -62,12 +62,13 @@ The archiver is also able to archive the message. >>> os.path.exists(pckpath) True -Note however that the prototype archiver can't archive messages. +The `prototype` archiver archives messages to a maildir. >>> archivers['prototype'].archive_message(mlist, msg) - Traceback (most recent call last): - ... - NotImplementedError + >>> archive_path = os.path.join( + ... config.ARCHIVE_DIR, 'prototype', mlist.fqdn_listname, 'new') + >>> len(os.listdir(archive_path)) + 1 The Mail-Archive.com diff --git a/src/mailman/archiving/prototype.py b/src/mailman/archiving/prototype.py index 55d78074e..453c6c770 100644 --- a/src/mailman/archiving/prototype.py +++ b/src/mailman/archiving/prototype.py @@ -25,11 +25,22 @@ __all__ = [ ] +import os +import errno +import logging + +from datetime import timedelta +from mailbox import Maildir from urlparse import urljoin + +from flufl.lock import Lock, TimeOutError from zope.interface import implements +from mailman.config import config from mailman.interfaces.archiver import IArchiver +log = logging.getLogger('mailman.error') + class Prototype: @@ -61,5 +72,49 @@ class Prototype: @staticmethod def archive_message(mlist, message): - """See `IArchiver`.""" - raise NotImplementedError + """See `IArchiver`. + + This archiver saves messages into a maildir. + """ + archive_dir = os.path.join(config.ARCHIVE_DIR, 'prototype') + try: + os.makedirs(archive_dir, 0775) + except OSError as error: + # If this already exists, then we're fine + if error.errno != errno.EEXIST: + raise + + # Maildir will throw an error if the directories are partially created + # (for instance the toplevel exists but cur, new, or tmp do not) + # therefore we don't create the toplevel as we did above. + list_dir = os.path.join(archive_dir, mlist.fqdn_listname) + mailbox = Maildir(list_dir, create=True, factory=None) + lock_file = os.path.join( + config.LOCK_DIR, '{0}-maildir.lock'.format(mlist.fqdn_listname)) + + # Lock the maildir as Maildir.add() is not threadsafe. Don't use the + # context manager because it's not an error if we can't acquire the + # archiver lock. We'll just log the problem and continue. + # + # XXX 2012-03-14 BAW: When we extend the chain/pipeline architecture + # to other runners, e.g. the archive runner, it would be better to let + # any TimeOutError propagate up. That would cause the message to be + # re-queued and tried again later, rather than being discarded as + # happens now below. + lock = Lock(lock_file) + try: + lock.lock(timeout=timedelta(seconds=1)) + # Add the message to the maildir. The return value could be used + # to construct the file path if necessary. E.g. + # + # os.path.join(archive_dir, mlist.fqdn_listname, 'new', + # message_key) + mailbox.add(message) + except TimeOutError: + # Log the error and go on. + log.error('Unable to acquire prototype archiver lock for {0}, ' + 'discarding: {1}'.format( + mlist.fqdn_listname, + message.get('message-id', 'n/a'))) + finally: + lock.unlock(unconditionally=True) diff --git a/src/mailman/archiving/tests/__init__.py b/src/mailman/archiving/tests/__init__.py new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/src/mailman/archiving/tests/__init__.py diff --git a/src/mailman/archiving/tests/test_prototype.py b/src/mailman/archiving/tests/test_prototype.py new file mode 100644 index 000000000..7f48c5cfd --- /dev/null +++ b/src/mailman/archiving/tests/test_prototype.py @@ -0,0 +1,174 @@ +# 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/>. + +"""Test the prototype archiver.""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ + 'TestPrototypeArchiver', + ] + + +import os +import shutil +import tempfile +import unittest +import threading + +from email import message_from_file +from flufl.lock import Lock + +from mailman.app.lifecycle import create_list +from mailman.archiving.prototype import Prototype +from mailman.config import config +from mailman.testing.helpers import LogFileMark +from mailman.testing.helpers import ( + specialized_message_from_string as mfs) +from mailman.testing.layers import ConfigLayer +from mailman.utilities.email import add_message_hash + + +class TestPrototypeArchiver(unittest.TestCase): + """Test the prototype archiver.""" + + layer = ConfigLayer + + def setUp(self): + # Create a fake mailing list and message object + self._msg = mfs("""\ +To: test@example.com +From: anne@example.com +Subject: Testing the test list +Message-ID: <ant> +X-Message-ID-Hash: MS6QLWERIJLGCRF44J7USBFDELMNT2BW + +Tests are better than no tests +but the water deserves to be swum. +""") + self._mlist = create_list('test@example.com') + config.db.commit() + # Set up a temporary directory for the prototype archiver so that it's + # easier to clean up. + self._tempdir = tempfile.mkdtemp() + config.push('prototype', """ + [paths.testing] + archive_dir: {0} + """.format(self._tempdir)) + # Capture the structure of a maildir. + self._expected_dir_structure = set( + (os.path.join(config.ARCHIVE_DIR, path) for path in ( + 'prototype', + os.path.join('prototype', self._mlist.fqdn_listname), + os.path.join('prototype', self._mlist.fqdn_listname, 'cur'), + os.path.join('prototype', self._mlist.fqdn_listname, 'new'), + os.path.join('prototype', self._mlist.fqdn_listname, 'tmp'), + ))) + self._expected_dir_structure.add(config.ARCHIVE_DIR) + + def tearDown(self): + shutil.rmtree(self._tempdir) + config.pop('prototype') + + def _find(self, path): + all_filenames = set() + for dirpath, dirnames, filenames in os.walk(path): + if not isinstance(dirpath, unicode): + dirpath = unicode(dirpath) + all_filenames.add(dirpath) + for filename in filenames: + new_filename = filename + if not isinstance(filename, unicode): + new_filename = unicode(filename) + all_filenames.add(os.path.join(dirpath, new_filename)) + return all_filenames + + def test_archive_maildir_created(self): + # Archiving a message to the prototype archiver should create the + # expected directory structure. + Prototype.archive_message(self._mlist, self._msg) + all_filenames = self._find(config.ARCHIVE_DIR) + # Check that the directory structure has been created and we have one + # more file (the archived message) than expected directories. + archived_messages = all_filenames - self._expected_dir_structure + self.assertEqual(len(archived_messages), 1) + self.assertTrue( + archived_messages.pop().startswith( + os.path.join(config.ARCHIVE_DIR, 'prototype', + self._mlist.fqdn_listname, 'new'))) + + def test_archive_maildir_existence_does_not_raise(self): + # Archiving a second message does not cause an EEXIST to be raised + # when a second message is archived. + new_dir = None + Prototype.archive_message(self._mlist, self._msg) + for directory in ('cur', 'new', 'tmp'): + path = os.path.join(config.ARCHIVE_DIR, 'prototype', + self._mlist.fqdn_listname, directory) + if directory == 'new': + new_dir = path + self.assertTrue(os.path.isdir(path)) + # There should be one message in the 'new' directory. + self.assertEqual(len(os.listdir(new_dir)), 1) + # Archive a second message. If an exception occurs, let it fail the + # test. Afterward, two messages should be in the 'new' directory. + del self._msg['message-id'] + del self._msg['x-message-id-hash'] + self._msg['Message-ID'] = '<bee>' + add_message_hash(self._msg) + Prototype.archive_message(self._mlist, self._msg) + self.assertEqual(len(os.listdir(new_dir)), 2) + + def test_archive_lock_used(self): + # Test that locking the maildir when adding works as a failure here + # could mean we lose mail. + lock_file = os.path.join( + config.LOCK_DIR, '{0}-maildir.lock'.format( + self._mlist.fqdn_listname)) + with Lock(lock_file): + # Acquire the archiver lock, then make sure the archiver logs the + # fact that it could not acquire the lock. + archive_thread = threading.Thread( + target=Prototype.archive_message, + args=(self._mlist, self._msg)) + mark = LogFileMark('mailman.error') + archive_thread.run() + # Test that the archiver output the correct error. + line = mark.readline() + self.assertEqual( + # Strip out the timestamp. + line[28:-1], + 'Unable to acquire prototype archiver lock for {0}, ' + 'discarding: {1}'.format( + self._mlist.fqdn_listname, + self._msg.get('message-id'))) + # Check that the message didn't get archived. + created_files = self._find(config.ARCHIVE_DIR) + self.assertEqual(self._expected_dir_structure, created_files) + + def test_prototype_archiver_good_path(self): + # Verify the good path; the message gets archived. + Prototype.archive_message(self._mlist, self._msg) + new_path = os.path.join( + config.ARCHIVE_DIR, 'prototype', self._mlist.fqdn_listname, 'new') + archived_messages = list(os.listdir(new_path)) + self.assertEqual(len(archived_messages), 1) + # Check that the email has been added. + with open(os.path.join(new_path, archived_messages[0])) as fp: + archived_message = message_from_file(fp) + self.assertEqual(self._msg.as_string(), archived_message.as_string()) diff --git a/src/mailman/commands/docs/info.rst b/src/mailman/commands/docs/info.rst index 34883711e..ad034a1a6 100644 --- a/src/mailman/commands/docs/info.rst +++ b/src/mailman/commands/docs/info.rst @@ -59,6 +59,7 @@ The File System Hierarchy layout is the same every by definition. Python ... ... File system paths: + ARCHIVE_DIR = /var/lib/mailman/archives BIN_DIR = /sbin DATA_DIR = /var/lib/mailman/data ETC_DIR = /etc diff --git a/src/mailman/config/config.py b/src/mailman/config/config.py index 034b76b4f..e3b4f88a7 100644 --- a/src/mailman/config/config.py +++ b/src/mailman/config/config.py @@ -173,6 +173,7 @@ class Configuration: lock_dir = category.lock_dir, log_dir = category.log_dir, messages_dir = category.messages_dir, + archive_dir = category.archive_dir, pipermail_private_dir = category.pipermail_private_dir, pipermail_public_dir = category.pipermail_public_dir, queue_dir = category.queue_dir, diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg index e662633e6..7e15ab82f 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -113,6 +113,9 @@ etc_dir: $var_dir/etc ext_dir: $var_dir/ext # Directory where the default IMessageStore puts its messages. messages_dir: $var_dir/messages +# Directory for archive backends to store their messages in. Archivers should +# create a subdirectory in here to store their files. +archive_dir: $var_dir/archives # Directory for public Pipermail archiver artifacts. pipermail_public_dir: $var_dir/archives/public # Directory for private Pipermail archiver artifacts. diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 71cdcae6e..7888ed8fd 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -46,6 +46,8 @@ Architecture attribute on the message object, instead of trusting a possibly incorrect value if it's already set. The individual `IArchiver` implementations no longer set the `X-Message-ID-Hash` header. + * The Prototype archiver now stores its files in maildir format inside of + `$var_dir/archives/prototype`, given by Toshio Kuratomi. Database -------- @@ -105,8 +107,8 @@ Commands Bug fixes --------- - * Subscription disabled warnings are now sent without a Precedence: - header. (LP: #808821) + * Subscription disabled probe warning notification messages are now sent + without a `Precedence:` header. Given by Mark Sapiro. (LP: #808821) * Fixed KeyError in retry runner, contributed by Stephen A. Goss. (LP: #872391) * Fixed bogus use of `bounce_processing` attribute (should have been diff --git a/src/mailman/email/message.py b/src/mailman/email/message.py index 28d94523f..efe8879b1 100644 --- a/src/mailman/email/message.py +++ b/src/mailman/email/message.py @@ -178,10 +178,20 @@ class UserNotification(Message): self['To'] = recipients self.recipients = set([recipients]) - def send(self, mlist, noprecedence=False, **_kws): + def send(self, mlist, add_precedence=True, **_kws): """Sends the message by enqueuing it to the 'virgin' queue. This is used for all internally crafted messages. + + :param mlist: The mailing list to send the message to. + :type mlist: `IMailingList` + :param add_precedence: Flag indicating whether a `Precedence: bulk` + header should be added to the message or not. + :type add_precedence: bool + + This function also accepts arbitrary keyword arguments. The key/value + pairs for **kws is added to the metadata dictionary associated with + the enqueued message. """ # Since we're crafting the message from whole cloth, let's make sure # this message has a Message-ID. @@ -193,7 +203,7 @@ class UserNotification(Message): # UserNotifications are typically for admin messages, and for messages # other than list explosions. Send these out as Precedence: bulk, but # don't override an existing Precedence: header. - if 'precedence' not in self and not noprecedence: + if 'precedence' not in self and add_precedence: self['Precedence'] = 'bulk' self._enqueue(mlist, **_kws) |