diff --git a/djrill/exceptions.py b/djrill/exceptions.py index 10d7c9d..6672739 100644 --- a/djrill/exceptions.py +++ b/djrill/exceptions.py @@ -1,4 +1,5 @@ from requests import HTTPError +import warnings class MandrillAPIError(HTTPError): @@ -32,3 +33,11 @@ class NotSupportedByMandrillError(ValueError): avoid duplicating Mandrill's validation logic locally.) """ + + +class RemovedInDjrill2(DeprecationWarning): + """Functionality due for deprecation in Djrill 2.0""" + + +def removed_in_djrill_2(message, stacklevel=1): + warnings.warn(message, category=RemovedInDjrill2, stacklevel=stacklevel + 1) diff --git a/djrill/mail/backends/djrill.py b/djrill/mail/backends/djrill.py index 8b0be95..a45bc06 100644 --- a/djrill/mail/backends/djrill.py +++ b/djrill/mail/backends/djrill.py @@ -6,6 +6,7 @@ from django.core.mail.message import sanitize_address, DEFAULT_ATTACHMENT_MIME_T # Oops: this file has the same name as our app, and cannot be renamed. #from djrill import MANDRILL_API_URL, MandrillAPIError, NotSupportedByMandrillError from ... import MANDRILL_API_URL, MandrillAPIError, NotSupportedByMandrillError +from ...exceptions import removed_in_djrill_2 from base64 import b64encode from datetime import date, datetime @@ -18,22 +19,36 @@ import requests DjrillBackendHTTPError = MandrillAPIError # Backwards-compat Djrill<=0.2.0 -class JSONDateUTCEncoder(json.JSONEncoder): - """JSONEncoder that encodes dates in string format used by Mandrill. +def encode_date_for_mandrill(dt): + """Format a date or datetime for use as a Mandrill API date field datetime becomes "YYYY-MM-DD HH:MM:SS" converted to UTC, if timezone-aware microseconds removed date becomes "YYYY-MM-DD 00:00:00" + anything else gets returned intact """ + if isinstance(dt, datetime): + dt = dt.replace(microsecond=0) + if dt.utcoffset() is not None: + dt = (dt - dt.utcoffset()).replace(tzinfo=None) + return dt.isoformat(' ') + elif isinstance(dt, date): + return dt.isoformat() + ' 00:00:00' + else: + return dt + + +class JSONDateUTCEncoder(json.JSONEncoder): + """[deprecated] JSONEncoder that encodes dates in string format used by Mandrill.""" def default(self, obj): - if isinstance(obj, datetime): - dt = obj.replace(microsecond=0) - if dt.utcoffset() is not None: - dt = (dt - dt.utcoffset()).replace(tzinfo=None) - return dt.isoformat(' ') - elif isinstance(obj, date): - return obj.isoformat() + ' 00:00:00' + if isinstance(obj, date): + removed_in_djrill_2( + "You've used the date '%r' as a Djrill message attribute " + "(perhaps in merge vars or metadata). Djrill 2.0 will require " + "you to explicitly convert this date to a string." % obj + ) + return encode_date_for_mandrill(obj) return super(JSONDateUTCEncoder, self).default(obj) @@ -104,7 +119,19 @@ class DjrillBackend(BaseEmailBackend): raise return False - response = requests.post(api_url, data=json.dumps(api_params, cls=JSONDateUTCEncoder)) + try: + api_data = json.dumps(api_params, cls=JSONDateUTCEncoder) + except TypeError as err: + # Add some context to the "not JSON serializable" message + if not err.args: + err.args = ('',) + err.args = ( + 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:] + raise err + + response = requests.post(api_url, data=api_data) if response.status_code != 200: @@ -175,12 +202,17 @@ class DjrillBackend(BaseEmailBackend): """Extend api_params to include Mandrill global-send options set on message""" # Mandrill attributes that can be copied directly: mandrill_attrs = [ - 'async', 'ip_pool', 'send_at' + 'async', 'ip_pool' ] for attr in mandrill_attrs: if hasattr(message, attr): api_params[attr] = getattr(message, attr) + # Mandrill attributes that require conversion: + if hasattr(message, 'send_at'): + api_params['send_at'] = encode_date_for_mandrill(message.send_at) + + def _make_mandrill_to_list(self, message, recipients, recipient_type="to"): """Create a Mandrill 'to' field from a list of emails. diff --git a/djrill/tests/mock_backend.py b/djrill/tests/mock_backend.py index bc46689..e119db2 100644 --- a/djrill/tests/mock_backend.py +++ b/djrill/tests/mock_backend.py @@ -5,12 +5,12 @@ import six from django.test import TestCase -from .utils import override_settings +from .utils import BackportedAssertions, override_settings @override_settings(MANDRILL_API_KEY="FAKE_API_KEY_FOR_TESTING", EMAIL_BACKEND="djrill.mail.backends.djrill.DjrillBackend") -class DjrillBackendMockAPITestCase(TestCase): +class DjrillBackendMockAPITestCase(TestCase, BackportedAssertions): """TestCase that uses Djrill EmailBackend with a mocked Mandrill API""" class MockResponse(requests.Response): diff --git a/djrill/tests/test_legacy.py b/djrill/tests/test_legacy.py index 64a12f7..3bebb49 100644 --- a/djrill/tests/test_legacy.py +++ b/djrill/tests/test_legacy.py @@ -1,10 +1,56 @@ # Tests deprecated Djrill features +from datetime import date, datetime +import warnings + +from django.core import mail from django.test import TestCase from djrill.mail import DjrillMessage from djrill import MandrillAPIError, NotSupportedByMandrillError +from .mock_backend import DjrillBackendMockAPITestCase + + +class DjrillBackendDeprecationTests(DjrillBackendMockAPITestCase): + + def test_deprecated_json_date_encoding(self): + """Djrill 2.0+ avoids a blanket JSONDateUTCEncoder""" + # Djrill allows dates for send_at, so shouldn't warn: + message = mail.EmailMessage('Subject', 'Body', 'from@example.com', ['to@example.com']) + message.send_at = datetime(2022, 10, 11, 12, 13, 14, 567) + self.assertNotWarns(DeprecationWarning, message.send) + + # merge_vars need to be json-serializable, so should generate a warning: + message = mail.EmailMessage('Subject', 'Body', 'from@example.com', ['to@example.com']) + message.global_merge_vars = {'DATE': date(2022, 10, 11)} + self.assertWarnsMessage(DeprecationWarning, + "Djrill 2.0 will require you to explicitly convert this date to a string", + message.send) + # ... but should still encode the date (for now): + data = self.get_api_call_data() + self.assertEqual(data['message']['global_merge_vars'], + [{'name': 'DATE', 'content': "2022-10-11 00:00:00"}]) + + def assertWarnsMessage(self, warning, message, callable, *args, **kwds): + """Checks that `callable` issues a warning of category `warning` containing `message`""" + with warnings.catch_warnings(record=True) as warned: + warnings.simplefilter("always") + callable(*args, **kwds) + self.assertGreater(len(warned), 0, msg="No warnings issued") + self.assertTrue( + any(issubclass(w.category, warning) and message in str(w.message) for w in warned), + msg="%r(%r) not found in %r" % (warning, message, [str(w) for w in warned])) + + def assertNotWarns(self, warning, callable, *args, **kwds): + """Checks that `callable` does not issue any warnings of category `warning`""" + with warnings.catch_warnings(record=True) as warned: + warnings.simplefilter("always") + callable(*args, **kwds) + relevant_warnings = [w for w in warned if issubclass(w.category, warning)] + self.assertEqual(len(relevant_warnings), 0, + msg="Unexpected warnings %r" % [str(w) for w in relevant_warnings]) + class DjrillMessageTests(TestCase): """Test the DjrillMessage class (deprecated as of Djrill v0.2.0) diff --git a/djrill/tests/test_mandrill_send.py b/djrill/tests/test_mandrill_send.py index fad7cf7..d84c5ce 100644 --- a/djrill/tests/test_mandrill_send.py +++ b/djrill/tests/test_mandrill_send.py @@ -4,6 +4,7 @@ from __future__ import unicode_literals from base64 import b64decode from datetime import date, datetime, timedelta, tzinfo +from decimal import Decimal from email.mime.base import MIMEBase from email.mime.image import MIMEImage import json @@ -507,6 +508,16 @@ class DjrillMandrillFeatureTests(DjrillBackendMockAPITestCase): self.assertEqual(sent, 0) self.assertIsNone(msg.mandrill_response) + def test_json_serialization_warnings(self): + """Try to provide more information about non-json-serializable data""" + self.message.global_merge_vars = {'PRICE': Decimal('19.99')} + with self.assertRaisesMessage( + TypeError, + "Decimal('19.99') is not JSON serializable in a Djrill message (perhaps " + "it's a merge var?). Try converting it to a string or number first." + ): + self.message.send() + @override_settings(EMAIL_BACKEND="djrill.mail.backends.djrill.DjrillBackend") class DjrillImproperlyConfiguredTests(TestCase): diff --git a/djrill/tests/utils.py b/djrill/tests/utils.py index 7871ce4..c001e3b 100644 --- a/djrill/tests/utils.py +++ b/djrill/tests/utils.py @@ -1,4 +1,8 @@ +import re +import six + __all__ = ( + 'BackportedAssertions', 'override_settings', ) @@ -66,3 +70,19 @@ except ImportError: # new_value = getattr(settings, key, None) # setting_changed.send(sender=settings._wrapped.__class__, # setting=key, value=new_value) + + +class BackportedAssertions(object): + """Handful of useful TestCase assertions backported to Python 2.6/Django 1.3""" + + # Backport from Python 2.7/3.1 + def assertIn(self, member, container, msg=None): + """Just like self.assertTrue(a in b), but with a nicer default message.""" + if member not in container: + self.fail(msg or '%r not found in %r' % (member, container)) + + # Backport from Django 1.4 + def assertRaisesMessage(self, expected_exception, expected_message, + callable_obj=None, *args, **kwargs): + return six.assertRaisesRegex(self, expected_exception, re.escape(expected_message), + callable_obj, *args, **kwargs) diff --git a/docs/history.rst b/docs/history.rst index 862e9f1..221d2de 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -7,6 +7,35 @@ Among other things, this means that minor updates and breaking changes will always increment the major version number (1.x to 2.0). +Upcoming Changes in Djrill 2.0 +------------------------------ + +Djrill 2.0 is under development and will include some breaking changes. +Although the changes won't impact most Djrill users, the current +version of Djrill (1.4) will try to warn you if you use things +that will change. (Warnings appear in the console when running Django +in debug mode.) + +* **Dates in merge data and other attributes** + + Djrill automatically converts :attr:`send_at` `date` and `datetime` + values to the ISO 8601 string format expected by the Mandrill API. + + Unintentionally, it also converts dates used in other Mandrill message + attributes (such as :attr:`merge_vars` or :attr:`metadata`) where it + might not be expected (or appropriate). + + Djrill 2.0 will remove this automatic date formatting, except + for attributes that are inherently dates (currently only `send_at`). + + To assist in detecting code relying on the (undocumented) current + behavior, Djrill 1.4 will report a `DeprecationWarning` for `date` + or `datetime` values used in any Mandrill message attributes other + than `send_at`. See :ref:`formatting-merge-data` for other options. + + +Change Log +---------- Version 1.4 (development): @@ -15,6 +44,9 @@ Version 1.4 (development): (Specifying a :ref:`Reply-To header ` still works, with any version of Django, and will override the reply_to param if you use both.) +* More-helpful exception when using a non-JSON-serializable + type in merge_vars and other Djrill message attributes +* Deprecation warnings for upcoming 2.0 changes (see above) Version 1.3: diff --git a/docs/usage/sending_mail.rst b/docs/usage/sending_mail.rst index 3864422..0104082 100644 --- a/docs/usage/sending_mail.rst +++ b/docs/usage/sending_mail.rst @@ -198,6 +198,9 @@ Most of the options from the Mandrill message.global_merge_vars = {'company': "ACME", 'offer': "10% off"} + Merge data must be strings or other JSON-serializable types. + (See :ref:`formatting-merge-data` for details.) + .. attribute:: merge_vars ``dict``: per-recipient merge variables (most useful with :ref:`mandrill-templates`). The keys @@ -209,6 +212,9 @@ Most of the options from the Mandrill 'rr@example.com': {'offer': "instant tunnel paint"} } + Merge data must be strings or other JSON-serializable types. + (See :ref:`formatting-merge-data` for details.) + .. attribute:: tags ``list`` of ``str``: tags to apply to the message, for filtering reports in the Mandrill @@ -245,12 +251,18 @@ Most of the options from the Mandrill message.metadata = {'customer': customer.id, 'order': order.reference_number} + Mandrill restricts metadata keys to alphanumeric characters and underscore, and + metadata values to numbers, strings, boolean values, and None (null). + .. attribute:: recipient_metadata ``dict``: per-recipient metadata values. Keys are the recipient email addresses, and values are dicts of metadata for each recipient (similar to :attr:`merge_vars`) + Mandrill restricts metadata keys to alphanumeric characters and underscore, and + metadata values to numbers, strings, boolean values, and None (null). + .. attribute:: async ``Boolean``: whether Mandrill should use an async mode optimized for bulk sending. diff --git a/docs/usage/templates.rst b/docs/usage/templates.rst index d3f8957..3673183 100644 --- a/docs/usage/templates.rst +++ b/docs/usage/templates.rst @@ -37,6 +37,45 @@ and will ignore any `body` text set on the `EmailMessage`. All of Djrill's other :ref:`Mandrill-specific options ` can be used with templates. + +.. _formatting-merge-data: + +Formatting Merge Data +~~~~~~~~~~~~~~~~~~~~~ + +If you're using dates, datetimes, Decimals, or anything other than strings and integers, +you'll need to format them into strings for use as merge data:: + + product = Product.objects.get(123) # A Django model + total_cost = Decimal('19.99') + ship_date = date(2015, 11, 18) + + # Won't work -- you'll get "not JSON serializable" exceptions: + msg.global_merge_vars = { + 'PRODUCT': product, + 'TOTAL_COST': total_cost, + 'SHIP_DATE': ship_date + } + + # Do something this instead: + msg.global_merge_vars = { + 'PRODUCT': product.name, # assuming name is a CharField + 'TOTAL_COST': "%.2f" % total_cost, + 'SHIP_DATE': ship_date.strftime('%B %d, %Y') # US-style "March 15, 2015" + } + +These are just examples. You'll need to determine the best way to format +your merge data as strings. + +Although floats are allowed in merge vars, you'll generally want to format them +into strings yourself to avoid surprises with floating-point precision. + +Technically, Djrill will accept anything serializable by the Python json package -- +which means advanced template users can include dicts and lists as merge vars +(for templates designed to handle objects and arrays). +See the Python :class:`json.JSONEncoder` docs for a list of allowable types. + + How To Use Default Mandrill Subject and From fields ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~