Skip to content

Commit ebe6710

Browse files
committed
Get rid of magic JSON serialization for Mailgun metadata.
Treat Mailgun metadata like all other ESPs: simple key-value dict, where values are strings. If you want to store JSON in metadata, you should serialize and deserialize it yourself.
1 parent 8e43f29 commit ebe6710

File tree

3 files changed

+11
-31
lines changed

3 files changed

+11
-31
lines changed

anymail/backends/mailgun.py

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -122,18 +122,8 @@ def add_attachment(self, attachment):
122122
)
123123

124124
def set_metadata(self, metadata):
125-
# The Mailgun docs are a little unclear on whether to send each var as a separate v: field,
126-
# or to send a single 'v:my-custom-data' field with a json blob of all vars.
127-
# (https://documentation.mailgun.com/user_manual.html#attaching-data-to-messages)
128-
# From experimentation, it seems like the first option works:
129125
for key, value in metadata.items():
130-
# Ensure the value is json-serializable (for Mailgun storage)
131-
json = self.serialize_json(value) # will raise AnymailSerializationError
132-
# Special case: a single string value should be sent bare (without quotes),
133-
# because Mailgun will add quotes when querying the value as json.
134-
if json.startswith('"'): # only a single string could be serialized as "...
135-
json = value
136-
self.data["v:%s" % key] = json
126+
self.data["v:%s" % key] = value
137127

138128
def set_send_at(self, send_at):
139129
# Mailgun expects RFC-2822 format dates

tests/test_mailgun_backend.py

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from __future__ import unicode_literals
44

55
from datetime import date, datetime
6-
from decimal import Decimal
76
from email.mime.base import MIMEBase
87
from email.mime.image import MIMEImage
98

@@ -13,7 +12,7 @@
1312
from django.test.utils import override_settings
1413
from django.utils.timezone import get_fixed_timezone, override as override_current_timezone
1514

16-
from anymail.exceptions import AnymailAPIError, AnymailSerializationError, AnymailUnsupportedFeature
15+
from anymail.exceptions import AnymailAPIError, AnymailUnsupportedFeature
1716
from anymail.message import attach_inline_image_file
1817

1918
from .mock_requests_backend import RequestsBackendMockAPITestCase, SessionSharingTestCasesMixin
@@ -270,12 +269,13 @@ class MailgunBackendAnymailFeatureTests(MailgunBackendMockAPITestCase):
270269
"""Test backend support for Anymail added features"""
271270

272271
def test_metadata(self):
273-
self.message.metadata = {'user_id': "12345", 'items': ['mail', 'gun']}
272+
# Each metadata value is just a string; you can serialize your own JSON if you'd like.
273+
# (The Mailgun docs are a little confusing on this point.)
274+
self.message.metadata = {'user_id': "12345", 'items': '["mail","gun"]'}
274275
self.message.send()
275276
data = self.get_api_call_data()
276-
# note values get serialized to json:
277-
self.assertEqual(data['v:user_id'], '12345') # simple values are transmitted as-is
278-
self.assertEqual(data['v:items'], '["mail", "gun"]') # complex values get json-serialized
277+
self.assertEqual(data['v:user_id'], '12345')
278+
self.assertEqual(data['v:items'], '["mail","gun"]')
279279

280280
def test_send_at(self):
281281
utc_plus_6 = get_fixed_timezone(6 * 60)
@@ -401,16 +401,8 @@ def test_send_unparsable_response(self):
401401
self.assertEqual(self.message.anymail_status.recipients, {})
402402
self.assertEqual(self.message.anymail_status.esp_response, mock_response)
403403

404-
def test_json_serialization_errors(self):
405-
"""Try to provide more information about non-json-serializable data"""
406-
self.message.metadata = {'total': Decimal('19.99')}
407-
with self.assertRaises(AnymailSerializationError) as cm:
408-
self.message.send()
409-
print(self.get_api_call_data())
410-
err = cm.exception
411-
self.assertIsInstance(err, TypeError) # compatibility with json.dumps
412-
self.assertIn("Don't know how to send this data to Mailgun", str(err)) # our added context
413-
self.assertIn("Decimal('19.99') is not JSON serializable", str(err)) # original message
404+
# test_json_serialization_errors: Mailgun payload isn't JSON, so we don't test this.
405+
# (Anything that requests can serialize as a form field will work with Mailgun)
414406

415407

416408
class MailgunBackendRecipientsRefusedTests(MailgunBackendMockAPITestCase):

tests/test_mailgun_integration.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ def test_all_options(self):
111111
reply_to=["[email protected]", "Reply 2 <[email protected]>"],
112112
headers={"X-Anymail-Test": "value"},
113113

114-
metadata={"meta1": "simple string", "meta2": 2, "meta3": {"complex": "value"}},
114+
metadata={"meta1": "simple string", "meta2": 2},
115115
send_at=send_at,
116116
tags=["tag 1", "tag 2"],
117117
track_clicks=False,
@@ -136,9 +136,7 @@ def test_all_options(self):
136136
event = events.pop()
137137
self.assertCountEqual(event["tags"], ["tag 1", "tag 2"]) # don't care about order
138138
self.assertEqual(event["user-variables"],
139-
{"meta1": "simple string",
140-
"meta2": "2", # numbers become strings
141-
"meta3": '{"complex": "value"}'}) # complex values become json
139+
{"meta1": "simple string", "meta2": "2"}) # all metadata values become strings
142140

143141
self.assertEqual(event["message"]["scheduled-for"], send_at_timestamp)
144142
self.assertCountEqual(event["message"]["recipients"],

0 commit comments

Comments
 (0)