diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/mailman/archiving/docs/common.rst | 13 | ||||
| -rw-r--r-- | src/mailman/archiving/prototype.py | 53 | ||||
| -rw-r--r-- | src/mailman/archiving/tests/test_prototype.py | 188 | ||||
| -rw-r--r-- | src/mailman/commands/docs/info.rst | 1 | ||||
| -rw-r--r-- | src/mailman/config/schema.cfg | 4 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 2 | ||||
| -rw-r--r-- | src/mailman/rest/helpers.py | 4 | ||||
| -rw-r--r-- | src/mailman/rules/administrivia.py | 2 | ||||
| -rw-r--r-- | src/mailman/rules/moderation.py | 4 | ||||
| -rw-r--r-- | src/mailman/runners/command.py | 6 |
10 files changed, 153 insertions, 124 deletions
diff --git a/src/mailman/archiving/docs/common.rst b/src/mailman/archiving/docs/common.rst index 7bee8c70e..5f7cfe42b 100644 --- a/src/mailman/archiving/docs/common.rst +++ b/src/mailman/archiving/docs/common.rst @@ -62,17 +62,14 @@ The archiver is also able to archive the message. >>> os.path.exists(pckpath) True -The prototype archiver is available to simplistically archive messages. -:: +The `prototype` archiver archives messages to a maildir. >>> archivers['prototype'].archive_message(mlist, msg) + >>> archive_path = os.path.join( + ... config.ARCHIVE_DIR, 'prototype', mlist.fqdn_listname, 'new') + >>> len(os.listdir(archive_path)) + 1 - >>> import os - >>> from mailman import config - >>> archivepath = os.path.join(config.ARCHIVE_DIR, 'prototype', - ... mlist.fqdn_listname, 'new') - >>> len (os.listdir(archivepath)) >= 1 - True The Mail-Archive.com ==================== diff --git a/src/mailman/archiving/prototype.py b/src/mailman/archiving/prototype.py index be6c60475..453c6c770 100644 --- a/src/mailman/archiving/prototype.py +++ b/src/mailman/archiving/prototype.py @@ -27,10 +27,8 @@ __all__ = [ import os import errno -import hashlib import logging -from base64 import b32encode from datetime import timedelta from mailbox import Maildir from urlparse import urljoin @@ -41,7 +39,8 @@ from zope.interface import implements from mailman.config import config from mailman.interfaces.archiver import IArchiver -elog = logging.getLogger('mailman.error') +log = logging.getLogger('mailman.error') + class Prototype: @@ -74,38 +73,48 @@ class Prototype: @staticmethod def archive_message(mlist, message): """See `IArchiver`. - - This sample archiver saves nmessages into a maildir + + 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 e: + except OSError as error: # If this already exists, then we're fine - if e.errno != errno.EEXIST: + 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 + # therefore we don't create the toplevel as we did above. list_dir = os.path.join(archive_dir, mlist.fqdn_listname) - mail_box = Maildir(list_dir, create=True, factory=None) + 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 - lock = Lock(os.path.join(config.LOCK_DIR, '%s-maildir.lock' - % 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 - # Message_key could be used to construct the file path if - # necessary:: - # os.path.join(archive_dir, mlist.fqdn_listname, 'new', - # message_key) - message_key = mail_box.add(message) + # 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 - elog.error('Unable to lock archive for %s, discarded' - ' message: %s' % (mlist.fqdn_listname, - message.get('message-id', '<unknown>'))) + # 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/test_prototype.py b/src/mailman/archiving/tests/test_prototype.py index 06c456076..9c229db1c 100644 --- a/src/mailman/archiving/tests/test_prototype.py +++ b/src/mailman/archiving/tests/test_prototype.py @@ -21,131 +21,151 @@ 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 import prototype +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 smfs +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.""" -class test_PrototypeArchiveMethod(unittest.TestCase): layer = ConfigLayer def setUp(self): # Create a fake mailing list and message object - self.message = smfs('''\ + self._msg = mfs("""\ To: test@example.com -From: admin@example.com +From: anne@example.com Subject: Testing the test list -Message-ID: <DEADBEEF@example.com> +Message-ID: <ant> +X-Message-ID-Hash: MS6QLWERIJLGCRF44J7USBFDELMNT2BW -Tests are better than not tests -but the water deserves to be swum - ''') - - self.mlist = create_list('test@example.com') +Tests are better than no tests +but the water deserves to be swum. +""") + self._mlist = create_list('test@example.com') config.db.commit() - - # Set some config directories where we won't bash real data - config.ARCHIVE_DIR = '%s%s' % (tempfile.mkdtemp(), os.path.sep) - config.LOCK_DIR = tempfile.mkdtemp() - - # Structure of a maildir - self.expected_dir_structure = frozenset( - (os.path.join(config.ARCHIVE_DIR, f) for f 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'), - ) - ) - ) + # 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(config.ARCHIVE_DIR) - shutil.rmtree(config.LOCK_DIR) + shutil.rmtree(self._tempdir) + config.pop('prototype') def _find(self, path): all_filenames = set() - for dirs in os.walk(path): - directory = dirs[0] - if not isinstance(directory, unicode): - directory = unicode(directory) - all_filenames.add(directory) - if dirs[2]: - for filename in dirs[2]: - new_filename = os.path.join(dirs[0], filename) - if not isinstance(new_filename, unicode): - new_filename = unicode(new_filename) - all_filenames.add(new_filename) + 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 = os.path.join(dirpath, filename) + if not isinstance(new_filename, unicode): + new_filename = unicode(new_filename) + all_filenames.add(new_filename) return all_filenames def test_archive_maildir_created(self): - prototype.Prototype.archive_message(self.mlist, self.message) + # 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 - self.assertTrue(self.expected_dir_structure.issubset(all_filenames)) - self.assertEqual(len(all_filenames), len(self.expected_dir_structure) + 1) - - def test_archive_maildir_existance_does_not_raise(self): - os.makedirs(os.path.join(config.ARCHIVE_DIR, 'prototype', - self.mlist.fqdn_listname, 'cur')) - os.mkdir(os.path.join(config.ARCHIVE_DIR, 'prototype', - self.mlist.fqdn_listname, 'new')) - os.mkdir(os.path.join(config.ARCHIVE_DIR, 'prototype', - self.mlist.fqdn_listname, 'tmp')) + # more file (the archived message) than expected directories. + archived_messages = all_filenames - self._expected_dir_structure + self.assertEqual(self._expected_dir_structure, + all_filenames - archived_messages) - # Checking that no exception is raised in this circumstance because it - # will be the common case (adding a new message to an archive whose - # directories have alreay been created) - try: - prototype.Prototype.archive_message(self.mlist, self.message) - except: - self.assertTrue(False, 'Exception raised when the archive' - ' directory structure already in place') + 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 = Lock(os.path.join(config.LOCK_DIR, '%s-maildir.lock' - % self.mlist.fqdn_listname)) - with lock: - # Take this lock. Then make sure the archiver fails while that's - # working. + # 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, Hen make sure the archiver logs the + # fact that it could not acquire the lock. archive_thread = threading.Thread( - target=prototype.Prototype.archive_message, - args=(self.mlist, self.message)) + target=Prototype.archive_message, + args=(self._mlist, self._msg)) mark = LogFileMark('mailman.error') archive_thread.run() - # Test that the archiver output the correct error + # Test that the archiver output the correct error. line = mark.readline() - self.assertTrue(line.endswith('Unable to lock archive for %s,' - ' discarded message: %s\n' % (self.mlist.fqdn_listname, - self.message.get('message-id')))) - - # Check that the file didn't get created + 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) + self.assertEqual(self._expected_dir_structure, created_files) - def test_mail_added(self): - prototype.Prototype.archive_message(self.mlist, self.message) - for filename in os.listdir(os.path.join(config.ARCHIVE_DIR, - 'prototype', self.mlist.fqdn_listname, 'new')): - # Check that the email has been added - email = open(os.path.join(config.ARCHIVE_DIR, 'prototype', - self.mlist.fqdn_listname, 'new', filename)) - self.assertTrue((repr(self.message)).endswith(email.read())) + 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/schema.cfg b/src/mailman/config/schema.cfg index 8edde2c8d..7e15ab82f 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -113,8 +113,8 @@ 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 archives in. -# Archivers should create a subdirectory in here to store their files +# 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 diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 6869e2889..c2e4d1e51 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 -------- diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py index 2ce62b0fe..2824a894e 100644 --- a/src/mailman/rest/helpers.py +++ b/src/mailman/rest/helpers.py @@ -77,10 +77,10 @@ class ExtendedEncoder(json.JSONEncoder): seconds = obj.seconds + obj.microseconds / 1000000.0 return '{0}d{1}s'.format(obj.days, seconds) return '{0}d'.format(obj.days) - elif hasattr(obj, 'enumclass') and issubclass(obj.enumclass, Enum): + elif hasattr(obj, 'enum') and issubclass(obj.enum, Enum): # It's up to the decoding validator to associate this name with # the right Enum class. - return obj.enumname + return obj.name return json.JSONEncoder.default(self, obj) diff --git a/src/mailman/rules/administrivia.py b/src/mailman/rules/administrivia.py index 790c16c19..41c6edf30 100644 --- a/src/mailman/rules/administrivia.py +++ b/src/mailman/rules/administrivia.py @@ -81,7 +81,7 @@ class Administrivia: lineno = 0 for line in lines: line = line.strip() - if line == '': + if len(line) == 0: continue lineno += 1 if lineno > config.mailman.email_commands_max_lines: diff --git a/src/mailman/rules/moderation.py b/src/mailman/rules/moderation.py index bcec47cba..cb27d89d8 100644 --- a/src/mailman/rules/moderation.py +++ b/src/mailman/rules/moderation.py @@ -57,7 +57,7 @@ class MemberModeration: elif action is not None: # We must stringify the moderation action so that it can be # stored in the pending request table. - msgdata['moderation_action'] = action.enumname + msgdata['moderation_action'] = action.name msgdata['moderation_sender'] = sender return True # The sender is not a member so this rule does not match. @@ -98,7 +98,7 @@ class NonmemberModeration: elif action is not None: # We must stringify the moderation action so that it can be # stored in the pending request table. - msgdata['moderation_action'] = action.enumname + msgdata['moderation_action'] = action.name msgdata['moderation_sender'] = sender return True # The sender must be a member, so this rule does not match. diff --git a/src/mailman/runners/command.py b/src/mailman/runners/command.py index f13b02229..ac611ed3a 100644 --- a/src/mailman/runners/command.py +++ b/src/mailman/runners/command.py @@ -85,7 +85,7 @@ class CommandFinder: # bogus characters. Otherwise, there's nothing in the subject # that we can use. if isinstance(raw_subject, unicode): - safe_subject = raw_subject.encode('us-ascii', errors='ignore') + safe_subject = raw_subject.encode('us-ascii', 'ignore') self.command_lines.append(safe_subject) # Find the first text/plain part of the message. part = None @@ -119,7 +119,7 @@ class CommandFinder: # ASCII commands and arguments, ignore anything else. parts = [(part if isinstance(part, unicode) - else part.decode('ascii', errors='ignore')) + else part.decode('ascii', 'ignore')) for part in parts] yield parts @@ -139,7 +139,7 @@ The results of your email command are provided below. def write(self, text): if not isinstance(text, unicode): - text = text.decode(self.charset, errors='ignore') + text = text.decode(self.charset, 'ignore') self._output.write(text) def __unicode__(self): |