diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/mailman/app/docs/moderator.rst | 36 | ||||
| -rw-r--r-- | src/mailman/app/moderator.py | 11 | ||||
| -rw-r--r-- | src/mailman/app/tests/test_moderation.py | 36 | ||||
| -rw-r--r-- | src/mailman/chains/tests/test_hold.py | 35 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 1 | ||||
| -rw-r--r-- | src/mailman/interfaces/messages.py | 9 | ||||
| -rw-r--r-- | src/mailman/model/messagestore.py | 6 | ||||
| -rw-r--r-- | src/mailman/model/tests/test_messagestore.py | 19 |
8 files changed, 86 insertions, 67 deletions
diff --git a/src/mailman/app/docs/moderator.rst b/src/mailman/app/docs/moderator.rst index bf8a8e5df..f4339667f 100644 --- a/src/mailman/app/docs/moderator.rst +++ b/src/mailman/app/docs/moderator.rst @@ -172,36 +172,11 @@ however the message metadata indicates that the message has been approved. version : 3 -Preserving and forwarding the message -------------------------------------- +Forwarding the message +---------------------- -In addition to any of the above dispositions, the message can also be -preserved for further study. Ordinarily the message is removed from the -global message store after its disposition (though approved messages may be -re-added to the message store later). When handling a message, we can ask for -a copy to be preserve, which skips deleting the message from the storage. -:: - - >>> msg = message_from_string("""\ - ... From: dave@example.org - ... To: ant@example.com - ... Subject: Something important - ... Message-ID: <dolphin> - ... - ... Here's something important about our mailing list. - ... """) - >>> id = hold_message(mlist, msg, {}, 'Needs approval') - >>> handle_message(mlist, id, Action.discard, preserve=True) - - >>> from mailman.interfaces.messages import IMessageStore - >>> from zope.component import getUtility - >>> message_store = getUtility(IMessageStore) - >>> print(message_store.get_message_by_id('<dolphin>')['message-id']) - <dolphin> - -Orthogonal to preservation, the message can also be forwarded to another -address. This is helpful for getting the message into the inbox of one of the -moderators. +The message can be forwarded to another address. This is helpful for getting +the message into the inbox of one of the moderators. :: >>> msg = message_from_string("""\ @@ -241,8 +216,9 @@ the unsubscribing address is required. Fred is a member of the mailing list... - >>> mlist.send_welcome_message = False >>> from mailman.interfaces.usermanager import IUserManager + >>> from zope.component import getUtility + >>> mlist.send_welcome_message = False >>> fred = getUtility(IUserManager).create_address( ... 'fred@example.com', 'Fred Person') >>> from mailman.interfaces.registrar import IRegistrar diff --git a/src/mailman/app/moderator.py b/src/mailman/app/moderator.py index 88ee79f8c..af2addea7 100644 --- a/src/mailman/app/moderator.py +++ b/src/mailman/app/moderator.py @@ -98,8 +98,7 @@ def hold_message(mlist, msg, msgdata=None, reason=None): -def handle_message(mlist, id, action, - comment=None, preserve=False, forward=None): +def handle_message(mlist, id, action, comment=None, forward=None): message_store = getUtility(IMessageStore) requestdb = IListRequests(mlist) key, msgdata = requestdb.get_request(id) @@ -108,9 +107,10 @@ def handle_message(mlist, id, action, message_id = msgdata['_mod_message_id'] sender = msgdata['_mod_sender'] subject = msgdata['_mod_subject'] + keep = False if action in (Action.defer, Action.hold): # Nothing to do, but preserve the message for later. - preserve = True + keep = True elif action is Action.discard: rejection = 'Discarded' elif action is Action.reject: @@ -174,9 +174,8 @@ def handle_message(mlist, id, action, fmsg.set_type('message/rfc822') fmsg.attach(msg) fmsg.send(mlist) - # Delete the message from the message store if it is not being preserved. - if not preserve: - message_store.delete_message(message_id) + # Delete the request if it's not being kept. + if not keep: requestdb.delete_request(id) # Log the rejection if rejection: diff --git a/src/mailman/app/tests/test_moderation.py b/src/mailman/app/tests/test_moderation.py index 9a8d2721a..e71b692c0 100644 --- a/src/mailman/app/tests/test_moderation.py +++ b/src/mailman/app/tests/test_moderation.py @@ -121,31 +121,12 @@ Message-ID: <alpha> key, data = self._request_db.get_request(request_id) self.assertEqual(data['received_time'], received_time) - def test_non_preserving_disposition(self): - # By default, disposed messages are not preserved. - request_id = hold_message(self._mlist, self._msg) - handle_message(self._mlist, request_id, Action.discard) - message_store = getUtility(IMessageStore) - self.assertIsNone(message_store.get_message_by_id('<alpha>')) - - def test_preserving_disposition(self): - # Preserving a message keeps it in the store. - request_id = hold_message(self._mlist, self._msg) - handle_message(self._mlist, request_id, Action.discard, preserve=True) - message_store = getUtility(IMessageStore) - preserved_message = message_store.get_message_by_id('<alpha>') - self.assertEqual(preserved_message['message-id'], '<alpha>') - - def test_preserve_and_forward(self): - # We can both preserve and forward the message. + def test_forward(self): + # We can forward the message to an email address. request_id = hold_message(self._mlist, self._msg) handle_message(self._mlist, request_id, Action.discard, - preserve=True, forward=['zack@example.com']) - # The message is preserved in the store. - message_store = getUtility(IMessageStore) - preserved_message = message_store.get_message_by_id('<alpha>') - self.assertEqual(preserved_message['message-id'], '<alpha>') - # And the forwarded message lives in the virgin queue. + forward=['zack@example.com']) + # The forwarded message lives in the virgin queue. messages = get_queue_messages('virgin') self.assertEqual(len(messages), 1) self.assertEqual(str(messages[0].msg['subject']), @@ -162,6 +143,15 @@ Message-ID: <alpha> handle_message(self._mlist, request_id, Action.discard) self.assertEqual(self._request_db.count, 0) + def test_handled_message_stays_in_store(self): + # The message is still available in the store, even when it's been + # disposed of. + request_id = hold_message(self._mlist, self._msg) + handle_message(self._mlist, request_id, Action.discard) + self.assertEqual(self._request_db.count, 0) + message = getUtility(IMessageStore).get_message_by_id('<alpha>') + self.assertEqual(message['subject'], 'hold me') + class TestUnsubscription(unittest.TestCase): diff --git a/src/mailman/chains/tests/test_hold.py b/src/mailman/chains/tests/test_hold.py index 7f5b9e3ba..c7b676f8b 100644 --- a/src/mailman/chains/tests/test_hold.py +++ b/src/mailman/chains/tests/test_hold.py @@ -30,6 +30,7 @@ from mailman.app.lifecycle import create_list from mailman.chains.hold import autorespond_to_sender from mailman.core.chains import process as process_chain from mailman.interfaces.autorespond import IAutoResponseSet, Response +from mailman.interfaces.messages import IMessageStore from mailman.interfaces.usermanager import IUserManager from mailman.testing.helpers import ( LogFileMark, configuration, get_queue_messages, @@ -165,3 +166,37 @@ A message body. self.assertEqual( msg['Subject'], 'test@example.com post from anne@example.com requires approval') + + def test_hold_chain_crosspost(self): + mlist2 = create_list('test2@example.com') + msg = mfs("""\ +From: anne@example.com +To: test@example.com, test2@example.com +Subject: A message +Message-ID: <ant> +MIME-Version: 1.0 + +A message body. +""") + process_chain(self._mlist, msg, {}, start_chain='hold') + process_chain(mlist2, msg, {}, start_chain='hold') + items = get_queue_messages('virgin') + # There are four items in the virgin queue. Two of them are for the + # list owners who need to moderate the held message, and the other is + # for anne telling her that her message was held for approval. + self.assertEqual(len(items), 4) + anne_froms = set() + owner_tos = set() + for item in items: + if item.msg['to'] == 'anne@example.com': + anne_froms.add(item.msg['from']) + else: + owner_tos.add(item.msg['to']) + self.assertEqual(anne_froms, set(['test-bounces@example.com', + 'test2-bounces@example.com'])) + self.assertEqual(owner_tos, set(['test-owner@example.com', + 'test2-owner@example.com'])) + # And the message appears in the store. + messages = list(getUtility(IMessageStore).messages) + self.assertEqual(len(messages), 1) + self.assertEqual(messages[0]['message-id'], '<ant>') diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 0ef59266c..f4407e62e 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -68,6 +68,7 @@ Bugs * Trying to subscribe an address as a list owner (or moderator or nonmember) which is already subscribed with that role produces a server error. Originally given by Anirudh Dahiya. (Closes #198) + * Cross-posting messages held on both lists no longer fails. (Closes #176) Configuration ------------- diff --git a/src/mailman/interfaces/messages.py b/src/mailman/interfaces/messages.py index bdc06c0fc..ed8dad4c7 100644 --- a/src/mailman/interfaces/messages.py +++ b/src/mailman/interfaces/messages.py @@ -64,11 +64,12 @@ class IMessageStore(Interface): :param message: An email.message.Message instance containing at least a unique Message-ID header. The message will be given an - Message-ID-Hash header, overriding any existing such header. - :returns: The calculated Message-ID-Hash header. + Message-ID-Hash header, overriding any existing such header. If + the message already exists in the store, it is not added again. + :returns: The calculated Message-ID-Hash header or None if the message + already exists in the store. + :rtype: str or None :raises ValueError: if the message is missing a Message-ID header. - The storage service is also allowed to raise this exception if it - find, but disallows collisions. """ def get_message_by_id(message_id): diff --git a/src/mailman/model/messagestore.py b/src/mailman/model/messagestore.py index f247f94c2..9d0cfbc76 100644 --- a/src/mailman/model/messagestore.py +++ b/src/mailman/model/messagestore.py @@ -56,13 +56,11 @@ class MessageStore: message_id = message_ids[0] if isinstance(message_id, bytes): message_id = message_id.decode('ascii') - # Complain if the Message-ID already exists in the storage. + # If the Message-ID already exists in the store, don't store it again. existing = store.query(Message).filter( Message.message_id == message_id).first() if existing is not None: - raise ValueError( - 'Message ID already exists in message store: {0}'.format( - message_id)) + return None hash32 = add_message_hash(message) # Calculate the path on disk where we're going to store this message # object, in pickled format. diff --git a/src/mailman/model/tests/test_messagestore.py b/src/mailman/model/tests/test_messagestore.py index 6e3142536..998fa36c0 100644 --- a/src/mailman/model/tests/test_messagestore.py +++ b/src/mailman/model/tests/test_messagestore.py @@ -137,3 +137,22 @@ X-Message-ID-Hash: abc self.assertEqual(len(x_message_id_hashes), 1) self.assertEqual(x_message_id_hashes[0], 'MS6QLWERIJLGCRF44J7USBFDELMNT2BW') + + def test_add_message_duplicate_okay(self): + msg = mfs("""\ +Subject: Once +Message-ID: <ant> + +""") + hash32 = self._store.add(msg) + stored_msg = self._store.get_message_by_id('<ant>') + self.assertEqual(msg['subject'], stored_msg['subject']) + self.assertEqual(msg['message-id-hash'], hash32) + # A second insertion, even if headers change, does not store the + # message twice. + del msg['subject'] + msg['Subject'] = 'Twice' + hash32 = self._store.add(msg) + stored_msg = self._store.get_message_by_id('<ant>') + self.assertNotEqual(msg['subject'], stored_msg['subject']) + self.assertIsNone(hash32) |