Barry Warsaw pushed to branch master at mailman / Mailman
Commits: f2620c6f by Barry Warsaw at 2015-12-19T17:23:56Z Digests improvements: * digestable -> digests_enabled * nondigestable: removed * Exposed digests_enabled, digest_send_periodic, digest_volume_frequency in REST. - - - - - d7bc81a6 by Barry Warsaw at 2015-12-19T17:28:36Z NEWS - - - - - bb45766f by Barry Warsaw at 2015-12-20T12:44:06Z Add a send-digests subcommand to send list digests right now. * Add a `mailman send-digests` subcommand which replaces the functionality of the MM2.1 senddigests.py cronjob. * Use mlist.data_path where appropriate instead of crafting it from config.LIST_DATA_DIR. This makes it more consistent to switch to using the list-id as the data subdirectory. * Refactor the to_digest handler so that we can implement maybe_send_digest_now() for the internal API. * Fix some typos in subcommand --help summaries. - - - - - 573e6094 by Barry Warsaw at 2015-12-20T13:53:25Z Move the data_paths. - - - - - f58d9ea9 by Barry Warsaw at 2015-12-20T14:10:05Z Add a test and a fix for the no-args version of `mailman send-digests`. - - - - - 8e244768 by Barry Warsaw at 2015-12-21T12:09:45Z Be liberal with the transaction. - - - - - 22 changed files: - − port_me/senddigests.py - src/mailman/app/lifecycle.py - src/mailman/app/tests/test_lifecycle.py - src/mailman/commands/cli_control.py - src/mailman/commands/cli_help.py - src/mailman/commands/cli_lists.py - + src/mailman/commands/cli_send_digests.py - + src/mailman/commands/tests/test_send_digests.py - src/mailman/core/tests/test_pipelines.py - + src/mailman/database/alembic/versions/70af5a4e5790_digests.py - src/mailman/database/tests/test_migrations.py - src/mailman/docs/NEWS.rst - src/mailman/docs/START.rst - src/mailman/handlers/docs/digests.rst - src/mailman/handlers/to_digest.py - src/mailman/interfaces/mailinglist.py - src/mailman/model/mailinglist.py - src/mailman/rest/docs/listconf.rst - src/mailman/rest/listconf.py - src/mailman/rest/tests/test_listconf.py - src/mailman/runners/docs/digester.rst - src/mailman/styles/base.py Changes: ===================================== port_me/senddigests.py deleted ===================================== --- a/port_me/senddigests.py +++ /dev/null @@ -1,83 +0,0 @@ -# Copyright (C) 1998-2015 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/>. - -import os -import sys -import optparse - -from mailman import MailList -from mailman.core.i18n import _ -from mailman.initialize import initialize -from mailman.version import MAILMAN_VERSION - -# Work around known problems with some RedHat cron daemons -import signal -signal.signal(signal.SIGCHLD, signal.SIG_DFL) - - - -def parseargs(): - parser = optparse.OptionParser(version=MAILMAN_VERSION, - usage=_("""\ -%prog [options] - -Dispatch digests for lists w/pending messages and digest_send_periodic -set.""")) - parser.add_option('-l', '--listname', - type='string', default=[], action='append', - dest='listnames', help=_("""\ -Send the digest for the given list only, otherwise the digests for all -lists are sent out. Multiple -l options may be given.""")) - parser.add_option('-C', '--config', - help=_('Alternative configuration file to use')) - opts, args = parser.parse_args() - if args: - parser.print_help() - print >> sys.stderr, _('Unexpected arguments') - sys.exit(1) - return opts, args, parser - - - -def main(): - opts, args, parser = parseargs() - initialize(opts.config) - - for listname in set(opts.listnames or config.list_manager.names): - mlist = MailList.MailList(listname, lock=False) - if mlist.digest_send_periodic: - mlist.Lock() - try: - try: - mlist.send_digest_now() - mlist.Save() - # We are unable to predict what exception may occur in digest - # processing and we don't want to lose the other digests, so - # we catch everything. - except Exception as errmsg: - print >> sys.stderr, \ - 'List: %s: problem processing %s:\n%s' % \ - (listname, - os.path.join(mlist.data_path, 'digest.mbox'), - errmsg) - finally: - mlist.Unlock() - - - -if __name__ == '__main__': - main() ===================================== src/mailman/app/lifecycle.py ===================================== --- a/src/mailman/app/lifecycle.py +++ b/src/mailman/app/lifecycle.py @@ -23,8 +23,6 @@ __all__ = [ ] -import os -import errno import shutil import logging @@ -94,14 +92,12 @@ def create_list(fqdn_listname, owners=None, style_name=None): def remove_list(mlist): """Remove the list and all associated artifacts and subscriptions.""" - fqdn_listname = mlist.fqdn_listname + # Remove the list's data directory, if it exists. + try: + shutil.rmtree(mlist.data_path) + except FileNotFoundError: + pass # Delete the mailing list from the database. getUtility(IListManager).delete(mlist) # Do the MTA-specific list deletion tasks call_name(config.mta.incoming).delete(mlist) - # Remove the list directory, if it exists. - try: - shutil.rmtree(os.path.join(config.LIST_DATA_DIR, fqdn_listname)) - except OSError as error: - if error.errno != errno.ENOENT: - raise ===================================== src/mailman/app/tests/test_lifecycle.py ===================================== --- a/src/mailman/app/tests/test_lifecycle.py +++ b/src/mailman/app/tests/test_lifecycle.py @@ -26,7 +26,6 @@ import os import shutil import unittest -from mailman.config import config from mailman.interfaces.address import InvalidEmailAddressError from mailman.interfaces.domain import BadDomainSpecificationError from mailman.app.lifecycle import create_list, remove_list @@ -47,13 +46,12 @@ class TestLifecycle(unittest.TestCase): def test_unregistered_domain(self): # Creating a list with an unregistered domain raises an exception. self.assertRaises(BadDomainSpecificationError, - create_list, 't...@nodomain.example.org') + create_list, 't...@nodomain.example.org') def test_remove_list_error(self): # An error occurs while deleting the list's data directory. mlist = create_list('t...@example.com') - data_dir = os.path.join(config.LIST_DATA_DIR, mlist.fqdn_listname) - os.chmod(data_dir, 0) - self.addCleanup(shutil.rmtree, data_dir) + os.chmod(mlist.data_path, 0) + self.addCleanup(shutil.rmtree, mlist.data_path) self.assertRaises(OSError, remove_list, mlist) - os.chmod(data_dir, 0o777) + os.chmod(mlist.data_path, 0o777) ===================================== src/mailman/commands/cli_control.py ===================================== --- a/src/mailman/commands/cli_control.py +++ b/src/mailman/commands/cli_control.py @@ -205,7 +205,7 @@ class Stop(SignalCommand): class Reopen(SignalCommand): - """Signal the Mailman processes to re-open their log files..""" + """Signal the Mailman processes to re-open their log files.""" name = 'reopen' message = _('Reopening the Mailman runners') ===================================== src/mailman/commands/cli_help.py ===================================== --- a/src/mailman/commands/cli_help.py +++ b/src/mailman/commands/cli_help.py @@ -30,7 +30,7 @@ from zope.interface import implementer @implementer(ICLISubCommand) class Help: # Lowercase, to match argparse's default --help text. - """show this help message and exit""" + """Show this help message and exit.""" name = 'help' ===================================== src/mailman/commands/cli_lists.py ===================================== --- a/src/mailman/commands/cli_lists.py +++ b/src/mailman/commands/cli_lists.py @@ -122,7 +122,7 @@ class Lists: @implementer(ICLISubCommand) class Create: - """Create a mailing list""" + """Create a mailing list.""" name = 'create' @@ -238,7 +238,7 @@ class Create: @implementer(ICLISubCommand) class Remove: - """Remove a mailing list""" + """Remove a mailing list.""" name = 'remove' ===================================== src/mailman/commands/cli_send_digests.py ===================================== --- /dev/null +++ b/src/mailman/commands/cli_send_digests.py @@ -0,0 +1,69 @@ +# Copyright (C) 2015 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/>. + +"""The `send_digests` subcommand.""" + +__all__ = [ + 'Send', + ] + + +import sys + +from mailman.core.i18n import _ +from mailman.handlers.to_digest import maybe_send_digest_now +from mailman.interfaces.command import ICLISubCommand +from mailman.interfaces.listmanager import IListManager +from zope.component import getUtility +from zope.interface import implementer + + + +@implementer(ICLISubCommand) +class Send: + """Send some mailing list digests right now.""" + + name = 'send-digests' + + def add(self, parser, command_parser): + """See `ICLISubCommand`.""" + + command_parser.add_argument( + '-l', '--list', + default=[], dest='lists', metavar='list', action='append', + help=_("""Send the digests for this mailing list. Multiple --list + options can be given. The argument can either be a List-ID + or a fully qualified list name. Without this option, the + digests for all mailing lists will be sent if possible.""")) + + def process(self, args): + """See `ICLISubCommand`.""" + if not args.lists: + # Send the digests for every list. + maybe_send_digest_now(force=True) + return + list_manager = getUtility(IListManager) + for list_spec in args.lists: + # We'll accept list-ids or fqdn list names. + if '@' in list_spec: + mlist = list_manager.get(list_spec) + else: + mlist = list_manager.get_by_list_id(list_spec) + if mlist is None: + print(_('No such list found: $list_spec'), file=sys.stderr) + continue + maybe_send_digest_now(mlist, force=True) ===================================== src/mailman/commands/tests/test_send_digests.py ===================================== --- /dev/null +++ b/src/mailman/commands/tests/test_send_digests.py @@ -0,0 +1,353 @@ +# Copyright (C) 2015 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/>. + +"""Test the send-digests subcommand.""" + +__all__ = [ + 'TestSendDigests', + ] + + +import os +import unittest + +from io import StringIO +from mailman.app.lifecycle import create_list +from mailman.commands.cli_send_digests import Send +from mailman.config import config +from mailman.interfaces.member import DeliveryMode +from mailman.runners.digest import DigestRunner +from mailman.testing.helpers import ( + get_queue_messages, make_testable_runner, + specialized_message_from_string as mfs, subscribe) +from mailman.testing.layers import ConfigLayer +from unittest.mock import patch + + + +class FakeArgs: + def __init__(self): + self.lists = [] + + + +class TestSendDigests(unittest.TestCase): + """Test the send-digests subcommand.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('a...@example.com') + self._mlist.digests_enabled = True + self._mlist.digest_size_threshold = 100000 + self._mlist.send_welcome_message = False + self._command = Send() + self._handler = config.handlers['to-digest'] + self._runner = make_testable_runner(DigestRunner, 'digest') + # The mailing list needs at least one digest recipient. + member = subscribe(self._mlist, 'Anne') + member.preferences.delivery_mode = DeliveryMode.plaintext_digests + + def test_send_one_digest_by_list_id(self): + msg = mfs("""\ +To: a...@example.com +From: a...@example.com +Subject: message 1 + +""") + self._handler.process(self._mlist, msg, {}) + del msg['subject'] + msg['subject'] = 'message 2' + self._handler.process(self._mlist, msg, {}) + # There are no digests already being sent, but the ant mailing list + # does have a digest mbox collecting messages. + items = get_queue_messages('digest') + self.assertEqual(len(items), 0) + mailbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(mailbox_path), 0) + args = FakeArgs() + args.lists.append('ant.example.com') + self._command.process(args) + self._runner.run() + # Now, there's no digest mbox and there's a plaintext digest in the + # outgoing queue. + self.assertFalse(os.path.exists(mailbox_path)) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + digest_contents = str(items[0].msg) + self.assertIn('Subject: message 1', digest_contents) + self.assertIn('Subject: message 2', digest_contents) + + def test_send_one_digest_by_fqdn_listname(self): + msg = mfs("""\ +To: a...@example.com +From: a...@example.com +Subject: message 1 + +""") + self._handler.process(self._mlist, msg, {}) + del msg['subject'] + msg['subject'] = 'message 2' + self._handler.process(self._mlist, msg, {}) + # There are no digests already being sent, but the ant mailing list + # does have a digest mbox collecting messages. + items = get_queue_messages('digest') + self.assertEqual(len(items), 0) + mailbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(mailbox_path), 0) + args = FakeArgs() + args.lists.append('a...@example.com') + self._command.process(args) + self._runner.run() + # Now, there's no digest mbox and there's a plaintext digest in the + # outgoing queue. + self.assertFalse(os.path.exists(mailbox_path)) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + digest_contents = str(items[0].msg) + self.assertIn('Subject: message 1', digest_contents) + self.assertIn('Subject: message 2', digest_contents) + + def test_send_one_digest_to_missing_list_id(self): + msg = mfs("""\ +To: a...@example.com +From: a...@example.com +Subject: message 1 + +""") + self._handler.process(self._mlist, msg, {}) + del msg['subject'] + msg['subject'] = 'message 2' + self._handler.process(self._mlist, msg, {}) + # There are no digests already being sent, but the ant mailing list + # does have a digest mbox collecting messages. + items = get_queue_messages('digest') + self.assertEqual(len(items), 0) + mailbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(mailbox_path), 0) + args = FakeArgs() + args.lists.append('bee.example.com') + stderr = StringIO() + with patch('mailman.commands.cli_send_digests.sys.stderr', stderr): + self._command.process(args) + self._runner.run() + # The warning was printed to stderr. + self.assertEqual(stderr.getvalue(), + 'No such list found: bee.example.com\n') + # And no digest was prepared. + self.assertGreater(os.path.getsize(mailbox_path), 0) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 0) + + def test_send_one_digest_to_missing_fqdn_listname(self): + msg = mfs("""\ +To: a...@example.com +From: a...@example.com +Subject: message 1 + +""") + self._handler.process(self._mlist, msg, {}) + del msg['subject'] + msg['subject'] = 'message 2' + self._handler.process(self._mlist, msg, {}) + # There are no digests already being sent, but the ant mailing list + # does have a digest mbox collecting messages. + items = get_queue_messages('digest') + self.assertEqual(len(items), 0) + mailbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(mailbox_path), 0) + args = FakeArgs() + args.lists.append('b...@example.com') + stderr = StringIO() + with patch('mailman.commands.cli_send_digests.sys.stderr', stderr): + self._command.process(args) + self._runner.run() + # The warning was printed to stderr. + self.assertEqual(stderr.getvalue(), + 'No such list found: b...@example.com\n') + # And no digest was prepared. + self.assertGreater(os.path.getsize(mailbox_path), 0) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 0) + + def test_send_digest_to_one_missing_and_one_existing_list(self): + msg = mfs("""\ +To: a...@example.com +From: a...@example.com +Subject: message 1 + +""") + self._handler.process(self._mlist, msg, {}) + del msg['subject'] + msg['subject'] = 'message 2' + self._handler.process(self._mlist, msg, {}) + # There are no digests already being sent, but the ant mailing list + # does have a digest mbox collecting messages. + items = get_queue_messages('digest') + self.assertEqual(len(items), 0) + mailbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(mailbox_path), 0) + args = FakeArgs() + args.lists.extend(('ant.example.com', 'bee.example.com')) + stderr = StringIO() + with patch('mailman.commands.cli_send_digests.sys.stderr', stderr): + self._command.process(args) + self._runner.run() + # The warning was printed to stderr. + self.assertEqual(stderr.getvalue(), + 'No such list found: bee.example.com\n') + # But ant's digest was still prepared. + self.assertFalse(os.path.exists(mailbox_path)) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + digest_contents = str(items[0].msg) + self.assertIn('Subject: message 1', digest_contents) + self.assertIn('Subject: message 2', digest_contents) + + def test_send_digests_for_two_lists(self): + # Populate ant's digest. + msg = mfs("""\ +To: a...@example.com +From: a...@example.com +Subject: message 1 + +""") + self._handler.process(self._mlist, msg, {}) + del msg['subject'] + msg['subject'] = 'message 2' + self._handler.process(self._mlist, msg, {}) + # Create the second list. + bee = create_list('b...@example.com') + bee.digests_enabled = True + bee.digest_size_threshold = 100000 + bee.send_welcome_message = False + member = subscribe(bee, 'Bart') + member.preferences.delivery_mode = DeliveryMode.plaintext_digests + # Populate bee's digest. + msg = mfs("""\ +To: b...@example.com +From: b...@example.com +Subject: message 3 + +""") + self._handler.process(bee, msg, {}) + del msg['subject'] + msg['subject'] = 'message 4' + self._handler.process(bee, msg, {}) + # There are no digests for either list already being sent, but the + # mailing lists do have a digest mbox collecting messages. + ant_mailbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(ant_mailbox_path), 0) + # Check bee's digest. + bee_mailbox_path = os.path.join(bee.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(bee_mailbox_path), 0) + # Both. + items = get_queue_messages('digest') + self.assertEqual(len(items), 0) + # Process both list's digests. + args = FakeArgs() + args.lists.extend(('ant.example.com', 'b...@example.com')) + self._command.process(args) + self._runner.run() + # Now, neither list has a digest mbox and but there are plaintext + # digest in the outgoing queue for both. + self.assertFalse(os.path.exists(ant_mailbox_path)) + self.assertFalse(os.path.exists(bee_mailbox_path)) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 2) + # Figure out which digest is going to ant and which to bee. + if items[0].msg['to'] == 'a...@example.com': + ant = items[0].msg + bee = items[1].msg + else: + assert items[0].msg['to'] == 'b...@example.com' + ant = items[1].msg + bee = items[0].msg + # Check ant's digest. + digest_contents = str(ant) + self.assertIn('Subject: message 1', digest_contents) + self.assertIn('Subject: message 2', digest_contents) + # Check bee's digest. + digest_contents = str(bee) + self.assertIn('Subject: message 3', digest_contents) + self.assertIn('Subject: message 4', digest_contents) + + def test_send_digests_for_all_lists(self): + # Populate ant's digest. + msg = mfs("""\ +To: a...@example.com +From: a...@example.com +Subject: message 1 + +""") + self._handler.process(self._mlist, msg, {}) + del msg['subject'] + msg['subject'] = 'message 2' + self._handler.process(self._mlist, msg, {}) + # Create the second list. + bee = create_list('b...@example.com') + bee.digests_enabled = True + bee.digest_size_threshold = 100000 + bee.send_welcome_message = False + member = subscribe(bee, 'Bart') + member.preferences.delivery_mode = DeliveryMode.plaintext_digests + # Populate bee's digest. + msg = mfs("""\ +To: b...@example.com +From: b...@example.com +Subject: message 3 + +""") + self._handler.process(bee, msg, {}) + del msg['subject'] + msg['subject'] = 'message 4' + self._handler.process(bee, msg, {}) + # There are no digests for either list already being sent, but the + # mailing lists do have a digest mbox collecting messages. + ant_mailbox_path = os.path.join(self._mlist.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(ant_mailbox_path), 0) + # Check bee's digest. + bee_mailbox_path = os.path.join(bee.data_path, 'digest.mmdf') + self.assertGreater(os.path.getsize(bee_mailbox_path), 0) + # Both. + items = get_queue_messages('digest') + self.assertEqual(len(items), 0) + # Process all mailing list digests by not setting any arguments. + self._command.process(FakeArgs()) + self._runner.run() + # Now, neither list has a digest mbox and but there are plaintext + # digest in the outgoing queue for both. + self.assertFalse(os.path.exists(ant_mailbox_path)) + self.assertFalse(os.path.exists(bee_mailbox_path)) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 2) + # Figure out which digest is going to ant and which to bee. + if items[0].msg['to'] == 'a...@example.com': + ant = items[0].msg + bee = items[1].msg + else: + assert items[0].msg['to'] == 'b...@example.com' + ant = items[1].msg + bee = items[0].msg + # Check ant's digest. + digest_contents = str(ant) + self.assertIn('Subject: message 1', digest_contents) + self.assertIn('Subject: message 2', digest_contents) + # Check bee's digest. + digest_contents = str(bee) + self.assertIn('Subject: message 3', digest_contents) + self.assertIn('Subject: message 4', digest_contents) ===================================== src/mailman/core/tests/test_pipelines.py ===================================== --- a/src/mailman/core/tests/test_pipelines.py +++ b/src/mailman/core/tests/test_pipelines.py @@ -151,7 +151,7 @@ testing def test_only_decorate_output(self): # Ensure that decoration is not done on the archive, digest, or # usenet copy of the message. - self.assertTrue(self._mlist.digestable) + self.assertTrue(self._mlist.digests_enabled) # Set up NNTP. self._mlist.gateway_to_news = True self._mlist.linked_newsgroup = 'testing' ===================================== src/mailman/database/alembic/versions/70af5a4e5790_digests.py ===================================== --- /dev/null +++ b/src/mailman/database/alembic/versions/70af5a4e5790_digests.py @@ -0,0 +1,45 @@ +"""digests + +Revision ID: 70af5a4e5790 +Revises: 47294d3a604 +Create Date: 2015-12-19 12:05:42.202998 + +""" + +# revision identifiers, used by Alembic. +revision = '70af5a4e5790' +down_revision = '47294d3a604' + +import os +import sqlalchemy as sa + +from alembic import op +from mailman.config import config + + +def upgrade(): + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.alter_column('digestable', new_column_name='digests_enabled') + batch_op.drop_column('nondigestable') + # Non-database migration: rename the list's data-path. + for dirname in os.listdir(config.LIST_DATA_DIR): + if '@' in dirname: + old_name = os.path.join(config.LIST_DATA_DIR, dirname) + listname, at, domain = dirname.partition('@') + new_name = os.path.join(config.LIST_DATA_DIR, + '{}.{}'.format(listname, domain)) + os.rename(old_name, new_name) + + +def downgrade(): + with op.batch_alter_table('mailinglist') as batch_op: + batch_op.alter_column('digests_enabled', new_column_name='digestable') + # The data for this column is lost, it's not used anyway. + batch_op.add_column(sa.Column('nondigestable', sa.Boolean)) + for dirname in os.listdir(config.LIST_DATA_DIR): + if '@' not in dirname: + old_name = os.path.join(config.LIST_DATA_DIR, dirname) + listname, domain = dirname.split('.', 1) + new_name = os.path.join(config.LIST_DATA_DIR, + '{}@{}'.format(listname, domain)) + os.rename(old_name, new_name) ===================================== src/mailman/database/tests/test_migrations.py ===================================== --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -22,10 +22,12 @@ __all__ = [ ] +import os import unittest import alembic.command import sqlalchemy as sa +from mailman.app.lifecycle import create_list from mailman.config import config from mailman.database.alembic import alembic_cfg from mailman.database.helpers import exists_in_db @@ -169,3 +171,53 @@ class TestMigrations(unittest.TestCase): self.assertNotIn('type', results[i]) self.assertEqual(results[4]['type'], '"held message"') self.assertEqual(results[5]['type'], '"registration"') + + def test_70af5a4e5790_digests(self): + IDS_TO_DIGESTABLE = [ + (1, True), + (2, False), + (3, False), + (4, True), + ] + mlist_table = sa.sql.table( + 'mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('digests_enabled', sa.Boolean) + ) + # Downgrading. + with transaction(): + for table_id, enabled in IDS_TO_DIGESTABLE: + config.db.store.execute(mlist_table.insert().values( + id=table_id, digests_enabled=enabled)) + with transaction(): + alembic.command.downgrade(alembic_cfg, '47294d3a604') + results = config.db.store.execute( + 'SELECT id, digestable FROM mailinglist').fetchall() + self.assertEqual(results, IDS_TO_DIGESTABLE) + # Upgrading. + with transaction(): + alembic.command.upgrade(alembic_cfg, '70af5a4e5790') + results = config.db.store.execute( + 'SELECT id, digests_enabled FROM mailinglist').fetchall() + self.assertEqual(results, IDS_TO_DIGESTABLE) + + def test_70af5a4e5790_data_paths(self): + # Create a couple of mailing lists through the standard API. + with transaction(): + ant = create_list('a...@example.com') + bee = create_list('b...@example.com') + # Downgrade and verify that the old data paths exist. + alembic.command.downgrade(alembic_cfg, '47294d3a604') + self.assertTrue(os.path.exists( + os.path.join(config.LIST_DATA_DIR, 'a...@example.com'))) + self.assertTrue(os.path.exists( + os.path.join(config.LIST_DATA_DIR, 'a...@example.com'))) + # Upgrade and verify that the new data paths exists and the old ones + # no longer do. + alembic.command.upgrade(alembic_cfg, '70af5a4e5790') + self.assertFalse(os.path.exists( + os.path.join(config.LIST_DATA_DIR, 'a...@example.com'))) + self.assertFalse(os.path.exists( + os.path.join(config.LIST_DATA_DIR, 'a...@example.com'))) + self.assertTrue(os.path.exists(ant.data_path)) + self.assertTrue(os.path.exists(bee.data_path)) ===================================== src/mailman/docs/NEWS.rst ===================================== --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -120,6 +120,9 @@ REST Aurélien Bompard. * Fixed a number of corner cases for the return codes when PUTing or PATCHing list configuration variables. (Closes: #182) + * Expose ``digest_send_periodic``, ``digest_volume_frequency``, and + ``digests_enabled`` (renamed from ``digestable``) to the REST API. + (Closes: #159) Other ----- @@ -129,6 +132,12 @@ Other ``list_url`` or permalink. Given by Aurélien Bompard. * Large performance improvement in ``SubscriptionService.find_members()``. Given by Aurélien Bompard. + * Rework the digest machinery, and add a new `send-digests` subcommand, which + can be used from the command line or cron to immediately send out any + partially collected digests. + * The mailing list "data directory" has been renamed. Instead of using the + fqdn listname, the subdirectory inside ``[paths]list_data_dir`` now uses + the List-ID. 3.0.0 -- "Show Don't Tell" ===================================== src/mailman/docs/START.rst ===================================== --- a/src/mailman/docs/START.rst +++ b/src/mailman/docs/START.rst @@ -83,6 +83,9 @@ so:: $ MAILMAN_EXTRA_TESTING_CFG=/path/to/postgres.cfg .tox/pg/bin/python -m nose2 -vv -P user +Note that the path specified in `MAILMAN_EXTRA_TESTING_CFG` must be an +absolute path or some tests will fail. + Building for development ------------------------ ===================================== src/mailman/handlers/docs/digests.rst ===================================== --- a/src/mailman/handlers/docs/digests.rst +++ b/src/mailman/handlers/docs/digests.rst @@ -37,7 +37,7 @@ Short circuiting When a message is posted to the mailing list, it is generally added to a mailbox, unless the mailing list does not allow digests. - >>> mlist.digestable = False + >>> mlist.digests_enabled = False >>> msg = next(message_factory) >>> process = config.handlers['to-digest'].process >>> process(mlist, msg, {}) @@ -49,7 +49,7 @@ mailbox, unless the mailing list does not allow digests. ...or they may allow digests but the message is already a digest. - >>> mlist.digestable = True + >>> mlist.digests_enabled = True >>> process(mlist, msg, dict(isdigest=True)) >>> sum(1 for msg in digest_mbox(mlist)) 0 ===================================== src/mailman/handlers/to_digest.py ===================================== --- a/src/mailman/handlers/to_digest.py +++ b/src/mailman/handlers/to_digest.py @@ -19,6 +19,8 @@ __all__ = [ 'ToDigest', + 'bump_digest_number_and_volume', + 'maybe_send_digest_now', ] @@ -29,8 +31,10 @@ from mailman.core.i18n import _ from mailman.email.message import Message from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.handler import IHandler +from mailman.interfaces.listmanager import IListManager from mailman.utilities.datetime import now as right_now from mailman.utilities.mailbox import Mailbox +from zope.component import getUtility from zope.interface import implementer @@ -44,37 +48,16 @@ class ToDigest: def process(self, mlist, msg, msgdata): """See `IHandler`.""" - # Short circuit for non-digestable messages. - if not mlist.digestable or msgdata.get('isdigest'): + # Short-circuit if digests are not enabled or if this message already + # is a digest. + if not mlist.digests_enabled or msgdata.get('isdigest'): return # Open the mailbox that will be used to collect the current digest. mailbox_path = os.path.join(mlist.data_path, 'digest.mmdf') # Lock the mailbox and append the message. with Mailbox(mailbox_path, create=True) as mbox: mbox.add(msg) - # Calculate the current size of the mailbox file. This will not tell - # us exactly how big the resulting MIME and rfc1153 digest will - # actually be, but it's the most easily available metric to decide - # whether the size threshold has been reached. - size = os.path.getsize(mailbox_path) - if size >= mlist.digest_size_threshold * 1024.0: - # The digest is ready to send. Because we don't want to hold up - # this process with crafting the digest, we're going to move the - # digest file to a safe place, then craft a fake message for the - # DigestRunner as a trigger for it to build and send the digest. - mailbox_dest = os.path.join( - mlist.data_path, - 'digest.{0.volume}.{0.next_digest_number}.mmdf'.format(mlist)) - volume = mlist.volume - digest_number = mlist.next_digest_number - bump_digest_number_and_volume(mlist) - os.rename(mailbox_path, mailbox_dest) - config.switchboards['digest'].enqueue( - Message(), - listid=mlist.list_id, - digest_path=mailbox_dest, - volume=volume, - digest_number=digest_number) + maybe_send_digest_now(mlist) @@ -116,3 +99,54 @@ def bump_digest_number_and_volume(mlist): # Just bump the digest number. mlist.next_digest_number += 1 mlist.digest_last_sent_at = now + + + +def maybe_send_digest_now(mlist=None, force=False): + """Send this mailing list's digest now. + + If there are any messages in this mailing list's digest, the + digest is sent immediately, regardless of whether the size + threshold has been met. When called through the subcommand + `mailman send_digest` the value of .digest_send_periodic is + consulted. + + :param mlist: The mailing list whose digest should be sent. If this is + None, all mailing lists with non-zero sized digests will have theirs + sent immediately. + :type mlist: IMailingList or None + :param force: Should the digest be sent even if the size threshold hasn't + been met? + :type force: boolean + """ + if mlist is None: + digestable_lists = getUtility(IListManager).mailing_lists + else: + digestable_lists = [mlist] + for mailing_list in digestable_lists: + mailbox_path = os.path.join(mailing_list.data_path, 'digest.mmdf') + # Calculate the current size of the mailbox file. This will not tell + # us exactly how big the resulting MIME and rfc1153 digest will + # actually be, but it's the most easily available metric to decide + # whether the size threshold has been reached. + size = os.path.getsize(mailbox_path) + if (size >= mailing_list.digest_size_threshold * 1024.0 or + (force and size > 0)): + # Send the digest. Because we don't want to hold up this process + # with crafting the digest, we're going to move the digest file to + # a safe place, then craft a fake message for the DigestRunner as + # a trigger for it to build and send the digest. + mailbox_dest = os.path.join( + mailing_list.data_path, + 'digest.{0.volume}.{0.next_digest_number}.mmdf'.format( + mailing_list)) + volume = mailing_list.volume + digest_number = mailing_list.next_digest_number + bump_digest_number_and_volume(mailing_list) + os.rename(mailbox_path, mailbox_dest) + config.switchboards['digest'].enqueue( + Message(), + listid=mailing_list.list_id, + digest_path=mailbox_dest, + volume=volume, + digest_number=digest_number) ===================================== src/mailman/interfaces/mailinglist.py ===================================== --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -308,6 +308,20 @@ class IMailingList(Interface): # Digests. + digests_enabled = Attribute( + """Whether or not digests are enabled for this mailing list.""") + + digest_size_threshold = Attribute( + """The maximum (approximate) size in kilobytes of the digest currently + being collected.""") + + digest_send_periodic = Attribute( + """Should a digest be sent by the `mailman send_digest` command even + when the size threshold hasn't yet been met?""") + + digest_volume_frequency = Attribute( + """How often should a new digest volume be started?""") + digest_last_sent_at = Attribute( """The date and time a digest of this mailing list was last sent.""") @@ -322,10 +336,6 @@ class IMailingList(Interface): the digest volume number is bumped, the digest number is reset to 1.""") - digest_size_threshold = Attribute( - """The maximum (approximate) size in kilobytes of the digest currently - being collected.""") - def send_one_last_digest_to(address, delivery_mode): """Make sure to send one last digest to an address. ===================================== src/mailman/model/mailinglist.py ===================================== --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -135,13 +135,13 @@ class MailingList(Model): default_member_action = Column(Enum(Action)) default_nonmember_action = Column(Enum(Action)) description = Column(Unicode) + digests_enabled = Column(Boolean) digest_footer_uri = Column(Unicode) digest_header_uri = Column(Unicode) digest_is_default = Column(Boolean) digest_send_periodic = Column(Boolean) digest_size_threshold = Column(Float) digest_volume_frequency = Column(Enum(DigestFrequency)) - digestable = Column(Boolean) discard_these_nonmembers = Column(PickleType) emergency = Column(Boolean) encode_ascii_prefixes = Column(Boolean) @@ -164,7 +164,6 @@ class MailingList(Model): moderator_password = Column(LargeBinary) # TODO : was RawStr() newsgroup_moderation = Column(Enum(NewsgroupModeration)) nntp_prefix_subject_too = Column(Boolean) - nondigestable = Column(Boolean) nonmember_rejection_notice = Column(Unicode) obscure_addresses = Column(Boolean) owner_chain = Column(Unicode) @@ -262,7 +261,7 @@ class MailingList(Model): @property def data_path(self): """See `IMailingList`.""" - return os.path.join(config.LIST_DATA_DIR, self.fqdn_listname) + return os.path.join(config.LIST_DATA_DIR, self.list_id) # IMailingListAddresses ===================================== src/mailman/rest/docs/listconf.rst ===================================== --- a/src/mailman/rest/docs/listconf.rst +++ b/src/mailman/rest/docs/listconf.rst @@ -37,7 +37,10 @@ All readable attributes for a list are available on a sub-resource. default_nonmember_action: hold description: digest_last_sent_at: None + digest_send_periodic: True digest_size_threshold: 30.0 + digest_volume_frequency: monthly + digests_enabled: True display_name: Ant filter_content: False first_strip_reply_to: False @@ -97,7 +100,10 @@ When using ``PUT``, all writable attributes must be included. ... description='This is my mailing list', ... include_rfc2369_headers=False, ... allow_list_posts=False, + ... digest_send_periodic=False, ... digest_size_threshold=10.5, + ... digest_volume_frequency='yearly', + ... digests_enabled=False, ... posting_pipeline='virgin', ... filter_content=True, ... first_strip_reply_to=True, @@ -145,7 +151,10 @@ These values are changed permanently. default_nonmember_action: discard description: This is my mailing list ... + digest_send_periodic: False digest_size_threshold: 10.5 + digest_volume_frequency: yearly + digests_enabled: False display_name: Fnords filter_content: True first_strip_reply_to: True ===================================== src/mailman/rest/listconf.py ===================================== --- a/src/mailman/rest/listconf.py +++ b/src/mailman/rest/listconf.py @@ -29,6 +29,7 @@ from mailman.core.errors import ( from mailman.interfaces.action import Action from mailman.interfaces.archiver import ArchivePolicy from mailman.interfaces.autorespond import ResponseAction +from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.mailinglist import ( IAcceptableAliasSet, ReplyToMunging, SubscriptionPolicy) from mailman.rest.helpers import ( @@ -112,7 +113,10 @@ ATTRIBUTES = dict( default_nonmember_action=GetterSetter(enum_validator(Action)), description=GetterSetter(str), digest_last_sent_at=GetterSetter(None), + digest_send_periodic=GetterSetter(as_boolean), digest_size_threshold=GetterSetter(float), + digest_volume_frequency=GetterSetter(enum_validator(DigestFrequency)), + digests_enabled=GetterSetter(as_boolean), filter_content=GetterSetter(as_boolean), first_strip_reply_to=GetterSetter(as_boolean), fqdn_listname=GetterSetter(None), ===================================== src/mailman/rest/tests/test_listconf.py ===================================== --- a/src/mailman/rest/tests/test_listconf.py +++ b/src/mailman/rest/tests/test_listconf.py @@ -26,6 +26,7 @@ import unittest from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction +from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.mailinglist import ( IAcceptableAliasSet, SubscriptionPolicy) from mailman.testing.helpers import call_api @@ -58,9 +59,10 @@ RESOURCE = dict( description='This is my mailing list', include_rfc2369_headers=False, allow_list_posts=False, - #digest_send_periodic='yes', + digest_send_periodic=True, digest_size_threshold=10.5, - #digest_volume_frequency=1, + digest_volume_frequency='monthly', + digests_enabled=True, posting_pipeline='virgin', filter_content=True, first_strip_reply_to=True, @@ -198,7 +200,7 @@ class TestConfiguration(unittest.TestCase): def test_patch_attribute_double(self): with self.assertRaises(HTTPError) as cm: - resource, response = call_api( + call_api( 'http://localhost:9001/3.0/lists/ant.example.com' '/config/reply_to_address', dict(display_name='b...@ant.example.com', @@ -255,3 +257,121 @@ class TestConfiguration(unittest.TestCase): self.assertEqual(cm.exception.code, 400) self.assertEqual(cm.exception.reason, b'Cannot convert parameters: posting_pipeline') + + def test_get_digest_send_periodic(self): + with transaction(): + self._mlist.digest_send_periodic = False + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_send_periodic') + self.assertFalse(resource['digest_send_periodic']) + + def test_patch_digest_send_periodic(self): + with transaction(): + self._mlist.digest_send_periodic = False + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_send_periodic', + dict(digest_send_periodic=True), + 'PATCH') + self.assertEqual(response.status, 204) + self.assertTrue(self._mlist.digest_send_periodic) + + def test_put_digest_send_periodic(self): + with transaction(): + self._mlist.digest_send_periodic = False + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_send_periodic', + dict(digest_send_periodic=True), + 'PUT') + self.assertEqual(response.status, 204) + self.assertTrue(self._mlist.digest_send_periodic) + + def test_get_digest_volume_frequency(self): + with transaction(): + self._mlist.digest_volume_frequency = DigestFrequency.yearly + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_volume_frequency') + self.assertEqual(resource['digest_volume_frequency'], 'yearly') + + def test_patch_digest_volume_frequency(self): + with transaction(): + self._mlist.digest_volume_frequency = DigestFrequency.yearly + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_volume_frequency', + dict(digest_volume_frequency='monthly'), + 'PATCH') + self.assertEqual(response.status, 204) + self.assertEqual(self._mlist.digest_volume_frequency, + DigestFrequency.monthly) + + def test_put_digest_volume_frequency(self): + with transaction(): + self._mlist.digest_volume_frequency = DigestFrequency.yearly + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_volume_frequency', + dict(digest_volume_frequency='monthly'), + 'PUT') + self.assertEqual(response.status, 204) + self.assertEqual(self._mlist.digest_volume_frequency, + DigestFrequency.monthly) + + def test_bad_patch_digest_volume_frequency(self): + with transaction(): + self._mlist.digest_volume_frequency = DigestFrequency.yearly + with self.assertRaises(HTTPError) as cm: + call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_volume_frequency', + dict(digest_volume_frequency='once in a while'), + 'PATCH') + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.reason, + b'Cannot convert parameters: digest_volume_frequency') + + def test_bad_put_digest_volume_frequency(self): + with transaction(): + self._mlist.digest_volume_frequency = DigestFrequency.yearly + with self.assertRaises(HTTPError) as cm: + call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digest_volume_frequency', + dict(digest_volume_frequency='once in a while'), + 'PUT') + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.reason, + b'Cannot convert parameters: digest_volume_frequency') + + def test_get_digests_enabled(self): + with transaction(): + self._mlist.digests_enabled = False + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digests_enabled') + self.assertFalse(resource['digests_enabled']) + + def test_patch_digests_enabled(self): + with transaction(): + self._mlist.digests_enabled = False + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digests_enabled', + dict(digests_enabled=True), + 'PATCH') + self.assertEqual(response.status, 204) + self.assertTrue(self._mlist.digests_enabled) + + def test_put_digests_enabled(self): + with transaction(): + self._mlist.digests_enabled = False + resource, response = call_api( + 'http://localhost:9001/3.0/lists/ant.example.com/config' + '/digests_enabled', + dict(digests_enabled=True), + 'PUT') + self.assertEqual(response.status, 204) + self.assertTrue(self._mlist.digests_enabled) ===================================== src/mailman/runners/docs/digester.rst ===================================== --- a/src/mailman/runners/docs/digester.rst +++ b/src/mailman/runners/docs/digester.rst @@ -56,7 +56,7 @@ But the message metadata has a reference to the digest file. >>> dump_msgdata(entry.msgdata) _parsemsg : False digest_number: 1 - digest_path : .../lists/t...@example.com/digest.1.1.mmdf + digest_path : .../lists/test.example.com/digest.1.1.mmdf listid : test.example.com version : 3 volume : 1 ===================================== src/mailman/styles/base.py ===================================== --- a/src/mailman/styles/base.py +++ b/src/mailman/styles/base.py @@ -87,7 +87,7 @@ class BasicOperation: mlist.filter_action = FilterAction.discard mlist.filter_content = False # Digests. - mlist.digestable = True + mlist.digests_enabled = True mlist.digest_is_default = False mlist.mime_is_default_digest = False mlist.digest_size_threshold = 30 # KB @@ -97,7 +97,6 @@ class BasicOperation: 'mailman:///$listname/$language/footer-generic.txt') mlist.digest_volume_frequency = DigestFrequency.monthly mlist.next_digest_number = 1 - mlist.nondigestable = True # NNTP gateway mlist.nntp_host = '' mlist.linked_newsgroup = '' View it on GitLab: https://gitlab.com/mailman/mailman/compare/9f14de31ec1f78a2f2847d7a2c9b8efb775adab9...8e24476848de89302d9b0a8ea91116288201a95d
_______________________________________________ Mailman-checkins mailing list Mailman-checkins@python.org Unsubscribe: https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
