Refactor how requests are made

 - Create a single abstraction for making requests
 - Escape path paramaters for security reasons

Author: jonathanlloyd

pusher_push_notifications/__init__.py

@@ -5,6 +5,7 @@
 import json
 import re
 import time
+import urllib
 import warnings
 
 import jwt
@@ -49,8 +50,7 @@ class PusherBadResponseError(PusherError, Exception):
     """
 
 
-def handle_http_error(response_body, status_code):
-    """Handle different http error codes from the Push Notifications service"""
+def _handle_http_error(response_body, status_code):
     error_string = '{}: {}'.format(
         response_body.get('error', 'Unknown error'),
         response_body.get('description', 'no description'),
@@ -65,6 +65,17 @@ def handle_http_error(response_body, status_code):
         raise PusherServerError(error_string)
 
 
+def _make_url(scheme, host, path):
+    return urllib.parse.urlunparse([
+        scheme,
+        host,
+        path,
+        None,
+        None,
+        None,
+    ])
+
+
 class PushNotifications(object):
     """Pusher Push Notifications API client
     This client class can be used to publish notifications to the Pusher
@@ -97,6 +108,46 @@ def endpoint(self):
         ).lower()
         return self._endpoint or default_endpoint
 
+    def _make_request(self, method, path, path_params, body=None):
+        path_params = {
+            name: urllib.parse.quote(value)
+            for name, value in path_params.items()
+        }
+        path = path.format(**path_params)
+        url = _make_url(scheme='https', host=self.endpoint, path=path)
+
+        session = requests.Session()
+        request = requests.Request(
+            method,
+            url,
+            json=body,
+            headers={
+                'host': self.endpoint,
+                'authorization': 'Bearer {}'.format(self.secret_key),
+                'x-pusher-library': 'pusher-push-notifications-python {}'.format(
+                    SDK_VERSION,
+                )
+            },
+        )
+
+        response = session.send(request.prepare())
+
+        if response.status_code != 200:
+            try:
+                error_body = response.json()
+            except ValueError:
+                error_body = {}
+            _handle_http_error(error_body, response.status_code)
+
+        try:
+            response_body = response.json()
+        except ValueError:
+            raise PusherBadResponseError(
+                'The server returned a malformed response',
+            )
+
+        return response_body
+
     def publish(self, interests, publish_body):
         """Publish the given publish_body to the specified interests.
 
@@ -201,39 +252,16 @@ def publish_to_interests(self, interests, publish_body):
 
         publish_body = copy.deepcopy(publish_body)
         publish_body['interests'] = interests
-        session = requests.Session()
-        request = requests.Request(
-            'POST',
-            'https://{}/publish_api/v1/instances/{}/publishes'.format(
-                self.endpoint,
-                self.instance_id,
-            ),
-            json=publish_body,
-            headers={
-                'host': self.endpoint,
-                'authorization': 'Bearer {}'.format(self.secret_key),
-                'x-pusher-library': 'pusher-push-notifications-python {}'.format(
-                    SDK_VERSION,
-                )
+
+        response_body = self._make_request(
+            method='POST',
+            path='/publish_api/v1/instances/{instance_id}/publishes/interests',
+            path_params={
+                'instance_id': self.instance_id,
             },
+            body=publish_body,
         )
 
-        response = session.send(request.prepare())
-
-        if response.status_code != 200:
-            try:
-                response_body = response.json()
-            except ValueError:
-                response_body = {}
-            handle_http_error(response_body, response.status_code)
-
-        try:
-            response_body = response.json()
-        except ValueError:
-            raise PusherBadResponseError(
-                'The server returned a malformed response',
-            )
-
         return response_body
 
     def publish_to_users(self, user_ids, publish_body):
@@ -293,39 +321,16 @@ def publish_to_users(self, user_ids, publish_body):
 
         publish_body = copy.deepcopy(publish_body)
         publish_body['users'] = user_ids
-        session = requests.Session()
-        request = requests.Request(
-            'POST',
-            'https://{}/publish_api/v1/instances/{}/publishes/users'.format(
-                self.endpoint,
-                self.instance_id,
-            ),
-            json=publish_body,
-            headers={
-                'host': self.endpoint,
-                'authorization': 'Bearer {}'.format(self.secret_key),
-                'x-pusher-library': 'pusher-push-notifications-python {}'.format(
-                    SDK_VERSION,
-                )
+
+        response_body = self._make_request(
+            method='POST',
+            path='/publish_api/v1/instances/{instance_id}/publishes/users',
+            path_params={
+                'instance_id': self.instance_id,
             },
+            body=publish_body,
         )
 
-        response = session.send(request.prepare())
-
-        if response.status_code != 200:
-            try:
-                response_body = response.json()
-            except ValueError:
-                response_body = {}
-            handle_http_error(response_body, response.status_code)
-
-        try:
-            response_body = response.json()
-        except ValueError:
-            raise PusherBadResponseError(
-                'The server returned a malformed response',
-            )
-
         return response_body
 
     def authenticate_user(self, user_id):
@@ -385,20 +390,11 @@ def delete_user(self, user_id):
         if len(user_id) > USER_ID_MAX_LENGTH:
             raise ValueError('user_id longer than the maximum of 164 chars')
 
-        session = requests.Session()
-        request = requests.Request(
-            'DELETE',
-            'https://{}/user_api/v1/instances/{}/users/{}'.format(
-                self.endpoint,
-                self.instance_id,
-                user_id,
-            ),
-            headers={
-                'host': self.endpoint,
-                'authorization': 'Bearer {}'.format(self.secret_key),
-                'x-pusher-library': 'pusher-push-notifications-python {}'.format(
-                    SDK_VERSION,
-                )
+        self._make_request(
+            method='DELETE',
+            path='/user_api/v1/instances/{instance_id}/users/{user_id}',
+            path_params={
+                'instance_id': self.instance_id,
+                'user_id': user_id,
             },
         )
-        session.send(request.prepare())

tests/test_interests.py

@@ -53,7 +53,7 @@ def test_publish_to_interests_should_make_correct_http_request(self):
         )
         self.assertEqual(
             path,
-            '/publish_api/v1/instances/instance_id/publishes',
+            '/publish_api/v1/instances/instance_id/publishes/interests',
         )
         self.assertDictEqual(
             headers,
@@ -120,7 +120,7 @@ def test_deprecated_alias_still_works(self):
         )
         self.assertEqual(
             path,
-            '/publish_api/v1/instances/instance_id/publishes',
+            '/publish_api/v1/instances/instance_id/publishes/interests',
         )
         self.assertDictEqual(
             headers,