diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/mailman/chains/headers.py | 6 | ||||
| -rw-r--r-- | src/mailman/chains/tests/test_headers.py | 9 | ||||
| -rw-r--r-- | src/mailman/database/alembic/versions/cb7fc8476779_header_match_chain_renamed_to_action.py | 57 | ||||
| -rw-r--r-- | src/mailman/database/tests/test_migrations.py | 43 | ||||
| -rw-r--r-- | src/mailman/interfaces/mailinglist.py | 16 | ||||
| -rw-r--r-- | src/mailman/model/mailinglist.py | 12 | ||||
| -rw-r--r-- | src/mailman/model/tests/test_mailinglist.py | 15 | ||||
| -rw-r--r-- | src/mailman/rules/docs/header-matching.rst | 3 | ||||
| -rw-r--r-- | src/mailman/utilities/importer.py | 14 | ||||
| -rw-r--r-- | src/mailman/utilities/tests/test_import.py | 6 |
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')] ) |