Fix header/footer interpolations when personalizing messages.

- When doing individual deliveries, insert a 'member' key into the copy of the
  metadata dictionary for this recipient's delivery.  This will contain the
  IMember of the recipient, if the recipient is a member of the mailing list.
  There will still be a 'recipient' key which will contain just the email
  address to deliver the message to.

- Remove $user_password from header/footer placeholders.

- Remove the 'personalize' key from the metadata dictionary and change
  decorate.process() to search only for the 'member' key.  No need for both of
  them and the 'member' key contains more information.  Plus, it allows us to
  do a more efficient member query in the delivery module some time in the
  future.

- Move some of the LMTP log messages from mailman.runner to mailman.smtp.

5 files changed, 168 insertions, 29 deletions

diff --git a/src/mailman/mta/base.py b/src/mailman/mta/base.py
index 375d79fb8..7873d66e9 100644
--- a/src/mailman/mta/base.py
+++ b/src/mailman/mta/base.py
@@ -139,7 +139,7 @@ class IndividualDelivery(BaseDelivery):
 
     def __init__(self):
         """See `BaseDelivery`."""
-        # 
+        #
         super(IndividualDelivery, self).__init__()
         self.callbacks = []
 
@@ -162,6 +162,12 @@ class IndividualDelivery(BaseDelivery):
             # That way the subclass's _get_sender() override can encode the
             # recipient address in the sender, e.g. for VERP.
             msgdata_copy['recipient'] = recipient
+            # See if the recipient is a member of the mailing list, and if so,
+            # squirrel this information away for use by other modules, such as
+            # the header/footer decorator.  XXX 2012-03-05 this is probably
+            # highly inefficient on the database.
+            member = mlist.members.get_member(recipient)
+            msgdata_copy['member'] = member
             for callback in self.callbacks:
                 callback(mlist, message_copy, msgdata_copy)
             status = self._deliver_to_recipients(
diff --git a/src/mailman/mta/docs/decorating.rst b/src/mailman/mta/docs/decorating.rst
index 44559edb3..cf595b0d5 100644
--- a/src/mailman/mta/docs/decorating.rst
+++ b/src/mailman/mta/docs/decorating.rst
@@ -43,7 +43,6 @@ We start by writing the site-global header and footer template.
     >>> with open(myfooter_path, 'w') as fp:
     ...     print >> fp, """\
     ... User name: $user_name
-    ... Password: $user_password
     ... Language: $user_language
     ... Options: $user_optionsurl
     ... """
@@ -54,7 +53,7 @@ these are site-global templates, we can use a shorted URL.
     >>> mlist = create_list('test@example.com')
     >>> mlist.header_uri = 'mailman:///myheader.txt'
     >>> mlist.footer_uri = 'mailman:///myfooter.txt'
-    
+
     >>> transaction.commit()
 
     >>> msg = message_from_string("""\
@@ -87,17 +86,14 @@ list.
     >>> user_manager = getUtility(IUserManager)
 
     >>> anne = user_manager.create_user('aperson@example.com', 'Anne Person')
-    >>> anne.password = b'AAA'
     >>> mlist.subscribe(list(anne.addresses)[0], MemberRole.member)
     <Member: Anne Person <aperson@example.com> ...
 
     >>> bart = user_manager.create_user('bperson@example.com', 'Bart Person')
-    >>> bart.password = b'BBB'
     >>> mlist.subscribe(list(bart.addresses)[0], MemberRole.member)
     <Member: Bart Person <bperson@example.com> ...
 
     >>> cris = user_manager.create_user('cperson@example.com', 'Cris Person')
-    >>> cris.password = b'CCC'
     >>> mlist.subscribe(list(cris.addresses)[0], MemberRole.member)
     <Member: Cris Person <cperson@example.com> ...
 
@@ -129,7 +125,6 @@ The decorations happen when the message is delivered.
     Subscribed address: aperson@example.com
     This is a test.
     User name: Anne Person
-    Password: AAA
     Language: English (USA)
     Options: http://example.com/aperson@example.com
     ----------
@@ -148,7 +143,6 @@ The decorations happen when the message is delivered.
     Subscribed address: bperson@example.com
     This is a test.
     User name: Bart Person
-    Password: BBB
     Language: English (USA)
     Options: http://example.com/bperson@example.com
     ----------
@@ -167,7 +161,6 @@ The decorations happen when the message is delivered.
     Subscribed address: cperson@example.com
     This is a test.
     User name: Cris Person
-    Password: CCC
     Language: English (USA)
     Options: http://example.com/cperson@example.com
     ----------
diff --git a/src/mailman/mta/tests/test_delivery.py b/src/mailman/mta/tests/test_delivery.py
new file mode 100644
index 000000000..1e362d755
--- /dev/null
+++ b/src/mailman/mta/tests/test_delivery.py
@@ -0,0 +1,144 @@
+# 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 various aspects of email delivery."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'TestIndividualDelivery',
+    ]
+
+
+import os
+import shutil
+import tempfile
+import unittest
+
+from mailman.app.lifecycle import create_list
+from mailman.app.membership import add_member
+from mailman.config import config
+from mailman.interfaces.mailinglist import Personalization
+from mailman.interfaces.member import DeliveryMode
+from mailman.mta.deliver import Deliver
+from mailman.testing.helpers import (
+    specialized_message_from_string as mfs)
+from mailman.testing.layers import ConfigLayer
+
+
+
+# Global test capture.
+_deliveries = []
+
+# Derive this from the default individual delivery class.  The point being
+# that we don't want to *actually* attempt delivery of the message to the MTA,
+# we just want to capture the messages and metadata dictionaries for
+# inspection.
+class DeliverTester(Deliver):
+    def _deliver_to_recipients(self, mlist, msg, msgdata, recipients):
+        _deliveries.append((mlist, msg, msgdata, recipients))
+        # Nothing gets refused.
+        return []
+
+
+
+class TestIndividualDelivery(unittest.TestCase):
+    """Test personalized delivery details."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+        self._mlist.personalize = Personalization.individual
+        # Make Anne a member of this mailing list.
+        self._anne = add_member(self._mlist,
+                                'anne@example.org', 'Anne Person',
+                                'xyz', DeliveryMode.regular, 'en')
+        # Clear out any results from the previous test.
+        del _deliveries[:]
+        self._msg = mfs("""\
+From: anne@example.org
+To: test@example.com
+Subject: test
+
+""")
+        # Set up a personalized footer for decoration.
+        self._template_dir = tempfile.mkdtemp()
+        path = os.path.join(self._template_dir,
+                            'site', 'en', 'member-footer.txt')
+        os.makedirs(os.path.dirname(path))
+        with open(path, 'w') as fp:
+            print("""\
+address  : $user_address
+delivered: $user_delivered_to
+language : $user_language
+name     : $user_name
+options  : $user_optionsurl
+""", file=fp)
+        config.push('templates', """
+        [paths.testing]
+        template_dir: {0}
+        """.format(self._template_dir))
+        self._mlist.footer_uri = 'mailman:///member-footer.txt'
+
+    def tearDown(self):
+        # Free references.
+        del _deliveries[:]
+        shutil.rmtree(self._template_dir)
+        config.pop('templates')
+
+    def test_member_key(self):
+        # 'personalize' should end up in the metadata dictionary so that
+        # $user_* keys in headers and footers get filled in correctly.
+        msgdata = dict(recipients=['anne@example.org'])
+        agent = DeliverTester()
+        refused = agent.deliver(self._mlist, self._msg, msgdata)
+        self.assertEqual(len(refused), 0)
+        self.assertEqual(len(_deliveries), 1)
+        _mlist, _msg, _msgdata, _recipients = _deliveries[0]
+        member = _msgdata.get('member')
+        self.assertEqual(member, self._anne)
+
+    def test_decoration(self):
+        msgdata = dict(recipients=['anne@example.org'])
+        agent = DeliverTester()
+        refused = agent.deliver(self._mlist, self._msg, msgdata)
+        self.assertEqual(len(refused), 0)
+        self.assertEqual(len(_deliveries), 1)
+        _mlist, _msg, _msgdata, _recipients = _deliveries[0]
+        try:
+            eq = self.assertMultiLineEqual
+        except AttributeError:
+            # Python 2.6
+            eq = self.assertEqual
+        eq(_msg.as_string(), """\
+From: anne@example.org
+To: test@example.com
+Subject: test
+MIME-Version: 1.0
+Content-Type: text/plain; charset="us-ascii"
+Content-Transfer-Encoding: 7bit
+
+
+address  : anne@example.org
+delivered: anne@example.org
+language : English (USA)
+name     : Anne Person
+options  : http://example.com/anne@example.org
+
+""")
diff --git a/src/mailman/pipeline/decorate.py b/src/mailman/pipeline/decorate.py
index a8fc340f8..b785e5ecf 100644
--- a/src/mailman/pipeline/decorate.py
+++ b/src/mailman/pipeline/decorate.py
@@ -51,21 +51,16 @@ def process(mlist, msg, msgdata):
     if msgdata.get('isdigest') or msgdata.get('nodecorate'):
         return
     d = {}
-    if msgdata.get('personalize'):
-        # Calculate the extra personalization dictionary.  Note that the
-        # length of the recips list better be exactly 1.
-        recipient = msgdata['recipient']
-        user = getUtility(IUserManager).get_user(recipient)
-        member = mlist.members.get_member(recipient)
+    member = msgdata.get('member')
+    if member is not None:
+        # Calculate the extra personalization dictionary.
+        recipient = msgdata.get('recipient', member.address.original_email)
         d['user_address'] = recipient
-        if user is not None and member is not None:
-            d['user_delivered_to'] = member.address.original_email
-            # BAW: Hmm, should we allow this?
-            d['user_password'] = user.password
-            d['user_language'] = member.preferred_language.description
-            d['user_name'] = (user.real_name if user.real_name
-                              else member.address.original_email)
-            d['user_optionsurl'] = member.options_url
+        d['user_delivered_to'] = member.address.original_email
+        d['user_language'] = member.preferred_language.description
+        d['user_name'] = (member.user.real_name if member.user.real_name
+                          else member.address.original_email)
+        d['user_optionsurl'] = member.options_url
     # These strings are descriptive for the log file and shouldn't be i18n'd
     d.update(msgdata.get('decoration-data', {}))
     try:
diff --git a/src/mailman/runners/lmtp.py b/src/mailman/runners/lmtp.py
index 483b63049..6824491eb 100644
--- a/src/mailman/runners/lmtp.py
+++ b/src/mailman/runners/lmtp.py
@@ -47,6 +47,7 @@ from mailman.interfaces.listmanager import IListManager
 
 elog = logging.getLogger('mailman.error')
 qlog = logging.getLogger('mailman.runner')
+slog = logging.getLogger('mailman.smtp')
 
 
 # We only care about the listname and the sub-addresses as in listname@ or
@@ -147,7 +148,7 @@ class LMTPRunner(Runner, smtpd.SMTPServer):
     def handle_accept(self):
         conn, addr = self.accept()
         Channel(self, conn, addr)
-        qlog.debug('LMTP accept from %s', addr)
+        slog.debug('LMTP accept from %s', addr)
 
     @txn
     def process_message(self, peer, mailfrom, rcpttos, data):
@@ -156,7 +157,7 @@ class LMTPRunner(Runner, smtpd.SMTPServer):
             # since the set of mailing lists could have changed.
             listnames = set(getUtility(IListManager).names)
             # Parse the message data.  If there are any defects in the
-            # message, reject it right away; it's probably spam. 
+            # message, reject it right away; it's probably spam.
             msg = email.message_from_string(data, Message)
             msg.original_size = len(data)
             if msg.defects:
@@ -177,7 +178,7 @@ class LMTPRunner(Runner, smtpd.SMTPServer):
             try:
                 to = parseaddr(to)[1].lower()
                 listname, subaddress, domain = split_recipient(to)
-                qlog.debug('%s to: %s, list: %s, sub: %s, dom: %s',
+                slog.debug('%s to: %s, list: %s, sub: %s, dom: %s',
                            message_id, to, listname, subaddress, domain)
                 listname += '@' + domain
                 if listname not in listnames:
@@ -197,7 +198,7 @@ class LMTPRunner(Runner, smtpd.SMTPServer):
                     queue = 'in'
                 elif canonical_subaddress is None:
                     # The subaddress was bogus.
-                    elog.error('%s unknown sub-address: %s',
+                    slog.error('%s unknown sub-address: %s',
                                message_id, subaddress)
                     status.append(ERR_550)
                     continue
@@ -214,11 +215,11 @@ class LMTPRunner(Runner, smtpd.SMTPServer):
                 # a success status for this recipient.
                 if queue is not None:
                     config.switchboards[queue].enqueue(msg, msgdata)
-                    qlog.debug('%s subaddress: %s, queue: %s',
+                    slog.debug('%s subaddress: %s, queue: %s',
                                message_id, canonical_subaddress, queue)
                     status.append('250 Ok')
             except Exception:
-                elog.exception('Queue detection: %s', msg['message-id'])
+                slog.exception('Queue detection: %s', msg['message-id'])
                 config.db.abort()
                 status.append(ERR_550)
         # All done; returning this big status string should give the expected