diff options
| -rw-r--r-- | src/mailman/app/digests.py | 3 | ||||
| -rw-r--r-- | src/mailman/bin/mailman.py | 4 | ||||
| -rw-r--r-- | src/mailman/bin/tests/test_mailman.py | 57 | ||||
| -rw-r--r-- | src/mailman/commands/cli_digests.py | 26 | ||||
| -rw-r--r-- | src/mailman/commands/tests/test_digests.py | 28 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 3 |
6 files changed, 116 insertions, 5 deletions
diff --git a/src/mailman/app/digests.py b/src/mailman/app/digests.py index f8b217142..0d47df328 100644 --- a/src/mailman/app/digests.py +++ b/src/mailman/app/digests.py @@ -56,8 +56,7 @@ def bump_digest_number_and_volume(mlist): bump = (now.toordinal() > mlist.digest_last_sent_at.toordinal()) else: raise AssertionError( - 'Bad DigestFrequency: {0}'.format( - mlist.digest_volume_frequency)) + 'Bad DigestFrequency: {}'.format(mlist.digest_volume_frequency)) if bump: mlist.volume += 1 mlist.next_digest_number = 1 diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py index 7a443e9d6..b4d144b7c 100644 --- a/src/mailman/bin/mailman.py +++ b/src/mailman/bin/mailman.py @@ -24,6 +24,7 @@ from functools import cmp_to_key from mailman import public from mailman.core.i18n import _ from mailman.core.initialize import initialize +from mailman.database.transaction import transaction from mailman.interfaces.command import ICLISubCommand from mailman.utilities.modules import find_components from mailman.version import MAILMAN_VERSION_FULL @@ -95,4 +96,5 @@ def main(): else os.path.abspath(os.path.expanduser(args.config))) initialize(config_path) # Perform the subcommand option. - args.func(args) + with transaction(): + args.func(args) diff --git a/src/mailman/bin/tests/test_mailman.py b/src/mailman/bin/tests/test_mailman.py index 7c9d2f2bc..c678ff560 100644 --- a/src/mailman/bin/tests/test_mailman.py +++ b/src/mailman/bin/tests/test_mailman.py @@ -19,12 +19,21 @@ import unittest +from contextlib import ExitStack +from datetime import timedelta from io import StringIO +from mailman.app.lifecycle import create_list from mailman.bin.mailman import main +from mailman.config import config +from mailman.database.transaction import transaction +from mailman.testing.layers import ConfigLayer +from mailman.utilities.datetime import now from unittest.mock import patch class TestMailmanCommand(unittest.TestCase): + layer = ConfigLayer + def test_mailman_command_without_subcommand_prints_help(self): # Issue #137: Running `mailman` without a subcommand raises an # AttributeError. @@ -34,3 +43,51 @@ class TestMailmanCommand(unittest.TestCase): with self.assertRaises(SystemExit): main() self.assertIn('usage', output.getvalue()) + + def test_transaction_commit_after_successful_subcommand(self): + # Issue #223: Subcommands which change the database need to commit or + # abort the transaction. + with transaction(): + mlist = create_list('ant@example.com') + mlist.volume = 5 + mlist.next_digest_number = 3 + mlist.digest_last_sent_at = now() - timedelta(days=60) + testargs = ['mailman', 'digests', '-b', '-l', 'ant@example.com'] + output = StringIO() + with ExitStack() as resources: + enter = resources.enter_context + enter(patch('sys.argv', testargs)) + enter(patch('sys.stdout', output)) + # Everything is already initialized. + enter(patch('mailman.bin.mailman.initialize')) + main() + # Clear the current transaction to force a database reload. + config.db.abort() + self.assertEqual(mlist.volume, 6) + self.assertEqual(mlist.next_digest_number, 1) + + def test_transaction_abort_after_failing_subcommand(self): + with transaction(): + mlist = create_list('ant@example.com') + mlist.volume = 5 + mlist.next_digest_number = 3 + mlist.digest_last_sent_at = now() - timedelta(days=60) + testargs = ['mailman', 'digests', '-b', '-l', 'ant@example.com', + '--send'] + output = StringIO() + with ExitStack() as resources: + enter = resources.enter_context + enter(patch('sys.argv', testargs)) + enter(patch('sys.stdout', output)) + # Force an exception in the subcommand. + enter(patch('mailman.commands.cli_digests.maybe_send_digest_now', + side_effect=RuntimeError)) + # Everything is already initialized. + enter(patch('mailman.bin.mailman.initialize')) + with self.assertRaises(RuntimeError): + main() + # Clear the current transaction to force a database reload. + config.db.abort() + # The volume and number haven't changed. + self.assertEqual(mlist.volume, 5) + self.assertEqual(mlist.next_digest_number, 3) diff --git a/src/mailman/commands/cli_digests.py b/src/mailman/commands/cli_digests.py index c4623259e..3a4d6614f 100644 --- a/src/mailman/commands/cli_digests.py +++ b/src/mailman/commands/cli_digests.py @@ -57,6 +57,15 @@ class Digests: help=_("""Increment the digest volume number and reset the digest number to one. If given with --send, the volume number is incremented before any current digests are sent.""")) + command_parser.add_argument( + '-n', '--dry-run', + default=False, action='store_true', + help=_("""Don't actually do anything, but in conjunction with + --verbose, show what would happen.""")) + command_parser.add_argument( + '-v', '--verbose', + default=False, action='store_true', + help=_("""Print some additional status.""")) def process(self, args): """See `ICLISubCommand`.""" @@ -77,7 +86,20 @@ class Digests: lists = list(list_manager.mailing_lists) if args.bump: for mlist in lists: - bump_digest_number_and_volume(mlist) + if args.verbose: + print(_('\ +$mlist.list_id is at volume $mlist.volume, number \ +${mlist.next_digest_number}')) + if not args.dry_run: + bump_digest_number_and_volume(mlist) + if args.verbose: + print(_('\ +$mlist.list_id bumped to volume $mlist.volume, number \ +${mlist.next_digest_number}')) if args.send: for mlist in lists: - maybe_send_digest_now(mlist, force=True) + if args.verbose: + print(_('\ +$mlist.list_id sent volume $mlist.volume, number ${mlist.next_digest_number}')) + if not args.dry_run: + maybe_send_digest_now(mlist, force=True) diff --git a/src/mailman/commands/tests/test_digests.py b/src/mailman/commands/tests/test_digests.py index b14820cbd..4c62cea38 100644 --- a/src/mailman/commands/tests/test_digests.py +++ b/src/mailman/commands/tests/test_digests.py @@ -41,6 +41,8 @@ class FakeArgs: self.lists = [] self.send = False self.bump = False + self.dry_run = False + self.verbose = False class TestSendDigests(unittest.TestCase): @@ -421,3 +423,29 @@ class TestBumpVolume(unittest.TestCase): self.assertEqual(self._mlist.volume, 8) self.assertEqual(self._mlist.next_digest_number, 1) self.assertEqual(self._mlist.digest_last_sent_at, self.right_now) + + def test_bump_verbose(self): + args = FakeArgs() + args.bump = True + args.verbose = True + args.lists.append('ant.example.com') + output = StringIO() + with patch('sys.stdout', output): + self._command.process(args) + self.assertMultiLineEqual(output.getvalue(), """\ +ant.example.com is at volume 7, number 4 +ant.example.com bumped to volume 7, number 5 +""") + + def test_send_verbose(self): + args = FakeArgs() + args.send = True + args.verbose = True + args.dry_run = True + args.lists.append('ant.example.com') + output = StringIO() + with patch('sys.stdout', output): + self._command.process(args) + self.assertMultiLineEqual(output.getvalue(), """\ +ant.example.com sent volume 7, number 4 +""") diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index b14738330..10473e8ce 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -94,6 +94,9 @@ Command line compatibility, but now there is a ``-D``/``--no-domain`` option to prevent missing domains from being create, forcing an error in those cases. Given by Gurkirpal Singh. (Closes #39) + * ``mailman`` subcommands now properly commit any outstanding transactions. + (Closes #223) + * ``mailman digests`` has grown ``--verbose`` and ``-dry-run`` options. Interfaces ---------- |