diff options
| -rw-r--r-- | src/mailman/app/docs/bans.rst | 125 | ||||
| -rw-r--r-- | src/mailman/app/membership.py | 2 | ||||
| -rw-r--r-- | src/mailman/app/tests/test_membership.py | 28 | ||||
| -rw-r--r-- | src/mailman/config/configure.zcml | 17 | ||||
| -rw-r--r-- | src/mailman/database/docs/migration.rst | 7 | ||||
| -rw-r--r-- | src/mailman/database/schema/mm_20120407000000.py | 2 | ||||
| -rw-r--r-- | src/mailman/database/schema/mm_20121015000000.py | 88 | ||||
| -rw-r--r-- | src/mailman/database/schema/sqlite_20120407000000_01.sql | 2 | ||||
| -rw-r--r-- | src/mailman/database/schema/sqlite_20121015000000_01.sql | 22 | ||||
| -rw-r--r-- | src/mailman/database/tests/test_migrations.py | 192 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 11 | ||||
| -rw-r--r-- | src/mailman/interfaces/bans.py | 42 | ||||
| -rw-r--r-- | src/mailman/model/bans.py | 61 |
13 files changed, 389 insertions, 210 deletions
diff --git a/src/mailman/app/docs/bans.rst b/src/mailman/app/docs/bans.rst index bb6d5902e..5cb56e409 100644 --- a/src/mailman/app/docs/bans.rst +++ b/src/mailman/app/docs/bans.rst @@ -6,20 +6,26 @@ Email addresses can be banned from ever subscribing, either to a specific mailing list or globally within the Mailman system. Both explicit email addresses and email address patterns can be banned. -Bans are managed through the `Ban Manager`. +Bans are managed through the `Ban Manager`. There are ban managers for +specific lists, and there is a global ban manager. To get access to the +global ban manager, adapt ``None``. - >>> from zope.component import getUtility >>> from mailman.interfaces.bans import IBanManager - >>> ban_manager = getUtility(IBanManager) + >>> global_bans = IBanManager(None) -At first, no email addresses are banned, either globally... +At first, no email addresses are banned globally. - >>> ban_manager.is_banned('anne@example.com') + >>> global_bans.is_banned('anne@example.com') False -...or for a specific mailing list. +To get a list-specific ban manager, adapt the mailing list object. - >>> ban_manager.is_banned('bart@example.com', 'test@example.com') + >>> mlist = create_list('test@example.com') + >>> test_bans = IBanManager(mlist) + +There are no bans for this particular list. + + >>> test_bans.is_banned('bart@example.com') False @@ -27,17 +33,17 @@ Specific bans ============= An email address can be banned from a specific mailing list by adding a ban to -the ban manager. +the list's ban manager. - >>> ban_manager.ban('cris@example.com', 'test@example.com') - >>> ban_manager.is_banned('cris@example.com', 'test@example.com') + >>> test_bans.ban('cris@example.com') + >>> test_bans.is_banned('cris@example.com') True - >>> ban_manager.is_banned('bart@example.com', 'test@example.com') + >>> test_bans.is_banned('bart@example.com') False However, this is not a global ban. - >>> ban_manager.is_banned('cris@example.com') + >>> global_bans.is_banned('cris@example.com') False @@ -47,48 +53,53 @@ Global bans An email address can be banned globally, so that it cannot be subscribed to any mailing list. - >>> ban_manager.ban('dave@example.com') + >>> global_bans.ban('dave@example.com') -Dave is banned from the test mailing list... +Because there is a global ban, Dave is also banned from the mailing list. - >>> ban_manager.is_banned('dave@example.com', 'test@example.com') + >>> test_bans.is_banned('dave@example.com') True -...and the sample mailing list. +Even when a new mailing list is created, Dave is still banned from this list +because of his global ban. - >>> ban_manager.is_banned('dave@example.com', 'sample@example.com') + >>> sample = create_list('sample@example.com') + >>> sample_bans = IBanManager(sample) + >>> sample_bans.is_banned('dave@example.com') True -Dave is also banned globally. +Dave is of course banned globally. - >>> ban_manager.is_banned('dave@example.com') + >>> global_bans.is_banned('dave@example.com') True Cris however is not banned globally. - >>> ban_manager.is_banned('cris@example.com') + >>> global_bans.is_banned('cris@example.com') False Even though Cris is not banned globally, we can add a global ban for her. - >>> ban_manager.ban('cris@example.com') - >>> ban_manager.is_banned('cris@example.com') + >>> global_bans.ban('cris@example.com') + >>> global_bans.is_banned('cris@example.com') True -Cris is obviously still banned from specific mailing lists. +Cris is now banned from all mailing lists. - >>> ban_manager.is_banned('cris@example.com', 'test@example.com') + >>> test_bans.is_banned('cris@example.com') True - >>> ban_manager.is_banned('cris@example.com', 'sample@example.com') + >>> sample_bans.is_banned('cris@example.com') True -We can remove the global ban to once again just ban her address from the test -list. +We can remove the global ban to once again just ban her address from just the +test list. - >>> ban_manager.unban('cris@example.com') - >>> ban_manager.is_banned('cris@example.com', 'test@example.com') + >>> global_bans.unban('cris@example.com') + >>> global_bans.is_banned('cris@example.com') + False + >>> test_bans.is_banned('cris@example.com') True - >>> ban_manager.is_banned('cris@example.com', 'sample@example.com') + >>> sample_bans.is_banned('cris@example.com') False @@ -100,52 +111,56 @@ and globally, just as specific addresses can be banned. Use this for example, when an entire domain is a spam faucet. When using a pattern, the email address must start with a caret (^). - >>> ban_manager.ban('^.*@example.org', 'test@example.com') + >>> test_bans.ban('^.*@example.org') -Now, no one from example.org can subscribe to the test list. +Now, no one from example.org can subscribe to the test mailing list. - >>> ban_manager.is_banned('elle@example.org', 'test@example.com') + >>> test_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('eperson@example.org', 'test@example.com') + >>> test_bans.is_banned('eperson@example.org') True - >>> ban_manager.is_banned('elle@example.com', 'test@example.com') + +example.com addresses are not banned. + + >>> test_bans.is_banned('elle@example.com') False -They are not, however banned globally. +example.org addresses are not banned globally, nor for any other mailing +list. - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> sample_bans.is_banned('elle@example.org') False - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') False Of course, we can ban everyone from example.org globally too. - >>> ban_manager.ban('^.*@example.org') - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> global_bans.ban('^.*@example.org') + >>> sample_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') True We can remove the mailing list ban on the pattern, though the global ban will still be in place. - >>> ban_manager.unban('^.*@example.org', 'test@example.com') - >>> ban_manager.is_banned('elle@example.org', 'test@example.com') + >>> test_bans.unban('^.*@example.org') + >>> test_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> sample_bans.is_banned('elle@example.org') True - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') True But once the global ban is removed, everyone from example.org can subscribe to the mailing lists. - >>> ban_manager.unban('^.*@example.org') - >>> ban_manager.is_banned('elle@example.org', 'test@example.com') + >>> global_bans.unban('^.*@example.org') + >>> test_bans.is_banned('elle@example.org') False - >>> ban_manager.is_banned('elle@example.org', 'sample@example.com') + >>> sample_bans.is_banned('elle@example.org') False - >>> ban_manager.is_banned('elle@example.org') + >>> global_bans.is_banned('elle@example.org') False @@ -154,14 +169,14 @@ Adding and removing bans It is not an error to add a ban more than once. These are just ignored. - >>> ban_manager.ban('fred@example.com', 'test@example.com') - >>> ban_manager.ban('fred@example.com', 'test@example.com') - >>> ban_manager.is_banned('fred@example.com', 'test@example.com') + >>> test_bans.ban('fred@example.com') + >>> test_bans.ban('fred@example.com') + >>> test_bans.is_banned('fred@example.com') True Nor is it an error to remove a ban more than once. - >>> ban_manager.unban('fred@example.com', 'test@example.com') - >>> ban_manager.unban('fred@example.com', 'test@example.com') - >>> ban_manager.is_banned('fred@example.com', 'test@example.com') + >>> test_bans.unban('fred@example.com') + >>> test_bans.unban('fred@example.com') + >>> test_bans.is_banned('fred@example.com') False diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py index c73735b35..3f012e5a5 100644 --- a/src/mailman/app/membership.py +++ b/src/mailman/app/membership.py @@ -73,7 +73,7 @@ def add_member(mlist, email, display_name, password, delivery_mode, language, # Let's be extra cautious. getUtility(IEmailValidator).validate(email) # Check to see if the email address is banned. - if getUtility(IBanManager).is_banned(email, mlist.fqdn_listname): + if IBanManager(mlist).is_banned(email): raise MembershipIsBannedError(mlist, email) # See if there's already a user linked with the given address. user_manager = getUtility(IUserManager) diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py index 00c279910..113ae6a52 100644 --- a/src/mailman/app/tests/test_membership.py +++ b/src/mailman/app/tests/test_membership.py @@ -69,7 +69,7 @@ class AddMemberTest(unittest.TestCase): def test_add_member_banned(self): # Test that members who are banned by specific address cannot # subscribe to the mailing list. - getUtility(IBanManager).ban('anne@example.com', 'test@example.com') + IBanManager(self._mlist).ban('anne@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', @@ -78,43 +78,43 @@ class AddMemberTest(unittest.TestCase): def test_add_member_globally_banned(self): # Test that members who are banned by specific address cannot # subscribe to the mailing list. - getUtility(IBanManager).ban('anne@example.com') + IBanManager(None).ban('anne@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) def test_add_member_banned_from_different_list(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('anne@example.com', 'sample@example.com') + # Test that members who are banned by on a different list can still be + # subscribed to other mlists. + sample_list = create_list('sample@example.com') + IBanManager(sample_list).ban('anne@example.com') member = add_member(self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) self.assertEqual(member.address.email, 'anne@example.com') def test_add_member_banned_by_pattern(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('^.*@example.com', 'test@example.com') + # Addresses matching regexp ban patterns cannot subscribe. + IBanManager(self._mlist).ban('^.*@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) def test_add_member_globally_banned_by_pattern(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('^.*@example.com') + # Addresses matching global regexp ban patterns cannot subscribe. + IBanManager(None).ban('^.*@example.com') self.assertRaises( MembershipIsBannedError, add_member, self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) def test_add_member_banned_from_different_list_by_pattern(self): - # Test that members who are banned by specific address cannot - # subscribe to the mailing list. - getUtility(IBanManager).ban('^.*@example.com', 'sample@example.com') + # Addresses matching regexp ban patterns on one list can still + # subscribe to other mailing lists. + sample_list = create_list('sample@example.com') + IBanManager(sample_list).ban('^.*@example.com') member = add_member(self._mlist, 'anne@example.com', 'Anne Person', '123', DeliveryMode.regular, system_preferences.preferred_language) diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml index ed85ae1a6..efb449538 100644 --- a/src/mailman/config/configure.zcml +++ b/src/mailman/config/configure.zcml @@ -6,6 +6,18 @@ <adapter for="mailman.interfaces.mailinglist.IMailingList" + provides="mailman.interfaces.bans.IBanManager" + factory="mailman.model.bans.BanManager" + /> + + <adapter + for="None" + provides="mailman.interfaces.bans.IBanManager" + factory="mailman.model.bans.BanManager" + /> + + <adapter + for="mailman.interfaces.mailinglist.IMailingList" provides="mailman.interfaces.autorespond.IAutoResponseSet" factory="mailman.model.autorespond.AutoResponseSet" /> @@ -37,11 +49,6 @@ /> <utility - provides="mailman.interfaces.bans.IBanManager" - factory="mailman.model.bans.BanManager" - /> - - <utility provides="mailman.interfaces.bounce.IBounceProcessor" factory="mailman.model.bounce.BounceProcessor" /> diff --git a/src/mailman/database/docs/migration.rst b/src/mailman/database/docs/migration.rst index 6216f9bbc..2988579d3 100644 --- a/src/mailman/database/docs/migration.rst +++ b/src/mailman/database/docs/migration.rst @@ -30,12 +30,13 @@ already applied. >>> from mailman.model.version import Version >>> results = config.db.store.find(Version, component='schema') >>> results.count() - 2 + 3 >>> versions = sorted(result.version for result in results) >>> for version in versions: ... print version 00000000000000 20120407000000 + 20121015000000 Migrations @@ -96,6 +97,7 @@ This will load the new migration, since it hasn't been loaded before. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 >>> test = config.db.store.find(Version, component='test').one() >>> print test.version @@ -126,6 +128,7 @@ The first time we load this new migration, we'll get the 801 marker. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 20129999000001 >>> test = config.db.store.find(Version, component='test') @@ -142,6 +145,7 @@ We do not get an 802 marker because the migration has already been loaded. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 20129999000001 >>> test = config.db.store.find(Version, component='test') @@ -176,6 +180,7 @@ You'll notice that the ...04 version is not present. ... print result 00000000000000 20120407000000 + 20121015000000 20129999000000 20129999000001 20129999000002 diff --git a/src/mailman/database/schema/mm_20120407000000.py b/src/mailman/database/schema/mm_20120407000000.py index d6e647c1a..3910e9438 100644 --- a/src/mailman/database/schema/mm_20120407000000.py +++ b/src/mailman/database/schema/mm_20120407000000.py @@ -149,7 +149,7 @@ def upgrade_sqlite(database, store, version, module_path): def upgrade_postgres(database, store, version, module_path): # Get the old values from the mailinglist table. results = store.execute(""" - SELECT id, archive, archive_private, list_name, mail_host + SELECT id, archive, archive_private, list_name, mail_host FROM mailinglist; """) # Do the simple renames first. diff --git a/src/mailman/database/schema/mm_20121015000000.py b/src/mailman/database/schema/mm_20121015000000.py new file mode 100644 index 000000000..51e0602e7 --- /dev/null +++ b/src/mailman/database/schema/mm_20121015000000.py @@ -0,0 +1,88 @@ +# Copyright (C) 2012 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see <http://www.gnu.org/licenses/>. + +"""3.0b2 -> 3.0b3 schema migrations. + +* bans.mailing_list -> bans.list_id +""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ + 'upgrade', + ] + + +VERSION = '20121015000000' + + + +def upgrade(database, store, version, module_path): + if database.TAG == 'sqlite': + upgrade_sqlite(database, store, version, module_path) + else: + upgrade_postgres(database, store, version, module_path) + + + +def _make_listid(fqdn_listname): + list_name, at, mail_host = fqdn_listname.partition('@') + if at == '': + # If there is no @ sign in the value, assume it already contains the + # list-id. + return fqdn_listname + return '{0}.{1}'.format(list_name, mail_host) + + + +def upgrade_sqlite(database, store, version, module_path): + database.load_schema( + store, version, 'sqlite_{0}_01.sql'.format(version), module_path) + results = store.execute(""" + SELECT id, mailing_list + FROM ban; + """) + for id, mailing_list in results: + # Skip global bans since there's nothing to update. + if mailing_list is None: + continue + store.execute(""" + UPDATE ban_backup SET list_id = '{0}' + WHERE id = {1}; + """.format(_make_listid(mailing_list), id)) + # Pivot the backup table to the real thing. + store.execute('DROP TABLE ban;') + store.execute('ALTER TABLE ban_backup RENAME TO ban;') + + + +def upgrade_postgres(database, store, version, module_path): + # Get the old values from the ban table. + results = store.execute('SELECT id, mailing_list FROM ban;') + store.execute('ALTER TABLE ban ADD COLUMN list_id TEXT;') + for id, mailing_list in results: + # Skip global bans since there's nothing to update. + if mailing_list is None: + continue + store.execute(""" + UPDATE ban SET list_id = '{0}' + WHERE id = {1}; + """.format(_make_listid(mailing_list), id)) + store.execute('ALTER TABLE ban DROP COLUMN mailing_list;') + # Record the migration in the version table. + database.load_schema(store, version, None, module_path) diff --git a/src/mailman/database/schema/sqlite_20120407000000_01.sql b/src/mailman/database/schema/sqlite_20120407000000_01.sql index b93e214c4..53bab70dd 100644 --- a/src/mailman/database/schema/sqlite_20120407000000_01.sql +++ b/src/mailman/database/schema/sqlite_20120407000000_01.sql @@ -269,7 +269,7 @@ INSERT INTO mem_backup SELECT preferences_id, user_id FROM member; - + -- Add the new columns. They'll get inserted at the Python layer. ALTER TABLE ml_backup ADD COLUMN archive_policy INTEGER; diff --git a/src/mailman/database/schema/sqlite_20121015000000_01.sql b/src/mailman/database/schema/sqlite_20121015000000_01.sql new file mode 100644 index 000000000..c0df75111 --- /dev/null +++ b/src/mailman/database/schema/sqlite_20121015000000_01.sql @@ -0,0 +1,22 @@ +-- THIS FILE CONTAINS THE SQLITE3 SCHEMA MIGRATION FROM +-- 3.0b2 TO 3.0b3 +-- +-- AFTER 3.0b3 IS RELEASED YOU MAY NOT EDIT THIS FILE. + +-- REMOVALS from the ban table. +-- REM mailing_list + +-- ADDS to the ban table. +-- ADD list_id + +CREATE TABLE ban_backup ( + id INTEGER NOT NULL, + email TEXT, + PRIMARY KEY (id) + ); + +INSERT INTO ban_backup SELECT + id, email + FROM ban; + +ALTER TABLE ban_backup ADD COLUMN list_id TEXT; diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index f50ce35d3..4401b030f 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -29,6 +29,7 @@ __all__ = [ import unittest +from operator import attrgetter from pkg_resources import resource_string from storm.exceptions import DatabaseError from zope.component import getUtility @@ -40,30 +41,14 @@ from mailman.interfaces.listmanager import IListManager from mailman.interfaces.mailinglist import IAcceptableAliasSet from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.subscriptions import ISubscriptionService +from mailman.model.bans import Ban from mailman.testing.helpers import temporary_db from mailman.testing.layers import ConfigLayer class MigrationTestBase(unittest.TestCase): - """Test the dated migration (LP: #971013) - - Circa: 3.0b1 -> 3.0b2 - - table mailinglist: - * news_moderation -> newsgroup_moderation - * news_prefix_subject_too -> nntp_prefix_subject_too - * include_list_post_header -> allow_list_posts - * ADD archive_policy - * ADD list_id - * REMOVE archive - * REMOVE archive_private - * REMOVE archive_volume_frequency - * REMOVE nntp_host - - table member: - * mailing_list -> list_id - """ + """Test database migrations.""" layer = ConfigLayer @@ -73,6 +58,30 @@ class MigrationTestBase(unittest.TestCase): def tearDown(self): self._database._cleanup() + def _missing_present(self, table, migrations, missing, present): + """The appropriate migrations leave columns missing and present. + + :param table: The table to test columns from. + :param migrations: Sequence of migrations to load. + :param missing: Set of columns which should be missing after the + migrations are loaded. + :param present: Set of columns which should be present after the + migrations are loaded. + """ + for migration in migrations: + self._database.load_migrations(migration) + self._database.store.commit() + for column in missing: + self.assertRaises(DatabaseError, + self._database.store.execute, + 'select {0} from {1};'.format(column, table)) + self._database.store.rollback() + for column in present: + # This should not produce an exception. Is there some better test + # that we can perform? + self._database.store.execute( + 'select {0} from {1};'.format(column, table)) + class TestMigration20120407Schema(MigrationTestBase): @@ -81,70 +90,52 @@ class TestMigration20120407Schema(MigrationTestBase): def test_pre_upgrade_columns_migration(self): # Test that before the migration, the old table columns are present # and the new database columns are not. - # - # Load all the migrations to just before the one we're testing. - self._database.load_migrations('20120406999999') - self._database.store.commit() - # Verify that the database has not yet been migrated. - for missing in ('allow_list_posts', - 'archive_policy', - 'list_id', - 'nntp_prefix_subject_too'): - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select {0} from mailinglist;'.format(missing)) - self._database.store.rollback() - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select list_id from member;') - self._database.store.rollback() - for present in ('archive', - 'archive_private', - 'archive_volume_frequency', - 'generic_nonmember_action', - 'include_list_post_header', - 'news_moderation', - 'news_prefix_subject_too', - 'nntp_host'): - # This should not produce an exception. Is there some better test - # that we can perform? - self._database.store.execute( - 'select {0} from mailinglist;'.format(present)) - # Again, this should not produce an exception. - self._database.store.execute('select mailing_list from member;') + self._missing_present('mailinglist', + ['20120406999999'], + # New columns are missing. + ('allow_list_posts', + 'archive_policy', + 'list_id', + 'nntp_prefix_subject_too'), + # Old columns are present. + ('archive', + 'archive_private', + 'archive_volume_frequency', + 'generic_nonmember_action', + 'include_list_post_header', + 'news_moderation', + 'news_prefix_subject_too', + 'nntp_host')) + self._missing_present('member', + ['20120406999999'], + ('list_id',), + ('mailing_list',)) def test_post_upgrade_columns_migration(self): # Test that after the migration, the old table columns are missing # and the new database columns are present. - # - # Load all the migrations up to and including the one we're testing. - self._database.load_migrations('20120406999999') - self._database.load_migrations('20120407000000') - # Verify that the database has been migrated. - for present in ('allow_list_posts', - 'archive_policy', - 'list_id', - 'nntp_prefix_subject_too'): - # This should not produce an exception. Is there some better test - # that we can perform? - self._database.store.execute( - 'select {0} from mailinglist;'.format(present)) - self._database.store.execute('select list_id from member;') - for missing in ('archive', - 'archive_private', - 'archive_volume_frequency', - 'generic_nonmember_action', - 'include_list_post_header', - 'news_moderation', - 'news_prefix_subject_too', - 'nntp_host'): - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select {0} from mailinglist;'.format(missing)) - self._database.store.rollback() - self.assertRaises(DatabaseError, - self._database.store.execute, - 'select mailing_list from member;') + self._missing_present('mailinglist', + ['20120406999999', + '20120407000000'], + # The old columns are missing. + ('archive', + 'archive_private', + 'archive_volume_frequency', + 'generic_nonmember_action', + 'include_list_post_header', + 'news_moderation', + 'news_prefix_subject_too', + 'nntp_host'), + # The new columns are present. + ('allow_list_posts', + 'archive_policy', + 'list_id', + 'nntp_prefix_subject_too')) + self._missing_present('member', + ['20120406999999', + '20120407000000'], + ('mailing_list',), + ('list_id',)) @@ -367,3 +358,48 @@ class TestMigration20120407MigratedData(MigrationTestBase): with temporary_db(self._database): mlist = getUtility(IListManager).get('test@example.com') self.assertTrue(mlist.allow_list_posts) + + + +class TestMigration20121015Schema(MigrationTestBase): + """Test column migrations.""" + + def test_pre_upgrade_column_migrations(self): + self._missing_present('ban', + ['20121014999999'], + ('list_id',), + ('mailing_list',)) + + def test_post_upgrade_column_migrations(self): + self._missing_present('ban', + ['20121014999999', + '20121015000000'], + ('mailing_list',), + ('list_id',)) + + +class TestMigration20121015MigratedData(MigrationTestBase): + """Test non-migrated data.""" + + def test_migration_bans(self): + # Load all the migrations to just before the one we're testing. + self._database.load_migrations('20121014999999') + # Insert a list-specific ban. + self._database.store.execute(""" + INSERT INTO ban VALUES ( + 1, 'anne@example.com', 'test@example.com'); + """) + # Insert a global ban. + self._database.store.execute(""" + INSERT INTO ban VALUES ( + 2, 'bart@example.com', NULL); + """) + # Update to the current migration we're testing. + self._database.load_migrations('20121015000000') + # Now both the local and global bans should still be present. + bans = sorted(self._database.store.find(Ban), + key=attrgetter('email')) + self.assertEqual(bans[0].email, 'anne@example.com') + self.assertEqual(bans[0].list_id, 'test.example.com') + self.assertEqual(bans[1].email, 'bart@example.com') + self.assertEqual(bans[1].list_id, None) diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 367c6c53a..11f52a3de 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -29,6 +29,17 @@ REST unverify an address more than once, but verifying an already verified address does not change its `.verified_on` date. (LP: #1054730) +Database +-------- + * The `ban` table now uses list-ids to cross-reference the mailing list, + since these cannot change even if the mailing list is moved or renamed. + +Interfaces +---------- + * The `IBanManager` is no longer a global utility. Instead, you adapt an + `IMailingList` to an `IBanManager` to manage the bans for a specific + mailing list. To manage the global bans, adapt ``None``. + Integration ----------- * Added support for Postfix `relay_domains` setting for better virtual domain diff --git a/src/mailman/interfaces/bans.py b/src/mailman/interfaces/bans.py index 233a374b1..e09d48575 100644 --- a/src/mailman/interfaces/bans.py +++ b/src/mailman/interfaces/bans.py @@ -17,7 +17,7 @@ """Manager of email address bans.""" -from __future__ import absolute_import, unicode_literals +from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ @@ -33,49 +33,48 @@ from zope.interface import Attribute, Interface class IBan(Interface): """A specific ban. - In general, this interface isn't publicly useful. + In general, this interface isn't used publicly. Instead, bans are managed + through the `IBanManager` interface. """ email = Attribute('The banned email address, or pattern.') - mailing_list = Attribute( - """The fqdn name of the mailing list the ban applies to. + list_id = Attribute( + """The list-id of the mailing list the ban applies to. - Use None if this is a global ban. + Use ``None`` if this is a global ban. """) class IBanManager(Interface): - """The global manager of email address bans.""" + """The global manager of email address bans. - def ban(email, mailing_list=None): + To manage bans for a specific mailing list, adapt that `IMailingList` + to an `IBanManager`. To manage global bans, adapt ``None``. + """ + + def ban(email): """Ban an email address from subscribing to a mailing list. When an email address is banned, it will not be allowed to subscribe - to a the named mailing list. This does not affect any email address - already subscribed to the mailing list. With the default arguments, - an email address can be banned globally from subscribing to any - mailing list on the system. + to the mailing list. This does not affect any email address that may + already be subscribed to a mailing list. It is also possible to add a 'ban pattern' whereby all email addresses matching a Python regular expression can be banned. This is accomplished by using a `^` as the first character in `email`. - When an email address is already banned for the given mailing list (or - globally), then this method does nothing. However, it is possible to + When an email address is already banned. However, it is possible to extend a ban for a specific mailing list into a global ban; both bans would be in place and they can be removed individually. :param email: The text email address being banned or, if the string starts with a caret (^), the email address pattern to ban. :type email: str - :param mailing_list: The fqdn name of the mailing list to which the - ban applies. If None, then the ban is global. - :type mailing_list: string """ - def unban(email, mailing_list=None): + def unban(email): """Remove an email address ban. This removes a specific or global email address ban, which would have @@ -85,12 +84,9 @@ class IBanManager(Interface): :param email: The text email address being unbanned or, if the string starts with a caret (^), the email address pattern to unban. :type email: str - :param mailing_list: The fqdn name of the mailing list to which the - unban applies. If None, then the unban is global. - :type mailing_list: string """ - def is_banned(email, mailing_list=None): + def is_banned(email): """Check whether a specific email address is banned. `email` must be a text email address; it cannot be a pattern. The @@ -100,10 +96,6 @@ class IBanManager(Interface): :param email: The text email address being checked. :type email: str - :param mailing_list: The fqdn name of the mailing list being checked. - Note that if not None, both specific and global bans will be - checked. If None, then only global bans will be checked. - :type mailing_list: string :return: A flag indicating whether the given email address is banned or not. :rtype: bool diff --git a/src/mailman/model/bans.py b/src/mailman/model/bans.py index b6de9336f..8687cd993 100644 --- a/src/mailman/model/bans.py +++ b/src/mailman/model/bans.py @@ -42,12 +42,12 @@ class Ban(Model): id = Int(primary=True) email = Unicode() - mailing_list = Unicode() + list_id = Unicode() - def __init__(self, email, mailing_list): + def __init__(self, email, list_id): super(Ban, self).__init__() self.email = email - self.mailing_list = mailing_list + self.list_id = list_id @@ -55,55 +55,58 @@ class Ban(Model): class BanManager: """See `IBanManager`.""" + def __init__(self, mailing_list=None): + self._list_id = (None if mailing_list is None + else mailing_list.list_id) + @dbconnection - def ban(self, store, email, mailing_list=None): + def ban(self, store, email): """See `IBanManager`.""" - bans = store.find(Ban, email=email, mailing_list=mailing_list) + bans = store.find(Ban, email=email, list_id=self._list_id) if bans.count() == 0: - ban = Ban(email, mailing_list) + ban = Ban(email, self._list_id) store.add(ban) @dbconnection - def unban(self, store, email, mailing_list=None): + def unban(self, store, email): """See `IBanManager`.""" - ban = store.find(Ban, email=email, mailing_list=mailing_list).one() + ban = store.find(Ban, email=email, list_id=self._list_id).one() if ban is not None: store.remove(ban) @dbconnection - def is_banned(self, store, email, mailing_list=None): + def is_banned(self, store, email): """See `IBanManager`.""" - # A specific mailing list ban is being checked, however the email - # address could be banned specifically, or globally. - if mailing_list is not None: - # Try specific bans first. - bans = store.find(Ban, email=email, mailing_list=mailing_list) + list_id = self._list_id + if list_id is None: + # The client is asking for global bans. Look up bans on the + # specific email address first. + bans = store.find(Ban, email=email, list_id=None) + if bans.count() > 0: + return True + # And now look for global pattern bans. + bans = store.find(Ban, list_id=None) + for ban in bans: + if (ban.email.startswith('^') and + re.match(ban.email, email, re.IGNORECASE) is not None): + return True + else: + # This is a list-specific ban. + bans = store.find(Ban, email=email, list_id=list_id) if bans.count() > 0: return True # Try global bans next. - bans = store.find(Ban, email=email, mailing_list=None) + bans = store.find(Ban, email=email, list_id=None) if bans.count() > 0: return True # Now try specific mailing list bans, but with a pattern. - bans = store.find(Ban, mailing_list=mailing_list) + bans = store.find(Ban, list_id=list_id) for ban in bans: if (ban.email.startswith('^') and re.match(ban.email, email, re.IGNORECASE) is not None): return True # And now try global pattern bans. - bans = store.find(Ban, mailing_list=None) - for ban in bans: - if (ban.email.startswith('^') and - re.match(ban.email, email, re.IGNORECASE) is not None): - return True - else: - # The client is asking for global bans. Look up bans on the - # specific email address first. - bans = store.find(Ban, email=email, mailing_list=None) - if bans.count() > 0: - return True - # And now look for global pattern bans. - bans = store.find(Ban, mailing_list=None) + bans = store.find(Ban, list_id=None) for ban in bans: if (ban.email.startswith('^') and re.match(ban.email, email, re.IGNORECASE) is not None): |