8 files changed, 90 insertions, 88 deletions

diff --git a/src/mailman/database/alembic/versions/d4fbb4fd34ca_header_match_order.py b/src/mailman/database/alembic/versions/d4fbb4fd34ca_header_match_order.py
index 4dade211b..66ffb0169 100644
--- a/src/mailman/database/alembic/versions/d4fbb4fd34ca_header_match_order.py
+++ b/src/mailman/database/alembic/versions/d4fbb4fd34ca_header_match_order.py
@@ -12,7 +12,6 @@ down_revision = 'bfda02ab3a9b'
 
 import sqlalchemy as sa
 from alembic import op
-from mailman.database.helpers import is_sqlite
 
 
 def upgrade():
diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py
index c3558abea..331f386d0 100644
--- a/src/mailman/database/tests/test_migrations.py
+++ b/src/mailman/database/tests/test_migrations.py
@@ -36,7 +36,6 @@ from mailman.database.transaction import transaction
 from mailman.testing.layers import ConfigLayer
 
 
-
 class TestMigrations(unittest.TestCase):
     layer = ConfigLayer
 
diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py
index 53cea9d61..4b98ff6a7 100644
--- a/src/mailman/interfaces/mailinglist.py
+++ b/src/mailman/interfaces/mailinglist.py
@@ -36,7 +36,6 @@ from mailman.interfaces.member import MemberRole
 from zope.interface import Interface, Attribute
 
 
-
 class Personalization(Enum):
     none = 0
     # Everyone gets a unique copy of the message, and there are a few more
@@ -68,7 +67,6 @@ class SubscriptionPolicy(Enum):
     confirm_then_moderate = 3
 
 
-
 class IMailingList(Interface):
     """A mailing list."""
 
@@ -780,7 +778,6 @@ class IMailingList(Interface):
         )
 
 
-
 class IAcceptableAlias(Interface):
     """An acceptable alias for implicit destinations."""
 
@@ -819,7 +816,6 @@ class IAcceptableAliasSet(Interface):
         """An iterator over all the acceptable aliases.""")
 
 
-
 class IListArchiver(Interface):
     """An archiver for a mailing list.
 
@@ -853,7 +849,6 @@ class IListArchiverSet(Interface):
         """
 
 
-
 class IHeaderMatch(Interface):
     """A mailing list-specific message header matching rule."""
 
@@ -866,6 +861,12 @@ class IHeaderMatch(Interface):
     pattern = Attribute(
         """The regular expression to match.""")
 
+    position = Attribute(
+        """The ordinal position of this header match.
+
+        Set this to change the position of this header match.
+        """)
+
     chain = Attribute(
         """The chain to jump to on a match.
 
@@ -889,15 +890,17 @@ class IHeaderMatchList(Interface):
         :param pattern: The regular expression to use.
         :type pattern: string
         :param chain: The chain to jump to, or None to use the site-wide
-            configuration.  Defaults to None.
+            antispam jump chain via the configuration.  Defaults to None.
         :type chain: string or None
         :raises ValueError: if the header/pattern pair already exists for this
             mailing list.
         """
 
     def insert(index, header, pattern, chain=None):
-        """Insert the given rule at the given index position in this mailing
-        list's header match list.
+        """Insert a header match rule.
+
+        Inserts the given rule at the given index position in this
+        mailing list's header match list.
 
         :param index: The index to insert the rule at.
         :type index: integer
@@ -907,7 +910,7 @@ class IHeaderMatchList(Interface):
         :param pattern: The regular expression to use.
         :type pattern: string
         :param chain: The chain to jump to, or None to use the site-wide
-            configuration.  Defaults to None.
+            antispam jump chain via the configuration.  Defaults to None.
         :type chain: string or None
         :raises ValueError: if the header/pattern pair already exists for this
             mailing list.
@@ -920,25 +923,26 @@ class IHeaderMatchList(Interface):
         :type header: string
         :param pattern: The regular expression part of the rule to be removed.
         :type pattern: string
+        :raises ValueError: if the header does not exist in the list of
+            header matches.
         """
 
-    def __getitem__(key):
+    def __getitem__(index):
         """Return the header match at the given index for this mailing list.
 
-        :param key: The index of the header match to return.
-        :type key: integer
+        :param index: The index of the header match to return.
+        :type index: integer
         :return: The header match at this index.
-        :rtype: `IHeaderMatch`.
+        :rtype: `IHeaderMatch`
         :raises IndexError: if there is no header match at this index for
             this mailing list.
         """
 
-    def __delitem__(key):
-        """Remove the rule at the given index from this mailing list's header
-        match list.
+    def __delitem__(index):
+        """Remove the rule at the given index.
 
-        :param key: The index of the header match to remove.
-        :type key: integer
+        :param index: The index of the header match to remove.
+        :type index: integer
         :raises IndexError: if there is no header match at this index for
             this mailing list.
         """
diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py
index 7761f2842..07b5b4fed 100644
--- a/src/mailman/model/mailinglist.py
+++ b/src/mailman/model/mailinglist.py
@@ -70,7 +70,6 @@ SPACE = ' '
 UNDERSCORE = '_'
 
 
-
 @implementer(IMailingList)
 class MailingList(Model):
     """See `IMailingList`."""
@@ -189,9 +188,10 @@ class MailingList(Model):
     topics_bodylines_limit = Column(Integer)
     topics_enabled = Column(Boolean)
     welcome_message_uri = Column(Unicode)
-    # ORM relationships
+    # ORM relationships.
     header_matches = relationship(
-        'HeaderMatch', backref='mailing_list', cascade="all, delete-orphan",
+        'HeaderMatch', backref='mailing_list',
+        cascade="all, delete-orphan",
         order_by="HeaderMatch._position")
 
     def __init__(self, fqdn_listname):
@@ -500,7 +500,6 @@ class MailingList(Model):
         return member
 
 
-
 @implementer(IAcceptableAlias)
 class AcceptableAlias(Model):
     """See `IAcceptableAlias`."""
@@ -521,7 +520,6 @@ class AcceptableAlias(Model):
         self.alias = alias
 
 
-
 @implementer(IAcceptableAliasSet)
 class AcceptableAliasSet:
     """See `IAcceptableAliasSet`."""
@@ -557,7 +555,6 @@ class AcceptableAliasSet:
             yield alias.alias
 
 
-
 @implementer(IListArchiver)
 class ListArchiver(Model):
     """See `IListArchiver`."""
@@ -627,7 +624,6 @@ class ListArchiverSet:
             ListArchiver.name == archiver_name).first()
 
 
-
 @implementer(IHeaderMatch)
 class HeaderMatch(Model):
     """See `IHeaderMatch`."""
@@ -637,7 +633,8 @@ class HeaderMatch(Model):
     id = Column(Integer, primary_key=True)
 
     mailing_list_id = Column(
-        Integer, ForeignKey('mailinglist.id'),
+        Integer,
+        ForeignKey('mailinglist.id'),
         index=True, nullable=False)
 
     _position = Column('position', Integer, index=True, default=0)
@@ -646,9 +643,9 @@ class HeaderMatch(Model):
     chain = Column(Unicode, nullable=True)
 
     def __init__(self, **kw):
-        if 'position' in kw:
-            kw['_position'] = kw['position']
-            del kw['position']
+        position = kw.pop('position', None)
+        if position is not None:
+            kw['_position'] = position
         super().__init__(**kw)
 
     @hybrid_property
@@ -663,7 +660,8 @@ class HeaderMatch(Model):
         if value < 0:
             raise ValueError('Negative indexes are not supported')
         if value == self.position:
-            return # Nothing to do
+            # Nothing to do.
+            return
         existing_count = store.query(HeaderMatch).filter(
             HeaderMatch.mailing_list == self.mailing_list).count()
         if value >= existing_count:
@@ -673,26 +671,25 @@ class HeaderMatch(Model):
                 count=existing_count))
         if value < self.position:
             # Moving up: header matches between the new position and the
-            # current one must be moved down the list to make room. Those after
-            # the current position must not be changed.
+            # current one must be moved down the list to make room. Those
+            # after the current position must not be changed.
             for header_match in store.query(HeaderMatch).filter(
-                HeaderMatch.mailing_list == self.mailing_list,
-                HeaderMatch.position >= value,
-                HeaderMatch.position < self.position):
+                    HeaderMatch.mailing_list == self.mailing_list,
+                    HeaderMatch.position >= value,
+                    HeaderMatch.position < self.position):
                 header_match._position = header_match.position + 1
         elif value > self.position:
             # Moving down: header matches between the current position and the
-            # new one must be moved up the list to make room. Those after
-            # the new position must not be changed.
+            # new one must be moved up the list to make room. Those after the
+            # new position must not be changed.
             for header_match in store.query(HeaderMatch).filter(
-                HeaderMatch.mailing_list == self.mailing_list,
-                HeaderMatch.position > self.position,
-                HeaderMatch.position <= value):
+                    HeaderMatch.mailing_list == self.mailing_list,
+                    HeaderMatch.position > self.position,
+                    HeaderMatch.position <= value):
                 header_match._position = header_match.position - 1
         self._position = value
 
 
-
 @implementer(IHeaderMatchList)
 class HeaderMatchList:
     """See `IHeaderMatchList`."""
@@ -761,22 +758,22 @@ class HeaderMatchList:
         store.expire(self._mailing_list, ['header_matches'])
 
     @dbconnection
-    def __getitem__(self, store, key):
-        if key < 0:
-            key = len(self) + key
+    def __getitem__(self, store, index):
+        if index < 0:
+            index = len(self) + index
         try:
             return store.query(HeaderMatch).filter(
                 HeaderMatch.mailing_list == self._mailing_list,
-                HeaderMatch.position == key).one()
+                HeaderMatch.position == index).one()
         except NoResultFound:
             raise IndexError
 
     @dbconnection
-    def __delitem__(self, store, key):
+    def __delitem__(self, store, index):
         try:
             existing = store.query(HeaderMatch).filter(
                 HeaderMatch.mailing_list == self._mailing_list,
-                HeaderMatch.position == key).one()
+                HeaderMatch.position == index).one()
         except NoResultFound:
             raise IndexError
         else:
@@ -797,15 +794,14 @@ class HeaderMatchList:
 
     @dbconnection
     def _restore_position_sequence(self, store):
-        """Restore a continuous position sequence for this mailing list's
-        header matches.
-
-        The header match positions may not be continuous after deleting an
-        item.  It won't prevent this component from working properly, but it's
-        cleaner to restore a continuous sequence.
-        """
-        for position, header_match in enumerate(store.query(HeaderMatch).filter(
-            HeaderMatch.mailing_list == self._mailing_list
-            ).order_by(HeaderMatch.position)):
-            header_match._position = position
+        # Restore a continuous position sequence for this mailing list's
+        # header matches.
+        #
+        # The header match positions may not be continuous after deleting an
+        # item.  It won't prevent this component from working properly, but
+        # it's cleaner to restore a continuous sequence.
+        for position, match in enumerate(store.query(HeaderMatch).filter(
+                HeaderMatch.mailing_list == self._mailing_list
+                ).order_by(HeaderMatch.position)):
+            match._position = position
         store.expire(self._mailing_list, ['header_matches'])
diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py
index 1fc7b2e0d..67693fb4b 100644
--- a/src/mailman/model/tests/test_mailinglist.py
+++ b/src/mailman/model/tests/test_mailinglist.py
@@ -43,7 +43,6 @@ from mailman.utilities.datetime import now
 from zope.component import getUtility
 
 
-
 class TestMailingList(unittest.TestCase):
     layer = ConfigLayer
 
@@ -103,7 +102,6 @@ class TestMailingList(unittest.TestCase):
                       items[0].msg.get_payload())
 
 
-
 class TestListArchiver(unittest.TestCase):
     layer = ConfigLayer
 
@@ -146,7 +144,6 @@ class TestListArchiver(unittest.TestCase):
         config.pop('enable prototype')
 
 
-
 class TestDisabledListArchiver(unittest.TestCase):
     layer = ConfigLayer
 
@@ -174,7 +171,6 @@ class TestDisabledListArchiver(unittest.TestCase):
         config.pop('enable prototype')
 
 
-
 class TestAcceptableAliases(unittest.TestCase):
     layer = ConfigLayer
 
@@ -192,7 +188,6 @@ class TestAcceptableAliases(unittest.TestCase):
         self.assertEqual(len(list(alias_set.aliases)), 0)
 
 
-
 class TestHeaderMatch(unittest.TestCase):
     layer = ConfigLayer
 
@@ -265,18 +260,18 @@ class TestHeaderMatch(unittest.TestCase):
         header_matches.append('header-1', 'pattern-1')
         header_matches.append('header-2', 'pattern-2')
         header_matches.append('header-3', 'pattern-3')
-        hm = header_matches[1]
-        self.assertEqual(hm.header, 'header-2')
-        self.assertEqual(hm.pattern, 'pattern-2')
+        match = header_matches[1]
+        self.assertEqual(match.header, 'header-2')
+        self.assertEqual(match.pattern, 'pattern-2')
 
     def test_get_by_negative_index(self):
         header_matches = IHeaderMatchList(self._mlist)
         header_matches.append('header-1', 'pattern-1')
         header_matches.append('header-2', 'pattern-2')
         header_matches.append('header-3', 'pattern-3')
-        hm = header_matches[-1]
-        self.assertEqual(hm.header, 'header-3')
-        self.assertEqual(hm.pattern, 'pattern-3')
+        match = header_matches[-1]
+        self.assertEqual(match.header, 'header-3')
+        self.assertEqual(match.pattern, 'pattern-3')
 
     def test_get_non_existent_by_index(self):
         header_matches = IHeaderMatchList(self._mlist)
diff --git a/src/mailman/rest/docs/listconf.rst b/src/mailman/rest/docs/listconf.rst
index 5e32844de..8b65f58f0 100644
--- a/src/mailman/rest/docs/listconf.rst
+++ b/src/mailman/rest/docs/listconf.rst
@@ -314,6 +314,7 @@ The list of header matches for a mailing list are returned on the
     total_size: 0
 
 New header matches can be created by POSTing to the resource.
+::
 
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches', {
@@ -335,10 +336,11 @@ New header matches can be created by POSTing to the resource.
     self_link: http://localhost:9001/3.0/lists/ant.example.com/header-matches/0
 
 To follow the global antispam action, the header match rule must not specify
-an ``action`` key.  If the default antispam action is changed in the
-configuration file and Mailman is restarted, those rules will get the new
-jump action.  If a specific action is desired, the ``action`` key must point
-to a valid action.
+an ``action`` key, which names the chain to jump to if the rule matches.  If
+the default antispam action is changed in the configuration file and Mailman
+is restarted, those rules will get the new jump action.  If a specific action
+is desired, the ``action`` key must name a valid chain to jump to.
+::
 
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches', {
@@ -364,6 +366,7 @@ to a valid action.
 The resource can be changed by PATCHing it.  The ``position`` key can be used
 to change the priority of the header match in the list.  If it is not supplied,
 the priority is not changed.
+::
 
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches/1',
@@ -398,19 +401,20 @@ the priority is not changed.
         http_etag: "..."
         pattern: ^No
         position: 0
-        self_link: http://localhost:9001/3.0/lists/ant.example.com/header-matches/0
+        self_link: .../lists/ant.example.com/header-matches/0
     entry 1:
         header: x-spam-flag
         http_etag: "..."
         pattern: ^Yes
         position: 1
-        self_link: http://localhost:9001/3.0/lists/ant.example.com/header-matches/1
+        self_link: .../lists/ant.example.com/header-matches/1
     http_etag: "..."
     start: 0
     total_size: 2
 
 The PUT method can replace an entire header match.  The ``position`` key is
-optional: if it is omitted, the order will not be changed.
+optional; if it is omitted, the order will not be changed.
+::
 
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches/1',
@@ -433,6 +437,7 @@ optional: if it is omitted, the order will not be changed.
     self_link: http://localhost:9001/3.0/lists/ant.example.com/header-matches/1
 
 A header match can be removed using the DELETE method.
+::
 
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches/1',
@@ -440,6 +445,7 @@ A header match can be removed using the DELETE method.
     content-length: 0
     ...
     status: 204
+
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches')
     entry 0:
@@ -448,13 +454,14 @@ A header match can be removed using the DELETE method.
         http_etag: "..."
         pattern: ^No
         position: 0
-        self_link: http://localhost:9001/3.0/lists/ant.example.com/header-matches/0
+        self_link: .../lists/ant.example.com/header-matches/0
     http_etag: "..."
     start: 0
     total_size: 1
 
 The mailing list's header matches can be cleared by issuing a DELETE request on
 the top resource.
+::
 
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches',
@@ -462,6 +469,7 @@ the top resource.
     content-length: 0
     ...
     status: 204
+
     >>> dump_json('http://localhost:9001/3.0/lists/ant.example.com'
     ...           '/header-matches')
     http_etag: "..."
diff --git a/src/mailman/rest/header_matches.py b/src/mailman/rest/header_matches.py
index 9d1afa546..f54c181ef 100644
--- a/src/mailman/rest/header_matches.py
+++ b/src/mailman/rest/header_matches.py
@@ -26,8 +26,8 @@ __all__ = [
 from mailman.interfaces.action import Action
 from mailman.interfaces.mailinglist import IHeaderMatchList
 from mailman.rest.helpers import (
-    CollectionMixin, GetterSetter, bad_request, child, created, etag,
-    no_content, not_found, okay)
+    CollectionMixin, bad_request, child, created, etag, no_content, not_found,
+    okay)
 from mailman.rest.validator import Validator, enum_validator
 
 
@@ -109,9 +109,9 @@ class HeaderMatch(_HeaderMatchBase):
         validator = Validator(**kws)
         try:
             arguments = validator(request)
-            if 'action' in arguments:
-                arguments['chain'] = arguments['action'].name
-                del arguments['action']
+            action = arguments.pop('action', None)
+            if action is not None:
+                arguments['chain'] = action.name
             for key, value in arguments.items():
                 setattr(header_match, key, value)
         except ValueError as error:
@@ -154,9 +154,9 @@ class HeaderMatches(_HeaderMatchBase, CollectionMixin):
         except ValueError as error:
             bad_request(response, str(error))
             return
-        if 'action' in arguments:
-            arguments['chain'] = arguments['action'].name
-            del arguments['action']
+        action = arguments.pop('action', None)
+        if action is not None:
+            arguments['chain'] = action.name
         try:
             self.header_matches.append(**arguments)
         except ValueError:
diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py
index 52de967cf..782841838 100644
--- a/src/mailman/utilities/importer.py
+++ b/src/mailman/utilities/importer.py
@@ -41,7 +41,8 @@ from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.bounce import UnrecognizedBounceDisposition
 from mailman.interfaces.digests import DigestFrequency
 from mailman.interfaces.languages import ILanguageManager
-from mailman.interfaces.mailinglist import IAcceptableAliasSet, IHeaderMatchList
+from mailman.interfaces.mailinglist import (
+    IAcceptableAliasSet, IHeaderMatchList)
 from mailman.interfaces.mailinglist import Personalization, ReplyToMunging
 from mailman.interfaces.mailinglist import SubscriptionPolicy
 from mailman.interfaces.member import DeliveryMode, DeliveryStatus, MemberRole