From 7af50e2b346b76daab8206c448e5e67f06cbacbd Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 14 May 2011 07:06:54 +0200 Subject: Add tests for the outgoing queue runner. * Use m.u.datetime.now() instead of datetime.now() * Add a predicate argument to make_testable_runner() so that we can e.g. pass in a function that causes the queue runner to run only once. * Minor improvement to get_queue_messages() so that it doesn't need a lambda. --- src/mailman/queue/tests/test_outgoing.py | 87 ++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 src/mailman/queue/tests/test_outgoing.py (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py new file mode 100644 index 000000000..fc4eb3070 --- /dev/null +++ b/src/mailman/queue/tests/test_outgoing.py @@ -0,0 +1,87 @@ +# Copyright (C) 2011 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 . + +"""Test the outgoing queue runner.""" + +from __future__ import absolute_import, unicode_literals + +__metaclass__ = type +__all__ = [ + 'test_suite', + ] + + +import datetime +import unittest + +from mailman.app.lifecycle import create_list +from mailman.config import config +from mailman.queue.outgoing import OutgoingRunner +from mailman.testing.helpers import ( + get_queue_messages, + make_testable_runner, + specialized_message_from_string as message_from_string) +from mailman.testing.layers import SMTPLayer +from mailman.utilities.datetime import now + + + +def run_once(qrunner): + """Predicate for make_testable_runner(). + + Ensures that the queue runner only runs once. + """ + return True + + + +class TestOnce(unittest.TestCase): + """Test outgoing runner message disposition.""" + + layer = SMTPLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + self._outq = config.switchboards['out'] + self._runner = make_testable_runner(OutgoingRunner, 'out', run_once) + self._msg = message_from_string("""\ +From: anne@example.com +To: test@example.com +Message-Id: + +""") + self._msgdata = {} + + def test_deliver_after(self): + # When the metadata has a deliver_after key in the future, the queue + # runner will re-enqueue the message rather than delivering it. + deliver_after = now() + datetime.timedelta(days=10) + self._msgdata['deliver_after'] = deliver_after + self._outq.enqueue(self._msg, self._msgdata, tolist=True, + listname='test@example.com') + self._runner.run() + items = get_queue_messages('out') + self.assertEqual(len(items), 1) + self.assertEqual(items[0].msgdata['deliver_after'], deliver_after) + self.assertEqual(items[0].msg['message-id'], '') + + + +def test_suite(): + suite = unittest.TestSuite() + suite.addTest(unittest.makeSuite(TestOnce)) + return suite -- cgit v1.2.3-70-g09d2 From c201299b4932ea153e765f6316bab4c02ff09e50 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 14 May 2011 13:01:10 +0200 Subject: checkpointing --- src/mailman/queue/outgoing.py | 2 +- src/mailman/queue/tests/test_outgoing.py | 109 +++++++++++++++++++++++++++++-- 2 files changed, 105 insertions(+), 6 deletions(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/outgoing.py b/src/mailman/queue/outgoing.py index 07d45c0bd..dfa60a02d 100644 --- a/src/mailman/queue/outgoing.py +++ b/src/mailman/queue/outgoing.py @@ -70,7 +70,7 @@ class OutgoingRunner(Runner, BounceMixin): # Also, if personalization is /not/ enabled, but # verp_delivery_interval is set (and we've hit this interval), then # again, this message should be VERP'd. Otherwise, no. - elif mlist.personalize <> Personalization.none: + elif mlist.personalize != Personalization.none: if as_boolean(config.mta.verp_personalized_deliveries): msgdata['verp'] = True elif interval == 0: diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index fc4eb3070..7fc1143bc 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -25,17 +25,20 @@ __all__ = [ ] -import datetime import unittest +from contextlib import contextmanager +from datetime import timedelta + from mailman.app.lifecycle import create_list from mailman.config import config +from mailman.interfaces.mailinglist import Personalization from mailman.queue.outgoing import OutgoingRunner from mailman.testing.helpers import ( get_queue_messages, make_testable_runner, specialized_message_from_string as message_from_string) -from mailman.testing.layers import SMTPLayer +from mailman.testing.layers import ConfigLayer, SMTPLayer from mailman.utilities.datetime import now @@ -48,6 +51,16 @@ def run_once(qrunner): return True +@contextmanager +def temporary_config(name, settings): + """Temporarily set a configuration (use in a with-statement).""" + config.push(name, settings) + try: + yield + finally: + config.pop(name) + + class TestOnce(unittest.TestCase): """Test outgoing runner message disposition.""" @@ -69,10 +82,10 @@ Message-Id: def test_deliver_after(self): # When the metadata has a deliver_after key in the future, the queue # runner will re-enqueue the message rather than delivering it. - deliver_after = now() + datetime.timedelta(days=10) + deliver_after = now() + timedelta(days=10) self._msgdata['deliver_after'] = deliver_after - self._outq.enqueue(self._msg, self._msgdata, tolist=True, - listname='test@example.com') + self._outq.enqueue(self._msg, self._msgdata, + tolist=True, listname='test@example.com') self._runner.run() items = get_queue_messages('out') self.assertEqual(len(items), 1) @@ -80,8 +93,94 @@ Message-Id: self.assertEqual(items[0].msg['message-id'], '') + +captured_mlist = None +captured_msg = None +captured_msgdata = None + +def capture(mlist, msg, msgdata): + global captured_mlist, captured_msg, captured_msgdata + captured_mlist = mlist + captured_msg = msg + captured_msgdata = msgdata + + +class TestVERPSettings(unittest.TestCase): + """Test the selection of VERP based on various criteria.""" + + layer = ConfigLayer + + def setUp(self): + global captured_mlist, captured_msg, captured_msgdata + # Push a config where actual delivery is handled by a dummy function. + # We generally don't care what this does, since we're just testing the + # setting of the 'verp' key in the metadata. + config.push('fake outgoing', """ + [mta] + outgoing: mailman.queue.tests.test_outgoing.capture + """) + # Reset the captured data. + captured_mlist = None + captured_msg = None + captured_msgdata = None + self._mlist = create_list('test@example.com') + self._outq = config.switchboards['out'] + self._runner = make_testable_runner(OutgoingRunner, 'out') + self._msg = message_from_string("""\ +From: anne@example.com +To: test@example.com +Message-Id: + +""") + + def tearDown(self): + config.pop('fake outgoing') + + def test_delivery_callback(self): + # Test that the configuration variable calls the appropriate callback. + self._outq.enqueue(self._msg, {}, listname='test@example.com') + self._runner.run() + self.assertEqual(captured_mlist, self._mlist) + self.assertEqual(captured_msg.as_string(), self._msg.as_string()) + # Of course, the message metadata will contain a bunch of keys added + # by the processing. We don't really care about the details, so this + # test is a good enough stand-in. + self.assertEqual(captured_msgdata['listname'], 'test@example.com') + + def test_verp_in_metadata(self): + # Test that if the metadata has a 'verp' key, it is unchanged. + marker = 'yepper' + msgdata = dict(verp=marker) + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + self._runner.run() + self.assertEqual(captured_msgdata['verp'], marker) + + def test_personalized_deliveries_verp(self): + # When deliveries are personalized, and the configuration setting + # indicates, messages will be VERP'd. + msgdata = {} + self._mlist.personalize = Personalization.individual + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + with temporary_config('personalize', """ + [mta] + verp_personalized_deliveries: yes + """): + self._runner.run() + self.assertTrue(msgdata['verp']) + msgdata = {} + self._mlist.personalize = Personalization.full + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + with temporary_config('personalize', """ + [mta] + verp_personalized_deliveries: yes + """): + self._runner.run() + self.assertTrue(msgdata['verp']) + + def test_suite(): suite = unittest.TestSuite() suite.addTest(unittest.makeSuite(TestOnce)) + suite.addTest(unittest.makeSuite(TestVERPSettings)) return suite -- cgit v1.2.3-70-g09d2 From 5a35aef83485f656ed2ef9c67216131074d62920 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 14 May 2011 13:57:26 +0200 Subject: oh well --- src/mailman/queue/tests/test_outgoing.py | 4 ++-- src/mailman/testing/helpers.py | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index 7fc1143bc..ad6c89f34 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -166,7 +166,7 @@ Message-Id: verp_personalized_deliveries: yes """): self._runner.run() - self.assertTrue(msgdata['verp']) + ## self.assertTrue(msgdata['verp']) msgdata = {} self._mlist.personalize = Personalization.full self._outq.enqueue(self._msg, msgdata, listname='test@example.com') @@ -175,7 +175,7 @@ Message-Id: verp_personalized_deliveries: yes """): self._runner.run() - self.assertTrue(msgdata['verp']) + ## self.assertTrue(msgdata['verp']) diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py index f3b0071d0..9f9ea6181 100644 --- a/src/mailman/testing/helpers.py +++ b/src/mailman/testing/helpers.py @@ -48,7 +48,6 @@ from contextlib import contextmanager from email import message_from_string from httplib2 import Http from lazr.config import as_timedelta -from operator import itemgetter from urllib import urlencode from urllib2 import HTTPError from zope import event @@ -126,7 +125,7 @@ def get_queue_messages(queue_name, sort_on=None): messages.append(_Bag(msg=msg, msgdata=msgdata)) queue.finish(filebase) if sort_on is not None: - messages.sort(key=itemgetter(sort_on)) + messages.sort(key=lambda item: item.msg[sort_on]) return messages -- cgit v1.2.3-70-g09d2 From 67bbe74667b29ab6f889017b6483fa468a79f019 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 14 May 2011 08:13:34 -0400 Subject: * Fixed tests * Fix obvious typo in __init__(). How can we get Storm to help us with this? --- src/mailman/model/mailinglist.py | 2 +- src/mailman/queue/tests/test_outgoing.py | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index 9294fe7cc..6952abcf0 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -195,7 +195,7 @@ class MailingList(Model): # For the pending database self.next_request_id = 1 self._restore() - self.personalization = Personalization.none + self.personalize = Personalization.none self.real_name = string.capwords( SPACE.join(listname.split(UNDERSCORE))) makedirs(self.data_path) diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index ad6c89f34..718fc4ba6 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -155,7 +155,7 @@ Message-Id: self._runner.run() self.assertEqual(captured_msgdata['verp'], marker) - def test_personalized_deliveries_verp(self): + def test_personalized_individual_deliveries_verp(self): # When deliveries are personalized, and the configuration setting # indicates, messages will be VERP'd. msgdata = {} @@ -166,7 +166,11 @@ Message-Id: verp_personalized_deliveries: yes """): self._runner.run() - ## self.assertTrue(msgdata['verp']) + self.assertTrue(captured_msgdata['verp']) + + def test_personalized_full_deliveries_verp(self): + # When deliveries are personalized, and the configuration setting + # indicates, messages will be VERP'd. msgdata = {} self._mlist.personalize = Personalization.full self._outq.enqueue(self._msg, msgdata, listname='test@example.com') @@ -175,7 +179,7 @@ Message-Id: verp_personalized_deliveries: yes """): self._runner.run() - ## self.assertTrue(msgdata['verp']) + self.assertTrue(captured_msgdata['verp']) -- cgit v1.2.3-70-g09d2 From 1bfc2990a5d9d6e9bb8ae7241a4af6a7ad576f1c Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 14 May 2011 08:17:03 -0400 Subject: Another test. --- src/mailman/queue/tests/test_outgoing.py | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index 718fc4ba6..6bc953beb 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -181,6 +181,15 @@ Message-Id: self._runner.run() self.assertTrue(captured_msgdata['verp']) + def test_personalized_deliveries_no_verp(self): + # When deliveries are personalized, but the configuration setting + # does not indicate, messages will not be VERP'd. + msgdata = {} + self._mlist.personalize = Personalization.full + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + self._runner.run() + self.assertTrue('verp' not in captured_msgdata) + def test_suite(): -- cgit v1.2.3-70-g09d2 From 520af4b499d2008e02b55f3f3e89fd20f79b48df Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 14 May 2011 08:20:07 -0400 Subject: More tests. --- src/mailman/queue/tests/test_outgoing.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index 6bc953beb..8ceda6066 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -188,7 +188,29 @@ Message-Id: self._mlist.personalize = Personalization.full self._outq.enqueue(self._msg, msgdata, listname='test@example.com') self._runner.run() - self.assertTrue('verp' not in captured_msgdata) + self.assertFalse('verp' in captured_msgdata) + + def test_verp_never(self): + # Never VERP when the interval is zero. + msgdata = {} + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + with temporary_config('personalize', """ + [mta] + verp_delivery_interval: 0 + """): + self._runner.run() + self.assertEqual(captured_msgdata['verp'], False) + + def test_verp_always(self): + # Always VERP when the interval is one. + msgdata = {} + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + with temporary_config('personalize', """ + [mta] + verp_delivery_interval: 1 + """): + self._runner.run() + self.assertEqual(captured_msgdata['verp'], True) -- cgit v1.2.3-70-g09d2 From 9d30e3d7f584093fd30d2cbdd1124b4d49adb132 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Sat, 14 May 2011 08:22:36 -0400 Subject: More tests --- src/mailman/queue/tests/test_outgoing.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index 8ceda6066..da090f31e 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -212,6 +212,30 @@ Message-Id: self._runner.run() self.assertEqual(captured_msgdata['verp'], True) + def test_verp_on_interval_match(self): + # VERP every so often, when the post_id matches. + self._mlist.post_id = 5 + msgdata = {} + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + with temporary_config('personalize', """ + [mta] + verp_delivery_interval: 5 + """): + self._runner.run() + self.assertEqual(captured_msgdata['verp'], True) + + def test_no_verp_on_interval_miss(self): + # VERP every so often, when the post_id matches. + self._mlist.post_id = 4 + msgdata = {} + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + with temporary_config('personalize', """ + [mta] + verp_delivery_interval: 5 + """): + self._runner.run() + self.assertEqual(captured_msgdata['verp'], False) + def test_suite(): -- cgit v1.2.3-70-g09d2 From ae5fe445251b22aaed0a986600b982a27279b2c7 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 17 May 2011 15:30:37 -0400 Subject: Test (and fix!) the path in the outgoing runner where a socket.error gets raised during the delivery function. Modify the ReopenableFileHandler so that the filename is a public attribute. --- src/mailman/core/logging.py | 6 +-- src/mailman/queue/outgoing.py | 7 ++- src/mailman/queue/tests/test_outgoing.py | 88 ++++++++++++++++++++++++++++++-- 3 files changed, 91 insertions(+), 10 deletions(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/core/logging.py b/src/mailman/core/logging.py index e15924177..0ff129897 100644 --- a/src/mailman/core/logging.py +++ b/src/mailman/core/logging.py @@ -57,11 +57,11 @@ class ReopenableFileHandler(logging.Handler): def __init__(self, name, filename): logging.Handler.__init__(self) self.name = name - self._filename = filename + self.filename = filename self._stream = self._open() def _open(self): - return codecs.open(self._filename, 'a', 'utf-8') + return codecs.open(self.filename, 'a', 'utf-8') def flush(self): if self._stream: @@ -98,7 +98,7 @@ class ReopenableFileHandler(logging.Handler): :type filename: string """ if filename is not None: - self._filename = filename + self.filename = filename self._stream.close() self._stream = self._open() diff --git a/src/mailman/queue/outgoing.py b/src/mailman/queue/outgoing.py index dfa60a02d..6a8e2b08d 100644 --- a/src/mailman/queue/outgoing.py +++ b/src/mailman/queue/outgoing.py @@ -89,13 +89,12 @@ class OutgoingRunner(Runner, BounceMixin): # There was a problem connecting to the SMTP server. Log this # once, but crank up our sleep time so we don't fill the error # log. - port = int(config.mta.port) + port = int(config.mta.smtp_port) if port == 0: - port = 'smtp' - # Log this just once. + port = 'smtp' # Log this just once. if not self._logged: log.error('Cannot connect to SMTP server %s on port %s', - config.mta.host, port) + config.mta.smtp_host, port) self._logged = True return True except SomeRecipientsFailed as error: diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index da090f31e..c86a1efa7 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -25,6 +25,9 @@ __all__ = [ ] +import os +import socket +import logging import unittest from contextlib import contextmanager @@ -230,16 +233,95 @@ Message-Id: msgdata = {} self._outq.enqueue(self._msg, msgdata, listname='test@example.com') with temporary_config('personalize', """ - [mta] - verp_delivery_interval: 5 - """): + [mta] + verp_delivery_interval: 5 + """): self._runner.run() self.assertEqual(captured_msgdata['verp'], False) + +def raise_socket_error(mlist, msg, msgdata): + raise socket.error + + +class TestSocketError(unittest.TestCase): + """Test socket.error occurring in the delivery function.""" + + layer = ConfigLayer + + def setUp(self): + # Push a config where actual delivery is handled by a dummy function. + # We generally don't care what this does, since we're just testing the + # setting of the 'verp' key in the metadata. + config.push('fake outgoing', """ + [mta] + outgoing: mailman.queue.tests.test_outgoing.raise_socket_error + """) + self._mlist = create_list('test@example.com') + self._outq = config.switchboards['out'] + self._runner = make_testable_runner(OutgoingRunner, 'out', run_once) + self._msg = message_from_string("""\ +From: anne@example.com +To: test@example.com +Message-Id: + +""") + + def tearDown(self): + config.pop('fake outgoing') + + def test_error_with_port_0(self): + # Test the code path where a socket.error is raised in the delivery + # function, and the MTA port is set to zero. The only real effect of + # that is a log message. Start by opening the error log and reading + # the current file position. + error_log = logging.getLogger('mailman.error') + filename = error_log.handlers[0].filename + filepos = os.stat(filename).st_size + self._outq.enqueue(self._msg, {}, listname='test@example.com') + with temporary_config('port 0', """ + [mta] + smtp_port: 0 + """): + self._runner.run() + with open(filename) as fp: + fp.seek(filepos) + line = fp.readline() + # The log line will contain a variable timestamp, the PID, and a + # trailing newline. Ignore these. + self.assertEqual( + line[-53:-1], + 'Cannot connect to SMTP server localhost on port smtp') + + def test_error_with_numeric_port(self): + # Test the code path where a socket.error is raised in the delivery + # function, and the MTA port is set to zero. The only real effect of + # that is a log message. Start by opening the error log and reading + # the current file position. + error_log = logging.getLogger('mailman.error') + filename = error_log.handlers[0].filename + filepos = os.stat(filename).st_size + self._outq.enqueue(self._msg, {}, listname='test@example.com') + with temporary_config('port 0', """ + [mta] + smtp_port: 2112 + """): + self._runner.run() + with open(filename) as fp: + fp.seek(filepos) + line = fp.readline() + # The log line will contain a variable timestamp, the PID, and a + # trailing newline. Ignore these. + self.assertEqual( + line[-53:-1], + 'Cannot connect to SMTP server localhost on port 2112') + + def test_suite(): suite = unittest.TestSuite() suite.addTest(unittest.makeSuite(TestOnce)) suite.addTest(unittest.makeSuite(TestVERPSettings)) + suite.addTest(unittest.makeSuite(TestSocketError)) return suite -- cgit v1.2.3-70-g09d2 From 8e86c361c33c5f51ce8215173b8e9703be4af7f9 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 17 May 2011 16:25:25 -0400 Subject: * Flesh out IBounceProcessor so that you can get an iterator over all events and over just the unprocessed events. * In the outgoing queue runner, work out the logic for when SomeRecipientsFailed with permanent failures in a probe message. --- src/mailman/interfaces/bounce.py | 6 ++ src/mailman/model/bounce.py | 22 +++++++- src/mailman/model/tests/test_bounce.py | 63 ++++++++++++++++++++- src/mailman/queue/outgoing.py | 25 +++++++- src/mailman/queue/tests/test_outgoing.py | 97 +++++++++++++++++++++++++++++++- 5 files changed, 204 insertions(+), 9 deletions(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/interfaces/bounce.py b/src/mailman/interfaces/bounce.py index 168ca7ee0..0b301aa98 100644 --- a/src/mailman/interfaces/bounce.py +++ b/src/mailman/interfaces/bounce.py @@ -113,3 +113,9 @@ class IBounceProcessor(Interface): :return: The registered bounce event. :rtype: IBounceEvent """ + + events = Attribute( + """An iterator over all events.""") + + unprocessed = Attribute( + """An iterator over all unprocessed bounce events.""") diff --git a/src/mailman/model/bounce.py b/src/mailman/model/bounce.py index 8abe3d149..20953b0ff 100644 --- a/src/mailman/model/bounce.py +++ b/src/mailman/model/bounce.py @@ -29,10 +29,11 @@ __all__ = [ from storm.locals import Bool, Int, DateTime, Unicode from zope.interface import implements -from mailman.interfaces.bounce import ( - BounceContext, IBounceEvent, IBounceProcessor) +from mailman.config import config from mailman.database.model import Model from mailman.database.types import Enum +from mailman.interfaces.bounce import ( + BounceContext, IBounceEvent, IBounceProcessor) from mailman.utilities.datetime import now @@ -63,4 +64,19 @@ class BounceProcessor: def register(self, mlist, email, msg, where=None): """See `IBounceProcessor`.""" - return BounceEvent(mlist.fqdn_listname, email, msg, where) + event = BounceEvent(mlist.fqdn_listname, email, msg, where) + config.db.store.add(event) + return event + + @property + def events(self): + """See `IBounceProcessor`.""" + for event in config.db.store.find(BounceEvent): + yield event + + @property + def unprocessed(self): + """See `IBounceProcessor`.""" + for event in config.db.store.find(BounceEvent, + BounceEvent.processed == False): + yield event diff --git a/src/mailman/model/tests/test_bounce.py b/src/mailman/model/tests/test_bounce.py index fb0bf0875..a232b37fd 100644 --- a/src/mailman/model/tests/test_bounce.py +++ b/src/mailman/model/tests/test_bounce.py @@ -27,7 +27,14 @@ __all__ = [ import unittest -from mailman.model.bounce import BounceEvent +from datetime import datetime +from zope.component import getUtility + +from mailman.app.lifecycle import create_list +from mailman.config import config +from mailman.interfaces.bounce import BounceContext, IBounceProcessor +from mailman.testing.helpers import ( + specialized_message_from_string as message_from_string) from mailman.testing.layers import ConfigLayer @@ -35,8 +42,60 @@ from mailman.testing.layers import ConfigLayer class TestBounceEvents(unittest.TestCase): layer = ConfigLayer - + def setUp(self): + self._processor = getUtility(IBounceProcessor) + self._mlist = create_list('test@example.com') + self._msg = message_from_string("""\ +From: mail-daemon@example.com +To: test-bounces@example.com +Message-Id: + +""") + + def test_events_iterator(self): + self._processor.register(self._mlist, 'anne@example.com', self._msg) + config.db.commit() + events = list(self._processor.events) + self.assertEqual(len(events), 1) + event = events[0] + self.assertEqual(event.list_name, 'test@example.com') + self.assertEqual(event.email, 'anne@example.com') + self.assertEqual(event.timestamp, datetime(2005, 8, 1, 7, 49, 23)) + self.assertEqual(event.message_id, '') + self.assertEqual(event.context, BounceContext.normal) + self.assertEqual(event.processed, False) + # The unprocessed list will be exactly the same right now. + unprocessed = list(self._processor.unprocessed) + self.assertEqual(len(unprocessed), 1) + event = unprocessed[0] + self.assertEqual(event.list_name, 'test@example.com') + self.assertEqual(event.email, 'anne@example.com') + self.assertEqual(event.timestamp, datetime(2005, 8, 1, 7, 49, 23)) + self.assertEqual(event.message_id, '') + self.assertEqual(event.context, BounceContext.normal) + self.assertEqual(event.processed, False) + def test_unprocessed_events_iterator(self): + self._processor.register(self._mlist, 'anne@example.com', self._msg) + self._processor.register(self._mlist, 'bart@example.com', self._msg) + config.db.commit() + events = list(self._processor.events) + self.assertEqual(len(events), 2) + unprocessed = list(self._processor.unprocessed) + # The unprocessed list will be exactly the same right now. + self.assertEqual(len(unprocessed), 2) + # Process one of the events. + events[0].processed = True + config.db.commit() + # Now there will be only one unprocessed event. + unprocessed = list(self._processor.unprocessed) + self.assertEqual(len(unprocessed), 1) + # Process the other event. + events[1].processed = True + config.db.commit() + # Now there will be no unprocessed events. + unprocessed = list(self._processor.unprocessed) + self.assertEqual(len(unprocessed), 0) diff --git a/src/mailman/queue/outgoing.py b/src/mailman/queue/outgoing.py index 6a8e2b08d..f20adc270 100644 --- a/src/mailman/queue/outgoing.py +++ b/src/mailman/queue/outgoing.py @@ -22,10 +22,14 @@ import logging from datetime import datetime from lazr.config import as_boolean, as_timedelta +from zope.component import getUtility from mailman.config import config +from mailman.interfaces.bounce import BounceContext, IBounceProcessor from mailman.interfaces.mailinglist import Personalization +from mailman.interfaces.membership import ISubscriptionService from mailman.interfaces.mta import SomeRecipientsFailed +from mailman.interfaces.pending import IPendings from mailman.queue import Runner from mailman.queue.bounce import BounceMixin from mailman.utilities.datetime import now @@ -98,9 +102,24 @@ class OutgoingRunner(Runner, BounceMixin): self._logged = True return True except SomeRecipientsFailed as error: - # Handle local rejects of probe messages differently. - if msgdata.get('probe_token') and error.permanent_failures: - self._probe_bounce(mlist, msgdata['probe_token']) + if 'probe_token' in msgdata: + # This is a failure of our local MTA to deliver to a probe + # message recipient. Register the bounce event for permanent + # failures. Start by grabbing and confirming (i.e. removing) + # the pendable record associated with this bounce token, + # regardless of what address was actually failing. + if len(error.permanent_failures) > 0: + pended = getUtility(IPendings).confirm( + msgdata['probe_token']) + # It's possible the token has been confirmed out of the + # database. Just ignore that. + if pended is not None: + member = getUtility(ISubscriptionService).get_member( + pended['member_id']) + processor = getUtility(IBounceProcessor) + processor.register( + mlist, member.address.email, msg, + BounceContext.probe) else: # Delivery failed at SMTP time for some or all of the # recipients. Permanent failures are registered as bounces, diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index c86a1efa7..da45dbdb5 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -31,11 +31,18 @@ import logging import unittest from contextlib import contextmanager -from datetime import timedelta +from datetime import datetime, timedelta +from zope.component import getUtility +from mailman.app.bounces import send_probe from mailman.app.lifecycle import create_list from mailman.config import config +from mailman.interfaces.bounce import BounceContext, IBounceProcessor from mailman.interfaces.mailinglist import Personalization +from mailman.interfaces.member import MemberRole +from mailman.interfaces.mta import SomeRecipientsFailed +from mailman.interfaces.pending import IPendings +from mailman.interfaces.usermanager import IUserManager from mailman.queue.outgoing import OutgoingRunner from mailman.testing.helpers import ( get_queue_messages, @@ -318,10 +325,98 @@ Message-Id: 'Cannot connect to SMTP server localhost on port 2112') + +temporary_failures = [] +permanent_failures = [] + + +def raise_SomeRecipientsFailed(mlist, msg, msgdata): + raise SomeRecipientsFailed(temporary_failures, permanent_failures) + + +class TestSomeRecipientsFailed(unittest.TestCase): + """Test socket.error occurring in the delivery function.""" + + layer = ConfigLayer + + def setUp(self): + global temporary_failures, permanent_failures + del temporary_failures[:] + del permanent_failures[:] + # Push a config where actual delivery is handled by a dummy function. + # We generally don't care what this does, since we're just testing the + # setting of the 'verp' key in the metadata. + config.push('fake outgoing', """ + [mta] + outgoing: mailman.queue.tests.test_outgoing.raise_SomeRecipientsFailed + """) + self._mlist = create_list('test@example.com') + self._outq = config.switchboards['out'] + self._runner = make_testable_runner(OutgoingRunner, 'out', run_once) + self._msg = message_from_string("""\ +From: anne@example.com +To: test@example.com +Message-Id: + +""") + + def tearDown(self): + config.pop('fake outgoing') + + def test_probe_failure(self): + # When a probe message fails during SMTP, a bounce event is recorded + # with the proper bounce context. + anne = getUtility(IUserManager).create_address('anne@example.com') + member = self._mlist.subscribe(anne, MemberRole.member) + token = send_probe(member, self._msg) + msgdata = dict(probe_token=token) + permanent_failures.append('anne@example.com') + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + self._runner.run() + events = list(getUtility(IBounceProcessor).unprocessed) + self.assertEqual(len(events), 1) + event = events[0] + self.assertEqual(event.list_name, 'test@example.com') + self.assertEqual(event.email, 'anne@example.com') + self.assertEqual(event.timestamp, datetime(2005, 8, 1, 7, 49, 23)) + self.assertEqual(event.message_id, '') + self.assertEqual(event.context, BounceContext.probe) + self.assertEqual(event.processed, False) + + def test_confirmed_probe_failure(self): + # This time, a probe also fails, but for some reason the probe token + # has already been confirmed and no longer exists in the database. + anne = getUtility(IUserManager).create_address('anne@example.com') + member = self._mlist.subscribe(anne, MemberRole.member) + token = send_probe(member, self._msg) + getUtility(IPendings).confirm(token) + msgdata = dict(probe_token=token) + permanent_failures.append('anne@example.com') + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + self._runner.run() + events = list(getUtility(IBounceProcessor).unprocessed) + self.assertEqual(len(events), 0) + + def test_probe_temporary_failure(self): + # This time, a probe also fails, but the failures are temporary so + # they are not registered. + anne = getUtility(IUserManager).create_address('anne@example.com') + member = self._mlist.subscribe(anne, MemberRole.member) + token = send_probe(member, self._msg) + getUtility(IPendings).confirm(token) + msgdata = dict(probe_token=token) + temporary_failures.append('anne@example.com') + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + self._runner.run() + events = list(getUtility(IBounceProcessor).unprocessed) + self.assertEqual(len(events), 0) + + def test_suite(): suite = unittest.TestSuite() suite.addTest(unittest.makeSuite(TestOnce)) suite.addTest(unittest.makeSuite(TestVERPSettings)) suite.addTest(unittest.makeSuite(TestSocketError)) + suite.addTest(unittest.makeSuite(TestSomeRecipientsFailed)) return suite -- cgit v1.2.3-70-g09d2 From 0a7fe8845bc20df2e934509df0e8830f4274d0c7 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 17 May 2011 17:10:47 -0400 Subject: Work out what happens for permanent and temporary non-probe failures. --- src/mailman/queue/outgoing.py | 28 +++++----- src/mailman/queue/tests/test_outgoing.py | 90 ++++++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 18 deletions(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/outgoing.py b/src/mailman/queue/outgoing.py index f20adc270..2939651d6 100644 --- a/src/mailman/queue/outgoing.py +++ b/src/mailman/queue/outgoing.py @@ -102,6 +102,12 @@ class OutgoingRunner(Runner, BounceMixin): self._logged = True return True except SomeRecipientsFailed as error: + processor = getUtility(IBounceProcessor) + # BAW: msg is the original message that failed delivery, not a + # bounce message. This may be confusing if this is what's sent to + # the user in the probe message. Maybe we should craft a + # bounce-like message containing information about the permanent + # SMTP failure? if 'probe_token' in msgdata: # This is a failure of our local MTA to deliver to a probe # message recipient. Register the bounce event for permanent @@ -116,7 +122,6 @@ class OutgoingRunner(Runner, BounceMixin): if pended is not None: member = getUtility(ISubscriptionService).get_member( pended['member_id']) - processor = getUtility(IBounceProcessor) processor.register( mlist, member.address.email, msg, BounceContext.probe) @@ -124,24 +129,17 @@ class OutgoingRunner(Runner, BounceMixin): # Delivery failed at SMTP time for some or all of the # recipients. Permanent failures are registered as bounces, # but temporary failures are retried for later. - # - # BAW: msg is going to be the original message that failed - # delivery, not a bounce message. This may be confusing if - # this is what's sent to the user in the probe message. Maybe - # we should craft a bounce-like message containing information - # about the permanent SMTP failure? - if error.permanent_failures: - self._queue_bounces( - mlist.fqdn_listname, error.permanent_failures, msg) + for email in error.permanent_failures: + processor.register(mlist, email, msg, BounceContext.normal) # Move temporary failures to the qfiles/retry queue which will # occasionally move them back here for another shot at # delivery. if error.temporary_failures: current_time = now() - recips = error.temporary_failures + recipients = error.temporary_failures last_recip_count = msgdata.get('last_recip_count', 0) deliver_until = msgdata.get('deliver_until', current_time) - if len(recips) == last_recip_count: + if len(recipients) == last_recip_count: # We didn't make any progress, so don't attempt # delivery any longer. BAW: is this the best # disposition? @@ -149,11 +147,11 @@ class OutgoingRunner(Runner, BounceMixin): return False else: # Keep trying to delivery this message for a while - deliver_until = now + as_timedelta( + deliver_until = current_time + as_timedelta( config.mta.delivery_retry_period) - msgdata['last_recip_count'] = len(recips) + msgdata['last_recip_count'] = len(recipients) msgdata['deliver_until'] = deliver_until - msgdata['recipients'] = recips + msgdata['recipients'] = recipients self._retryq.enqueue(msg, msgdata) # We've successfully completed handling of this message return False diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index da45dbdb5..eaef1578d 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -32,6 +32,7 @@ import unittest from contextlib import contextmanager from datetime import datetime, timedelta +from lazr.config import as_timedelta from zope.component import getUtility from mailman.app.bounces import send_probe @@ -343,6 +344,7 @@ class TestSomeRecipientsFailed(unittest.TestCase): global temporary_failures, permanent_failures del temporary_failures[:] del permanent_failures[:] + self._processor = getUtility(IBounceProcessor) # Push a config where actual delivery is handled by a dummy function. # We generally don't care what this does, since we're just testing the # setting of the 'verp' key in the metadata. @@ -373,7 +375,7 @@ Message-Id: permanent_failures.append('anne@example.com') self._outq.enqueue(self._msg, msgdata, listname='test@example.com') self._runner.run() - events = list(getUtility(IBounceProcessor).unprocessed) + events = list(self._processor.unprocessed) self.assertEqual(len(events), 1) event = events[0] self.assertEqual(event.list_name, 'test@example.com') @@ -394,7 +396,7 @@ Message-Id: permanent_failures.append('anne@example.com') self._outq.enqueue(self._msg, msgdata, listname='test@example.com') self._runner.run() - events = list(getUtility(IBounceProcessor).unprocessed) + events = list(self._processor.unprocessed) self.assertEqual(len(events), 0) def test_probe_temporary_failure(self): @@ -408,9 +410,91 @@ Message-Id: temporary_failures.append('anne@example.com') self._outq.enqueue(self._msg, msgdata, listname='test@example.com') self._runner.run() - events = list(getUtility(IBounceProcessor).unprocessed) + events = list(self._processor.unprocessed) self.assertEqual(len(events), 0) + def test_one_permanent_failure(self): + # Normal (i.e. non-probe) permanent failures just get registered. + permanent_failures.append('anne@example.com') + self._outq.enqueue(self._msg, {}, listname='test@example.com') + self._runner.run() + events = list(self._processor.unprocessed) + self.assertEqual(len(events), 1) + self.assertEqual(events[0].email, 'anne@example.com') + self.assertEqual(events[0].context, BounceContext.normal) + + def test_two_permanent_failures(self): + # Two normal (i.e. non-probe) permanent failures just get registered. + permanent_failures.append('anne@example.com') + permanent_failures.append('bart@example.com') + self._outq.enqueue(self._msg, {}, listname='test@example.com') + self._runner.run() + events = list(self._processor.unprocessed) + self.assertEqual(len(events), 2) + self.assertEqual(events[0].email, 'anne@example.com') + self.assertEqual(events[0].context, BounceContext.normal) + self.assertEqual(events[1].email, 'bart@example.com') + self.assertEqual(events[1].context, BounceContext.normal) + + def test_one_temporary_failure(self): + # The first time there are temporary failures, the message just gets + # put in the retry queue, but with some metadata to prevent infinite + # retries. + temporary_failures.append('cris@example.com') + self._outq.enqueue(self._msg, {}, listname='test@example.com') + self._runner.run() + events = list(self._processor.unprocessed) + self.assertEqual(len(events), 0) + items = get_queue_messages('retry') + self.assertEqual(len(items), 1) + self.assertEqual(self._msg.as_string(), items[0].msg.as_string()) + # The metadata has three keys which are used two decide whether the + # next temporary failure should be retried. + self.assertEqual(items[0].msgdata['last_recip_count'], 1) + deliver_until = (datetime(2005, 8, 1, 7, 49, 23) + + as_timedelta(config.mta.delivery_retry_period)) + self.assertEqual(items[0].msgdata['deliver_until'], deliver_until) + self.assertEqual(items[0].msgdata['recipients'], ['cris@example.com']) + + def test_two_temporary_failures(self): + # The first time there are temporary failures, the message just gets + # put in the retry queue, but with some metadata to prevent infinite + # retries. + temporary_failures.append('cris@example.com') + temporary_failures.append('dave@example.com') + self._outq.enqueue(self._msg, {}, listname='test@example.com') + self._runner.run() + events = list(self._processor.unprocessed) + self.assertEqual(len(events), 0) + items = get_queue_messages('retry') + # There's still only one item in the retry queue, but the metadata + # contains both temporary failures. + self.assertEqual(len(items), 1) + self.assertEqual(items[0].msgdata['last_recip_count'], 2) + self.assertEqual(items[0].msgdata['recipients'], + ['cris@example.com', 'dave@example.com']) + + def test_mixed_failures(self): + # Some temporary and some permanent failures. + permanent_failures.append('elle@example.com') + permanent_failures.append('fred@example.com') + temporary_failures.append('gwen@example.com') + temporary_failures.append('herb@example.com') + self._outq.enqueue(self._msg, {}, listname='test@example.com') + self._runner.run() + # Let's look at the permanent failures. + events = list(self._processor.unprocessed) + self.assertEqual(len(events), 2) + self.assertEqual(events[0].email, 'elle@example.com') + self.assertEqual(events[0].context, BounceContext.normal) + self.assertEqual(events[1].email, 'fred@example.com') + self.assertEqual(events[1].context, BounceContext.normal) + # Let's look at the temporary failures. + items = get_queue_messages('retry') + self.assertEqual(len(items), 1) + self.assertEqual(items[0].msgdata['recipients'], + ['gwen@example.com', 'herb@example.com']) + def test_suite(): -- cgit v1.2.3-70-g09d2 From 2884180c24a27a3ecee3c804dad2270928c5d19b Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 18 May 2011 23:05:44 -0400 Subject: More testing and refactoring the temporary failures branch. --- src/mailman/queue/outgoing.py | 13 ++++-- src/mailman/queue/tests/test_outgoing.py | 69 ++++++++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 11 deletions(-) (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/queue/outgoing.py b/src/mailman/queue/outgoing.py index 2939651d6..e591353f9 100644 --- a/src/mailman/queue/outgoing.py +++ b/src/mailman/queue/outgoing.py @@ -41,6 +41,7 @@ from mailman.utilities.modules import find_name DEAL_WITH_PERMFAILURES_EVERY = 10 log = logging.getLogger('mailman.error') +smtp_log = logging.getLogger('mailman.smtp') @@ -140,13 +141,17 @@ class OutgoingRunner(Runner, BounceMixin): last_recip_count = msgdata.get('last_recip_count', 0) deliver_until = msgdata.get('deliver_until', current_time) if len(recipients) == last_recip_count: - # We didn't make any progress, so don't attempt - # delivery any longer. BAW: is this the best - # disposition? + # We didn't make any progress. If we've exceeded the + # configured retry period, log this failure and + # discard the message. if current_time > deliver_until: + smtp_log.error('Discarding message with ' + 'persistent temporary failures: ' + '{0}'.format(msg['message-id'])) return False else: - # Keep trying to delivery this message for a while + # We made some progress, so keep trying to delivery + # this message for a while longer. deliver_until = current_time + as_timedelta( config.mta.delivery_retry_period) msgdata['last_recip_count'] = len(recipients) diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index eaef1578d..74850ce75 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -50,7 +50,20 @@ from mailman.testing.helpers import ( make_testable_runner, specialized_message_from_string as message_from_string) from mailman.testing.layers import ConfigLayer, SMTPLayer -from mailman.utilities.datetime import now +from mailman.utilities.datetime import factory, now + + + +class LogFileMark: + def __init__(self, log_name): + self._log = logging.getLogger(log_name) + self._filename = self._log.handlers[0].filename + self._filepos = os.stat(self._filename).st_size + + def readline(self): + with open(self._filename) as fp: + fp.seek(self._filepos) + return fp.readline() @@ -307,18 +320,14 @@ Message-Id: # function, and the MTA port is set to zero. The only real effect of # that is a log message. Start by opening the error log and reading # the current file position. - error_log = logging.getLogger('mailman.error') - filename = error_log.handlers[0].filename - filepos = os.stat(filename).st_size + mark = LogFileMark('mailman.error') self._outq.enqueue(self._msg, {}, listname='test@example.com') with temporary_config('port 0', """ [mta] smtp_port: 2112 """): self._runner.run() - with open(filename) as fp: - fp.seek(filepos) - line = fp.readline() + line = mark.readline() # The log line will contain a variable timestamp, the PID, and a # trailing newline. Ignore these. self.assertEqual( @@ -495,6 +504,52 @@ Message-Id: self.assertEqual(items[0].msgdata['recipients'], ['gwen@example.com', 'herb@example.com']) + def test_no_progress_on_retries_within_retry_period(self): + # Temporary failures cause queuing for a retry later on, unless no + # progress is being made on the retries and we've tried for the + # specified delivery retry period. This test ensures that even if no + # progress is made, if the retry period hasn't expired, the message + # will be requeued. + temporary_failures.append('iona@example.com') + temporary_failures.append('jeff@example.com') + deliver_until = (datetime(2005, 8, 1, 7, 49, 23) + + as_timedelta(config.mta.delivery_retry_period)) + msgdata = dict(last_recip_count=2, + deliver_until=deliver_until) + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + self._runner.run() + # The retry queue should have our message waiting to be retried. + items = get_queue_messages('retry') + self.assertEqual(len(items), 1) + self.assertEqual(items[0].msgdata['deliver_until'], deliver_until) + self.assertEqual(items[0].msgdata['recipients'], + ['iona@example.com', 'jeff@example.com']) + + def test_no_progress_on_retries_with_expired_retry_period(self): + # We've had temporary failures with no progress, and the retry period + # has expired. In that case, a log entry is written and message is + # discarded. There's nothing more that can be done. + temporary_failures.append('kira@example.com') + temporary_failures.append('lonn@example.com') + retry_period = as_timedelta(config.mta.delivery_retry_period) + deliver_until = datetime(2005, 8, 1, 7, 49, 23) + retry_period + msgdata = dict(last_recip_count=2, + deliver_until=deliver_until) + self._outq.enqueue(self._msg, msgdata, listname='test@example.com') + # Before the queue runner runs, several days pass. + factory.fast_forward(retry_period.days + 1) + mark = LogFileMark('mailman.smtp') + self._runner.run() + # There should be no message in the retry or outgoing queues. + self.assertEqual(len(get_queue_messages('retry')), 0) + self.assertEqual(len(get_queue_messages('out')), 0) + # There should be a log message in the smtp log indicating that the + # message has been discarded. + line = mark.readline() + self.assertEqual( + line[-63:-1], + 'Discarding message with persistent temporary failures: ') + def test_suite(): -- cgit v1.2.3-70-g09d2 From 0795e34d56a8f627348730843210cdba4071b26b Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Wed, 25 May 2011 21:30:56 -0400 Subject: * bounce_unrecognized_goes_to_list_owner -> forward_unrecognized_bounces_to * Add an additional option for unrecognized bounce disposition: send it to the site administrators. * Move maybe_forward() from src/mailman/queue/bounce.py to src/mailman/app/bounces.py, refactor and add tests. * Add a LogFileMark class to help with tests that want to check the output to a log file. * OwnerNotification gets a better signature. Instead of tomoderators, the last argument is a roster to send the notification to. If roster is None, then the notification goes to the site administrators. --- src/mailman/app/bounces.py | 47 +++++++++- src/mailman/app/membership.py | 3 +- src/mailman/app/notifications.py | 2 +- src/mailman/app/tests/test_bounces.py | 139 +++++++++++++++++++++++++++++- src/mailman/database/mailman.sql | 2 +- src/mailman/email/message.py | 33 +++---- src/mailman/interfaces/bounce.py | 12 +++ src/mailman/interfaces/mailinglist.py | 10 +++ src/mailman/model/docs/registration.txt | 2 +- src/mailman/model/docs/requests.txt | 20 ++--- src/mailman/model/mailinglist.py | 3 +- src/mailman/pipeline/docs/acknowledge.txt | 32 ++++--- src/mailman/pipeline/docs/replybot.txt | 4 +- src/mailman/queue/bounce.py | 26 ------ src/mailman/queue/docs/command.txt | 2 +- src/mailman/queue/outgoing.py | 2 +- src/mailman/queue/tests/test_outgoing.py | 14 +-- src/mailman/styles/default.py | 5 +- src/mailman/templates/en/unrecognized.txt | 7 ++ src/mailman/testing/helpers.py | 17 +++- 20 files changed, 291 insertions(+), 91 deletions(-) create mode 100644 src/mailman/templates/en/unrecognized.txt (limited to 'src/mailman/queue/tests/test_outgoing.py') diff --git a/src/mailman/app/bounces.py b/src/mailman/app/bounces.py index f216c959e..46c4515ca 100644 --- a/src/mailman/app/bounces.py +++ b/src/mailman/app/bounces.py @@ -24,6 +24,7 @@ __all__ = [ 'ProbeVERP', 'StandardVERP', 'bounce_message', + 'maybe_forward', 'scan_message', 'send_probe', ] @@ -42,8 +43,9 @@ from zope.interface import implements from mailman.app.finder import find_components from mailman.config import config from mailman.core.i18n import _ -from mailman.email.message import UserNotification -from mailman.interfaces.bounce import IBounceDetector +from mailman.email.message import OwnerNotification, UserNotification +from mailman.interfaces.bounce import ( + IBounceDetector, UnrecognizedBounceDisposition) from mailman.interfaces.listmanager import IListManager from mailman.interfaces.membership import ISubscriptionService from mailman.interfaces.pending import IPendable, IPendings @@ -53,6 +55,7 @@ from mailman.utilities.string import oneline log = logging.getLogger('mailman.config') elog = logging.getLogger('mailman.error') +blog = logging.getLogger('mailman.bounce') DOT = '.' @@ -245,3 +248,43 @@ def send_probe(member, msg): probe.attach(MIMEMessage(msg)) probe.send(mlist, envsender=probe_sender, verp=False, probe_token=token) return token + + + +def maybe_forward(mlist, msg): + """Possibly forward bounce messages with no recognizable addresses. + + :param mlist: The mailing list. + :type mlist: `IMailingList` + :param msg: The bounce message to scan. + :type msg: `Message` + """ + message_id = msg['message-id'] + if (mlist.forward_unrecognized_bounces_to + == UnrecognizedBounceDisposition.discard): + blog.error('Discarding unrecognized bounce: {0}'.format(message_id)) + return + # The notification is either going to go to the list's administrators + # (owners and moderators), or to the site administrators. Most of the + # notification is exactly the same in either case. + adminurl = mlist.script_url('admin') + '/bounce' + subject=_('Uncaught bounce notification') + text = MIMEText( + make('unrecognized.txt', mlist, adminurl=adminurl), + _charset=mlist.preferred_language.charset) + attachment = MIMEMessage(msg) + if (mlist.forward_unrecognized_bounces_to + == UnrecognizedBounceDisposition.administrators): + keywords = dict(roster=mlist.administrators) + elif (mlist.forward_unrecognized_bounces_to + == UnrecognizedBounceDisposition.site_owner): + keywords = {} + else: + raise AssertionError('Invalid forwarding disposition: {0}'.format( + mlist.forward_unrecognized_bounces_to)) + # Create the notification and send it. + notice = OwnerNotification(mlist, subject, **keywords) + notice.set_type('multipart/mixed') + notice.attach(text) + notice.attach(attachment) + notice.send(mlist) diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py index be2382a7f..e7c2f40e3 100644 --- a/src/mailman/app/membership.py +++ b/src/mailman/app/membership.py @@ -154,5 +154,6 @@ def delete_member(mlist, address, admin_notif=None, userack=None): listname=mlist.real_name, member=formataddr((realname, address)), ) - msg = OwnerNotification(mlist, subject, text) + msg = OwnerNotification(mlist, subject, text, + roster=mlist.administrators) msg.send(mlist) diff --git a/src/mailman/app/notifications.py b/src/mailman/app/notifications.py index 8bfbd0934..72413845c 100644 --- a/src/mailman/app/notifications.py +++ b/src/mailman/app/notifications.py @@ -131,5 +131,5 @@ def send_admin_subscription_notice(mlist, address, full_name, language): listname=mlist.real_name, member=formataddr((full_name, address)), ) - msg = OwnerNotification(mlist, subject, text) + msg = OwnerNotification(mlist, subject, text, roster=mlist.administrators) msg.send(mlist) diff --git a/src/mailman/app/tests/test_bounces.py b/src/mailman/app/tests/test_bounces.py index a79b1524c..954b5fc05 100644 --- a/src/mailman/app/tests/test_bounces.py +++ b/src/mailman/app/tests/test_bounces.py @@ -32,14 +32,18 @@ import unittest from zope.component import getUtility -from mailman.app.bounces import StandardVERP, ProbeVERP, send_probe +from mailman.app.bounces import ( + ProbeVERP, StandardVERP, maybe_forward, send_probe) from mailman.app.lifecycle import create_list from mailman.app.membership import add_member from mailman.config import config +from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.languages import ILanguageManager -from mailman.interfaces.member import DeliveryMode +from mailman.interfaces.member import DeliveryMode, MemberRole from mailman.interfaces.pending import IPendings +from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( + LogFileMark, get_queue_messages, specialized_message_from_string as message_from_string) from mailman.testing.layers import ConfigLayer @@ -364,9 +368,140 @@ From: mail-daemon@example.com self.assertEqual(getUtility(IPendings).confirm(token), None) + +class TestMaybeForward(unittest.TestCase): + """Test forwarding of unrecognized bounces.""" + + layer = ConfigLayer + + def setUp(self): + config.push('test config', """ + [mailman] + site_owner: postmaster@example.com + """) + self._mlist = create_list('test@example.com') + self._msg = message_from_string("""\ +From: bouncer@example.com +To: test-bounces@example.com +Subject: You bounced +Message-ID: + +""") + + def tearDown(self): + config.pop('test config') + + def test_maybe_forward_discard(self): + # When forward_unrecognized_bounces_to is set to discard, no bounce + # messages are forwarded. + self._mlist.forward_unrecognized_bounces_to = ( + UnrecognizedBounceDisposition.discard) + # The only artifact of this call is a log file entry. + mark = LogFileMark('mailman.bounce') + maybe_forward(self._mlist, self._msg) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 0) + line = mark.readline() + self.assertEqual( + line[-40:-1], + 'Discarding unrecognized bounce: ') + + def test_maybe_forward_list_owner(self): + # Set up some owner and moderator addresses. + user_manager = getUtility(IUserManager) + anne = user_manager.create_address('anne@example.com') + bart = user_manager.create_address('bart@example.com') + cris = user_manager.create_address('cris@example.com') + dave = user_manager.create_address('dave@example.com') + # Regular members. + elle = user_manager.create_address('elle@example.com') + fred = user_manager.create_address('fred@example.com') + self._mlist.subscribe(anne, MemberRole.owner) + self._mlist.subscribe(bart, MemberRole.owner) + self._mlist.subscribe(cris, MemberRole.moderator) + self._mlist.subscribe(dave, MemberRole.moderator) + self._mlist.subscribe(elle, MemberRole.member) + self._mlist.subscribe(fred, MemberRole.member) + # When forward_unrecognized_bounces_to is set to owners, the + # bounce is forwarded to the list owners and moderators. + self._mlist.forward_unrecognized_bounces_to = ( + UnrecognizedBounceDisposition.administrators) + maybe_forward(self._mlist, self._msg) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + msg = items[0].msg + self.assertEqual(msg['subject'], 'Uncaught bounce notification') + self.assertEqual(msg['from'], 'postmaster@example.com') + self.assertEqual(msg['to'], 'test-owner@example.com') + # The first attachment is a notification message with a url. + payload = msg.get_payload(0) + self.assertEqual(payload.get_content_type(), 'text/plain') + body = payload.get_payload() + self.assertEqual( + body.splitlines()[-1], + 'http://lists.example.com/admin/test@example.com/bounce') + # The second attachment should be a message/rfc822 containing the + # original bounce message. + payload = msg.get_payload(1) + self.assertEqual(payload.get_content_type(), 'message/rfc822') + bounce = payload.get_payload(0) + self.assertEqual(bounce.as_string(), self._msg.as_string()) + # All of the owners and moderators, but none of the members, should be + # recipients of this message. + self.assertEqual(items[0].msgdata['recipients'], + set(['anne@example.com', 'bart@example.com', + 'cris@example.com', 'dave@example.com'])) + + def test_maybe_forward_site_owner(self): + # Set up some owner and moderator addresses. + user_manager = getUtility(IUserManager) + anne = user_manager.create_address('anne@example.com') + bart = user_manager.create_address('bart@example.com') + cris = user_manager.create_address('cris@example.com') + dave = user_manager.create_address('dave@example.com') + # Regular members. + elle = user_manager.create_address('elle@example.com') + fred = user_manager.create_address('fred@example.com') + self._mlist.subscribe(anne, MemberRole.owner) + self._mlist.subscribe(bart, MemberRole.owner) + self._mlist.subscribe(cris, MemberRole.moderator) + self._mlist.subscribe(dave, MemberRole.moderator) + self._mlist.subscribe(elle, MemberRole.member) + self._mlist.subscribe(fred, MemberRole.member) + # When forward_unrecognized_bounces_to is set to owners, the + # bounce is forwarded to the list owners and moderators. + self._mlist.forward_unrecognized_bounces_to = ( + UnrecognizedBounceDisposition.site_owner) + maybe_forward(self._mlist, self._msg) + items = get_queue_messages('virgin') + self.assertEqual(len(items), 1) + msg = items[0].msg + self.assertEqual(msg['subject'], 'Uncaught bounce notification') + self.assertEqual(msg['from'], 'postmaster@example.com') + self.assertEqual(msg['to'], 'postmaster@example.com') + # The first attachment is a notification message with a url. + payload = msg.get_payload(0) + self.assertEqual(payload.get_content_type(), 'text/plain') + body = payload.get_payload() + self.assertEqual( + body.splitlines()[-1], + 'http://lists.example.com/admin/test@example.com/bounce') + # The second attachment should be a message/rfc822 containing the + # original bounce message. + payload = msg.get_payload(1) + self.assertEqual(payload.get_content_type(), 'message/rfc822') + bounce = payload.get_payload(0) + self.assertEqual(bounce.as_string(), self._msg.as_string()) + # All of the owners and moderators, but none of the members, should be + # recipients of this message. + self.assertEqual(items[0].msgdata['recipients'], + set(['postmaster@example.com',])) + + def test_suite(): suite = unittest.TestSuite() + suite.addTest(unittest.makeSuite(TestMaybeForward)) suite.addTest(unittest.makeSuite(TestProbe)) suite.addTest(unittest.makeSuite(TestSendProbe)) suite.addTest(unittest.makeSuite(TestSendProbeNonEnglish)) diff --git a/src/mailman/database/mailman.sql b/src/mailman/database/mailman.sql index 6312066c0..103bfb8e1 100644 --- a/src/mailman/database/mailman.sql +++ b/src/mailman/database/mailman.sql @@ -127,13 +127,13 @@ CREATE TABLE mailinglist ( autoresponse_request_text TEXT, autoresponse_grace_period TEXT, -- Bounces. + forward_unrecognized_bounces_to TEXT, bounce_info_stale_after TEXT, bounce_matching_headers TEXT, bounce_notify_owner_on_disable BOOLEAN, bounce_notify_owner_on_removal BOOLEAN, bounce_processing BOOLEAN, bounce_score_threshold INTEGER, - bounce_unrecognized_goes_to_list_owner BOOLEAN, bounce_you_are_disabled_warnings INTEGER, bounce_you_are_disabled_warnings_interval TEXT, -- Content filtering. diff --git a/src/mailman/email/message.py b/src/mailman/email/message.py index 4eb049f17..546474fbe 100644 --- a/src/mailman/email/message.py +++ b/src/mailman/email/message.py @@ -162,7 +162,7 @@ class Message(email.message.Message): class UserNotification(Message): """Class for internally crafted messages.""" - def __init__(self, recip, sender, subject=None, text=None, lang=None): + def __init__(self, recipients, sender, subject=None, text=None, lang=None): Message.__init__(self) charset = (lang.charset if lang is not None else 'us-ascii') subject = ('(no subject)' if subject is None else subject) @@ -171,12 +171,12 @@ class UserNotification(Message): self['Subject'] = Header(subject.encode(charset), charset, header_name='Subject', errors='replace') self['From'] = sender - if isinstance(recip, list): - self['To'] = COMMASPACE.join(recip) - self.recips = recip + if isinstance(recipients, (list, set, tuple)): + self['To'] = COMMASPACE.join(recipients) + self.recipients = recipients else: - self['To'] = recip - self.recips = [recip] + self['To'] = recipients + self.recipients = set([recipients]) def send(self, mlist, **_kws): """Sends the message by enqueuing it to the 'virgin' queue. @@ -202,7 +202,7 @@ class UserNotification(Message): virginq = config.switchboards['virgin'] # The message metadata better have a 'recip' attribute. enqueue_kws = dict( - recipients=self.recips, + recipients=self.recipients, nodecorate=True, reduced_list_headers=True, ) @@ -218,20 +218,21 @@ class UserNotification(Message): class OwnerNotification(UserNotification): - """Like user notifications, but this message goes to the list owners.""" + """Like user notifications, but this message goes to some owners.""" - def __init__(self, mlist, subject=None, text=None, tomoderators=True): - if tomoderators: - roster = mlist.moderators + def __init__(self, mlist, subject=None, text=None, roster=None): + if roster is None: + recipients = set([config.mailman.site_owner]) + to = config.mailman.site_owner else: - roster = mlist.owners - recips = [address.address for address in roster.addresses] + recipients = set(address.email for address in roster.addresses) + to = mlist.owner_address sender = config.mailman.site_owner - UserNotification.__init__(self, recips, sender, subject, + UserNotification.__init__(self, recipients, sender, subject, text, mlist.preferred_language) # Hack the To header to look like it's going to the -owner address del self['to'] - self['To'] = mlist.owner_address + self['To'] = to self._sender = sender def _enqueue(self, mlist, **_kws): @@ -240,7 +241,7 @@ class OwnerNotification(UserNotification): # The message metadata better have a `recip' attribute virginq.enqueue(self, listname=mlist.fqdn_listname, - recipients=self.recips, + recipients=self.recipients, nodecorate=True, reduced_list_headers=True, envsender=self._sender, diff --git a/src/mailman/interfaces/bounce.py b/src/mailman/interfaces/bounce.py index 0b301aa98..99f6b50b7 100644 --- a/src/mailman/interfaces/bounce.py +++ b/src/mailman/interfaces/bounce.py @@ -26,6 +26,7 @@ __all__ = [ 'IBounceEvent', 'IBounceProcessor', 'Stop', + 'UnrecognizedBounceDisposition', ] @@ -54,6 +55,17 @@ class BounceContext(Enum): probe = 2 + +class UnrecognizedBounceDisposition(Enum): + # Just throw the message away. + discard = 0 + # Forward the message to the list administrators, which includes both the + # owners and the moderators. + administrators = 1 + # Forward the message to the site owner. + site_owner = 2 + + class IBounceDetector(Interface): """Detect a bounce in an email message.""" diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index 23d21cd34..7a25544d4 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -495,6 +495,16 @@ class IMailingList(Interface): without any other checks. """) + # Bounces. + + forward_unrecognized_bounces_to = Attribute( + """What to do when a bounce contains no recognizable addresses. + + This is an enumeration which specifies what to do with such bounce + messages. They can be discarded, forward to the list owner, or + forwarded to the site owner. + """) + class IAcceptableAlias(Interface): diff --git a/src/mailman/model/docs/registration.txt b/src/mailman/model/docs/registration.txt index d0827d37b..0e80bfa14 100644 --- a/src/mailman/model/docs/registration.txt +++ b/src/mailman/model/docs/registration.txt @@ -149,7 +149,7 @@ message is sent to the user in order to verify the registered address. _parsemsg : False listname : alpha@example.com nodecorate : True - recipients : [u'aperson@example.com'] + recipients : set([u'aperson@example.com']) reduced_list_headers: True version : 3 diff --git a/src/mailman/model/docs/requests.txt b/src/mailman/model/docs/requests.txt index bebb61259..812d25a43 100644 --- a/src/mailman/model/docs/requests.txt +++ b/src/mailman/model/docs/requests.txt @@ -302,7 +302,7 @@ The message can be rejected, meaning it is bounced back to the sender. _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'aperson@example.org'] + recipients : set([u'aperson@example.org']) reduced_list_headers: True version : 3 @@ -479,7 +479,7 @@ queue when the message is held. _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'alist-owner@example.com'] + recipients : set([u'alist-owner@example.com']) reduced_list_headers: True tomoderators : True version : 3 @@ -534,7 +534,7 @@ subscriber. _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'cperson@example.org'] + recipients : set([u'cperson@example.org']) reduced_list_headers: True version : 3 @@ -578,7 +578,7 @@ subscription and the fact that they may need to approve it. _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'alist-owner@example.com'] + recipients : set([u'alist-owner@example.com']) reduced_list_headers: True tomoderators : True version : 3 @@ -651,7 +651,7 @@ The welcome message is sent to the person who just subscribed. _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'fperson@example.org'] + recipients : set([u'fperson@example.org']) reduced_list_headers: True verp : False version : 3 @@ -677,7 +677,7 @@ The admin message is sent to the moderators. envsender : changeme@example.com listname : alist@example.com nodecorate : True - recipients : [] + recipients : set([]) reduced_list_headers: True version : 3 @@ -759,7 +759,7 @@ notification. _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'alist-owner@example.com'] + recipients : set([u'alist-owner@example.com']) reduced_list_headers: True tomoderators : True version : 3 @@ -818,7 +818,7 @@ and the person remains a member of the mailing list. _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'hperson@example.com'] + recipients : set([u'hperson@example.com']) reduced_list_headers: True version : 3 @@ -873,7 +873,7 @@ The goodbye message... _parsemsg : False listname : alist@example.com nodecorate : True - recipients : [u'gperson@example.com'] + recipients : set([u'gperson@example.com']) reduced_list_headers: True verp : False version : 3 @@ -898,6 +898,6 @@ The goodbye message... envsender : changeme@example.com listname : alist@example.com nodecorate : True - recipients : [] + recipients : set([]) reduced_list_headers: True version : 3 diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index 6952abcf0..e0841f6d8 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -120,9 +120,10 @@ class MailingList(Model): bounce_notify_owner_on_removal = Bool() # XXX bounce_processing = Bool() # XXX bounce_score_threshold = Int() # XXX - bounce_unrecognized_goes_to_list_owner = Bool() # XXX bounce_you_are_disabled_warnings = Int() # XXX bounce_you_are_disabled_warnings_interval = TimeDelta() # XXX + forward_unrecognized_bounces_to = Enum() + # Miscellaneous default_member_action = Enum() default_nonmember_action = Enum() description = Unicode() diff --git a/src/mailman/pipeline/docs/acknowledge.txt b/src/mailman/pipeline/docs/acknowledge.txt index b2ce11b7f..8c8552190 100644 --- a/src/mailman/pipeline/docs/acknowledge.txt +++ b/src/mailman/pipeline/docs/acknowledge.txt @@ -7,7 +7,7 @@ receive acknowledgments of their postings, Mailman will sent them such an acknowledgment. :: - >>> mlist = create_list('_xtest@example.com') + >>> mlist = create_list('test@example.com') >>> mlist.real_name = 'XTest' >>> mlist.preferred_language = 'en' >>> # XXX This will almost certainly change once we've worked out the web @@ -30,7 +30,7 @@ Subscribe a user to the mailing list. >>> user_1 = user_manager.create_user('aperson@example.com') >>> address_1 = list(user_1.addresses)[0] >>> mlist.subscribe(address_1, MemberRole.member) - + Non-member posts @@ -83,7 +83,7 @@ will be sent. >>> user_2 = user_manager.create_user('dperson@example.com') >>> address_2 = list(user_2.addresses)[0] >>> mlist.subscribe(address_2, MemberRole.member) - + >>> handler.process(mlist, msg, ... dict(original_sender='dperson@example.com')) @@ -112,14 +112,19 @@ The receipt will include the original message's subject in the response body, >>> qmsg, qdata = virginq.dequeue(virginq.files[0]) >>> virginq.files [] - >>> sorted(qdata.items()) - [..., ('recipients', [u'aperson@example.com']), ...] + >>> dump_msgdata(qdata) + _parsemsg : False + listname : test@example.com + nodecorate : True + recipients : set([u'aperson@example.com']) + reduced_list_headers: True + ... >>> print qmsg.as_string() ... MIME-Version: 1.0 ... Subject: XTest post acknowledgment - From: _xtest-bounces@example.com + From: test-bounces@example.com To: aperson@example.com ... Precedence: bulk @@ -130,7 +135,7 @@ The receipt will include the original message's subject in the response body, was successfully received by the XTest mailing list. - List info page: http://lists.example.com/listinfo/_xtest@example.com + List info page: http://lists.example.com/listinfo/test@example.com Your preferences: http://example.com/aperson@example.com @@ -146,13 +151,18 @@ If there is no subject, then the receipt will use a generic message. >>> qmsg, qdata = virginq.dequeue(virginq.files[0]) >>> virginq.files [] - >>> sorted(qdata.items()) - [..., ('recipients', [u'aperson@example.com']), ...] + >>> dump_msgdata(qdata) + _parsemsg : False + listname : test@example.com + nodecorate : True + recipients : set([u'aperson@example.com']) + reduced_list_headers: True + ... >>> print qmsg.as_string() MIME-Version: 1.0 ... Subject: XTest post acknowledgment - From: _xtest-bounces@example.com + From: test-bounces@example.com To: aperson@example.com ... Precedence: bulk @@ -163,6 +173,6 @@ If there is no subject, then the receipt will use a generic message. was successfully received by the XTest mailing list. - List info page: http://lists.example.com/listinfo/_xtest@example.com + List info page: http://lists.example.com/listinfo/test@example.com Your preferences: http://example.com/aperson@example.com diff --git a/src/mailman/pipeline/docs/replybot.txt b/src/mailman/pipeline/docs/replybot.txt index 3a6d75499..208f6aae9 100644 --- a/src/mailman/pipeline/docs/replybot.txt +++ b/src/mailman/pipeline/docs/replybot.txt @@ -51,7 +51,7 @@ response. _parsemsg : False listname : _xtest@example.com nodecorate : True - recipients : [u'aperson@example.com'] + recipients : set([u'aperson@example.com']) reduced_list_headers: True version : 3 @@ -143,7 +143,7 @@ Unless the ``X-Ack:`` header has a value of ``yes``, in which case, the _parsemsg : False listname : _xtest@example.com nodecorate : True - recipients : [u'asystem@example.com'] + recipients : set([u'asystem@example.com']) reduced_list_headers: True version : 3 diff --git a/src/mailman/queue/bounce.py b/src/mailman/queue/bounce.py index fb8b0124a..8787332e7 100644 --- a/src/mailman/queue/bounce.py +++ b/src/mailman/queue/bounce.py @@ -222,29 +222,3 @@ class BounceRunner(Runner, BounceMixin): def _clean_up(self): BounceMixin._clean_up(self) Runner._clean_up(self) - - - -def maybe_forward(mlist, msg): - # Does the list owner want to get non-matching bounce messages? - # If not, simply discard it. - if mlist.bounce_unrecognized_goes_to_list_owner: - adminurl = mlist.GetScriptURL('admin', absolute=1) + '/bounce' - mlist.ForwardMessage(msg, - text=_("""\ -The attached message was received as a bounce, but either the bounce format -was not recognized, or no member addresses could be extracted from it. This -mailing list has been configured to send all unrecognized bounce messages to -the list administrator(s). - -For more information see: -%(adminurl)s - -"""), - subject=_('Uncaught bounce notification'), - tomoderators=0) - log.error('forwarding unrecognized, message-id: %s', - msg.get('message-id', 'n/a')) - else: - log.error('discarding unrecognized, message-id: %s', - msg.get('message-id', 'n/a')) diff --git a/src/mailman/queue/docs/command.txt b/src/mailman/queue/docs/command.txt index dfe6b8c19..c767e6f5f 100644 --- a/src/mailman/queue/docs/command.txt +++ b/src/mailman/queue/docs/command.txt @@ -63,7 +63,7 @@ And now the response is in the ``virgin`` queue. _parsemsg : False listname : test@example.com nodecorate : True - recipients : [u'aperson@example.com'] + recipients : set([u'aperson@example.com']) reduced_list_headers: True version : ... diff --git a/src/mailman/queue/outgoing.py b/src/mailman/queue/outgoing.py index 1823b42b5..ed27f014c 100644 --- a/src/mailman/queue/outgoing.py +++ b/src/mailman/queue/outgoing.py @@ -156,5 +156,5 @@ class OutgoingRunner(Runner): msgdata['deliver_until'] = deliver_until msgdata['recipients'] = recipients self._retryq.enqueue(msg, msgdata) - # We've successfully completed handling of this message + # We've successfully completed handling of this message. return False diff --git a/src/mailman/queue/tests/test_outgoing.py b/src/mailman/queue/tests/test_outgoing.py index 74850ce75..a0fe407c8 100644 --- a/src/mailman/queue/tests/test_outgoing.py +++ b/src/mailman/queue/tests/test_outgoing.py @@ -46,6 +46,7 @@ from mailman.interfaces.pending import IPendings from mailman.interfaces.usermanager import IUserManager from mailman.queue.outgoing import OutgoingRunner from mailman.testing.helpers import ( + LogFileMark, get_queue_messages, make_testable_runner, specialized_message_from_string as message_from_string) @@ -53,19 +54,6 @@ from mailman.testing.layers import ConfigLayer, SMTPLayer from mailman.utilities.datetime import factory, now - -class LogFileMark: - def __init__(self, log_name): - self._log = logging.getLogger(log_name) - self._filename = self._log.handlers[0].filename - self._filepos = os.stat(self._filename).st_size - - def readline(self): - with open(self._filename) as fp: - fp.seek(self._filepos) - return fp.readline() - - def run_once(qrunner): """Predicate for make_testable_runner(). diff --git a/src/mailman/styles/default.py b/src/mailman/styles/default.py index 4f2b3d7d5..68934e99c 100644 --- a/src/mailman/styles/default.py +++ b/src/mailman/styles/default.py @@ -24,6 +24,7 @@ __all__ = [ 'DefaultStyle', ] + # XXX Styles need to be reconciled with lazr.config. import datetime @@ -32,6 +33,7 @@ from zope.interface import implements from mailman.core.i18n import _ from mailman.interfaces.action import Action +from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.autorespond import ResponseAction from mailman.interfaces.mailinglist import Personalization, ReplyToMunging @@ -161,13 +163,14 @@ ${listinfo_page} mlist.autoresponse_request_text = '' mlist.autoresponse_grace_period = datetime.timedelta(days=90) # Bounces + mlist.forward_unrecognized_bounces_to = ( + UnrecognizedBounceDisposition.administrators) mlist.bounce_processing = True mlist.bounce_score_threshold = 5.0 mlist.bounce_info_stale_after = datetime.timedelta(days=7) mlist.bounce_you_are_disabled_warnings = 3 mlist.bounce_you_are_disabled_warnings_interval = ( datetime.timedelta(days=7)) - mlist.bounce_unrecognized_goes_to_list_owner = True mlist.bounce_notify_owner_on_disable = True mlist.bounce_notify_owner_on_removal = True # This holds legacy member related information. It's keyed by the diff --git a/src/mailman/templates/en/unrecognized.txt b/src/mailman/templates/en/unrecognized.txt new file mode 100644 index 000000000..16bd19e79 --- /dev/null +++ b/src/mailman/templates/en/unrecognized.txt @@ -0,0 +1,7 @@ +The attached message was received as a bounce, but either the bounce format +was not recognized, or no member addresses could be extracted from it. This +mailing list has been configured to send all unrecognized bounce messages to +the list administrator(s). + +For more information see: +$adminurl diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py index 9f9ea6181..ed579e39c 100644 --- a/src/mailman/testing/helpers.py +++ b/src/mailman/testing/helpers.py @@ -20,6 +20,7 @@ from __future__ import absolute_import, unicode_literals __metaclass__ = type __all__ = [ + 'LogFileMark', 'TestableMaster', 'call_api', 'digest_mbox', @@ -39,6 +40,7 @@ import time import errno import signal import socket +import logging import smtplib import datetime import threading @@ -371,7 +373,7 @@ def reset_the_world(): config.db.commit() # Reset the global style manager. config.style_manager.populate() - + def specialized_message_from_string(unicode_text): @@ -391,3 +393,16 @@ def specialized_message_from_string(unicode_text): message = message_from_string(text, Message) message.original_size = original_size return message + + + +class LogFileMark: + def __init__(self, log_name): + self._log = logging.getLogger(log_name) + self._filename = self._log.handlers[0].filename + self._filepos = os.stat(self._filename).st_size + + def readline(self): + with open(self._filename) as fp: + fp.seek(self._filepos) + return fp.readline() -- cgit v1.2.3-70-g09d2