fix: ensure hook added if client is re-set

Author: ricellis

ibmcloudant/__init__.py

@@ -19,7 +19,7 @@
 from ibm_cloud_sdk_core import IAMTokenManager, DetailedResponse, BaseService, ApiException, get_authenticator
 from .couchdb_session_authenticator import CouchDbSessionAuthenticator
 from .couchdb_session_get_authenticator_patch import new_construct_authenticator
-from .cloudant_base_service import new_init, new_prepare_request, new_set_service_url, new_set_default_headers
+from .cloudant_base_service import new_init, new_prepare_request, new_set_default_headers, new_set_http_client, new_set_service_url
 from .couchdb_session_token_manager import CouchDbSessionTokenManager
 from .cloudant_v1 import CloudantV1
 from .features.changes_follower import ChangesFollower
@@ -34,3 +34,5 @@
 CloudantV1.set_default_headers = new_set_default_headers
 
 CloudantV1.prepare_request = new_prepare_request
+
+CloudantV1.set_http_client = new_set_http_client

ibmcloudant/cloudant_base_service.py

@@ -25,7 +25,7 @@
 from io import BytesIO
 
 from ibm_cloud_sdk_core.authenticators import Authenticator
-from requests import Response
+from requests import Response, Session
 from requests.cookies import RequestsCookieJar
 
 from .common import get_sdk_headers
@@ -72,12 +72,10 @@ def __hash__(self):
         # Since Py3.6 dict is ordered so use a key only dict for our set
         rules_by_operation.setdefault(operation_id, dict()).setdefault(rule)
 
-
-old_init = CloudantV1.__init__
-
+_old_init = CloudantV1.__init__
 
 def new_init(self, authenticator: Authenticator = None):
-    old_init(self, authenticator)
+    _old_init(self, authenticator)
     # Overwrite default read timeout to 2.5 minutes
     if not ('timeout' in self.http_config):
         new_http_config = self.http_config.copy()
@@ -88,27 +86,22 @@ def new_init(self, authenticator: Authenticator = None):
         # Replacing BaseService's http.cookiejar.CookieJar as RequestsCookieJar supports update(CookieJar)
         self.jar = RequestsCookieJar(self.jar)
         self.authenticator.set_jar(self.jar)  # Authenticators don't have access to cookie jars by default
-    response_hooks = self.get_http_client().hooks['response']
-    if _error_response_hook not in response_hooks:
-        response_hooks.append(_error_response_hook)
-
-old_set_service_url = CloudantV1.set_service_url
+    add_hooks(self)
 
+_old_set_service_url = CloudantV1.set_service_url
 
 def new_set_service_url(self, service_url: str):
-    old_set_service_url(self, service_url)
+    _old_set_service_url(self, service_url)
     try:
         if isinstance(self.authenticator, CouchDbSessionAuthenticator):
             self.authenticator.token_manager.set_service_url(service_url)
     except AttributeError:
         pass  # in case no authenticator is configured yet, pass
 
-
-old_set_default_headers = CloudantV1.set_default_headers
-
+_old_set_default_headers = CloudantV1.set_default_headers
 
 def new_set_default_headers(self, headers: Dict[str, str]):
-    old_set_default_headers(self, headers)
+    _old_set_default_headers(self, headers)
     if isinstance(self.authenticator, CouchDbSessionAuthenticator):
         combined_headers = {}
         combined_headers.update(headers)
@@ -119,12 +112,13 @@ def new_set_default_headers(self, headers: Dict[str, str]):
         )
         self.authenticator.token_manager.set_default_headers(combined_headers)
 
+_old_set_disable_ssl_verification = CloudantV1.set_disable_ssl_verification
 
-old_set_disable_ssl_verification = CloudantV1.set_disable_ssl_verification
-
-
+# Note this is currently unused, but probably should be enabled.
+# To enable it we need to resolve whether CouchDbSessionAuthenticator
+# should ever be allowed to have a different value from the service client.
 def new_set_disable_ssl_verification(self, status: bool = False) -> None:
-    old_set_disable_ssl_verification(self, status)
+    _old_set_disable_ssl_verification(self, status)
     if isinstance(self.authenticator, CouchDbSessionAuthenticator):
         self.authenticator.token_manager.set_disable_ssl_verification(status)
 
@@ -193,7 +187,7 @@ def _error_response_hook(response:Response, *args, **kwargs) -> Optional[Respons
                 pass
     return response
 
-old_prepare_request = CloudantV1.prepare_request
+_old_prepare_request = CloudantV1.prepare_request
 
 def new_prepare_request(self,
                         method: str,
@@ -229,4 +223,15 @@ def new_prepare_request(self,
                 if segment_to_validate.startswith('_'):
                     raise ValueError('{0} {1} starts with the invalid _ character.'.format(rule.error_parameter_name,
                         unquote(segment_to_validate)))
-    return old_prepare_request(self, method, url, *args, headers=headers, params=params, data=data, files=files, **kwargs)
+    return _old_prepare_request(self, method, url, *args, headers=headers, params=params, data=data, files=files, **kwargs)
+
+def add_hooks(self):
+    response_hooks = self.get_http_client().hooks['response']
+    if _error_response_hook not in response_hooks:
+        response_hooks.append(_error_response_hook)
+
+_old_set_http_client = CloudantV1.set_http_client
+
+def new_set_http_client(self, http_client: Session) -> None:
+    _old_set_http_client(self, http_client)
+    add_hooks(self)

test/unit/test_cloudant_base_error_augment.py

@@ -167,7 +167,22 @@ def _response_callback(cls, resp):
         return _error_response_hook(resp)
 
     def test_response_hook_added(self):
-         self.assertEqual(_error_response_hook, self._service.get_http_client().hooks['response'][0])
+        self.assertEqual(_error_response_hook, self._service.get_http_client().hooks['response'][0])
+
+    def test_response_hook_not_double_added_via_set_http_client(self):
+        client = self._service.get_http_client()
+        self._service.set_http_client(client)
+        response_hooks =  self._service.get_http_client().hooks['response']
+        self.assertEqual(1, len(response_hooks))
+        self.assertEqual(_error_response_hook, response_hooks[0])
+
+    def test_response_hook_added_via_set_http_client(self):
+        client = self._service.get_http_client()
+        client.hooks['response'].clear()
+        self._service.set_http_client(client)
+        response_hooks =  self._service.get_http_client().hooks['response']
+        self.assertEqual(1, len(response_hooks))
+        self.assertEqual(_error_response_hook, response_hooks[0])
 
     def test_augment_error(self):
         self._run_test(