diff --git a/djrill/mail/backends/djrill.py b/djrill/mail/backends/djrill.py index 3c7d43d..dd10038 100644 --- a/djrill/mail/backends/djrill.py +++ b/djrill/mail/backends/djrill.py @@ -66,42 +66,61 @@ class DjrillBackend(BaseEmailBackend): self.api_send_template = self.api_url + "/messages/send-template.json" def open(self): - if not self.session: - try: - self.session = requests.Session() - self.session.headers["User-Agent"] = "Djrill/%s %s" % ( - __version__, self.session.headers.get("User-Agent", "")) - except: - if not self.fail_silently: - raise - return False - return True - - - def close(self, error=False): + """ + Ensure we have a requests Session to connect to the Mandrill API. + Returns True if a new session was created (and the caller must close it). + """ if self.session: - try: - self.session.close() - except: - if not self.fail_silently and not error: - raise + return False # already exists + + try: + self.session = requests.Session() + except requests.RequestException: + if not self.fail_silently: + raise + else: + self.session.headers["User-Agent"] = "Djrill/%s %s" % ( + __version__, self.session.headers.get("User-Agent", "")) + return True + + def close(self): + """ + Close the Mandrill API Session unconditionally. + + (You should call this only if you called open and it returned True; + else someone else created the session and will clean it up themselves.) + """ + if self.session is None: + return + try: + self.session.close() + except requests.RequestException: + if not self.fail_silently: + raise + finally: self.session = None def send_messages(self, email_messages): + """ + Sends one or more EmailMessage objects and returns the number of email + messages sent. + """ if not email_messages: return 0 - if not self.open(): - return + created_session = self.open() + if not self.session: + return 0 # exception in self.open with fail_silently num_sent = 0 - for message in email_messages: - sent = self._send(message) - - if sent: - num_sent += 1 - - self.close() + try: + for message in email_messages: + sent = self._send(message) + if sent: + num_sent += 1 + finally: + if created_session: + self.close() return num_sent @@ -134,13 +153,14 @@ class DjrillBackend(BaseEmailBackend): except NotSupportedByMandrillError: if not self.fail_silently: - self.close(True) raise return False try: api_data = json.dumps(api_params) except TypeError as err: + if self.fail_silently: + return False # Add some context to the "not JSON serializable" message if not err.args: err.args = ('',) @@ -148,7 +168,6 @@ class DjrillBackend(BaseEmailBackend): err.args[0] + " in a Djrill message (perhaps it's a merge var?)." " Try converting it to a string or number first.", ) + err.args[1:] - self.close(True) raise err response = self.session.post(api_url, data=api_data) @@ -165,7 +184,6 @@ class DjrillBackend(BaseEmailBackend): to['email'] for to in msg_dict.get('to', []) if 'email' in to) if 'from_email' in msg_dict: log_message += " from %s" % msg_dict['from_email'] - self.close(True) raise MandrillAPIError( status_code=response.status_code, response=response, @@ -378,6 +396,7 @@ class DjrillBackend(BaseEmailBackend): # b64encode requires bytes, so let's convert our content. try: + # noinspection PyUnresolvedReferences if isinstance(content, unicode): # Python 2.X unicode string content = content.encode(str_encoding) diff --git a/djrill/tests/__init__.py b/djrill/tests/__init__.py index 1d0efac..afefe93 100644 --- a/djrill/tests/__init__.py +++ b/djrill/tests/__init__.py @@ -1,6 +1,7 @@ from djrill.tests.test_mandrill_send import * from djrill.tests.test_mandrill_send_template import * -from djrill.tests.test_mandrill_webhook import * +from djrill.tests.test_mandrill_session_sharing import * from djrill.tests.test_mandrill_subaccounts import * +from djrill.tests.test_mandrill_webhook import * from djrill.tests.test_mandrill_integration import * diff --git a/djrill/tests/test_mandrill_session_sharing.py b/djrill/tests/test_mandrill_session_sharing.py new file mode 100644 index 0000000..ba128ee --- /dev/null +++ b/djrill/tests/test_mandrill_session_sharing.py @@ -0,0 +1,72 @@ +from decimal import Decimal +from mock import patch + +from django.core import mail + +from .mock_backend import DjrillBackendMockAPITestCase + + +class DjrillSessionSharingTests(DjrillBackendMockAPITestCase): + """Test Djrill backend sharing of single Mandrill API connection""" + + @patch('requests.Session.close', autospec=True) + def test_connection_sharing(self, mock_close): + """Djrill reuses one requests session when sending multiple messages""" + datatuple = ( + ('Subject 1', 'Body 1', 'from@example.com', ['to@example.com']), + ('Subject 2', 'Body 2', 'from@example.com', ['to@example.com']), + ) + mail.send_mass_mail(datatuple) + self.assertEqual(self.mock_post.call_count, 2) + session1 = self.mock_post.call_args_list[0][0] # arg[0] (self) is session + session2 = self.mock_post.call_args_list[1][0] + self.assertEqual(session1, session2) + self.assertEqual(mock_close.call_count, 1) + + @patch('requests.Session.close', autospec=True) + def test_caller_managed_connections(self, mock_close): + """Calling code can created long-lived connection that it opens and closes""" + connection = mail.get_connection() + connection.open() + mail.send_mail('Subject 1', 'body', 'from@example.com', ['to@example.com'], connection=connection) + session1 = self.mock_post.call_args[0] + self.assertEqual(mock_close.call_count, 0) # shouldn't be closed yet + + mail.send_mail('Subject 2', 'body', 'from@example.com', ['to@example.com'], connection=connection) + self.assertEqual(mock_close.call_count, 0) # still shouldn't be closed + session2 = self.mock_post.call_args[0] + self.assertEqual(session1, session2) # should have reused same session + + connection.close() + self.assertEqual(mock_close.call_count, 1) + + def test_session_closed_after_exception(self): + # fail loud case: + msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'],) + msg.global_merge_vars = {'PRICE': Decimal('19.99')} # will cause JSON serialization error + with patch('requests.Session.close', autospec=True) as mock_close: + with self.assertRaises(TypeError): + msg.send() + self.assertEqual(mock_close.call_count, 1) + + # fail silently case (EmailMessage caches backend on send, so must create new one): + msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'],) + msg.global_merge_vars = {'PRICE': Decimal('19.99')} + with patch('requests.Session.close', autospec=True) as mock_close: + sent = msg.send(fail_silently=True) + self.assertEqual(sent, 0) + self.assertEqual(mock_close.call_count, 1) + + # caller-supplied connection case: + with patch('requests.Session.close', autospec=True) as mock_close: + connection = mail.get_connection() + connection.open() + msg = mail.EmailMessage('Subject', 'Message', 'from@example.com', ['to1@example.com'], + connection=connection) + msg.global_merge_vars = {'PRICE': Decimal('19.99')} + with self.assertRaises(TypeError): + msg.send() + self.assertEqual(mock_close.call_count, 0) # wait for us to close it + + connection.close() + self.assertEqual(mock_close.call_count, 1) diff --git a/docs/history.rst b/docs/history.rst index 5f4d8c0..0567b56 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -68,6 +68,15 @@ Removed DjrillBackendHTTPError with :exc:`djrill.MandrillAPIError`. +Other Djrill 2.0 Changes +~~~~~~~~~~~~~~~~~~~~~~~~ + +* Use a single HTTP connection to the Mandrill API to improve performance + when sending multiple messages at once using :func:`~django.core.mail.send_mass_mail`. + (You can also directly manage your own long-lived Djrill connection across multiple sends, + by calling open and close on :ref:`Django's email backend `.) + + Older Releases --------------