fix(base service): Separate out request into prepare_request and send

Author: ehdsouza

ibm_cloud_sdk_core/base_service.py

@@ -25,7 +25,7 @@
 from .utils import has_bad_first_or_last_char, remove_null_values, cleanup_values
 from .detailed_response import DetailedResponse
 from .api_exception import ApiException
-from .authenticators import Authenticator, BasicAuthenticator, BearerAuthenticator, CP4DAuthenticator, IAMAuthenticator, NoAuthAuthenticator
+from .authenticators import Authenticator, BasicAuthenticator, BearerTokenAuthenticator, CloudPakForDataAuthenticator, IamAuthenticator, NoauthAuthenticator
 from http.cookiejar import CookieJar
 
 # Uncomment this to enable http debugging
@@ -88,7 +88,7 @@ def __init__(self,
                 self._set_service_properties(vcap_service_credentials)
 
         if not self.authenticator:
-            self.authenticator = NoAuthAuthenticator()
+            self.authenticator = NoauthAuthenticator()
 
     def _load_from_credential_file(self, service_name, separator='='):
         """
@@ -139,23 +139,23 @@ def _set_authenticator_properties(self, properties):
                 username=properties.get('username'),
                 password=properties.get('password'))
         elif auth_type == 'bearerToken':
-            self.authenticator = BearerAuthenticator(
+            self.authenticator = BearerTokenAuthenticator(
                 bearer_token=properties.get('bearer_token'))
         elif auth_type == 'cp4d':
-            self.authenticator = CP4DAuthenticator(
+            self.authenticator = CloudPakForDataAuthenticator(
                 username=properties.get('username'),
                 password=properties.get('password'),
                 url=properties.get('auth_url'),
                 disable_ssl_verification=properties.get('auth_disable_ssl'))
         elif auth_type == 'iam':
-            self.authenticator = IAMAuthenticator(
+            self.authenticator = IamAuthenticator(
                 apikey=properties.get('apikey'),
                 url=properties.get('auth_url'),
                 client_id=properties.get('client_id'),
                 client_secret=properties.get('client_secret'),
                 disable_ssl_verification=properties.get('auth_disable_ssl'))
         elif auth_type == 'noauth':
-            self.authenticator = NoAuthAuthenticator()
+            self.authenticator = NoauthAuthenticator()
 
     def _set_service_properties(self, properties):
         if 'url' in properties:
@@ -227,59 +227,68 @@ def set_default_headers(self, headers):
         else:
             raise TypeError("headers parameter must be a dictionary")
 
-    def request(self,
-                method,
-                url,
-                accept_json=False,
-                headers=None,
-                params=None,
-                json=None,
-                data=None,
-                files=None,
-                **kwargs):
-        full_url = self.url + url
+    def send(self, request, **kwargs):
+        # Use a one minute timeout when our caller doesn't give a timeout.
+        # http://docs.python-requests.org/en/master/user/quickstart/#timeouts
+        kwargs = dict({"timeout": 60}, **kwargs)
+        kwargs = dict(kwargs, **self.http_config)
+
+        if self.disable_ssl_verification:
+            kwargs['verify'] = False
+
+        response = requests.request(**request, cookies=self.jar, **kwargs)
+        print(response.headers)
+
+        if 200 <= response.status_code <= 299:
+            if response.status_code == 204 or request['method'] == 'HEAD':
+                # There is no body content for a HEAD request or a 204 response
+                result = None
+            elif not response.text:
+                result = None
+            else:
+                try:
+                    result = response.json()
+                except:
+                    result = response
+            return DetailedResponse(result, response.headers,
+                                    response.status_code)
+        else:
+            error_message = None
+            if response.status_code == 401:
+                error_message = 'Unauthorized: Access is denied due to ' \
+                                'invalid credentials'
+            raise ApiException(
+                response.status_code, error_message, http_response=response)
+
+
+    def prepare_request(self, method, url, headers=None,
+                        params=None, data=None, files=None, **kwargs):
+        request = {'method': method}
+
+        request['url'] = self.url + url
 
         headers = remove_null_values(headers) if headers else {}
         headers = cleanup_values(headers)
         headers = CaseInsensitiveDict(headers)
-
         if self.default_headers is not None:
             headers.update(self.default_headers)
-        if accept_json:
-            headers['accept'] = 'application/json'
-
         if not any(key in headers for key in ['user-agent', 'User-Agent']):
             headers.update(self.user_agent_header)
+        request['headers'] = headers
 
-        # Remove keys with None values
         params = remove_null_values(params)
         params = cleanup_values(params)
-        json = remove_null_values(json)
-        data = remove_null_values(data)
-        files = remove_null_values(files)
+        request['params'] = params
 
+        data = remove_null_values(data)
         if sys.version_info >= (3, 0) and isinstance(data, str):
             data = data.encode('utf-8')
+        request['data'] = data
 
-        # Support versions of requests older than 2.4.2 without the json input
-        if not data and json is not None:
-            data = json_import.dumps(json)
-            headers.update({'content-type': 'application/json'})
-
-        auth = None
-        if self.authenticator._is_bearer_authentication():
-            headers['Authorization'] = self.authenticator.authenticate()
-        elif self.authenticator._is_basic_authentication():
-            auth = self.authenticator.authenticate()
-
-        # Use a one minute timeout when our caller doesn't give a timeout.
-        # http://docs.python-requests.org/en/master/user/quickstart/#timeouts
-        kwargs = dict({"timeout": 60}, **kwargs)
-        kwargs = dict(kwargs, **self.http_config)
-
-        if self.disable_ssl_verification:
-            kwargs['verify'] = False
+        if self.authenticator:
+            self.authenticator.authenticate(request)
 
+        files = remove_null_values(files)
         if files is not None:
             for k, file_tuple in files.items():
                 if file_tuple and len(
@@ -288,41 +297,9 @@ def request(self,
                     if file and hasattr(file, 'name'):
                         filename = basename(file.name)
                         files[k] = (filename, file_tuple[1], file_tuple[2])
+        request['files'] = files
+        return request
 
-        response = requests.request(
-            method=method,
-            url=full_url,
-            cookies=self.jar,
-            auth=auth,
-            headers=headers,
-            params=params,
-            data=data,
-            files=files,
-            **kwargs)
-
-        if 200 <= response.status_code <= 299:
-            if response.status_code == 204 or method == 'HEAD':
-                # There is no body content for a HEAD request or a 204 response
-                return DetailedResponse(None, response.headers,
-                                        response.status_code)
-            if accept_json:
-                try:
-                    response_json = response.json()
-                except:
-                    # deserialization fails because there is no text
-                    return DetailedResponse(None, response.headers,
-                                            response.status_code)
-                return DetailedResponse(response_json, response.headers,
-                                        response.status_code)
-            return DetailedResponse(response, response.headers,
-                                    response.status_code)
-        else:
-            error_message = None
-            if response.status_code == 401:
-                error_message = 'Unauthorized: Access is denied due to ' \
-                                'invalid credentials'
-            raise ApiException(
-                response.status_code, error_message, http_response=response)
 
     @staticmethod
     def _convert_model(val, classname=None):

test/test_base_service.py

@@ -8,7 +8,7 @@
 from ibm_cloud_sdk_core import BaseService
 from ibm_cloud_sdk_core import ApiException
 from ibm_cloud_sdk_core import CP4DTokenManager
-from ibm_cloud_sdk_core.authenticators import IAMAuthenticator, Authenticator, BasicAuthenticator, CP4DAuthenticator
+from ibm_cloud_sdk_core.authenticators import IamAuthenticator, Authenticator, BasicAuthenticator, CloudPakForDataAuthenticator
 
 
 class AnyServiceV1(BaseService):
@@ -36,21 +36,24 @@ def op_with_path_params(self, path0, path1):
         params = {'version': self.version}
         url = '/v1/foo/{0}/bar/{1}/baz'.format(
             *self._encode_path_vars(path0, path1))
-        response = self.request(
-            method='GET', url=url, params=params, accept_json=True)
+        request = self.prepare_request(method='GET', url=url, params=params)
+        response = self.send(request)
         return response
 
     def with_http_config(self, http_config):
         self.set_http_config(http_config)
-        response = self.request(method='GET', url='', accept_json=True)
+        request = self.prepare_request(method='GET', url='', accept_json=True)
+        response = self.send(request)
         return response
 
     def any_service_call(self):
-        response = self.request(method='GET', url='', accept_json=True)
+        request = self.prepare_request(method='GET', url='', accept_json=True)
+        response = self.send(request)
         return response
 
     def head_request(self):
-        response = self.request(method='HEAD', url='', accept_json=True)
+        request = self.prepare_request(method='HEAD', url='', accept_json=True)
+        response = self.send(request)
         return response
 
 
@@ -133,14 +136,10 @@ def test_fail_http_config():
 
 @responses.activate
 def test_iam():
-    url = "https://iam.cloud.ibm.com/identity/token"
-    iam_authenticator = IAMAuthenticator('my_apikey')
+    iam_authenticator = IamAuthenticator('my_apikey', 'https://iam-test.cloud.ibm.com/identity/token')
     service = AnyServiceV1('2017-07-07', authenticator=iam_authenticator)
     assert service.authenticator is not None
 
-    iam_authenticator.set_url('https://iam-test.cloud.ibm.com/identity/token')
-    assert service.authenticator.token_manager.url == 'https://iam-test.cloud.ibm.com/identity/token'
-
     response = {
         "access_token": get_access_token(),
         "token_type": "Bearer",
@@ -178,20 +177,9 @@ def __init__(self):
     assert isinstance(service.authenticator, Authenticator)
 
 
-def test_set_username_and_password():
-    basic_authenticator = BasicAuthenticator('my_username', 'my_password')
-    service = AnyServiceV1('2017-07-07', authenticator=basic_authenticator)
-    assert service.authenticator.username == 'my_username'
-    assert service.authenticator.password == 'my_password'
-
-    basic_authenticator.set_username_and_password('hello', 'ibm')
-    assert service.authenticator.username == 'hello'
-    assert service.authenticator.password == 'ibm'
-
-
 def test_for_cp4d():
-    cp4d_authenticator = CP4DAuthenticator('my_username', 'my_password',
-                                           'my_url')
+    cp4d_authenticator = CloudPakForDataAuthenticator('my_username', 'my_password',
+                                                      'my_url')
     service = AnyServiceV1('2017-07-07', authenticator=cp4d_authenticator)
     assert service.authenticator.token_manager is not None
     assert service.authenticator.token_manager.username == 'my_username'
@@ -207,8 +195,8 @@ def test_disable_ssl_verification():
     service1.set_disable_ssl_verification(False)
     assert service1.disable_ssl_verification is False
 
-    cp4d_authenticator = CP4DAuthenticator('my_username', 'my_password',
-                                           'my_url')
+    cp4d_authenticator = CloudPakForDataAuthenticator('my_username', 'my_password',
+                                                      'my_url')
     service2 = AnyServiceV1('2017-07-07', authenticator=cp4d_authenticator)
     assert service2.disable_ssl_verification is False
     cp4d_authenticator.set_disable_ssl_verification(True)
@@ -330,7 +318,8 @@ def test_request_server_error():
         content_type='application/json')
     service = AnyServiceV1('2018-11-20')
     try:
-        service.request('GET', url='')
+        prepped = service.prepare_request('GET', url='')
+        service.send(prepped)
     except ApiException as err:
         assert err.message == 'internal server error'
 
@@ -345,13 +334,15 @@ def test_request_success_json():
         }),
         content_type='application/json')
     service = AnyServiceV1('2018-11-20')
-    detailed_response = service.request('GET', url='', accept_json=True)
+    prepped = service.prepare_request('GET', url='')
+    detailed_response = service.send(prepped)
     assert detailed_response.get_result() == {'foo': 'bar'}
 
     service = AnyServiceV1('2018-11-20', authenticator=BasicAuthenticator('my_username', 'my_password'))
     service.set_default_headers({'test': 'header'})
     service.set_disable_ssl_verification(True)
-    detailed_response = service.request('GET', url='', accept_json=True)
+    prepped = service.prepare_request('GET', url='')
+    detailed_response = service.send(prepped)
     assert detailed_response.get_result() == {'foo': 'bar'}
 
 @responses.activate
@@ -365,8 +356,9 @@ def test_request_success_response():
         }),
         content_type='application/json')
     service = AnyServiceV1('2018-11-20')
-    detailed_response = service.request('GET', url='')
-    assert detailed_response.get_result().text == '{"foo": "bar"}'
+    prepped = service.prepare_request('GET', url='')
+    detailed_response = service.send(prepped)
+    assert detailed_response.get_result() == {"foo": "bar"}
 
 @responses.activate
 def test_request_fail_401():
@@ -380,7 +372,8 @@ def test_request_fail_401():
         content_type='application/json')
     service = AnyServiceV1('2018-11-20')
     try:
-        service.request('GET', url='')
+        prepped = service.prepare_request('GET', url='')
+        service.send(prepped)
     except ApiException as err:
         assert err.message == 'Unauthorized: Access is denied due to invalid credentials'
 
@@ -448,18 +441,16 @@ def test_user_agent_header():
         responses.GET,
         'https://gateway.watsonplatform.net/test/api',
         status=200,
-        body=json.dumps({
-            'foo': 'bar'
-        }),
-        content_type='application/json')
-    response = service.request(
-        'GET', url='', headers={
-            'user-agent': 'my_user_agent'
-        })
+        body='some text')
+    prepped = service.prepare_request('GET', url='', headers={
+        'user-agent': 'my_user_agent'
+    })
+    response = service.send(prepped)
     assert response.get_result().request.headers.__getitem__(
         'user-agent') == 'my_user_agent'
 
-    response = service.request('GET', url='', headers=None)
+    prepped = service.prepare_request('GET', url='', headers=None)
+    response = service.send(prepped)
     assert response.get_result().request.headers.__getitem__(
         'user-agent') == user_agent_header['User-Agent']
 
@@ -481,4 +472,4 @@ def test_files():
             os.path.dirname(__file__), '../resources/ibm-credentials-iam.env'), 'r')
     form_data['file1'] = (None, file, 'application/octet-stream')
     form_data['string1'] = (None, 'hello', 'text.plain')
-    service.request('GET', url='', headers={'X-opt-out': True}, files=form_data)
+    service.prepare_request('GET', url='', headers={'X-opt-out': True}, files=form_data)