Less code duplication in hc.api.transports

hc/api/migrations/0023_auto_20160131_1919.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9 on 2016-01-31 19:19
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('api', '0022_auto_20160130_2042'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='notification',
+            options={'get_latest_by': 'created'},
+        ),
+    ]

hc/api/tests/test_notify.py

@@ -20,17 +20,17 @@ class NotifyTestCase(BaseTestCase):
         self.channel.save()
         self.channel.checks.add(self.check)
 
-    @patch("hc.api.transports.requests.get")
+    @patch("hc.api.transports.requests.request")
     def test_webhook(self, mock_get):
         self._setup_data("webhook", "http://example")
         mock_get.return_value.status_code = 200
 
         self.channel.notify(self.check)
         mock_get.assert_called_with(
-            u"http://example", headers={"User-Agent": "healthchecks.io"},
-            timeout=5)
+            "get",  u"http://example",
+            headers={"User-Agent": "healthchecks.io"}, timeout=5)
 
-    @patch("hc.api.transports.requests.get", side_effect=Timeout)
+    @patch("hc.api.transports.requests.request", side_effect=Timeout)
     def test_webhooks_handle_timeouts(self, mock_get):
         self._setup_data("webhook", "http://example")
         self.channel.notify(self.check)
@@ -38,7 +38,7 @@ class NotifyTestCase(BaseTestCase):
         n = Notification.objects.get()
         self.assertEqual(n.error, "Connection timed out")
 
-    @patch("hc.api.transports.requests.get", side_effect=ConnectionError)
+    @patch("hc.api.transports.requests.request", side_effect=ConnectionError)
     def test_webhooks_handle_connection_errors(self, mock_get):
         self._setup_data("webhook", "http://example")
         self.channel.notify(self.check)
@@ -46,7 +46,7 @@ class NotifyTestCase(BaseTestCase):
         n = Notification.objects.get()
         self.assertEqual(n.error, "Connection failed")
 
-    @patch("hc.api.transports.requests.get")
+    @patch("hc.api.transports.requests.request")
     def test_webhooks_ignore_up_events(self, mock_get):
         self._setup_data("webhook", "http://example", status="up")
         self.channel.notify(self.check)
@@ -54,7 +54,7 @@ class NotifyTestCase(BaseTestCase):
         self.assertFalse(mock_get.called)
         self.assertEqual(Notification.objects.count(), 0)
 
-    @patch("hc.api.transports.requests.get")
+    @patch("hc.api.transports.requests.request")
     def test_webhooks_handle_500(self, mock_get):
         self._setup_data("webhook", "http://example")
         mock_get.return_value.status_code = 500
@@ -83,19 +83,19 @@ class NotifyTestCase(BaseTestCase):
         self.assertEqual(n.error, "Email not verified")
         self.assertEqual(len(mail.outbox), 0)
 
-    @patch("hc.api.transports.JsonTransport.post")
+    @patch("hc.api.transports.requests.request")
     def test_pd(self, mock_post):
         self._setup_data("pd", "123")
-        mock_post.return_value = None
+        mock_post.return_value.status_code = 200
 
         self.channel.notify(self.check)
         assert Notification.objects.count() == 1
 
         args, kwargs = mock_post.call_args
-        payload = args[1]
-        self.assertEqual(payload["event_type"], "trigger")
+        json = kwargs["json"]
+        self.assertEqual(json["event_type"], "trigger")
 
-    @patch("hc.api.transports.requests.post")
+    @patch("hc.api.transports.requests.request")
     def test_slack(self, mock_post):
         self._setup_data("slack", "123")
         mock_post.return_value.status_code = 200
@@ -109,7 +109,7 @@ class NotifyTestCase(BaseTestCase):
         fields = {f["title"]: f["value"] for f in attachment["fields"]}
         self.assertEqual(fields["Last Ping"], "Never")
 
-    @patch("hc.api.transports.requests.post")
+    @patch("hc.api.transports.requests.request")
     def test_slack_handles_500(self, mock_post):
         self._setup_data("slack", "123")
         mock_post.return_value.status_code = 500
@@ -119,7 +119,7 @@ class NotifyTestCase(BaseTestCase):
         n = Notification.objects.get()
         self.assertEqual(n.error, "Received status code 500")
 
-    @patch("hc.api.transports.requests.post", side_effect=Timeout)
+    @patch("hc.api.transports.requests.request", side_effect=Timeout)
     def test_slack_handles_timeout(self, mock_post):
         self._setup_data("slack", "123")
 
@@ -128,7 +128,7 @@ class NotifyTestCase(BaseTestCase):
         n = Notification.objects.get()
         self.assertEqual(n.error, "Connection timed out")
 
-    @patch("hc.api.transports.requests.post")
+    @patch("hc.api.transports.requests.request")
     def test_hipchat(self, mock_post):
         self._setup_data("hipchat", "123")
         mock_post.return_value.status_code = 204
@@ -141,7 +141,7 @@ class NotifyTestCase(BaseTestCase):
         json = kwargs["json"]
         self.assertIn("DOWN", json["message"])
 
-    @patch("hc.api.transports.requests.post")
+    @patch("hc.api.transports.requests.request")
     def test_pushover(self, mock_post):
         self._setup_data("po", "123|0")
         mock_post.return_value.status_code = 200

hc/api/transports.py

@@ -53,20 +53,14 @@ class Email(Transport):
         emails.alert(self.channel.value, ctx)
 
 
-class Webhook(Transport):
-    def notify(self, check):
-        # Webhook integration only fires when check goes down.
-        if check.status != "down":
-            return "no-op"
-
-        # Webhook transport sends no arguments, so the
-        # notify and test actions are the same
-        return self.test()
+class HttpTransport(Transport):
 
-    def test(self):
-        headers = {"User-Agent": "healthchecks.io"}
+    def request(self, method, url, **kwargs):
         try:
-            r = requests.get(self.channel.value, timeout=5, headers=headers)
+            options = dict(kwargs)
+            options["timeout"] = 5
+            options["headers"] = {"User-Agent": "healthchecks.io"}
+            r = requests.request(method, url, **options)
             if r.status_code not in (200, 201, 204):
                 return "Received status code %d" % r.status_code
         except requests.exceptions.Timeout:
@@ -75,29 +69,36 @@ class Webhook(Transport):
         except requests.exceptions.ConnectionError:
             return "Connection failed"
 
+    def get(self, url):
+        return self.request("get", url)
 
-class JsonTransport(Transport):
-    def post(self, url, payload):
-        headers = {"User-Agent": "healthchecks.io"}
-        try:
-            r = requests.post(url, json=payload, timeout=5, headers=headers)
-            if r.status_code not in (200, 201, 204):
-                return "Received status code %d" % r.status_code
-        except requests.exceptions.Timeout:
-            # Well, we tried
-            return "Connection timed out"
-        except requests.exceptions.ConnectionError:
-            return "Connection failed"
+    def post(self, url, json):
+        return self.request("post", url, json=json)
+
+    def post_form(self, url, data):
+        return self.request("post", url, data=data)
+
+
+class Webhook(HttpTransport):
+    def notify(self, check):
+        # Webhook integration only fires when check goes down.
+        if check.status != "down":
+            return "no-op"
 
+        return self.get(self.channel.value)
 
-class Slack(JsonTransport):
+    def test(self):
+        return self.get(self.channel.value)
+
+
+class Slack(HttpTransport):
     def notify(self, check):
         text = tmpl("slack_message.json", check=check)
         payload = json.loads(text)
         return self.post(self.channel.value, payload)
 
 
-class HipChat(JsonTransport):
+class HipChat(HttpTransport):
     def notify(self, check):
         text = tmpl("hipchat_message.html", check=check)
         payload = {
@@ -107,7 +108,7 @@ class HipChat(JsonTransport):
         return self.post(self.channel.value, payload)
 
 
-class PagerDuty(JsonTransport):
+class PagerDuty(HttpTransport):
     URL = "https://events.pagerduty.com/generic/2010-04-15/create_event.json"
 
     def notify(self, check):
@@ -124,21 +125,9 @@ class PagerDuty(JsonTransport):
         return self.post(self.URL, payload)
 
 
-class Pushover(Transport):
+class Pushover(HttpTransport):
     URL = "https://api.pushover.net/1/messages.json"
 
-    def post(self, url, payload):
-        headers = {"User-Agent": "healthchecks.io"}
-        try:
-            r = requests.post(url, data=payload, timeout=5, headers=headers)
-            if r.status_code not in (200, 201, 204):
-                return "Received status code %d" % r.status_code
-        except requests.exceptions.Timeout:
-            # Well, we tried
-            return "Connection timed out"
-        except requests.exceptions.ConnectionError:
-            return "Connection failed"
-
     def notify(self, check):
         others = self.checks().filter(status="down").exclude(code=check.code)
         ctx = {
@@ -162,4 +151,4 @@ class Pushover(Transport):
             payload["retry"] = settings.PUSHOVER_EMERGENCY_RETRY_DELAY
             payload["expire"] = settings.PUSHOVER_EMERGENCY_EXPIRATION
 
-        return self.post(self.URL, payload)
+        return self.post_form(self.URL, payload)