4 files changed, 373 insertions, 206 deletions

diff --git a/src/mailman/mta/deliver.py b/src/mailman/mta/deliver.py
index 85bb09991..a3a32a0ff 100644
--- a/src/mailman/mta/deliver.py
+++ b/src/mailman/mta/deliver.py
@@ -75,9 +75,6 @@ def deliver(mlist, msg, msgdata):
     if not recipients:
         # Could be None, could be an empty sequence.
         return
-    # Calculate whether we should VERP this message or not.  The results of
-    # this set the 'verp' key in the message metadata.
-    _calculate_verp(mlist, msg, msgdata)
     # Which delivery agent should we use?  Several situations can cause us to
     # use individual delivery.  If not specified, use bulk delivery.  See the
     # to-outgoing handler for when the 'verp' key is set in the metadata.
@@ -158,33 +155,3 @@ def deliver(mlist, msg, msgdata):
     # Return the results
     if temporary_failures or permanent_failures:
         raise SomeRecipientsFailed(temporary_failures, permanent_failures)
-
-
-
-def _calculate_verp(mlist, msg, msgdata):
-    """Calculate whether this message should be VERP'd or not.
-
-    This function works by side-effect.  If the message should be VERP'd, then
-    the 'verp' key in msgdata is set to True, otherwise it is set to False.
-    """
-    if 'verp' in msgdata:
-        # Honor existing settings.
-        return
-    # If personalization is enabled for this list and we've configured Mailman
-    # to always VERP personalized deliveries, then yes we VERP it.  Also, if
-    # personalization is /not/ enabled, but verp_delivery_interval is set (and
-    # we've hit this interval), then again, this message should be
-    # VERP'd. Otherwise, no.
-    interval = int(config.mta.verp_delivery_interval)
-    if mlist.personalize <> Personalization.none:
-        if as_boolean(config.mta.verp_personalized_deliveries):
-            msgdata['verp'] = True
-    elif interval == 0:
-        # Never VERP.
-        msgdata['verp'] = False
-    elif interval == 1:
-        # VERP every time.
-        msgdata['verp'] = True
-    else:
-        # VERP every 'interval' number of times.
-        msgdata['verp'] = (int(mlist.post_id) % interval == 0)
diff --git a/src/mailman/pipeline/docs/to-outgoing.txt b/src/mailman/pipeline/docs/to-outgoing.txt
index e04620f8b..c7adca444 100644
--- a/src/mailman/pipeline/docs/to-outgoing.txt
+++ b/src/mailman/pipeline/docs/to-outgoing.txt
@@ -3,22 +3,10 @@ The outgoing handler
 ====================
 
 Mailman's outgoing queue is used as the wrapper around SMTP delivery to the
-upstream mail server.  The ToOutgoing handler does little more than drop the
-message into the outgoing queue, after calculating whether the message should
-be VERP'd or not.  VERP means Variable Envelope Return Path; we're using that
-term somewhat incorrectly, but within the spirit of the standard, which
-basically describes how to encode the recipient's address in the originator
-headers for unambigous bounce processing.
+upstream mail server.  The to-outgoing handler does little more than drop the
+message into the outgoing queue.
 
-    >>> mlist = create_list('_xtest@example.com')
-
-    >>> switchboard = config.switchboards['out']
-    >>> def queue_size():
-    ...     size = len(switchboard.files)
-    ...     for filebase in switchboard.files:
-    ...         msg, msgdata = switchboard.dequeue(filebase)
-    ...         switchboard.finish(filebase)
-    ...     return size
+    >>> mlist = create_list('test@example.com')
 
 Craft a message destined for the outgoing queue.  Include some random metadata
 as if this message had passed through some other handlers.
@@ -29,147 +17,25 @@ as if this message had passed through some other handlers.
     ... Something of great import.
     ... """)
 
-When certain conditions are met, the message will be VERP'd.  For example, if
-the message metadata already has a VERP key, this message will be VERP'd.
-
     >>> msgdata = dict(foo=1, bar=2, verp=True)
-
     >>> handler = config.handlers['to-outgoing']
     >>> handler.process(mlist, msg, msgdata)
-    >>> print msg.as_string()
-    Subject: Here is a message
-    <BLANKLINE>
-    Something of great import.
-    >>> msgdata['verp']
-    True
 
 While the queued message will not be changed, the queued metadata will have an
 additional key set: the mailing list name.
 
-    >>> filebase = switchboard.files[0]
-    >>> qmsg, qmsgdata = switchboard.dequeue(filebase)
-    >>> switchboard.finish(filebase)
-    >>> print qmsg.as_string()
+    >>> from mailman.testing.helpers import get_queue_messages
+    >>> messages = get_queue_messages('out')
+    >>> len(messages)
+    1
+    >>> print messages[0].msg.as_string()
     Subject: Here is a message
     <BLANKLINE>
     Something of great import.
-    >>> dump_msgdata(qmsgdata)
+    >>> dump_msgdata(messages[0].msgdata)
     _parsemsg: False
     bar      : 2
     foo      : 1
-    listname : _xtest@example.com
+    listname : test@example.com
     verp     : True
     version  : 3
-    >>> queue_size()
-    0
-
-If the list is set to personalize deliveries, and the global configuration
-option to VERP personalized deliveries is set, then the message will be
-VERP'd.
-
-    >>> config.push('test config', """
-    ... [mta]
-    ... verp_personalized_deliveries: yes
-    ... """)
-
-    >>> from mailman.interfaces.mailinglist import Personalization
-    >>> mlist.personalize = Personalization.individual
-    >>> msgdata = dict(foo=1, bar=2)
-    >>> handler.process(mlist, msg, msgdata)
-    >>> msgdata['verp']
-    True
-    >>> queue_size()
-    1
-
-However, if the global configuration variable prohibits VERP'ing, even
-personalized lists will not VERP.
-
-    >>> config.pop('test config')
-    >>> config.push('test config', """
-    ... [mta]
-    ... verp_personalized_deliveries: no
-    ... """)
-
-    >>> msgdata = dict(foo=1, bar=2)
-    >>> handler.process(mlist, msg, msgdata)
-    >>> print msgdata.get('verp')
-    None
-    >>> queue_size()
-    1
-
-If the list is not personalized, then the message may still be VERP'd based on
-the global configuration variable VERP_DELIVERY_INTERVAL.  This variable tells
-Mailman how often to VERP even non-personalized mailing lists.  It can be set
-to zero, which means non-personalized messages will never be VERP'd.
-
-    >>> config.pop('test config')
-    >>> config.push('test config', """
-    ... [mta]
-    ... verp_delivery_interval: 0
-    ... """)
-
-    >>> mlist.personalize = Personalization.none
-    >>> msgdata = dict(foo=1, bar=2)
-    >>> handler.process(mlist, msg, msgdata)
-    >>> print msgdata.get('verp')
-    None
-    >>> queue_size()
-    1
-
-If the interval is set to 1, then every message will be VERP'd.
-
-    >>> config.pop('test config')
-    >>> config.push('test config', """
-    ... [mta]
-    ... verp_delivery_interval: 1
-    ... """)
-
-    >>> for i in range(10):
-    ...     msgdata = dict(foo=1, bar=2)
-    ...     handler.process(mlist, msg, msgdata)
-    ...     print i, msgdata['verp']
-    0 True
-    1 True
-    2 True
-    3 True
-    4 True
-    5 True
-    6 True
-    7 True
-    8 True
-    9 True
-    >>> queue_size()
-    10
-
-If the interval is set to some other number, then one out of that many posts
-will be VERP'd.
-
-    >>> config.pop('test config')
-    >>> config.push('test config', """
-    ... [mta]
-    ... verp_delivery_interval: 3
-    ... """)
-
-    >>> for i in range(10):
-    ...     mlist.post_id = i
-    ...     msgdata = dict(foo=1, bar=2)
-    ...     handler.process(mlist, msg, msgdata)
-    ...     print i, msgdata.get('verp', False)
-    0 True
-    1 False
-    2 False
-    3 True
-    4 False
-    5 False
-    6 True
-    7 False
-    8 False
-    9 True
-    >>> queue_size()
-    10
-
-
-Clean up
-========
-
-    >>> config.pop('test config')
diff --git a/src/mailman/queue/docs/outgoing.txt b/src/mailman/queue/docs/outgoing.txt
index b121fe716..6188ab35c 100644
--- a/src/mailman/queue/docs/outgoing.txt
+++ b/src/mailman/queue/docs/outgoing.txt
@@ -3,7 +3,7 @@ Outgoing queue runner
 =====================
 
 The outgoing queue runner is the process that delivers messages to the
-directly upstream SMTP server.  It is this external SMTP server that performs
+directly upstream SMTP server.  It is this upstream SMTP server that performs
 final delivery to the intended recipients.
 
 Messages that appear in the outgoing queue are processed individually through
@@ -23,12 +23,11 @@ move messages to the 'retry queue' for handling delivery failures.
     >>> add_member(mlist, 'cperson@example.com', 'Cris Person',
     ...            'password', DeliveryMode.regular, 'en')
 
-By setting the mailing list to personalize messages, each recipient will get a
-unique copy of the message, with certain headers tailored for that recipient.
-
-    >>> from mailman.interfaces.mailinglist import Personalization
-    >>> mlist.personalize = Personalization.individual
-    >>> transaction.commit()
+Normally, messages would show up in the outgoing queue after the message has
+been processed by the rule set and pipeline.  But we can simulate that here by
+injecting a message directly into the outgoing queue.  First though, we must
+call the calculate-recipients handler so that the message metadata will be
+populated with the list of addresses to deliver the message to.
 
     >>> msg = message_from_string("""\
     ... From: aperson@example.com
@@ -39,37 +38,354 @@ unique copy of the message, with certain headers tailored for that recipient.
     ... First post!
     ... """)
 
-Normally, messages would show up in the outgoing queue after the message has
-been processed by the rule set and pipeline.  But we can simulate that here by
-injecting a message directly into the outgoing queue.
-
     >>> msgdata = {}
     >>> handler = config.handlers['calculate-recipients']
     >>> handler.process(mlist, msg, msgdata)
-
     >>> outgoing_queue = config.switchboards['out']
+
+The to-outgoing handler populates the message metadata with the destination
+mailing list name.  Simulate that here too.
+
     >>> ignore = outgoing_queue.enqueue(
     ...     msg, msgdata,
-    ...     verp=True, listname=mlist.fqdn_listname, tolist=True,
-    ...     _plaintext=True)
+    ...     tolist=True,
+    ...     listname=mlist.fqdn_listname)
 
 Running the outgoing queue runner processes the message, delivering it to the
-upstream SMTP, which happens to be our test server.
+upstream SMTP.
 
     >>> from mailman.queue.outgoing import OutgoingRunner
     >>> from mailman.testing.helpers import make_testable_runner
     >>> outgoing = make_testable_runner(OutgoingRunner, 'out')
     >>> outgoing.run()
 
-Three messages have been delivered to our SMTP server, one for each recipient.
+Every recipient got the same copy of the message.
+
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    1
+
+    >>> print messages[0].as_string()
+    From: aperson@example.com
+    To: test@example.com
+    Subject: My first post
+    Message-ID: <first>
+    Sender: test-bounces@example.com
+    Errors-To: test-bounces@example.com
+    X-Peer: ...
+    X-MailFrom: test-bounces@example.com
+    X-RcptTo: cperson@example.com, bperson@example.com, aperson@example.com
+    <BLANKLINE>
+    First post!
+
+
+Personalization
+===============
+
+Mailman supports sending individual messages to each recipient by
+personalizing delivery.  This increases the bandwidth between Mailman and the
+upstream mail server, and between the upstream mail server and the remote
+recipient mail servers.  The benefit is that personalization provides for a
+much better user experience, because the messages can be tailored for each
+recipient.
+
+    >>> from mailman.interfaces.mailinglist import Personalization
+    >>> mlist.personalize = Personalization.individual
+    >>> transaction.commit()
+
+Now when we send the message, our mail server will get three copies instead of
+just one.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    3
+
+Since we've done no other configuration, the only difference in the messages
+is the recipient address.  Specifically, the Sender header is the same for all
+recipients.
 
     >>> from operator import itemgetter
-    >>> messages = sorted(smtpd.messages, key=itemgetter('sender'))
+    >>> def show_headers(messages):
+    ...     for message in sorted(messages, key=itemgetter('x-rcptto')):
+    ...         print message['X-RcptTo'], message['Sender']
+
+    >>> show_headers(messages)
+    aperson@example.com   test-bounces@example.com
+    bperson@example.com   test-bounces@example.com
+    cperson@example.com   test-bounces@example.com
+
+
+VERP
+====
+
+An even more interesting personalization opportunity arises if VERP_ is
+enabled.  Here, Mailman takes advantage of the fact that it's sending
+individualized messages anyway, so it also encodes the recipients address in
+the Sender header.
+
+.. _VERP: ../../mta/docs/verp.html
+
+
+Forcing VERP
+------------
+
+A handler can force VERP by setting the 'verp' key in the message metadata.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     verp=True,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    3
+
+    >>> show_headers(messages)
+    aperson@example.com   test-bounces+aperson=example.com@example.com
+    bperson@example.com   test-bounces+bperson=example.com@example.com
+    cperson@example.com   test-bounces+cperson=example.com@example.com
+
+
+VERP personalized deliveries
+----------------------------
+
+The site administrator can enable VERP whenever messages are personalized.
+
+    >>> config.push('verp', """
+    ... [mta]
+    ... verp_personalized_deliveries: yes
+    ... """)
+
+Again, we get three individual messages, with VERP'd Sender headers.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
     >>> len(messages)
     3
 
-    >>> for message in messages:
-    ...     print message['sender']
-    test-bounces+aperson=example.com@example.com
-    test-bounces+bperson=example.com@example.com
-    test-bounces+cperson=example.com@example.com
+    >>> show_headers(messages)
+    aperson@example.com   test-bounces+aperson=example.com@example.com
+    bperson@example.com   test-bounces+bperson=example.com@example.com
+    cperson@example.com   test-bounces+cperson=example.com@example.com
+
+    >>> config.pop('verp')
+    >>> mlist.personalize = Personalization.none
+    >>> transaction.commit()
+
+
+VERP every once in a while
+--------------------------
+
+Perhaps personalization is too much of an overhead, but the list owners would
+still like to occasionally get the benefits of VERP.  The site administrator
+can enable occasional VERPing of messages every so often, by setting a
+delivery interval.  Every N non-personalized deliveries turns on VERP for just
+the next one.
+
+    >>> config.push('verp occasionally', """
+    ... [mta]
+    ... verp_delivery_interval: 3
+    ... """)
+
+    # Reset the list's post_id, which is used to calculate the intervals.
+    >>> mlist.post_id = 1
+    >>> transaction.commit()
+
+The first message is sent to the list, and it is neither personalized nor
+VERP'd.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    1
+
+    >>> show_headers(messages)
+    cperson@example.com, bperson@example.com, aperson@example.com
+    test-bounces@example.com
+
+    # Perform post-delivery bookkeeping.
+    >>> after = config.handlers['after-delivery']
+    >>> after.process(mlist, msg, msgdata)
+    >>> transaction.commit()
+
+The second message sent to the list is also not VERP'd.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    1
+
+    >>> show_headers(messages)
+    cperson@example.com, bperson@example.com, aperson@example.com
+    test-bounces@example.com
+
+    # Perform post-delivery bookkeeping.
+    >>> after.process(mlist, msg, msgdata)
+    >>> transaction.commit()
+
+The third message though is VERP'd.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    3
+
+    >>> show_headers(messages)
+    aperson@example.com   test-bounces+aperson=example.com@example.com
+    bperson@example.com   test-bounces+bperson=example.com@example.com
+    cperson@example.com   test-bounces+cperson=example.com@example.com
+
+    # Perform post-delivery bookkeeping.
+    >>> after.process(mlist, msg, msgdata)
+    >>> transaction.commit()
+
+The next one is back to bulk delivery.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    1
+
+    >>> show_headers(messages)
+    cperson@example.com, bperson@example.com, aperson@example.com
+    test-bounces@example.com
+
+    >>> config.pop('verp occasionally')
+
+
+VERP every time
+---------------
+
+If the site administrator wants to enable VERP for every delivery, even if no
+personalization is going on, they can set the interval to 1.
+
+    >>> config.push('always verp', """
+    ... [mta]
+    ... verp_delivery_interval: 1
+    ... """)
+
+    # Reset the list's post_id, which is used to calculate the intervals.
+    >>> mlist.post_id = 1
+    >>> transaction.commit()
+
+The first message is VERP'd.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    3
+
+    >>> show_headers(messages)
+    aperson@example.com   test-bounces+aperson=example.com@example.com
+    bperson@example.com   test-bounces+bperson=example.com@example.com
+    cperson@example.com   test-bounces+cperson=example.com@example.com
+
+    # Perform post-delivery bookkeeping.
+    >>> after.process(mlist, msg, msgdata)
+    >>> transaction.commit()
+
+As is the second message.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    3
+
+    >>> show_headers(messages)
+    aperson@example.com   test-bounces+aperson=example.com@example.com
+    bperson@example.com   test-bounces+bperson=example.com@example.com
+    cperson@example.com   test-bounces+cperson=example.com@example.com
+
+    # Perform post-delivery bookkeeping.
+    >>> after.process(mlist, msg, msgdata)
+    >>> transaction.commit()
+
+And the third message.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    3
+
+    >>> show_headers(messages)
+    aperson@example.com   test-bounces+aperson=example.com@example.com
+    bperson@example.com   test-bounces+bperson=example.com@example.com
+    cperson@example.com   test-bounces+cperson=example.com@example.com
+
+    # Perform post-delivery bookkeeping.
+    >>> after.process(mlist, msg, msgdata)
+    >>> transaction.commit()
+
+    >>> config.pop('always verp')
+
+
+Never VERP
+----------
+
+Similarly, the site administrator can disable occasional VERP'ing of
+non-personalized messages by setting the interval to zero.
+
+    >>> config.push('never verp', """
+    ... [mta]
+    ... verp_delivery_interval: 0
+    ... """)
+
+    # Reset the list's post_id, which is used to calculate the intervals.
+    >>> mlist.post_id = 1
+    >>> transaction.commit()
+
+Neither the first message...
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    1
+
+    >>> show_headers(messages)
+    cperson@example.com, bperson@example.com, aperson@example.com
+    test-bounces@example.com
+
+...nor the second message is VERP'd.
+
+    >>> ignore = outgoing_queue.enqueue(
+    ...     msg, msgdata,
+    ...     listname=mlist.fqdn_listname)
+    >>> outgoing.run()
+    >>> messages = list(smtpd.messages)
+    >>> len(messages)
+    1
+
+    >>> show_headers(messages)
+    cperson@example.com, bperson@example.com, aperson@example.com
+    test-bounces@example.com
diff --git a/src/mailman/queue/outgoing.py b/src/mailman/queue/outgoing.py
index b1adbe411..5a191ad61 100644
--- a/src/mailman/queue/outgoing.py
+++ b/src/mailman/queue/outgoing.py
@@ -23,9 +23,10 @@ import socket
 import logging
 
 from datetime import datetime
-from lazr.config import as_timedelta
+from lazr.config import as_boolean, as_timedelta
 
 from mailman.config import config
+from mailman.interfaces.mailinglist import Personalization
 from mailman.interfaces.mta import SomeRecipientsFailed
 from mailman.queue import Runner
 from mailman.queue.bounce import BounceMixin
@@ -59,14 +60,31 @@ class OutgoingRunner(Runner, BounceMixin):
         deliver_after = msgdata.get('deliver_after', datetime.fromtimestamp(0))
         if datetime.now() < deliver_after:
             return True
-        # Make sure we have the most up-to-date state
+        # Calculate whether we should VERP this message or not.  The results of
+        # this set the 'verp' key in the message metadata.
+        interval = int(config.mta.verp_delivery_interval)
+        if 'verp' in msgdata:
+            # Honor existing settings.
+            pass
+        # If personalization is enabled for this list and we've configured
+        # Mailman to always VERP personalized deliveries, then yes we VERP it.
+        # Also, if personalization is /not/ enabled, but
+        # verp_delivery_interval is set (and we've hit this interval), then
+        # again, this message should be VERP'd. Otherwise, no.
+        elif mlist.personalize <> Personalization.none:
+            if as_boolean(config.mta.verp_personalized_deliveries):
+                msgdata['verp'] = True
+        elif interval == 0:
+            # Never VERP.
+            msgdata['verp'] = False
+        elif interval == 1:
+            # VERP every time.
+            msgdata['verp'] = True
+        else:
+            # VERP every 'interval' number of times.
+            msgdata['verp'] = (mlist.post_id % interval == 0)
         try:
-            pid = os.getpid()
             self._func(mlist, msg, msgdata)
-            # Failsafe -- a child may have leaked through.
-            if pid <> os.getpid():
-                log.error('child process leaked thru: %s', pid)
-                os._exit(1)
             self._logged = False
         except socket.error:
             # There was a problem connecting to the SMTP server.  Log this