From 3a936484819681a83779658113ba23a0c074cdc5 Mon Sep 17 00:00:00 2001 From: medmunds Date: Sat, 5 Mar 2016 17:22:27 -0800 Subject: [PATCH] Normalize ESP response and recipient status --- anymail/backends/base.py | 81 +++++++++++++++------- anymail/backends/base_requests.py | 6 +- anymail/backends/mandrill.py | 39 +++++------ anymail/tests/test_mandrill_integration.py | 35 ++++++---- anymail/tests/test_mandrill_send.py | 40 ++++++----- 5 files changed, 118 insertions(+), 83 deletions(-) diff --git a/anymail/backends/base.py b/anymail/backends/base.py index af85db2..edf8dc5 100644 --- a/anymail/backends/base.py +++ b/anymail/backends/base.py @@ -4,10 +4,46 @@ from django.conf import settings from django.core.mail.backends.base import BaseEmailBackend from django.utils.timezone import is_naive, get_current_timezone, make_aware, utc -from ..exceptions import AnymailError, AnymailUnsupportedFeature +from ..exceptions import AnymailError, AnymailUnsupportedFeature, AnymailRecipientsRefused from ..utils import Attachment, ParsedEmail, UNSET, combine, last, get_anymail_setting +# The AnymailStatus* stuff should get moved to an anymail.message module +ANYMAIL_STATUSES = [ + 'sent', # the ESP has sent the message (though it may or may not get delivered) + 'queued', # the ESP will try to send the message later + 'invalid', # the recipient email was not valid + 'rejected', # the recipient is blacklisted + 'unknown', # anything else +] + + +class AnymailRecipientStatus(object): + """Information about an EmailMessage's send status for a single recipient""" + + def __init__(self, message_id, status): + self.message_id = message_id # ESP message id + self.status = status # one of ANYMAIL_STATUSES, or None for not yet sent to ESP + + +class AnymailStatus(object): + """Information about an EmailMessage's send status for all recipients""" + + def __init__(self): + self.message_id = None # set of ESP message ids across all recipients, or bare id if only one, or None + self.status = None # set of ANYMAIL_STATUSES across all recipients, or None for not yet sent to ESP + self.recipients = {} # per-recipient: { email: AnymailRecipientStatus, ... } + self.esp_response = None + + def set_recipient_status(self, recipients): + self.recipients.update(recipients) + recipient_statuses = self.recipients.values() + self.message_id = set([recipient.message_id for recipient in recipient_statuses]) + if len(self.message_id) == 1: + self.message_id = self.message_id.pop() # de-set-ify if single message_id + self.status = set([recipient.status for recipient in recipient_statuses]) + + class AnymailBaseBackend(BaseEmailBackend): """ Base Anymail email backend @@ -100,24 +136,24 @@ class AnymailBaseBackend(BaseEmailBackend): Implementations must raise exceptions derived from AnymailError for anticipated failures that should be suppressed in fail_silently mode. """ - message.anymail_status = None - esp_response_attr = "%s_response" % self.esp_name.lower() # e.g., message.mandrill_response - setattr(message, esp_response_attr, None) # until we have a response + message.anymail_status = AnymailStatus() if not message.recipients(): return False - payload = self.build_message_payload(message) + payload = self.build_message_payload(message, self.send_defaults) # FUTURE: if pre-send-signal OK... response = self.post_to_esp(payload, message) + message.anymail_status.esp_response = response - parsed_response = self.deserialize_response(response, payload, message) - setattr(message, esp_response_attr, parsed_response) - message.anymail_status = self.validate_response(parsed_response, response, payload, message) + recipient_status = self.parse_recipient_status(response, payload, message) + message.anymail_status.set_recipient_status(recipient_status) + + self.raise_for_recipient_status(message.anymail_status, response, payload, message) # FUTURE: post-send signal return True - def build_message_payload(self, message): + def build_message_payload(self, message, defaults): """Returns a payload that will allow message to be sent via the ESP. Derived classes must implement, and should subclass :class:BasePayload @@ -127,6 +163,7 @@ class AnymailBaseBackend(BaseEmailBackend): cannot be communicated to the ESP. :param message: :class:EmailMessage + :param defaults: dict :return: :class:BasePayload """ raise NotImplementedError("%s.%s must implement build_message_payload" % @@ -144,27 +181,21 @@ class AnymailBaseBackend(BaseEmailBackend): raise NotImplementedError("%s.%s must implement post_to_esp" % (self.__class__.__module__, self.__class__.__name__)) - def deserialize_response(self, response, payload, message): - """Deserialize a raw ESP response + def parse_recipient_status(self, response, payload, message): + """Return a dict mapping email to AnymailRecipientStatus for each recipient. Can raise AnymailAPIError (or derived exception) if response is unparsable """ - raise NotImplementedError("%s.%s must implement deserialize_response" % + raise NotImplementedError("%s.%s must implement parse_recipient_status" % (self.__class__.__module__, self.__class__.__name__)) - def validate_response(self, parsed_response, response, payload, message): - """Validate parsed_response, raising exceptions for any problems, and return normalized status. - - Extend this to provide your own validation checks. - Validation exceptions should inherit from anymail.exceptions.AnymailError - for proper fail_silently behavior. - - If *all* recipients are refused or invalid, should raise AnymailRecipientsRefused - - Returns one of "sent", "queued", "refused", "error" or "multi" - """ - raise NotImplementedError("%s.%s must implement validate_response" % - (self.__class__.__module__, self.__class__.__name__)) + def raise_for_recipient_status(self, anymail_status, response, payload, message): + """If *all* recipients are refused or invalid, raises AnymailRecipientsRefused""" + if not self.ignore_recipient_status: + # Error if *all* recipients are invalid or refused + # (This behavior parallels smtplib.SMTPRecipientsRefused from Django's SMTP EmailBackend) + if anymail_status.status.issubset({"invalid", "rejected"}): + raise AnymailRecipientsRefused(email_message=message, payload=payload, response=response) @property def esp_name(self): diff --git a/anymail/backends/base_requests.py b/anymail/backends/base_requests.py index 294e823..83f5557 100644 --- a/anymail/backends/base_requests.py +++ b/anymail/backends/base_requests.py @@ -74,10 +74,10 @@ class AnymailRequestsBackend(AnymailBaseBackend): raise AnymailRequestsAPIError(email_message=message, payload=payload, response=response) return response - def deserialize_response(self, response, payload, message): - """Return parsed ESP API response + def deserialize_json_response(self, response, payload, message): + """Deserialize an ESP API response that's in json. - Can raise AnymailRequestsAPIError if response is unparsable + Useful for implementing deserialize_response """ try: return response.json() diff --git a/anymail/backends/mandrill.py b/anymail/backends/mandrill.py index 55b8801..4e2057e 100644 --- a/anymail/backends/mandrill.py +++ b/anymail/backends/mandrill.py @@ -1,6 +1,7 @@ -from datetime import date, datetime +from datetime import datetime -from ..exceptions import AnymailRequestsAPIError, AnymailRecipientsRefused +from anymail.backends.base import AnymailRecipientStatus, ANYMAIL_STATUSES +from ..exceptions import AnymailRequestsAPIError from ..utils import last, combine, get_anymail_setting from .base_requests import AnymailRequestsBackend, RequestsPayload @@ -19,31 +20,25 @@ class MandrillBackend(AnymailRequestsBackend): api_url += "/" super(MandrillBackend, self).__init__(api_url, **kwargs) - def build_message_payload(self, message): - return MandrillPayload(message, self.send_defaults, self) + def build_message_payload(self, message, defaults): + return MandrillPayload(message, defaults, self) - def validate_response(self, parsed_response, response, payload, message): - """Validate parsed_response, raising exceptions for any problems. - """ + def parse_recipient_status(self, response, payload, message): + parsed_response = self.deserialize_json_response(response, payload, message) + recipient_status = {} try: - unique_statuses = set([item["status"] for item in parsed_response]) + # Mandrill returns a list of { email, status, _id, reject_reason } for each recipient + for item in parsed_response: + email = item['email'] + status = item['status'] + if status not in ANYMAIL_STATUSES: + status = 'unknown' + message_id = item.get('_id', None) # can be missing for invalid/rejected recipients + recipient_status[email] = AnymailRecipientStatus(message_id=message_id, status=status) except (KeyError, TypeError): raise AnymailRequestsAPIError("Invalid Mandrill API response format", email_message=message, payload=payload, response=response) - - if unique_statuses == {"sent"}: - return "sent" - elif unique_statuses == {"queued"}: - return "queued" - elif unique_statuses.issubset({"invalid", "rejected"}): - if self.ignore_recipient_status: - return "refused" - else: - # Error if *all* recipients are invalid or refused - # (This behavior parallels smtplib.SMTPRecipientsRefused from Django's SMTP EmailBackend) - raise AnymailRecipientsRefused(email_message=message, payload=payload, response=response) - else: - return "multi" + return recipient_status def _expand_merge_vars(vardict): diff --git a/anymail/tests/test_mandrill_integration.py b/anymail/tests/test_mandrill_integration.py index c1b8514..00b4a03 100644 --- a/anymail/tests/test_mandrill_integration.py +++ b/anymail/tests/test_mandrill_integration.py @@ -38,11 +38,17 @@ class DjrillIntegrationTests(TestCase): # Example of getting the Mandrill send status and _id from the message sent_count = self.message.send() self.assertEqual(sent_count, 1) + # noinspection PyUnresolvedReferences - response = self.message.mandrill_response - self.assertIn(response[0]['status'], ['sent', 'queued']) # successful send (could still bounce later) - self.assertEqual(response[0]['email'], 'to@example.com') - self.assertGreater(len(response[0]['_id']), 0) + anymail_status = self.message.anymail_status + sent_status = anymail_status.recipients['to@example.com'].status + message_id = anymail_status.recipients['to@example.com'].message_id + + self.assertIn(sent_status, ['sent', 'queued']) # successful send (could still bounce later) + self.assertGreater(len(message_id), 0) # don't know what it'll be, but it should exist + + self.assertEqual(anymail_status.status, {sent_status}) # set of all recipient statuses + self.assertEqual(anymail_status.message_id, message_id) # because only a single recipient (else would be a set) def test_invalid_from(self): # Example of trying to send from an invalid address @@ -61,15 +67,15 @@ class DjrillIntegrationTests(TestCase): try: self.message.send() except AnymailRecipientsRefused: - # Mandrill refused to deliver the mail -- message.mandrill_response will tell you why: + # Mandrill refused to deliver the mail -- message.anymail_status will tell you why: # noinspection PyUnresolvedReferences - response = self.message.mandrill_response - self.assertEqual(response[0]['status'], 'invalid') + anymail_status = self.message.anymail_status + self.assertEqual(anymail_status.recipients['invalid@localhost'].status, 'invalid') + self.assertEqual(anymail_status.status, {'invalid'}) else: # Sometimes Mandrill queues these test sends # noinspection PyUnresolvedReferences - response = self.message.mandrill_response - if response[0]['status'] == 'queued': + if self.message.anymail_status.status == {'queued'}: self.skipTest("Mandrill queued the send -- can't complete this test") else: self.fail("Djrill did not raise AnymailRecipientsRefused for invalid recipient") @@ -80,16 +86,15 @@ class DjrillIntegrationTests(TestCase): try: self.message.send() except AnymailRecipientsRefused: - # Mandrill refused to deliver the mail -- message.mandrill_response will tell you why: + # Mandrill refused to deliver the mail -- message.anymail_status will tell you why: # noinspection PyUnresolvedReferences - response = self.message.mandrill_response - self.assertEqual(response[0]['status'], 'rejected') - self.assertEqual(response[0]['reject_reason'], 'test') + anymail_status = self.message.anymail_status + self.assertEqual(anymail_status.recipients['reject@test.mandrillapp.com'].status, 'rejected') + self.assertEqual(anymail_status.status, {'rejected'}) else: # Sometimes Mandrill queues these test sends # noinspection PyUnresolvedReferences - response = self.message.mandrill_response - if response[0]['status'] == 'queued': + if self.message.anymail_status.status == {'queued'}: self.skipTest("Mandrill queued the send -- can't complete this test") else: self.fail("Djrill did not raise AnymailRecipientsRefused for blacklist recipient") diff --git a/anymail/tests/test_mandrill_send.py b/anymail/tests/test_mandrill_send.py index 9513c91..f277e25 100644 --- a/anymail/tests/test_mandrill_send.py +++ b/anymail/tests/test_mandrill_send.py @@ -525,39 +525,43 @@ class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase): self.assertFalse('async' in data) self.assertFalse('ip_pool' in data) - def test_send_attaches_mandrill_response(self): - """ The mandrill_response should be attached to the message when it is sent """ - response = [{'email': 'to1@example.com', 'status': 'sent'}] + # noinspection PyUnresolvedReferences + def test_send_attaches_anymail_status(self): + """ The anymail_status should be attached to the message when it is sent """ + response = [{'email': 'to1@example.com', 'status': 'sent', '_id': 'abc123'}] self.mock_post.return_value = self.MockResponse(raw=six.b(json.dumps(response))) msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'],) sent = msg.send() self.assertEqual(sent, 1) - # noinspection PyUnresolvedReferences - self.assertEqual(msg.mandrill_response, response) - # noinspection PyUnresolvedReferences - self.assertEqual(msg.anymail_status, "sent") + self.assertEqual(msg.anymail_status.status, {'sent'}) + self.assertEqual(msg.anymail_status.message_id, 'abc123') + self.assertEqual(msg.anymail_status.recipients['to1@example.com'].status, 'sent') + self.assertEqual(msg.anymail_status.recipients['to1@example.com'].message_id, 'abc123') + self.assertEqual(msg.anymail_status.esp_response.json(), response) - def test_send_failed_mandrill_response(self): - """ If the send fails, mandrill_response should be set to None """ + # noinspection PyUnresolvedReferences + def test_send_failed_anymail_status(self): + """ If the send fails, anymail_status should contain initial values""" self.mock_post.return_value = self.MockResponse(status_code=500) msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'],) sent = msg.send(fail_silently=True) self.assertEqual(sent, 0) - # noinspection PyUnresolvedReferences - self.assertIsNone(msg.mandrill_response) - # noinspection PyUnresolvedReferences - self.assertIsNone(msg.anymail_status) + self.assertIsNone(msg.anymail_status.status) + self.assertIsNone(msg.anymail_status.message_id) + self.assertEqual(msg.anymail_status.recipients, {}) + self.assertIsNone(msg.anymail_status.esp_response) + # noinspection PyUnresolvedReferences def test_send_unparsable_mandrill_response(self): """If the send succeeds, but a non-JSON API response, should raise an API exception""" - self.mock_post.return_value = self.MockResponse(status_code=500, raw=b"this isn't json") + self.mock_post.return_value = self.MockResponse(status_code=200, raw=b"this isn't json") msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'],) with self.assertRaises(AnymailAPIError): msg.send() - # noinspection PyUnresolvedReferences - self.assertIsNone(msg.mandrill_response) - # noinspection PyUnresolvedReferences - self.assertIsNone(msg.anymail_status) + self.assertIsNone(msg.anymail_status.status) + self.assertIsNone(msg.anymail_status.message_id) + self.assertEqual(msg.anymail_status.recipients, {}) + self.assertEqual(msg.anymail_status.esp_response, self.mock_post.return_value) def test_json_serialization_errors(self): """Try to provide more information about non-json-serializable data"""