Merge pull request #88 from 3lnc/client_refactor

Client refactor

Author: thinkingserious

.travis.yml

@@ -15,6 +15,7 @@ matrix:
 fail_fast: true
 before_install:
 - pip install pycodestyle
+- if [[ "$TRAVIS_PYTHON_VERSION" == 2.7* ]]; then pip install mock; fi
 install:
 - pip install codecov
 - travis_retry pip install coverage

python_http_client/client.py

@@ -1,5 +1,6 @@
 """HTTP Client library"""
 import json
+
 from .exceptions import handle_error
 
 try:
@@ -62,6 +63,9 @@ def to_dict(self):
 class Client(object):
     """Quickly and easily access any REST or REST-like API."""
 
+    # These are the supported HTTP verbs
+    methods = {'delete', 'get', 'patch', 'post', 'put'}
+
     def __init__(self,
                  host,
                  request_headers=None,
@@ -88,8 +92,6 @@ def __init__(self,
         self._version = version
         # _url_path keeps track of the dynamically built url
         self._url_path = url_path or []
-        # These are the supported HTTP verbs
-        self.methods = ['delete', 'get', 'patch', 'post', 'put']
         # APPEND SLASH set
         self.append_slash = append_slash
         self.timeout = timeout
@@ -211,44 +213,51 @@ def get_version(*args, **kwargs):
         if name in self.methods:
             method = name.upper()
 
-            def http_request(*_, **kwargs):
+            def http_request(
+                    request_body=None,
+                    query_params=None,
+                    request_headers=None,
+                    timeout=None,
+                    **_):
                 """Make the API call
-                :param args: unused
+                :param timeout: HTTP request timeout. Will be propagated to
+                    urllib client
+                :type timeout: float
+                :param request_headers: HTTP headers. Will be merged into
+                    current client object state
+                :type request_headers: dict
+                :param query_params: HTTP query parameters
+                :type query_params: dict
+                :param request_body: HTTP request body
+                :type request_body: string or json-serializable object
                 :param kwargs:
-                :return: Client object
+                :return: Response object
                 """
-                if 'request_headers' in kwargs:
-                    self._update_headers(kwargs['request_headers'])
-                if 'request_body' not in kwargs:
+                if request_headers:
+                    self._update_headers(request_headers)
+
+                if request_body is None:
                     data = None
                 else:
                     # Don't serialize to a JSON formatted str
                     # if we don't have a JSON Content-Type
-                    if 'Content-Type' in self.request_headers:
-                        if self.request_headers['Content-Type'] != \
-                           'application/json':
-                            data = kwargs['request_body'].encode('utf-8')
-                        else:
-                            data = json.dumps(
-                                kwargs['request_body']).encode('utf-8')
+                    if 'Content-Type' in self.request_headers and \
+                            self.request_headers['Content-Type'] != \
+                            'application/json':
+                        data = request_body.encode('utf-8')
                     else:
-                        data = json.dumps(
-                            kwargs['request_body']).encode('utf-8')
-
-                if 'query_params' in kwargs:
-                    params = kwargs['query_params']
-                else:
-                    params = None
+                        self.request_headers.setdefault(
+                            'Content-Type', 'application/json')
+                        data = json.dumps(request_body).encode('utf-8')
 
                 opener = urllib.build_opener()
-                request = urllib.Request(self._build_url(params), data=data)
-                if self.request_headers:
-                    for key, value in self.request_headers.items():
-                        request.add_header(key, value)
-                if data and ('Content-Type' not in self.request_headers):
-                    request.add_header('Content-Type', 'application/json')
+                request = urllib.Request(
+                    self._build_url(query_params),
+                    headers=self.request_headers,
+                    data=data,
+                )
                 request.get_method = lambda: method
-                timeout = kwargs.pop('timeout', None)
+
                 return Response(
                     self._make_request(opener, request, timeout=timeout)
                 )

tests/test_unit.py

@@ -3,20 +3,21 @@
 
 from python_http_client.client import Client
 from python_http_client.exceptions import (
-    handle_error,
-    HTTPError,
-    BadRequestsError,
+    BadRequestsError, HTTPError,
     NotFoundError,
+    ServiceUnavailableError,
     UnsupportedMediaTypeError,
-    ServiceUnavailableError
+    handle_error
 )
 
 try:
     # Python 3
     import urllib.request as urllib
+    from unittest import mock
 except ImportError:
     # Python 2
     import urllib2 as urllib
+    import mock
 
 try:
     basestring
@@ -81,7 +82,7 @@ def test__init__(self):
         self.assertEqual(default_client.host, self.host)
         self.assertEqual(default_client.request_headers, {})
         self.assertIs(default_client.timeout, None)
-        methods = ['delete', 'get', 'patch', 'post', 'put']
+        methods = {'delete', 'get', 'patch', 'post', 'put'}
         self.assertEqual(default_client.methods, methods)
         self.assertIsNone(default_client._version)
         self.assertEqual(default_client._url_path, [])
@@ -94,7 +95,7 @@ def test__init__(self):
                         timeout=10)
         self.assertEqual(client.host, self.host)
         self.assertEqual(client.request_headers, request_headers)
-        methods = ['delete', 'get', 'patch', 'post', 'put']
+        methods = {'delete', 'get', 'patch', 'post', 'put'}
         self.assertEqual(client.methods, methods)
         self.assertEqual(client._version, 3)
         self.assertEqual(client._url_path, [])
@@ -120,6 +121,19 @@ def test__build_url(self):
         built_url = self.client._build_url(query_params)
         self.assertEqual(built_url, url)
 
+    @mock.patch('python_http_client.client.Client._make_request')
+    def test__urllib_headers(self, maker):
+        self.client._update_headers({'X-test': 'Test'})
+        self.client.get()
+        request = maker.call_args[0][1]
+        self.assertIn('X-test', request.headers)
+
+    @mock.patch('python_http_client.client.Client._make_request')
+    def test__urllib_method(self, maker):
+        self.client.delete()
+        request = maker.call_args[0][1]
+        self.assertEqual(request.get_method(), 'DELETE')
+
     def test__update_headers(self):
         request_headers = {'X-Test': 'Test'}
         self.client._update_headers(request_headers)
@@ -148,7 +162,7 @@ def test__getattr__(self):
         self.assertEqual(client._version, 3)
 
         # Test GET
-        mock_client._url_path + ['test']
+        mock_client._url_path += ['test']
         r = mock_client.get()
         self.assertEqual(r.status_code, 200)