Improve and document JSON serialization for Mandrill API

* Add some context to exceptions on unserializable
  values (addresses #89).
* Document need to format merge data
  (into something JSON-serializable).
* Add RemovedInDjrill2 DeprecationWarning.
* Deprecate blanket date/datetime serialization.

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)

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.
 

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):

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)

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):

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)

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 <message-headers>`
   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:

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.

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 <mandrill-send-support>`
 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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~