10 files changed, 144 insertions, 37 deletions

diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py
index a952c4ffd..dd5cd1be0 100644
--- a/src/mailman/chains/headers.py
+++ b/src/mailman/chains/headers.py
@@ -150,4 +150,8 @@ class HeaderMatchChain(Chain):
         yield Link('any', LinkAction.jump, config.antispam.jump_chain)
         # Then return all the list-specific header matches.
         for entry in mlist.header_matches:
-            yield make_link(entry.header, entry.pattern, entry.chain)
+            if entry.action is None:
+                chain = None
+            else:
+                chain = entry.action.name
+            yield make_link(entry.header, entry.pattern, chain)
diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py
index d42baa55e..c2e01e5e3 100644
--- a/src/mailman/chains/tests/test_headers.py
+++ b/src/mailman/chains/tests/test_headers.py
@@ -29,6 +29,7 @@ from mailman.chains.headers import HeaderMatchRule, make_link
 from mailman.config import config
 from mailman.core.chains import process
 from mailman.email.message import Message
+from mailman.interfaces.action import Action
 from mailman.interfaces.chain import LinkAction, HoldEvent
 from mailman.interfaces.mailinglist import IHeaderMatchList
 from mailman.testing.helpers import (
@@ -160,9 +161,9 @@ class TestHeaderChain(unittest.TestCase):
         # properly.
         chain = config.chains['header-match']
         header_matches = IHeaderMatchList(self._mlist)
-        header_matches.append('Foo', 'a+', 'reject')
-        header_matches.append('Bar', 'b+', 'discard')
-        header_matches.append('Baz', 'z+', 'accept')
+        header_matches.append('Foo', 'a+', Action.reject)
+        header_matches.append('Bar', 'b+', Action.discard)
+        header_matches.append('Baz', 'z+', Action.accept)
         links = [link for link in chain.get_links(self._mlist, Message(), {})
                  if link.rule.name != 'any']
         self.assertEqual(len(links), 3)
@@ -193,7 +194,7 @@ A message body.
 """)
         msgdata = {}
         header_matches = IHeaderMatchList(self._mlist)
-        header_matches.append('Foo', 'foo', 'accept')
+        header_matches.append('Foo', 'foo', Action.accept)
         # This event subscriber records the event that occurs when the message
         # is processed by the owner chain.
         events = []
diff --git a/src/mailman/database/alembic/versions/cb7fc8476779_header_match_chain_renamed_to_action.py b/src/mailman/database/alembic/versions/cb7fc8476779_header_match_chain_renamed_to_action.py
new file mode 100644
index 000000000..45a09b936
--- /dev/null
+++ b/src/mailman/database/alembic/versions/cb7fc8476779_header_match_chain_renamed_to_action.py
@@ -0,0 +1,57 @@
+"""HeaderMatch chain renamed to action
+
+Revision ID: cb7fc8476779
+Revises: d4fbb4fd34ca
+Create Date: 2016-02-02 17:23:36.199207
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'cb7fc8476779'
+down_revision = 'd4fbb4fd34ca'
+
+import sqlalchemy as sa
+from alembic import op
+from mailman.database.helpers import is_sqlite, exists_in_db
+from mailman.database.types import Enum
+from mailman.interfaces.action import Action
+
+
+# Don't import the table definition from the models, it may break this
+# migration when the model is updated in the future (see the Alembic doc)
+hm_table = sa.sql.table('headermatch',
+    sa.sql.column('action', Enum(Action)),
+    sa.sql.column('chain', sa.VARCHAR())
+    )
+
+
+def upgrade():
+    if not exists_in_db(op.get_bind(), 'headermatch', 'action'):
+        op.add_column(
+            'headermatch', sa.Column('action', Enum(Action), nullable=True))
+
+    # Now migrate the data
+    for action_enum in Action:
+        op.execute(hm_table.update(
+            ).values(action=action_enum
+            ).where(hm_table.c.chain == action_enum.name))
+    # Now that data is migrated, drop the old column (except on SQLite which
+    # does not support this)
+    if not is_sqlite(op.get_bind()):
+        op.drop_column('headermatch', 'chain')
+
+
+def downgrade():
+    if not exists_in_db(op.get_bind(), 'headermatch', 'chain'):
+        op.add_column(
+            'headermatch', sa.Column('chain', sa.VARCHAR(), nullable=True))
+
+    # Now migrate the data
+    for action_enum in Action:
+        op.execute(hm_table.update(
+            ).values(chain=action_enum.name
+            ).where(hm_table.c.action == action_enum))
+    # Now that data is migrated, drop the new column (except on SQLite which
+    # does not support this)
+    if not is_sqlite(op.get_bind()):
+        op.drop_column('headermatch', 'action')
diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py
index c3558abea..1791a2453 100644
--- a/src/mailman/database/tests/test_migrations.py
+++ b/src/mailman/database/tests/test_migrations.py
@@ -33,6 +33,9 @@ from mailman.database.alembic import alembic_cfg
 from mailman.database.helpers import exists_in_db
 from mailman.database.model import Model
 from mailman.database.transaction import transaction
+from mailman.database.types import Enum
+from mailman.interfaces.action import Action
+from mailman.interfaces.mailinglist import IHeaderMatchList
 from mailman.testing.layers import ConfigLayer
 
 
@@ -223,3 +226,43 @@ class TestMigrations(unittest.TestCase):
             os.path.join(config.LIST_DATA_DIR, 'ant@example.com')))
         self.assertTrue(os.path.exists(ant.data_path))
         self.assertTrue(os.path.exists(bee.data_path))
+
+    def test_cb7fc8476779_header_match_action(self):
+        # Create a mailing list through the standard API.
+        with transaction():
+            ant = create_list('ant@example.com')
+            # Create header matches
+            header_matches = IHeaderMatchList(ant)
+            header_matches.append('header-1', 'pattern', Action.accept)
+            header_matches.append('header-2', 'pattern', Action.hold)
+            header_matches.append('header-3', 'pattern', Action.discard)
+            header_matches.append('header-4', 'pattern', Action.reject)
+        hm_table = sa.sql.table(
+            'headermatch',
+            sa.sql.column('action', Enum(Action)),
+            sa.sql.column('chain', sa.Unicode)
+            )
+        # Downgrade and verify that the chain names are correct.
+        with transaction():
+            alembic.command.downgrade(alembic_cfg, 'd4fbb4fd34ca')
+        results = config.db.store.execute(
+            hm_table.select()).fetchall()
+        self.assertEqual(len(results), 4)
+        for action, chain in results:
+            self.assertEqual(action.name, chain)
+        # Clear the previous values for testing.
+        with transaction():
+            config.db.store.execute(
+                hm_table.update().values(action=None))
+        for action, chain in config.db.store.execute(
+            hm_table.select()).fetchall():
+            self.assertEqual(action, None)
+        # Upgrade and verify that the actions are back to normal.
+        with transaction():
+            alembic.command.upgrade(alembic_cfg, 'cb7fc8476779')
+        results = config.db.store.execute(
+            hm_table.select()).fetchall()
+        self.assertEqual(len(results), 4)
+        for action, chain in results:
+            self.assertIsNotNone(action)
+            self.assertEqual(action.name, chain)
diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py
index 533bf89a7..d8a9977eb 100644
--- a/src/mailman/interfaces/mailinglist.py
+++ b/src/mailman/interfaces/mailinglist.py
@@ -880,7 +880,7 @@ class IHeaderMatchList(Interface):
     def clear():
         """Clear the list of header matching rules."""
 
-    def append(header, pattern, chain=None):
+    def append(header, pattern, action=None):
         """Append the given rule to this mailing list's header match list.
 
         :param header: The email header to filter on.  It will be converted to
@@ -888,14 +888,14 @@ class IHeaderMatchList(Interface):
         :type header: string
         :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.
-        :type chain: string or None
+        :param action: The Action enum pointing to the chain to jump to, or
+            None to use the site-wide configuration.  Defaults to None.
+        :type action: `Action` enum or None
         :raises ValueError: if the header/pattern pair already exists for this
             mailing list.
         """
 
-    def insert(index, header, pattern, chain=None):
+    def insert(index, header, pattern, action=None):
         """Insert the given rule at the given index position in this mailing
         list's header match list.
 
@@ -906,9 +906,9 @@ class IHeaderMatchList(Interface):
         :type header: string
         :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.
-        :type chain: string or None
+        :param action: The Action enum pointing to the chain to jump to, or
+            None to use the site-wide configuration.  Defaults to None.
+        :type action: `Action` enum or None
         :raises ValueError: if the header/pattern pair already exists for this
             mailing list.
         """
diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py
index 0f8bcd212..1d7bdd3a3 100644
--- a/src/mailman/model/mailinglist.py
+++ b/src/mailman/model/mailinglist.py
@@ -643,7 +643,7 @@ class HeaderMatch(Model):
     _index = Column('index', Integer, index=True, default=0)
     header = Column(Unicode)
     pattern = Column(Unicode)
-    chain = Column(Unicode, nullable=True)
+    action = Column(Enum(Action), nullable=True)
 
     def __init__(self, **kw):
         if 'index' in kw:
@@ -713,7 +713,7 @@ class HeaderMatchList:
         store.expire(self._mailing_list, ['header_matches'])
 
     @dbconnection
-    def append(self, store, header, pattern, chain=None):
+    def append(self, store, header, pattern, action=None):
         header = header.lower()
         existing = store.query(HeaderMatch).filter(
             HeaderMatch.mailing_list == self._mailing_list,
@@ -728,20 +728,20 @@ class HeaderMatchList:
             last_index = -1
         header_match = HeaderMatch(
             mailing_list=self._mailing_list,
-            header=header, pattern=pattern, chain=chain,
+            header=header, pattern=pattern, action=action,
             index=last_index + 1)
         store.add(header_match)
         store.expire(self._mailing_list, ['header_matches'])
 
     @dbconnection
-    def insert(self, store, index, header, pattern, chain=None):
-        self.append(header, pattern, chain)
+    def insert(self, store, index, header, pattern, action=None):
+        self.append(header, pattern, action)
         # Get the header match that was just added.
         header_match = store.query(HeaderMatch).filter(
             HeaderMatch.mailing_list == self._mailing_list,
             HeaderMatch.header == header.lower(),
             HeaderMatch.pattern == pattern,
-            HeaderMatch.chain == chain).one()
+            HeaderMatch.action == action).one()
         header_match.index = index
         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 1431b1be8..61ab0901d 100644
--- a/src/mailman/model/tests/test_mailinglist.py
+++ b/src/mailman/model/tests/test_mailinglist.py
@@ -30,6 +30,7 @@ import unittest
 from mailman.app.lifecycle import create_list
 from mailman.config import config
 from mailman.database.transaction import transaction
+from mailman.interfaces.action import Action
 from mailman.interfaces.listmanager import IListManager
 from mailman.interfaces.mailinglist import (
     IAcceptableAliasSet, IHeaderMatchList, IListArchiverSet)
@@ -205,11 +206,11 @@ class TestHeaderMatch(unittest.TestCase):
         self.assertEqual(len(self._mlist.header_matches), 1)
         self.assertEqual(self._mlist.header_matches[0].header, 'header')
 
-    def test_chain_defaults_to_none(self):
+    def test_action_defaults_to_none(self):
         header_matches = IHeaderMatchList(self._mlist)
         header_matches.append('header', 'pattern')
         self.assertEqual(len(self._mlist.header_matches), 1)
-        self.assertEqual(self._mlist.header_matches[0].chain, None)
+        self.assertEqual(self._mlist.header_matches[0].action, None)
 
     def test_duplicate(self):
         header_matches = IHeaderMatchList(self._mlist)
@@ -240,16 +241,16 @@ class TestHeaderMatch(unittest.TestCase):
         header_matches = IHeaderMatchList(self._mlist)
         header_matches.append('Header', 'pattern')
         header_matches.append('Subject', 'patt.*')
-        header_matches.append('From', '.*@example.com', 'discard')
-        header_matches.append('From', '.*@example.org', 'accept')
-        matches = [(match.header, match.pattern, match.chain)
+        header_matches.append('From', '.*@example.com', Action.discard)
+        header_matches.append('From', '.*@example.org', Action.accept)
+        matches = [(match.header, match.pattern, match.action)
                    for match in IHeaderMatchList(self._mlist)]
         self.assertEqual(
             matches, [
             ('header', 'pattern', None),
             ('subject', 'patt.*', None),
-            ('from', '.*@example.com', 'discard'),
-            ('from', '.*@example.org', 'accept'),
+            ('from', '.*@example.com', Action.discard),
+            ('from', '.*@example.org', Action.accept),
             ])
 
     def test_clear(self):
diff --git a/src/mailman/rules/docs/header-matching.rst b/src/mailman/rules/docs/header-matching.rst
index 4e6c0853d..a4d539291 100644
--- a/src/mailman/rules/docs/header-matching.rst
+++ b/src/mailman/rules/docs/header-matching.rst
@@ -177,8 +177,9 @@ As does a message with a spam score of four pluses.
 Now, the list administrator wants to match on three plus signs, but wants
 those emails to be discarded instead of held.
 
+    >>> from mailman.interfaces.action import Action
     >>> header_matches.remove('x-spam-score', '[+]{3,}')
-    >>> header_matches.append('x-spam-score', '[+]{3,}', 'discard')
+    >>> header_matches.append('x-spam-score', '[+]{3,}', Action.discard)
 
 A message with a spam score of three pluses will still match, and the message
 will be discarded.
diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py
index 52de967cf..8a37d909e 100644
--- a/src/mailman/utilities/importer.py
+++ b/src/mailman/utilities/importer.py
@@ -129,7 +129,7 @@ def nonmember_action_mapping(value):
         }[value]
 
 
-def action_to_chain(value):
+def action_to_enum(value):
     # Converts an action number in Mailman 2.1 to the name of the corresponding
     # chain in 3.x.  The actions 'approve', 'subscribe' and 'unsubscribe' are
     # ignored.  The defer action is converted to None, because it is not
@@ -137,12 +137,12 @@ def action_to_chain(value):
     return {
         0: None,
         #1: 'approve',
-        2: 'reject',
-        3: 'discard',
+        2: Action.reject,
+        3: Action.discard,
         #4: 'subscribe',
         #5: 'unsubscribe',
-        6: 'accept',
-        7: 'hold',
+        6: Action.accept,
+        7: Action.hold,
         }[value]
 
 
@@ -337,7 +337,7 @@ def import_config_pck(mlist, config_dict):
     header_filter_rules = config_dict.get('header_filter_rules', [])
     for line_patterns, action, _unused in header_filter_rules:
         try:
-            chain = action_to_chain(action)
+            action = action_to_enum(action)
         except KeyError:
             log.warning('Unsupported header_filter_rules action: %r',
                         action)
@@ -374,7 +374,7 @@ def import_config_pck(mlist, config_dict):
                             'invalid regular expression: %r', line_pattern)
                 continue
             try:
-                header_matches.append(header, pattern, chain)
+                header_matches.append(header, pattern, action)
             except ValueError:
                 log.warning('Skipping duplicate header_filter rule: %r',
                             line_pattern)
diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py
index 68a005760..1b7c1d60e 100644
--- a/src/mailman/utilities/tests/test_import.py
+++ b/src/mailman/utilities/tests/test_import.py
@@ -367,7 +367,7 @@ class TestBasicImport(unittest.TestCase):
         error_log = LogFileMark('mailman.error')
         self._import()
         self.assertListEqual(
-            [(hm.header, hm.pattern, hm.chain)
+            [(hm.header, hm.pattern, hm.action.name)
              for hm in self._mlist.header_matches ], [
             ('x-spam-status', 'Yes.*', 'discard'),
             ('x-spam-status', 'Yes', 'reject'),
@@ -445,7 +445,7 @@ class TestBasicImport(unittest.TestCase):
             ]
         self._import()
         self.assertListEqual(
-            [(hm.header, hm.pattern, hm.chain)
+            [(hm.header, hm.pattern, hm.action)
              for hm in self._mlist.header_matches],
             [('x-spam-status', 'Yes', None)]
             )
@@ -476,7 +476,7 @@ class TestBasicImport(unittest.TestCase):
         error_log = LogFileMark('mailman.error')
         self._import()
         self.assertListEqual(
-            [(hm.header, hm.pattern, hm.chain)
+            [(hm.header, hm.pattern, hm.action.name)
              for hm in self._mlist.header_matches],
             [('someheadername', 'test-pattern', 'discard')]
             )