3 files changed, 56 insertions, 21 deletions

diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 6c5883814..6a10dcade 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -23,7 +23,8 @@ Architecture
    individual archiver implementations, since not all of them need a lock.  If
    they do, the implementations must acquire said lock themselves.
  * The `news` runner and queue has been renamed to the more accurate `nntp`.
-   Beta testers can can safely remove `$var_dir/queue/news`.
+   The runner has also been ported to Mailman 3 (LP: #967409).  Beta testers
+   can can safely remove `$var_dir/queue/news`.
 
 Configuration
 -------------
diff --git a/src/mailman/runners/nntp.py b/src/mailman/runners/nntp.py
index 32f0165da..411fbb6f2 100644
--- a/src/mailman/runners/nntp.py
+++ b/src/mailman/runners/nntp.py
@@ -30,6 +30,7 @@ from mailman.config import config
 from mailman.core.runner import Runner
 from mailman.interfaces.nntp import NewsModeration
 
+COMMA = ','
 COMMASPACE = ', '
 log = logging.getLogger('mailman.error')
 
@@ -101,33 +102,39 @@ def prepare_message(mlist, msg, msgdata):
     # came from mailing list user.
     stripped_subject = msgdata.get('stripped_subject',
                                    msgdata.get('original_subject'))
+    # XXX 2012-03-31 BAW: rename news_prefix_subject_too to nntp_.  This
+    # requires a schema change.
     if not mlist.news_prefix_subject_too and stripped_subject is not None:
         del msg['subject']
         msg['subject'] = stripped_subject
-    # Add the appropriate Newsgroups: header
-    ngheader = msg['newsgroups']
-    if ngheader is not None:
+    # Add the appropriate Newsgroups header.  Multiple Newsgroups headers are
+    # generally not allowed so we're not testing for them.
+    header = msg.get('newsgroups')
+    if header is None:
+        msg['Newsgroups'] = mlist.linked_newsgroup
+    else:
         # See if the Newsgroups: header already contains our linked_newsgroup.
         # If so, don't add it again.  If not, append our linked_newsgroup to
         # the end of the header list
-        ngroups = [s.strip() for s in ngheader.split(',')]
-        if mlist.linked_newsgroup not in ngroups:
-            ngroups.append(mlist.linked_newsgroup)
+        newsgroups = [value.strip() for value in header.split(COMMA)]
+        if mlist.linked_newsgroup not in newsgroups:
+            newsgroups.append(mlist.linked_newsgroup)
             # Subtitute our new header for the old one.
             del msg['newsgroups']
-            msg['Newsgroups'] = COMMASPACE.join(ngroups)
-    else:
-        # Newsgroups: isn't in the message
-        msg['Newsgroups'] = mlist.linked_newsgroup
+            msg['Newsgroups'] = COMMASPACE.join(newsgroups)
     # Note: We need to be sure two messages aren't ever sent to the same list
     # in the same process, since message ids need to be unique.  Further, if
-    # messages are crossposted to two Usenet-gated mailing lists, they each
-    # need to have unique message ids or the nntpd will only accept one of
-    # them.  The solution here is to substitute any existing message-id that
-    # isn't ours with one of ours, so we need to parse it to be sure we're not
-    # looping.
+    # messages are crossposted to two gated mailing lists, they must each have
+    # unique message ids or the nntpd will only accept one of them.  The
+    # solution here is to substitute any existing message-id that isn't ours
+    # with one of ours, so we need to parse it to be sure we're not looping.
     #
     # Our Message-ID format is <mailman.secs.pid.listname@hostname>
+    #
+    # XXX 2012-03-31 BAW: What we really want to do is try posting the message
+    # to the nntpd first, and only if that fails substitute a unique
+    # Message-ID.  The following should get moved out of prepare_message() and
+    # into _dispose() above.
     msgid = msg['message-id']
     hackmsgid = True
     if msgid:
@@ -139,14 +146,14 @@ def prepare_message(mlist, msg, msgdata):
     if hackmsgid:
         del msg['message-id']
         msg['Message-ID'] = email.utils.make_msgid()
-    # Lines: is useful
+    # Lines: is useful.
     if msg['Lines'] is None:
         # BAW: is there a better way?
-        count = len(list(email.Iterators.body_line_iterator(msg)))
+        count = len(list(email.iterators.body_line_iterator(msg)))
         msg['Lines'] = str(count)
     # Massage the message headers by remove some and rewriting others.  This
-    # woon't completely sanitize the message, but it will eliminate the bulk
-    # of the rejections based on message headers.  The NNTP server may still
+    # won't completely sanitize the message, but it will eliminate the bulk of
+    # the rejections based on message headers.  The NNTP server may still
     # reject the message because of other problems.
     for header in config.nntp.remove_headers.split():
         del msg[header]
@@ -163,5 +170,5 @@ def prepare_message(mlist, msg, msgdata):
         msg[header] = values[0]
         for v in values[1:]:
             msg[rewrite] = v
-    # Mark this message as prepared in case it has to be requeued
+    # Mark this message as prepared in case it has to be requeued.
     msgdata['prepped'] = True
diff --git a/src/mailman/runners/tests/test_nntp.py b/src/mailman/runners/tests/test_nntp.py
index 2df02d205..33a2f9a7c 100644
--- a/src/mailman/runners/tests/test_nntp.py
+++ b/src/mailman/runners/tests/test_nntp.py
@@ -43,6 +43,7 @@ class TestNNTP(unittest.TestCase):
 
     def setUp(self):
         self._mlist = create_list('test@example.com')
+        self._mlist.linked_newsgroup = 'example.test'
         self._msg = mfs("""\
 From: anne@example.com
 To: test@example.com
@@ -139,3 +140,29 @@ Testing
         headers = self._msg.get_all('subject')
         self.assertEqual(len(headers), 1)
         self.assertEqual(headers[0], 'Re: Your test')
+
+    def test_add_newsgroups_header(self):
+        # Prepared messages get a Newsgroups header.
+        msgdata = dict(original_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        self.assertEqual(self._msg['newsgroups'], 'example.test')
+
+    def test_add_newsgroups_header_to_existing(self):
+        # If the message already has a Newsgroups header, the linked newsgroup
+        # gets appended to that value, using comma-space separated lists.
+        self._msg['Newsgroups'] = 'foo.test, bar.test'
+        msgdata = dict(original_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        headers = self._msg.get_all('newsgroups')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'foo.test, bar.test, example.test')
+
+    def test_add_lines_header(self):
+        # A Lines: header seems useful.
+        nntp.prepare_message(self._mlist, self._msg, {})
+        self.assertEqual(self._msg['lines'], '1')
+
+    def test_the_message_has_been_prepared(self):
+        msgdata = {}
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        self.assertTrue(msgdata.get('prepped'))