From fbc78dc6204736f2939aa2c7cde14e12ade4c337 Mon Sep 17 00:00:00 2001
From: Lars Butler <Lars.Butler@gmail.com>
Date: Thu, 9 Jun 2016 11:53:41 +0200
Subject: [PATCH] Fix `Airbrake.notify` regression

1d7b191a2cc3b53f76f12d09f68c857183859e08 broke the `Airbrake.notify`
interface by assuming that the `payload` argument is already encoded as
a JSON string. Unfortunately, the tests didn't catch this because the
test weren't actually exercising the `notify` method. :sad_panda:

This patch fixes the regression introduced in
https://github.com/airbrake/airbrake-python/pull/27 and adds a test to
notify to ensure that this doesn't happen again.
---
 airbrake/notifier.py   | 18 ++++++++++++++----
 tests/test_notifier.py | 21 +++++++++++++++++++--
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/airbrake/notifier.py b/airbrake/notifier.py
index 891a33f..77f4623 100644
--- a/airbrake/notifier.py
+++ b/airbrake/notifier.py
@@ -186,14 +186,24 @@ def log(self, exc_info=None, message=None, filename=None,
                    'notifier': self.notifier,
                    'environment': environment,
                    'session': session}
-        return self.notify(json.dumps(payload, cls=utils.FailProofJSONEncoder))
+        return self.notify(payload)
 
     def notify(self, payload):
-        """Post the current errors payload body to airbrake.io."""
+        """Post the current errors payload body to airbrake.io.
+
+        :param dict payload:
+            Notification payload, in a dict/object form. The notification
+            payload will ultimately be sent as a JSON-encoded string, but here
+            it still needs to be in object form.
+        """
         headers = {'Content-Type': 'application/json'}
         api_key = {'key': self.api_key}
-        response = requests.post(self.api_url, data=payload,
-                                 headers=headers, params=api_key)
+        response = requests.post(
+            self.api_url,
+            data=json.dumps(payload, cls=utils.FailProofJSONEncoder,
+                            sort_keys=True),
+            headers=headers,
+            params=api_key)
         response.raise_for_status()
         return response
 
diff --git a/tests/test_notifier.py b/tests/test_notifier.py
index 2506d76..160faeb 100644
--- a/tests/test_notifier.py
+++ b/tests/test_notifier.py
@@ -1,7 +1,6 @@
 import airbrake
 import mock
 import unittest
-import json
 
 from airbrake.notifier import Airbrake
 
@@ -10,7 +9,6 @@ class TestAirbrakeNotifier(unittest.TestCase):
 
     def _create_notify(test, exception, session={}, environment={}, **params):
         def notify(self, payload):
-            payload = json.loads(payload)
             test.assertEqual(session, payload['session'])
             test.assertEqual(environment, payload['environment'])
             test.assertEqual(str(exception), payload['errors'][0]['message'])
@@ -67,5 +65,24 @@ def __repr__(self):
             extra = {'very': 'important', 'jsonify': non_serializable}
             self.logger.exception(Exception(msg), extra=extra)
 
+    def test_notify_payload(self):
+        # 1d7b191a2cc3b53f76f12d09f68c857183859e08 broke the `notify interface
+        # by making the assumption that the payload sent to `notify` is already
+        # encoded as a JSON string.
+        # This test ensures that such a regression can't happen again.
+        with mock.patch('requests.post') as requests_post:
+            ab = Airbrake(project_id=1234, api_key='fake', environment='test')
+            payload = dict(foo=1, bar=2)
+            ab.notify(payload)
+
+            expected_call_args = mock.call(
+                'https://airbrake.io/api/v3/projects/1234/notices',
+                data='{"bar": 2, "foo": 1}',
+                headers={'Content-Type': 'application/json'},
+                params={'key': 'fake'}
+            )
+            self.assertEqual(expected_call_args, requests_post.call_args)
+
+
 if __name__ == '__main__':
     unittest.main()