fix: use mailmanclient for mailman API instead of requests (#3685)

backend/mlarchive/archive/management/commands/get_subscriber_counts.py

@@ -1,7 +1,5 @@
 # encoding: utf-8
 
-import requests
-
 from django.config import settings
 from django.core.management.base import BaseCommand, CommandError
 from mlarchive.archive.utils import get_subscriber_counts
@@ -25,11 +23,9 @@ def handle(self, *args, **options):
         confirm_settings([
             'MAILMAN_API_USER',
             'MAILMAN_API_PASSWORD',
-            'MAILMAN_API_ORIGIN',
-            'MAILMAN_API_LISTS',
-            'MAILMAN_API_MEMBER'])
+            'MAILMAN_API_URL'])
         try:
             get_subscriber_counts()
-        except requests.RequestException as e:
+        except Exception as e:
             logger.error(f'command get_subscriber_counts failed: {e}')
             raise CommandError(f'Command failed. {e}')

backend/mlarchive/archive/utils.py

@@ -11,21 +11,23 @@
 import re
 import requests
 import subprocess
+from collections import defaultdict
 
-
+import mailmanclient
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.cache import cache
 from django.http import HttpResponse
 from django.utils.encoding import smart_bytes
+
 from mlarchive.archive.models import EmailList, Subscriber
 # from mlarchive.archive.signals import _export_lists, _list_save_handler
 
 
 logger = logging.getLogger(__name__)
 THREAD_SORT_FIELDS = ('-thread__date', 'thread_id', 'thread_order')
 LIST_LISTS_PATTERN = re.compile(r'\s*([\w\-]*) - (.*)$')
-
+MAILMAN_LISTID_PATTERN = re.compile(r'(.*)\.(ietf|irtf|iab|iesg|rfc-editor)\.org')
 
 # --------------------------------------------------
 # Helper Functions
@@ -217,13 +219,11 @@ def get_mailman_lists(private=None):
     Specify list.private value or leave out to retrieve all lists.
     Raises requests.RequestException if request fails.
     '''
-    response = requests.get(
-        settings.MAILMAN_API_LISTS,
-        auth=(settings.MAILMAN_API_USER, settings.MAILMAN_API_PASSWORD),
-        timeout=settings.DEFAULT_REQUESTS_TIMEOUT)
-    response.raise_for_status()
-    data = response.json()
-    mailman_lists = [e['list_name'] for e in data['entries']]
+    client = mailmanclient.Client(
+        settings.MAILMAN_API_URL,
+        settings.MAILMAN_API_USER,
+        settings.MAILMAN_API_PASSWORD)
+    mailman_lists = [x.list_name for x in client.lists]
     mlists = EmailList.objects.filter(name__in=mailman_lists)
     if isinstance(private, bool):
         mlists = mlists.filter(private=private)
@@ -240,17 +240,6 @@ def get_subscribers(listname):
     return output.split()
 
 
-def get_subscribers_3(listname):
-    '''Gets list of subscribers for listname from mailman 3 API'''
-    response = requests.get(
-        settings.MAILMAN_API_MEMBER.format(listname=listname),
-        auth=(settings.MAILMAN_API_USER, settings.MAILMAN_API_PASSWORD),
-        timeout=settings.DEFAULT_REQUESTS_TIMEOUT)
-    response.raise_for_status()
-    data = response.json()
-    return [e['email'] for e in data['entries']]
-
-
 def get_subscriber_count():
     '''Populates Subscriber table with subscriber counts from mailman'''
     for mlist in get_known_mailman_lists():
@@ -259,13 +248,16 @@ def get_subscriber_count():
 
 def get_subscriber_counts():
     '''Populates Subscriber table with subscriber counts from mailman 3 API'''
-    for mlist in get_mailman_lists():
-        try:
-            subscribers = get_subscribers_3(mlist.name)
-            Subscriber.objects.create(email_list=mlist, count=len(subscribers))
-        except requests.RequestException as e:
-            logger.error(f'get_subscribers_3 failed. listname={mlist.name}. {e}')
-            continue
+    client = mailmanclient.Client(
+        settings.MAILMAN_API_URL,
+        settings.MAILMAN_API_USER,
+        settings.MAILMAN_API_PASSWORD)
+    counts = {x.list_name: x.member_count for x in client.lists}
+    subscribers = []
+    for elist in EmailList.objects.all():
+        if elist.name in counts:
+            subscribers.append(Subscriber(email_list=elist, count=counts[elist.name]))
+    Subscriber.objects.bulk_create(subscribers)
 
 
 def get_membership(options, args):
@@ -294,29 +286,45 @@ def get_membership(options, args):
 
 def get_membership_3(quiet=False):
     '''For all private lists, get membership from mailman 3 API and update
-    list membership as needed'''
-    for mlist in get_mailman_lists(private=True):
+    list membership as needed.
+
+    Use client.members to get all list memberships rather than hitting the API for
+    every private list
+    '''
+    client = mailmanclient.Client(
+        settings.MAILMAN_API_URL,
+        settings.MAILMAN_API_USER,
+        settings.MAILMAN_API_PASSWORD)
+
+    # build dictionary of all list memberships
+    members = defaultdict(list)
+    no_match = set()
+    for member in client.members:
+        # list_name = member.list_id.split('.')[:-2]
+        match = MAILMAN_LISTID_PATTERN.match(member.list_id)
+        if match:
+            list_name = match.groups()[0]
+            members[list_name].append(member.email)
+        else:
+            no_match.add(member.list_id)
+    if no_match:
+        logger.error('Could not parse mailman list_ids: {}'.format(','.join(no_match)))
+
+    for mlist in EmailList.objects.filter(name__in=members.keys(), private=True):
         if not quiet:
             print("Processing: %s" % mlist.name)
-
-        # handle these exceptions locally because calls for
-        # other lists may succeed
-        try:
-            subscribers = get_subscribers_3(mlist.name)
-        except requests.RequestException as e:
-            logger.error(f'get_subscribers failed. listname={mlist.name}. {e}')
-            continue
-        sha = hashlib.sha1(smart_bytes(subscribers))
+        sha = hashlib.sha1(smart_bytes(members[mlist.name]))
         digest = base64.urlsafe_b64encode(sha.digest())
         digest = digest.decode()
         if mlist.members_digest != digest:
-            process_members(mlist, subscribers)
+            process_members(mlist, members[mlist.name])
             mlist.members_digest = digest
             mlist.save()
 
 
 def check_inactive(prompt=True):
     '''Check for inactive lists and mark them as inactive'''
+    # this won't work for mailman 3 or when postfix is moved
     active = []
     to_inactive = []
 

backend/mlarchive/settings/base.py

@@ -41,7 +41,7 @@
     SCOUT_MONITOR=(bool, False),
     SCOUT_KEY=(str, ''),
     CELERY_BROKER_URL=(str, 'amqp://'),
-    MAILMAN_API_ORIGIN=(str, 'http://localhost:8001'),
+    MAILMAN_API_URL=(str, 'http://localhost:8001/3.1'),
     MAILMAN_API_USER=(str, ''),
     MAILMAN_API_PASSWORD=(str, ''),
     IMPORT_MESSAGE_APIKEY=(str, ''),
@@ -246,9 +246,7 @@
 
 # MAILMAN SETTINGS
 MAILMAN_DIR = '/usr/lib/mailman'
-MAILMAN_API_ORIGIN = env('MAILMAN_API_ORIGIN')
-MAILMAN_API_LISTS = MAILMAN_API_ORIGIN + '/3.1/lists'
-MAILMAN_API_MEMBER = MAILMAN_API_ORIGIN + '/3.1/lists/{listname}/roster/member?fields=email'
+MAILMAN_API_URL = env('MAILMAN_API_URL')
 MAILMAN_API_USER = env('MAILMAN_API_USER')
 MAILMAN_API_PASSWORD = env('MAILMAN_API_PASSWORD')
 IMPORT_MESSAGE_APIKEY = env('IMPORT_MESSAGE_APIKEY')

backend/mlarchive/tests/archive/utils.py

@@ -14,11 +14,31 @@
 from mlarchive.archive.utils import (get_noauth, get_lists, get_lists_for_user,
     lookup_user, process_members, get_membership, check_inactive, EmailList,
     create_mbox_file, _get_lists_as_xml, get_subscribers, Subscriber,
-    get_known_mailman_lists, get_subscriber_count, get_subscribers_3,
+    get_known_mailman_lists, get_subscriber_count,
     get_mailman_lists, get_membership_3, get_subscriber_counts)
 from mlarchive.archive.models import User
 from factories import EmailListFactory
 
+# --------------------------------------------------
+# Helper Classes
+# --------------------------------------------------
+
+
+class MailmanList:
+    def __init__(self, list_name, member_count):
+        self.list_name = list_name
+        self.member_count = member_count
+
+
+class ListResponse:
+    def __init__(self, lists):
+        self.lists = lists
+
+
+# --------------------------------------------------
+# Tests
+# --------------------------------------------------
+
 
 @pytest.mark.django_db(transaction=True)
 def test_get_noauth():
@@ -82,19 +102,23 @@ def test_get_lists_for_user(admin_user):
 
 @patch('requests.post')
 def test_lookup_user(mock_post):
-    response = requests.Response()
+    # setup mocks
+    response1 = requests.Response()
+    response1.status_code = 404
+    response2 = requests.Response()
+    response2.status_code = 200
+    response2._content = b'{"person.person": {"1": {"user": ""}}}'
+    response3 = requests.Response()
+    response3.status_code = 200
+    response3._content = b'{"person.person": {"1": {"user": {"username": "[email protected]"}}}}'
+    mock_post.side_effect = [response1, response2, response3]
     # test error status
-    response.status_code = 404
-    mock_post.return_value = response
     user = lookup_user('[email protected]')
     assert user is None
     # test no user object
-    response.status_code = 200
-    response._content = b'{"person.person": {"1": {"user": ""}}}'
     user = lookup_user('[email protected]')
     assert user is None
     # test success
-    response._content = b'{"person.person": {"1": {"user": {"username": "[email protected]"}}}}'
     user = lookup_user('[email protected]')
     assert user == '[email protected]'
 
@@ -154,10 +178,10 @@ def test_get_membership(mock_post, mock_output):
     assert os.path.exists(path)
 
 
-@patch('requests.get')
+@patch('mailmanclient.restbase.connection.Connection.call')
 @patch('requests.post')
 @pytest.mark.django_db(transaction=True)
-def test_get_membership_3(mock_post, mock_get):
+def test_get_membership_3(mock_post, mock_client):
     # setup
     path = os.path.join(settings.EXPORT_DIR, 'email_lists.xml')
     if os.path.exists(path):
@@ -166,24 +190,17 @@ def test_get_membership_3(mock_post, mock_get):
     private = EmailListFactory.create(name='bee', private=True)
 
     # prep mock
-    response_lists = requests.Response()
-    response_lists.status_code = 200
-    response_lists._content = b'{"start": 0, "total_size": 1, "entries": [{"advertised": true, "display_name": "Bee", "fqdn_listname": "[email protected]", "list_id": "bee.lists.example.com", "list_name": "bee", "mail_host": "lists.example.com", "member_count": 1, "volume": 1, "description": "", "self_link": "http://172.19.199.3:8001/3.1/lists/bee.lists.example.com", "http_etag": "fb6d81b0f573936532b0b02d4d2116023a9e56a8"}], "http_etag": "ef59c8ea7baa670940fd87f99fce83ba5013381f"}'
-    response_subscribers = requests.Response()
-    response_subscribers.status_code = 200
-    response_subscribers._content = b'{"start": 0, "total_size": 1, "entries": [{"email": "[email protected]", "http_etag": "\\"9baeb8580e60c8f5d3f0bab8d369ec8bff31a4b6\\""}], "http_etag": "\\"36668874b31dc412bf586dcdb7009f6524be1035\\""}'
+    response_members = requests.Response()
+    response_members.status_code = 200
+    response_members._content = b'{"start": 0, "total_size": 1, "entries": [{"address": "http://172.19.199.3:8001/3.1/addresses/[email protected]", "bounce_score": 0, "last_warning_sent": "0001-01-01T00:00:00", "total_warnings_sent": 0, "delivery_mode": "regular", "email": "[email protected]", "list_id": "bee.ietf.org", "subscription_mode": "as_address", "role": "member", "user": "http://172.19.199.3:8001/3.1/users/bb94f3e8ee504f788aaee97d0b76c3d1", "display_name": "Bart Person", "self_link": "http://172.19.199.3:8001/3.1/members/a963ef52752e4f679ee3fe8253208690", "member_id": "a963ef52752e4f679ee3fe8253208690", "http_etag": "\\"36b75489a29b268a1cf7a08c1c6d9a54f72b1c1e\\""}], "http_etag": "\\"a613273c96620e5eea18627b9c7dd4b50a23be8d\\""}'
+
     response_lookup = requests.Response()
     response_lookup.status_code = 200
     response_lookup._content = b'{"person.person": {"1": {"user": {"username": "[email protected]"}}}}'
-
-    # handle multiple calls to mock_get
-    mock_get.side_effect = [
-        response_lists,
-        response_subscribers]
+
+    mock_client.return_value = response_members, response_members.json()
     mock_post.return_value = response_lookup
     assert private.members.count() == 0
-    options = DummyOptions()
-    options.quiet = True
     get_membership_3(quiet=True)
     assert private.members.count() == 1
     assert private.members.first().username == '[email protected]'
@@ -285,19 +302,10 @@ def test_get_subscriber_count(mock_output):
     assert subscriber.count == 3
 
 
-@patch('requests.get')
+@patch('mailmanclient.client.Client.get_lists')
 @pytest.mark.django_db(transaction=True)
-def test_get_subscriber_counts(mock_get):
-    response_lists = requests.Response()
-    response_lists.status_code = 200
-    response_lists._content = b'{"start": 0, "total_size": 1, "entries": [{"advertised": true, "display_name": "Bee", "fqdn_listname": "[email protected]", "list_id": "bee.lists.example.com", "list_name": "bee", "mail_host": "lists.example.com", "member_count": 1, "volume": 1, "description": "", "self_link": "http://172.19.199.3:8001/3.1/lists/bee.lists.example.com", "http_etag": "fb6d81b0f573936532b0b02d4d2116023a9e56a8"}], "http_etag": "ef59c8ea7baa670940fd87f99fce83ba5013381f"}'
-    response_subscribers = requests.Response()
-    response_subscribers.status_code = 200
-    response_subscribers._content = b'{"start": 0, "total_size": 1, "entries": [{"email": "[email protected]", "http_etag": "\\"9baeb8580e60c8f5d3f0bab8d369ec8bff31a4b6\\""}], "http_etag": "\\"36668874b31dc412bf586dcdb7009f6524be1035\\""}'
-    # handle multiple calls to mock_get
-    mock_get.side_effect = [
-        response_lists,
-        response_subscribers]
+def test_get_subscriber_counts(mock_client):
+    mock_client.return_value = [MailmanList(list_name='bee', member_count=1)]
     public = EmailListFactory.create(name='bee')
     assert Subscriber.objects.count() == 0
     get_subscriber_counts()
@@ -318,27 +326,15 @@ def test_get_known_mailman_lists(mock_output):
     assert list(mlists) == [private]
 
 
-@patch('requests.get')
+@patch('mailmanclient.restbase.connection.Connection.call')
 @pytest.mark.django_db(transaction=True)
-def test_get_mailman_lists(mock_get):
+def test_get_mailman_lists(mock_client):
     ant = EmailListFactory.create(name='ant')
     bee = EmailListFactory.create(name='bee', private=True)
     response = requests.Response()
     response.status_code = 200
     response._content = b'{"start": 0, "total_size": 2, "entries": [{"advertised": true, "display_name": "Ant", "fqdn_listname": "[email protected]", "list_id": "ant.lists.example.com", "list_name": "ant", "mail_host": "lists.example.com", "member_count": 0, "volume": 1, "description": "", "self_link": "http://172.19.199.3:8001/3.1/lists/ant.lists.example.com", "http_etag": "cb67744198a68efb427c24fab35d9fc593186d6c"}, {"advertised": true, "display_name": "Bee", "fqdn_listname": "[email protected]", "list_id": "bee.lists.example.com", "list_name": "bee", "mail_host": "lists.example.com", "member_count": 1, "volume": 1, "description": "", "self_link": "http://172.19.199.3:8001/3.1/lists/bee.lists.example.com", "http_etag": "fb6d81b0f573936532b0b02d4d2116023a9e56a8"}], "http_etag": "ef59c8ea7baa670940fd87f99fce83ba5013381f"}'
-    mock_get.return_value = response
+    mock_client.return_value = response, response.json()
     mlists = get_mailman_lists(private=True)
     assert len(mlists) == 1
     assert list(mlists) == [bee]
-
-
-@patch('requests.get')
-@pytest.mark.django_db(transaction=True)
-def test_get_subscribers_3(mock_get):
-    response = requests.Response()
-    response.status_code = 200
-    response._content = b'{"start": 0, "total_size": 1, "entries": [{"email": "[email protected]", "http_etag": "\\"9baeb8580e60c8f5d3f0bab8d369ec8bff31a4b6\\""}], "http_etag": "\\"36668874b31dc412bf586dcdb7009f6524be1035\\""}'
-    mock_get.return_value = response
-    public = EmailListFactory.create(name='public')
-    subs = get_subscribers_3('public')
-    assert subs == ['[email protected]']

requirements.txt

@@ -27,6 +27,7 @@ gunicorn==20.1.0
 html5lib
 jedi==0.17.2              # 0.18 has bug with ipython
 lxml>=3.3.5
+mailmanclient==3.3.5
 mock>=2.0.0
 mozilla-django-oidc==2.0.0
 passlib