Convert TestFileRecips to a doctest, and update the handler to more modern

Python idioms.  The recipients are now returned as a set instead of a list, so
duplicates are quashed.

In MailList.InitTempVars() we need to create the list's data directory when
the list is initialized.  If the directory already exists, this does nothing.

4 files changed, 115 insertions, 101 deletions

diff --git a/Mailman/Handlers/FileRecips.py b/Mailman/Handlers/FileRecips.py
index 51c90b45a..8ce07b432 100644
--- a/Mailman/Handlers/FileRecips.py
+++ b/Mailman/Handlers/FileRecips.py
@@ -17,6 +17,8 @@
 
 """Get the normal delivery recipients from a Sendmail style :include: file."""
 
+from __future__ import with_statement
+
 import os
 import errno
 
@@ -25,25 +27,20 @@ from Mailman import Errors
 
 
 def process(mlist, msg, msgdata):
-    if msgdata.has_key('recips'):
+    if 'recips' in msgdata:
         return
-    filename = os.path.join(mlist.fullpath(), 'members.txt')
+    filename = os.path.join(mlist.full_path, 'members.txt')
     try:
-        fp = open(filename)
+        with open(filename) as fp:
+            addrs = set(line.strip() for line in fp)
     except IOError, e:
         if e.errno <> errno.ENOENT:
             raise
-        # If the file didn't exist, just set an empty recipients list
-        msgdata['recips'] = []
+        msgdata['recips'] = set()
         return
-    # Read all the lines out of the file, and strip them of the trailing nl
-    addrs = [line.strip() for line in fp.readlines()]
-    # If the sender is in that list, remove him
+    # If the sender is a member of the list, remove them from the file recips.
     sender = msg.get_sender()
-    if mlist.isMember(sender):
-        try:
-            addrs.remove(mlist.getMemberCPAddress(sender))
-        except ValueError:
-            # Don't worry if the sender isn't in the list
-            pass
+    member = mlist.members.get_member(sender)
+    if member is not None:
+        addrs.discard(member.address.address)
     msgdata['recips'] = addrs
diff --git a/Mailman/MailList.py b/Mailman/MailList.py
index 9dff467ce..6f08ca674 100644
--- a/Mailman/MailList.py
+++ b/Mailman/MailList.py
@@ -304,8 +304,9 @@ class MailList(object, HTMLFormatter, Deliverer, ListAdmin,
         mod = sys.modules[adaptor_module]
         self._memberadaptor = getattr(mod, adaptor_class)(self)
         self._make_lock(self.fqdn_listname)
-        self._full_path = os.path.join(config.LIST_DATA_DIR,
-                                       self.fqdn_listname)
+        # Create the list's data directory.
+        self._full_path = os.path.join(config.LIST_DATA_DIR, self.fqdn_listname)
+        Utils.makedirs(self._full_path)
         # Only one level of mixin inheritance allowed
         for baseclass in self.__class__.__bases__:
             if hasattr(baseclass, 'InitTempVars'):
diff --git a/Mailman/docs/file-recips.txt b/Mailman/docs/file-recips.txt
new file mode 100644
index 000000000..2c42d3dd4
--- /dev/null
+++ b/Mailman/docs/file-recips.txt
@@ -0,0 +1,101 @@
+File recipients
+===============
+
+Mailman can calculate the recipients for a message from a Sendmail-style
+include file.  This file must be called members.txt and it must live in the
+list's data directory.
+
+    >>> from email import message_from_string
+    >>> from Mailman.Message import Message
+    >>> from Mailman.Handlers.FileRecips import process
+    >>> from Mailman.configuration import config
+    >>> from Mailman.database import flush
+    >>> mlist = config.list_manager.create('_xtest@example.com')
+    >>> flush()
+
+
+Short circuiting
+----------------
+
+If the message's metadata already has recipients, this handler immediately
+returns.
+
+    >>> msg = message_from_string("""\
+    ... From: aperson@example.com
+    ...
+    ... A message.
+    ... """, Message)
+    >>> msgdata = {'recips': 7}
+    >>> process(mlist, msg, msgdata)
+    >>> print msg.as_string()
+    From: aperson@example.com
+    <BLANKLINE>
+    A message.
+    <BLANKLINE>
+    >>> msgdata
+    {'recips': 7}
+
+
+Missing file
+------------
+
+The include file must live inside the list's data directory, under the name
+members.txt.  If the file doesn't exist, the list of recipients will be
+empty.
+
+    >>> import os
+    >>> file_path = os.path.join(mlist.full_path, 'members.txt')
+    >>> open(file_path)
+    Traceback (most recent call last):
+    ...
+    IOError: [Errno ...]
+    No such file or directory: '.../_xtest@example.com/members.txt'
+    >>> msgdata = {}
+    >>> process(mlist, msg, msgdata)
+    >>> sorted(msgdata['recips'])
+    []
+
+
+Existing file
+-------------
+
+If the file exists, it contains a list of addresses, one per line.  These
+addresses are returned as the set of recipients.
+
+    >>> fp = open(file_path, 'w')
+    >>> try:
+    ...     print >> fp, 'bperson@example.com'
+    ...     print >> fp, 'cperson@example.com'
+    ...     print >> fp, 'dperson@example.com'
+    ...     print >> fp, 'eperson@example.com'
+    ...     print >> fp, 'fperson@example.com'
+    ...     print >> fp, 'gperson@example.com'
+    ... finally:
+    ...     fp.close()
+
+    >>> msgdata = {}
+    >>> process(mlist, msg, msgdata)
+    >>> sorted(msgdata['recips'])
+    ['bperson@example.com', 'cperson@example.com', 'dperson@example.com',
+     'eperson@example.com', 'fperson@example.com', 'gperson@example.com']
+
+However, if the sender of the original message is a member of the list and
+their address is in the include file, the sender's address is /not/ included
+in the recipients list.
+
+    >>> from Mailman.constants import MemberRole
+    >>> address_1 = config.user_manager.create_address('cperson@example.com')
+    >>> address_1.subscribe(mlist, MemberRole.member)
+    <Member: cperson@example.com on _xtest@example.com as MemberRole.member>
+    >>> flush()
+
+    >>> msg = message_from_string("""\
+    ... From: cperson@example.com
+    ...
+    ... A message.
+    ... """, Message)
+    >>> msgdata = {}
+    >>> process(mlist, msg, msgdata)
+    >>> sorted(msgdata['recips'])
+    ['bperson@example.com', 'dperson@example.com',
+     'eperson@example.com', 'fperson@example.com', 'gperson@example.com']
diff --git a/Mailman/testing/test_handlers.py b/Mailman/testing/test_handlers.py
index b44a1c2cc..bf6b883fc 100644
--- a/Mailman/testing/test_handlers.py
+++ b/Mailman/testing/test_handlers.py
@@ -39,7 +39,6 @@ from Mailman.testing.base import TestBase
 from Mailman.Handlers import Acknowledge
 from Mailman.Handlers import AfterDelivery
 from Mailman.Handlers import Approve
-from Mailman.Handlers import FileRecips
 from Mailman.Handlers import Hold
 from Mailman.Handlers import MimeDel
 from Mailman.Handlers import Moderate
@@ -135,89 +134,6 @@ X-BeenThere: %s
 
 
 
-class TestFileRecips(TestBase):
-    def test_short_circuit(self):
-        msgdata = {'recips': 1}
-        rtn = FileRecips.process(self._mlist, None, msgdata)
-        # Not really a great test, but there's little else to assert
-        self.assertEqual(rtn, None)
-
-    def test_file_nonexistant(self):
-        msgdata = {}
-        FileRecips.process(self._mlist, None, msgdata)
-        self.assertEqual(msgdata.get('recips'), [])
-
-    def test_file_exists_no_sender(self):
-        msg = email.message_from_string("""\
-To: yall@example.com
-
-""", Message.Message)
-        msgdata = {}
-        file = os.path.join(self._mlist.fullpath(), 'members.txt')
-        addrs = ['aperson@example.org', 'bperson@example.com',
-                 'cperson@example.com', 'dperson@example.com']
-        fp = open(file, 'w')
-        try:
-            for addr in addrs:
-                print >> fp, addr
-            fp.close()
-            FileRecips.process(self._mlist, msg, msgdata)
-            self.assertEqual(msgdata.get('recips'), addrs)
-        finally:
-            try:
-                os.unlink(file)
-            except OSError, e:
-                if e.errno <> e.ENOENT: raise
-
-    def test_file_exists_no_member(self):
-        msg = email.message_from_string("""\
-From: eperson@example.com
-To: yall@example.com
-
-""", Message.Message)
-        msgdata = {}
-        file = os.path.join(self._mlist.fullpath(), 'members.txt')
-        addrs = ['aperson@example.org', 'bperson@example.com',
-                 'cperson@example.com', 'dperson@example.com']
-        fp = open(file, 'w')
-        try:
-            for addr in addrs:
-                print >> fp, addr
-            fp.close()
-            FileRecips.process(self._mlist, msg, msgdata)
-            self.assertEqual(msgdata.get('recips'), addrs)
-        finally:
-            try:
-                os.unlink(file)
-            except OSError, e:
-                if e.errno <> e.ENOENT: raise
-
-    def test_file_exists_is_member(self):
-        msg = email.message_from_string("""\
-From: aperson@example.org
-To: yall@example.com
-
-""", Message.Message)
-        msgdata = {}
-        file = os.path.join(self._mlist.fullpath(), 'members.txt')
-        addrs = ['aperson@example.org', 'bperson@example.com',
-                 'cperson@example.com', 'dperson@example.com']
-        fp = open(file, 'w')
-        try:
-            for addr in addrs:
-                print >> fp, addr
-                self._mlist.addNewMember(addr)
-            fp.close()
-            FileRecips.process(self._mlist, msg, msgdata)
-            self.assertEqual(msgdata.get('recips'), addrs[1:])
-        finally:
-            try:
-                os.unlink(file)
-            except OSError, e:
-                if e.errno <> e.ENOENT: raise
-
-
-
 class TestHold(TestBase):
     def setUp(self):
         TestBase.setUp(self)
@@ -1052,7 +968,6 @@ Mailman rocks!
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.makeSuite(TestApprove))
-    suite.addTest(unittest.makeSuite(TestFileRecips))
     suite.addTest(unittest.makeSuite(TestHold))
     suite.addTest(unittest.makeSuite(TestMimeDel))
     suite.addTest(unittest.makeSuite(TestModerate))