Support modern Flask, add tests

Replace the flask-script 'feedbackloop' entry point with Flask CLI
hooks. Add an option to store unparsable files to a mbox.

.gitignore

 *.pyc
 *.egg-info
+.coverage
+.tox
+build
+dist
+coverage.xml
+nosetests.xml
+htmlcov

Dockerfile

@@ -2,8 +2,10 @@ FROM registry.git.autistici.org/ai3/docker/s6-base:master
 
 RUN apt-get -q update && \
     env DEBIAN_FRONTEND=noninteractive apt-get -qy install --no-install-recommends \
-        python3-flask python3-sqlalchemy python3-flask-sqlalchemy python3-flask-script \
-        python3-setuptools ca-certificates
+        python3-flask python3-sqlalchemy python3-flask-sqlalchemy \
+        python3-setuptools ca-certificates && \
+    apt-get clean && \
+    rm -fr /var/lib/apt/lists/*
 
 ADD . /tmp/src
 WORKDIR /tmp/src

feedbackloop/debug.py

+import mailbox
+
+
+class DebugMbox(object):
+
+    def __init__(self, path):
+        self._path = path
+
+    def write_message(self, msg):
+        if not self._path:
+            return
+        mbox = mailbox.mbox(self._path)
+        mbox.lock()
+        try:
+            mbox.add(msg)
+        finally:
+            mbox.unlock()
+            mbox.close()

feedbackloop/main.py

 from cheroot import wsgi
 from datetime import datetime, timedelta
-from flask_script import Manager
+import click
 
 from .app import app, db
 
@@ -8,23 +8,19 @@ from .app import app, db
 from . import views
 from .model import FeedbackEntry
 from .parse import MailScanner
+from .debug import DebugMbox
 
 
 def create_app(config=None):
     app.config.from_envvar('APP_CONFIG', silent=True)
     if config:
         app.config.update(config)
-
-    db.create_all()
-
     return app
 
 
-manager = Manager(create_app)
-
-
-@manager.option('--unseen', action='store_true')
-@manager.option('--limit', type=int, default=0)
+@app.cli.command('ingest')
+@click.option('--unseen', is_flag=True, default=False)
+@click.option('--limit', type=int, default=0)
 def ingest(unseen, limit):
     """Read and ingest new messages from the IMAP mailbox."""
     with app.app_context():
@@ -33,13 +29,16 @@ def ingest(unseen, limit):
                 app.config['IMAP_USERNAME'],
                 app.config['IMAP_PASSWORD'],
                 app.config.get('IMAP_REMOVE_MESSAGES', True),
-                app.config.get('LIST_DOMAINS')) as scanner:
+                app.config.get('LIST_DOMAINS'),
+                DebugMbox(app.config.get('DEBUG_MBOX_PATH')),
+        ) as scanner:
             for entry in scanner.scan(unseen, limit):
                 db.session.add(entry)
         db.session.commit()
 
 
-@manager.option('--days', type=int, default=30)
+@app.cli.command('expire')
+@click.option('--days', type=int, default=30)
 def expire(days):
     """Expire old entries from the database."""
     cutoff = datetime.now() - timedelta(days)
@@ -50,15 +49,10 @@ def expire(days):
         db.session.commit()
 
 
-@manager.option('--addr', default='0.0.0.0')
-@manager.option('--port', type=int, default=3030)
+@app.cli.command('server')
+@click.option('--addr', default='0.0.0.0')
+@click.option('--port', type=int, default=3030)
 def server(addr, port):
-    wsgi.Server((addr, port), app).start()
-
-
-def main():
-    manager.run()
-
+    db.create_all()
 
-if __name__ == '__main__':
-    main()
+    wsgi.Server((addr, port), app).start()

feedbackloop/parse.py

 #!/usr/bin/python3
 
-import argparse
 import imaplib
 import re
 import time
@@ -55,13 +54,14 @@ class MailScanner():
     """
 
     def __init__(self, server, username, password, remove=True,
-                 list_domains=None):
+                 list_domains=None, debug_mbox=None):
         self._server = server
         self._username = username
         self._password = password
         self._remove = remove
         self._to_delete = []
         self._conn = None
+        self._debug_mbox = debug_mbox
         if list_domains:
             self._list_id_rx = re.compile(
                 r'<(.+)\.(%s)>' % (
@@ -102,7 +102,8 @@ class MailScanner():
 
     def _scan_mailbox(self, unseen=False, limit=None):
         i = 0
-        res, data = self._conn.uid('search', None, '(UNSEEN)' if unseen else 'ALL')
+        res, data = self._conn.uid('search', None,
+                                   '(UNSEEN)' if unseen else 'ALL')
         if res != 'OK':
             raise IMAPError(res)
         for uid in data[0].decode('utf-8').split():
@@ -122,6 +123,8 @@ class MailScanner():
                 yield entry
             except Exception as e:
                 print(f'error: {e}')
+                if self._debug_mbox:
+                    self._debug_mbox.write_message(raw_msg)
 
     def _parse_message(self, raw_msg):
         # Parse the ARF message body.
@@ -150,11 +153,13 @@ class MailScanner():
             timestamp = datetime.fromtimestamp(
                 time.mktime(parsedate(_hdr(hdrs, 'Date'))))
 
-        fr = msg.get_feedback_report()
-        if fr:
-            print(f'feedback report: {fr.get_feedback_type()} - {fr.get_original_mail_from()}')
-            original_from = fr.get_original_mail_from()
-            arrival_date = fr.get_arrival_date()
+        feedback_report = msg.get_feedback_report()
+        if feedback_report:
+            print('feedback report: %s - %s' % (
+                feedback_report.get_feedback_type(),
+                feedback_report.get_original_mail_from()))
+            original_from = feedback_report.get_original_mail_from()
+            arrival_date = feedback_report.get_arrival_date()
             if arrival_date:
                 timestamp = datetime.fromtimestamp(
                     time.mktime(parsedate(arrival_date)))
@@ -188,20 +193,3 @@ class MailScanner():
             timestamp=timestamp,
             message=raw_msg,
         )
-
-    
-def main():
-    parser = argparse.ArgumentParser()
-    parser.add_argument('--user')
-    parser.add_argument('--password')
-    parser.add_argument('--server', default='mail.autistici.org')
-    parser.add_argument('--unseen', action='store_true')
-    args = parser.parse_args()
-
-    with MailScanner(args.server, args.user, args.password, False) as scanner:
-        for entry in scanner.scan(args.unseen, 20):
-            print(entry)
-
-
-if __name__ == '__main__':
-    main()

feedbackloop/test/__init__.py

+
+TEST_MESSAGE = b'''From: <abusedesk@example.com>
+Date: Thu, 8 Mar 2005 17:40:36 EDT
+Subject: FW: Earn money
+To: <abuse@example.net>
+MIME-Version: 1.0
+Content-Type: multipart/report; report-type=feedback-report;
+     boundary="part1_13d.2e68ed54_boundary" 
+
+--part1_13d.2e68ed54_boundary
+Content-Disposition: inline
+Content-Transfer-Encoding: 8bit
+Content-Type: message/feedback-report
+
+Version: 1
+Original-Rcpt-To: c8d8eba9466e8d59deca1a60a1ec9bae@example.com
+Reported-Domain: example.net
+Source: Example
+Abuse-Type: complaint
+Feedback-Type: abuse
+User-Agent: ReturnPathFBL/2.0
+Arrival-Date: Thu, 20 Oct 2022 14:45:15 +0000
+Original-Mail-From: foo@example.net
+Source-Ip: 1.2.3.4
+
+--part1_13d.2e68ed54_boundary
+Content-Type: message/rfc822
+Content-Disposition: inline
+Content-Transfer-Encoding: 8bit
+
+From: <foo@example.net>
+Subject: Hello
+Date: Thu, 20 Oct 2022 14:40:36 +0000
+
+Hello world
+
+--part1_13d.2e68ed54_boundary--
+'''

feedbackloop/test/test_arf.py

+import unittest
+from feedbackloop.arf import ARFMessage
+from feedbackloop.test import TEST_MESSAGE
+
+
+class ARFTest(unittest.TestCase):
+
+    def test_parse_arf(self):
+        arf = ARFMessage(TEST_MESSAGE)
+        report = arf.get_feedback_report()
+        self.assertTrue(report)
+        self.assertEqual('1.2.3.4', report.get_source_ip())

feedbackloop/test/test_ingest.py

+import mock
+from flask_testing import TestCase
+from feedbackloop.main import create_app, ingest
+from feedbackloop.app import db
+from feedbackloop.test import TEST_MESSAGE
+from feedbackloop.model import FeedbackEntry
+
+
+class IngestTest(TestCase):
+
+    def create_app(self):
+        return create_app({
+            'TESTING': True,
+            'IMAP_SERVER': 'localhost',
+            'IMAP_USERNAME': 'user',
+            'IMAP_PASSWORD': 'password',
+        })
+
+    def setUp(self):
+        db.create_all()
+
+    def tearDown(self):
+        db.session.remove()
+        db.drop_all()
+
+    def assertExitStatus(self, exit_status, fn, *args, **kwargs):
+        try:
+            fn(*args, **kwargs)
+        except SystemExit as exc:
+            self.assertEqual(exit_status, exc.code)
+            return
+        raise AssertionError('exit was not called')
+
+    @mock.patch('imaplib.IMAP4_SSL')
+    def test_ingest_nothing(self, imap_mock):
+        imap_mock.return_value.uid.return_value = ('OK', [b''])
+
+        self.assertExitStatus(0, ingest, ['--limit', '10'])
+
+    @mock.patch('imaplib.IMAP4_SSL')
+    def test_ingest_onemsg(self, imap_mock):
+        imap_mock.return_value.uid.side_effect = [
+            ('OK', [b'42']),
+            ('OK', [[None, TEST_MESSAGE]]),
+            ('OK', []),
+        ]
+
+        self.assertExitStatus(0, ingest, [])
+
+        entry = FeedbackEntry.query.filter(
+            FeedbackEntry.sender=='foo@example.net').first()
+        self.assertTrue(entry)

feedbackloop/test/test_mail_scanner.py

+import unittest
+from feedbackloop.parse import MailScanner
+from feedbackloop.test import TEST_MESSAGE
+
+
+class MailScannerTest(unittest.TestCase):
+
+    def test_parse_message_ok(self):
+        ms = MailScanner('server', 'user', 'password')
+        entry = ms._parse_message(TEST_MESSAGE)
+        self.assertTrue(entry)
+        self.assertEqual('foo@example.net', entry.sender)
+        self.assertEqual('abusedesk@example.com', entry.reporter)
+        self.assertFalse(entry.is_list)

setup.py

@@ -8,13 +8,8 @@ setup(name='feedbackloop',
       url='https://git.autistici.org/ai3/tools/feedback-loop',
       packages=find_packages(),
       install_requires=[
-          'Flask', 'Flask-Script', 'cheroot', 'sqlalchemy', 'Flask-SQLAlchemy',
+          'Flask', 'cheroot', 'sqlalchemy', 'Flask-SQLAlchemy',
       ],
       zip_safe=False,
       include_package_data=True,
-      entry_points={
-          'console_scripts': [
-              'feedbackloop = feedbackloop.main:main',
-          ],
-      },
 )

tox.ini

+[tox]
+envlist = py3
+
+[testenv]
+deps=
+  Flask-Testing
+  coverage
+  nose
+  mock
+commands=
+  nosetests -vv --with-xunit --with-coverage --cover-package=feedbackloop --cover-erase --cover-html --cover-html-dir=htmlcov []
+  coverage xml
+