Merge lp:~sinzui/launchpad/mailman-email-addresses into lp:launchpad

[Curtis Hovey]

Send mailmain lowercased email addresses.

This is triggering nagios alerts, as the current MARK delta alert is set
to 40 minutes. Mailman is configured to write write MARKs to the log every
5 minutes.

Mailman's xmlrpc runner log shows it spends more than 99% of its time
acquiring locks to mailing lists to write email address cases changes to
disk. The email address are lower-cased though, so the change is no-op.

--------------------------------------------------------------------

RULES

    Pre-implementation: no one
    * Lowercase the email addresses that are sent to mailman.

QA

    * Review the staging mailman/xmlrc log or run
      fgrep MARK /path/to/mailman/logs/xmlrpc | tail -100
    * Verify the log does not show mix-case changes.
    * After release and after logs mailman logs are sync to carob
      review the tail of /srv/launchpad.net-logs/mailman/polevik/xmlrpc
    * verify the log does not show wasted work saving mixed-case email
      addresses.
    * Is MARK appearing closer to every 5 minutes?

LINT

    lib/lp/registry/doc/message-holds.txt
    lib/lp/registry/model/mailinglist.py
    lib/lp/registry/tests/test_mailinglist.py

TEST

    ./bin/test -vv -t TestMailinglistSet lp.registry.tests.test_mailinglist

LoC

    This branch adds two tests, but removes "Posting privileges" section
    from a doctest because the behaviour is test elsewhere in the doctest
    and explicitly in:
    TestMailinglistSetMessages.test_getSenderAddresses_approved_dict_values

IMPLEMENTATION

Lowercase the email address return by IMailingListSet getSenderAddresses()
and getSubscribedAddresses().
    lib/lp/registry/model/mailinglist.py
    lib/lp/registry/tests/test_mailinglist.py

Removed test that shows approving a held message add the use to
getSubscribedAddresses().
    lib/lp/registry/doc/message-holds.txt

[j.c.sackett (jcsackett)]

Thanks.

review: Approve

=== modified file 'lib/lp/registry/doc/message-holds.txt'
--- lib/lp/registry/doc/message-holds.txt	2012-01-04 06:52:11 +0000
+++ lib/lp/registry/doc/message-holds.txt	2012-09-13 18:04:22 +0000
@@ -372,90 +372,6 @@
     The Launchpad team
     ----------------------------------------
 
-
-== Posting privileges ==
-Message approvals are also used to derived posting privileges for non-team
-members.  To start with, only the team owner can post to the list.
-
-    >>> login('[e-mail]')
-    >>> team_three, list_three = mailinglists_helper.new_team(
-    ...     'test-three', True)
-    >>> sorted(list_three.getSenderAddresses())
-    [u'[e-mail]']
-
-Salgado posts a message to the list, but because he is not a team member, it
-gets held for approval.
-
-    >>> message = message_set.fromEmail("""\
-    ... From: [e-mail]
-    ... To: [e-mail]
-    ... Subject: My first test
-    ... Message-ID: <zebra>
-    ... Date: Fri, 01 Aug 2000 01:08:59 -0000
-    ...
-    ... This is my first post!
-    ... """)
-    >>> held_message = list_three.holdMessage(message)
-    >>> transaction.commit()
-
-If the message gets discarded (or rejected), Salgado is still not allowed to
-post to the mailing list.
-
-    >>> held_message.discard(owner)
-    >>> held_message.acknowledge()
-    >>> transaction.commit()
-
-    >>> sorted(list_three.getSenderAddresses())
-    [u'[e-mail]']
-
-    >>> from lp.registry.interfaces.mailinglist import IMailingListSet
-    >>> mailinglist_set = getUtility(IMailingListSet)
-    >>> bulk_addresses = mailinglist_set.getSenderAddresses(['test-three'])
-    >>> sorted(email for name, email in bulk_addresses['test-three'])
-    [u'[e-mail]']
-
-Salgado posts another message to the list, but this time it's approved.
-
-    >>> message = message_set.fromEmail("""\
-    ... From: [e-mail]
-    ... To: [e-mail]
-    ... Subject: My first test
-    ... Message-ID: <yeti>
-    ... Date: Fri, 01 Aug 2000 01:08:59 -0000
-    ...
-    ... This is my second post!
-    ... """)
-    >>> held_message = list_three.holdMessage(message)
-    >>> transaction.commit()
-
-    >>> held_message.approve(owner)
-    >>> held_message.acknowledge()
-    >>> transaction.commit()
-
-Now, Salgado is on the list of approved posters.
-
-    >>> sorted(list_three.getSenderAddresses())
-    [u'[e-mail]', u'[e-mail]']
-
-    >>> bulk_addresses = mailinglist_set.getSenderAddresses(['test-three'])
-    >>> sorted(email for name, email in bulk_addresses['test-three'])
-    [u'[e-mail]', u'[e-mail]']
-
-But he will still not get messages delivered to his address, since he's not
-subscribed to the team mailing list.
-
-    >>> sorted(list_three.getSubscribedAddresses())
-    []
-
-Of course, Salgado still cannot post to a mailing list that he has not been
-approved for, because he has not had three approved moderations.
-
-    >>> team_four, list_four = mailinglists_helper.new_team(
-    ...     'test-four', True)
-    >>> sorted(list_four.getSenderAddresses())
-    [u'[e-mail]']
-
-
 == Duplicates ==
 
 When a person responds to a bug email, the message can end up in the database

=== modified file 'lib/lp/registry/model/mailinglist.py'
--- lib/lp/registry/model/mailinglist.py	2012-01-04 22:30:45 +0000
+++ lib/lp/registry/model/mailinglist.py	2012-09-13 18:04:22 +0000
@@ -616,7 +616,7 @@
         for email, display_name, team_name in preferred:
             assert team_name in team_names, (
                 'Unexpected team name in results: %s' % team_name)
-            value = (display_name, email)
+            value = (display_name, email.lower())
             by_team[team_name].add(value)
         explicit = store.using(*tables).find(
             (EmailAddress.email, Person.displayname, Team.name),
@@ -628,7 +628,7 @@
         for email, display_name, team_name in explicit:
             assert team_name in team_names, (
                 'Unexpected team name in results: %s' % team_name)
-            value = (display_name, email)
+            value = (display_name, email.lower())
             by_team[team_name].add(value)
         # Turn the results into a mapping of lists.
         results = {}
@@ -688,7 +688,7 @@
         for team_name, person_displayname, email in all_posters:
             assert team_name in team_names, (
                 'Unexpected team name in results: %s' % team_name)
-            value = (person_displayname, email)
+            value = (person_displayname, email.lower())
             by_team[team_name].add(value)
         # Turn the results into a mapping of lists.
         results = {}

=== modified file 'lib/lp/registry/tests/test_mailinglist.py'
--- lib/lp/registry/tests/test_mailinglist.py	2012-08-13 19:51:16 +0000
+++ lib/lp/registry/tests/test_mailinglist.py	2012-09-13 18:04:22 +0000
@@ -108,6 +108,20 @@
             for m in team1.allmembers])
         self.assertEqual(list_senders, sorted(result[team1.name]))
 
+    def test_getSenderAddresses_multiple_and_lowercase_email(self):
+        # getSenderAddresses() contains multiple email addresses for
+        # users and they are lowercased for mailman.
+        # {team_name: [(member_displayname, member_email) ...]}
+        team1, member1 = self.factory.makeTeamWithMailingListSubscribers(
+            'team1', auto_subscribe=False)
+        email = self.factory.makeEmail('[e-mail]', member1)
+        result = self.mailing_list_set.getSenderAddresses([team1.name])
+        list_senders = sorted([
+            (m.displayname, m.preferredemail.email)
+            for m in team1.allmembers])
+        list_senders.append((member1.displayname, email.email.lower()))
+        self.assertContentEqual(list_senders, result[team1.name])
+
     def test_getSenderAddresses_participation_dict_values(self):
         # getSenderAddresses() dict values includes indirect participants.
         team1, member1 = self.factory.makeTeamWithMailingListSubscribers(
@@ -140,6 +154,25 @@
             (member1.displayname, member1.preferredemail.email)]
         self.assertEqual(list_subscribers, result[team1.name])
 
+    def test_getSubscribedAddresses_multiple_lowercase_email(self):
+        # getSubscribedAddresses() contains email addresses for
+        # users and they are lowercased for mailman. The email maybe
+        # explicitly set instead of the preferred email.
+        # {team_name: [(member_displayname, member_email) ...]}
+        team1, member1 = self.factory.makeTeamWithMailingListSubscribers(
+            'team1')
+        with person_logged_in(member1):
+            email1 = self.factory.makeEmail('[e-mail]', member1)
+            member1.setPreferredEmail(email1)
+        with person_logged_in(team1.teamowner):
+            email2 = self.factory.makeEmail('[e-mail]', team1.teamowner)
+            team1.mailing_list.subscribe(team1.teamowner, email2)
+        result = self.mailing_list_set.getSubscribedAddresses([team1.name])
+        list_subscribers = [
+            (member1.displayname, email1.email.lower()),
+            (team1.teamowner.displayname, email2.email.lower())]
+        self.assertContentEqual(list_subscribers, result[team1.name])
+
     def test_getSubscribedAddresses_participation_dict_values(self):
         # getSubscribedAddresses() dict values includes indirect participants.
         team1, member1 = self.factory.makeTeamWithMailingListSubscribers(