Merge lp:~vorlon/bzr-email/extra-headers into lp:bzr-email

[Steve Langasek]

Add support for including custom headers in generated mails, necessary for interfacing with the Debian PTS

[Robert Collins (lifeless)]

 review: needsfixing

One crucial thing, three smaller.

Crucially: please call the option
revision_mail_headers
and document it in the docstring in __init__.py - thats where the help
for 'bzr help email' is sourced. I think there are some docs in README
too, that are duplicated but should be kept in sync.

Smaller things:
Characters are nearly free: s/xhdrs/extra_mail_headers/ throughout
please: the 8 bit days are long gone ;)

A test case for this would be great.

In the sending function you do
xhdrs = self.extra_headers()
....
  (
   , xhdrs)

and don't use the variable at all. Please just call it when you call the
lower level function.

-Rob

review: Needs Fixing

[Robert Collins (lifeless)]

Thanks for this. There were some more nits:

Your docs were not enough to show someone how to set a header, so I've
improved on that.

Please set your editor not to use tabs when editing python code.

+ try:
+ key,value = line.split(": ")
+ result[key] = value
+ except ValueError: pass

Is something I missed - it seems to imply silently not doing what we
have been asked to do, so I'm deleting the try/except here.
Alternatively we could mutter it to the log.

In the same region, types should be compared with 'is', not ==.

try:
    for key in extra_mail_headers:
       msg[key] = extra_mail_headers[key]
except TypeError: pass

is a problem - a type error here could mean several different things, so
I've rewritten it to be safer.

In a related thing, its unidiomatic to do
except TypeError: pass
rather than
except TypeError:
    pass

Thanks again for this. Tests still pass as I am committing this, but I
note you didn't add a test that your extra headers /get/ all the way
through, so you may wish to do a manual check of trunk now.

-Rob

=== modified file '__init__.py'
--- __init__.py	2008-12-09 20:09:15 +0000
+++ __init__.py	2010-03-10 18:38:20 +0000
@@ -58,6 +58,9 @@
  - Any other value: Run the value expecting it to behave like ``/usr/bin/mail``
    - in particular supporting the -s and -a options.
 
+When using smtplib, you can specify additional headers to be included in the
+mail by setting the 'revision_mail_headers' configuration option.
+
 """
 
 

=== modified file 'emailer.py'
--- emailer.py	2008-12-09 20:09:15 +0000
+++ emailer.py	2010-03-10 18:38:20 +0000
@@ -167,6 +167,19 @@
             result = self.config.username()
         return result
 
+    def extra_headers(self):
+        """Additional headers to include when sending."""
+        result = {}
+        headers = self.config.get_user_option('revision_mail_headers')
+        if type(headers) != list:
+            headers = [headers]
+        for line in headers:
+            try:
+                key,value = line.split(": ")
+                result[key] = value
+            except ValueError: pass
+        return result
+
     def send(self):
         """Send the email.
 
@@ -229,7 +242,8 @@
         smtp = self._smtplib_implementation(self.config)
         smtp.send_text_and_attachment_email(from_addr, to_addrs,
                                             subject, body, diff,
-                                            self.diff_filename())
+                                            self.diff_filename(),
+                                            self.extra_headers())
 
     def should_send(self):
         result = self.config.get_user_option('post_commit_difflimit')

=== modified file 'smtp_connection.py'
--- smtp_connection.py	2009-03-10 02:24:28 +0000
+++ smtp_connection.py	2010-03-10 18:38:20 +0000
@@ -123,7 +123,8 @@
         """
         return parseaddr(address)
 
-    def _basic_message(self, from_address, to_addresses, subject):
+    def _basic_message(self, from_address, to_addresses, subject,
+                       extra_mail_headers=None):
         """Create the basic Message using the right Header info.
 
         This creates an email Message with no payload.
@@ -146,6 +147,12 @@
         msg['From'] = '%s <%s>' % (Header(unicode(from_user)), from_email)
         msg['User-Agent'] = 'bzr/%s' % _bzrlib_version
 
+        # MIMEMultipart doesn't support update()
+        try:
+            for key in extra_mail_headers:
+                msg[key] = extra_mail_headers[key]
+        except TypeError: pass
+
         to_emails = []
         to_header = []
         for addr in to_addresses:
@@ -157,7 +164,8 @@
         msg['Subject'] = Header(subject)
         return msg, from_email, to_emails
 
-    def create_email(self, from_address, to_addresses, subject, text):
+    def create_email(self, from_address, to_addresses, subject, text,
+                     extra_mail_headers=None):
         """Create an email.Message object.
 
         This function allows you to create a basic email, and then add extra
@@ -178,7 +186,8 @@
             to_emails: the list of email addresses extracted from to_addresses
         """
         msg, from_email, to_emails = self._basic_message(from_address,
-                                                         to_addresses, subject)
+                                                         to_addresses, subject,
+                                                         extra_mail_headers)
         payload = MIMEText(text.encode('utf-8'), 'plain', 'utf-8')
         msg.attach(payload)
         return msg, from_email, to_emails
@@ -214,7 +223,8 @@
 
     def send_text_and_attachment_email(self, from_address, to_addresses,
                                        subject, message, attachment_text,
-                                       attachment_filename='patch.diff'):
+                                       attachment_filename='patch.diff',
+                                       extra_mail_headers=None):
         """Send a Unicode message and an 8-bit attachment.
 
         See create_email for common parameter definitions.
@@ -225,7 +235,8 @@
             give a default name for email programs to save the attachment.
         """
         msg, from_email, to_emails = self.create_email(from_address,
-                                            to_addresses, subject, message)
+                                            to_addresses, subject, message,
+                                            extra_mail_headers)
         # Must be an 8-bit string
         assert isinstance(attachment_text, str)
 

=== modified file 'tests/testemail.py'
--- tests/testemail.py	2008-12-09 20:09:15 +0000
+++ tests/testemail.py	2010-03-10 18:38:20 +0000
@@ -34,7 +34,8 @@
 
 sample_config=("[DEFAULT]\n"
                "post_commit_to=[e-mail]\n"
-               "post_commit_sender=Sample <[e-mail]>\n")
+               "post_commit_sender=Sample <[e-mail]>\n"
+               "revision_mail_headers=X-Cheese: to the rescue!\n")
 
 unconfigured_config=("[DEFAULT]\n"
                      "email=Robert <[e-mail]>\n")
@@ -150,6 +151,10 @@
         sender = self.get_sender()
         self.assertEqual('patch-1.diff', sender.diff_filename())
 
+    def test_headers(self):
+        sender = self.get_sender()
+	self.assertEqual({'X-Cheese': 'to the rescue!'}, sender.extra_headers())
+
     def get_sender(self, text=sample_config):
         self.branch = BzrDir.create_branch_convenience('.')
         tree = self.branch.bzrdir.open_workingtree()