Add Response class to return request-id to caller

This change is required to return 'request_id' from client
to log request_id mappings of cross-project requests.

Instantiating class 'keystoneclient.v3.client.Client' using
'include_metadata=True' will cause manager response to return
a new 'Response' class instead of just the data. This 'Response'
class is going to have additional metadata properties available
like 'request_ids' and the original data will be available as
property 'data' to it.

This change is backward compatible since user has to set a new
parameter 'include_metadata=True' to client in order to get the
request_id returned.

Co-author: Dinesh Bhor <dinesh.bhor@nttdata.com>
Partially Implements: blueprint return-request-id-to-caller
Change-Id: Ibefaa484158ff08bfcacc1e2802d87fc26fd76a5

3 files changed, 318 insertions, 30 deletions

diff --git a/keystoneclient/base.py b/keystoneclient/base.py
index c466b1b..5824e6d 100644
--- a/keystoneclient/base.py
+++ b/keystoneclient/base.py
@@ -32,6 +32,23 @@ from keystoneclient import exceptions as ksc_exceptions
 from keystoneclient.i18n import _
 
 
+class Response(object):
+
+    def __init__(self, http_response, data):
+        self.request_ids = []
+        if isinstance(http_response, list):
+            # http_response is a list of <requests.Response> in case
+            # of pagination
+            for resp_obj in http_response:
+                # Extract 'x-openstack-request-id' from headers
+                self.request_ids.append(resp_obj.headers.get(
+                    'x-openstack-request-id'))
+        else:
+            self.request_ids.append(http_response.headers.get(
+                'x-openstack-request-id'))
+        self.data = data
+
+
 def getid(obj):
     """Return id if argument is a Resource.
 
@@ -107,6 +124,11 @@ class Manager(object):
             'may be removed in the 2.0.0 release', DeprecationWarning)
         return self.client
 
+    def _prepare_return_value(self, http_response, data):
+        if self.client.include_metadata:
+            return Response(http_response, data)
+        return data
+
     def _list(self, url, response_key, obj_class=None, body=None, **kwargs):
         """List the collection.
 
@@ -137,7 +159,8 @@ class Manager(object):
             # are already returned in a list (so simply utilize that list)
             pass
 
-        return [obj_class(self, res, loaded=True) for res in data if res]
+        return self._prepare_return_value(
+            resp, [obj_class(self, res, loaded=True) for res in data if res])
 
     def _get(self, url, response_key, **kwargs):
         """Get an object from collection.
@@ -148,7 +171,8 @@ class Manager(object):
         :param kwargs: Additional arguments will be passed to the request.
         """
         resp, body = self.client.get(url, **kwargs)
-        return self.resource_class(self, body[response_key], loaded=True)
+        return self._prepare_return_value(
+            resp, self.resource_class(self, body[response_key], loaded=True))
 
     def _head(self, url, **kwargs):
         """Retrieve request headers for an object.
@@ -157,7 +181,7 @@ class Manager(object):
         :param kwargs: Additional arguments will be passed to the request.
         """
         resp, body = self.client.head(url, **kwargs)
-        return resp.status_code == 204
+        return self._prepare_return_value(resp, resp.status_code == 204)
 
     def _post(self, url, body, response_key, return_raw=False, **kwargs):
         """Create an object.
@@ -174,7 +198,8 @@ class Manager(object):
         resp, body = self.client.post(url, body=body, **kwargs)
         if return_raw:
             return body[response_key]
-        return self.resource_class(self, body[response_key])
+        return self._prepare_return_value(
+            resp, self.resource_class(self, body[response_key]))
 
     def _put(self, url, body=None, response_key=None, **kwargs):
         """Update an object with PUT method.
@@ -190,9 +215,11 @@ class Manager(object):
         # PUT requests may not return a body
         if body is not None:
             if response_key is not None:
-                return self.resource_class(self, body[response_key])
+                return self._prepare_return_value(
+                    resp, self.resource_class(self, body[response_key]))
             else:
-                return self.resource_class(self, body)
+                return self._prepare_return_value(
+                    resp, self.resource_class(self, body))
 
     def _patch(self, url, body=None, response_key=None, **kwargs):
         """Update an object with PATCH method.
@@ -206,9 +233,11 @@ class Manager(object):
         """
         resp, body = self.client.patch(url, body=body, **kwargs)
         if response_key is not None:
-            return self.resource_class(self, body[response_key])
+            return self._prepare_return_value(
+                resp, self.resource_class(self, body[response_key]))
         else:
-            return self.resource_class(self, body)
+            return self._prepare_return_value(
+                resp, self.resource_class(self, body))
 
     def _delete(self, url, **kwargs):
         """Delete an object.
@@ -216,7 +245,8 @@ class Manager(object):
         :param url: a partial URL, e.g., '/servers/my-server'
         :param kwargs: Additional arguments will be passed to the request.
         """
-        return self.client.delete(url, **kwargs)
+        resp, body = self.client.delete(url, **kwargs)
+        return resp, self._prepare_return_value(resp, body)
 
     def _update(self, url, body=None, response_key=None, method="PUT",
                 **kwargs):
@@ -231,7 +261,10 @@ class Manager(object):
                                                  % method)
         # PUT requests may not return a body
         if body:
-            return self.resource_class(self, body[response_key])
+            return self._prepare_return_value(
+                resp, self.resource_class(self, body[response_key]))
+        else:
+            return self._prepare_return_value(resp, body)
 
 
 @six.add_metaclass(abc.ABCMeta)
@@ -249,16 +282,20 @@ class ManagerWithFind(Manager):
         the Python side.
         """
         rl = self.findall(**kwargs)
-        num = len(rl)
 
-        if num == 0:
+        if self.client.include_metadata:
+            base_response = rl
+            rl = rl.data
+            base_response.data = rl[0]
+
+        if len(rl) == 0:
             msg = _("No %(name)s matching %(kwargs)s.") % {
                 'name': self.resource_class.__name__, 'kwargs': kwargs}
             raise ksa_exceptions.NotFound(404, msg)
-        elif num > 1:
+        elif len(rl) > 1:
             raise ksc_exceptions.NoUniqueMatch
         else:
-            return rl[0]
+            return base_response if self.client.include_metadata else rl[0]
 
     def findall(self, **kwargs):
         """Find all items with attributes matching ``**kwargs``.
@@ -269,15 +306,23 @@ class ManagerWithFind(Manager):
         found = []
         searches = kwargs.items()
 
-        for obj in self.list():
-            try:
-                if all(getattr(obj, attr) == value
-                       for (attr, value) in searches):
-                    found.append(obj)
-            except AttributeError:
-                continue
+        def _extract_data(objs, response_data):
+            for obj in objs:
+                try:
+                    if all(getattr(obj, attr) == value
+                           for (attr, value) in searches):
+                        response_data.append(obj)
+                except AttributeError:
+                    continue
+            return response_data
+
+        objs = self.list()
+        if self.client.include_metadata:
+            # 'objs' is the object of 'Response' class.
+            objs.data = _extract_data(objs.data, found)
+            return objs
 
-        return found
+        return _extract_data(objs, found)
 
 
 class CrudManager(Manager):
@@ -376,6 +421,16 @@ class CrudManager(Manager):
 
     @filter_kwargs
     def list(self, fallback_to_auth=False, **kwargs):
+
+        def return_resp(resp, include_metadata=False):
+            base_response = None
+            list_data = resp
+            if include_metadata:
+                base_response = resp
+                list_data = resp.data
+                base_response.data = list_data
+            return base_response if include_metadata else list_data
+
         if 'id' in kwargs.keys():
             # Ensure that users are not trying to call things like
             # ``domains.list(id='default')`` when they should have used
@@ -392,15 +447,16 @@ class CrudManager(Manager):
         try:
             query = self._build_query(kwargs)
             url_query = '%(url)s%(query)s' % {'url': url, 'query': query}
-            return self._list(
-                url_query,
-                self.collection_key)
+            list_resp = self._list(url_query, self.collection_key)
+            return return_resp(list_resp,
+                               include_metadata=self.client.include_metadata)
         except ksa_exceptions.EmptyCatalog:
             if fallback_to_auth:
-                return self._list(
-                    url_query,
-                    self.collection_key,
-                    endpoint_filter={'interface': plugin.AUTH_INTERFACE})
+                list_resp = self._list(url_query, self.collection_key,
+                                       endpoint_filter={
+                                           'interface': plugin.AUTH_INTERFACE})
+                return return_resp(
+                    list_resp, include_metadata=self.client.include_metadata)
             else:
                 raise
 
@@ -439,6 +495,11 @@ class CrudManager(Manager):
             url_query,
             self.collection_key)
 
+        if self.client.include_metadata:
+            base_response = elements
+            elements = elements.data
+            base_response.data = elements[0]
+
         if not elements:
             msg = _("No %(name)s matching %(kwargs)s.") % {
                 'name': self.resource_class.__name__, 'kwargs': kwargs}
@@ -446,7 +507,8 @@ class CrudManager(Manager):
         elif len(elements) > 1:
             raise ksc_exceptions.NoUniqueMatch
         else:
-            return elements[0]
+            return (base_response if self.client.include_metadata
+                    else elements[0])
 
 
 class Resource(object):
diff --git a/keystoneclient/httpclient.py b/keystoneclient/httpclient.py
index 50d393a..8d157ce 100644
--- a/keystoneclient/httpclient.py
+++ b/keystoneclient/httpclient.py
@@ -389,6 +389,11 @@ class HTTPClient(baseclient.Client, base.BaseAuthPlugin):
                                          user_agent=user_agent,
                                          connect_retries=connect_retries)
 
+        # NOTE(dstanek): This allows me to not have to change keystoneauth or
+        # to write an adapter to the adapter here. Splitting thing into
+        # multiple project isn't always all sunshine and roses.
+        self._adapter.include_metadata = kwargs.pop('include_metadata', False)
+
         # keyring setup
         if use_keyring and keyring is None:
             _logger.warning('Failed to load keyring modules.')
diff --git a/keystoneclient/tests/unit/test_base.py b/keystoneclient/tests/unit/test_base.py
index 0a0fde1..bd5deb7 100644
--- a/keystoneclient/tests/unit/test_base.py
+++ b/keystoneclient/tests/unit/test_base.py
@@ -11,14 +11,29 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import uuid
+
 import fixtures
 from keystoneauth1.identity import v2
 from keystoneauth1 import session
+import requests
 
 from keystoneclient import base
+from keystoneclient import exceptions
 from keystoneclient.tests.unit import utils
+from keystoneclient import utils as base_utils
 from keystoneclient.v2_0 import client
 from keystoneclient.v2_0 import roles
+from keystoneclient.v3 import users
+
+TEST_REQUEST_ID = uuid.uuid4().hex
+TEST_REQUEST_ID_1 = uuid.uuid4().hex
+
+
+def create_response_with_request_id_header():
+    resp = requests.Response()
+    resp.headers['x-openstack-request-id'] = TEST_REQUEST_ID
+    return resp
 
 
 class HumanReadable(base.Resource):
@@ -202,3 +217,209 @@ class ManagerTest(utils.TestCase):
             management=True)
         put_mock.assert_called_once_with(self.url, management=True, body=None)
         self.assertEqual(rsrc.hi, 1)
+
+
+class ManagerRequestIdTest(utils.TestCase):
+    url = "/test-url"
+    resp = create_response_with_request_id_header()
+
+    def setUp(self):
+        super(ManagerRequestIdTest, self).setUp()
+
+        auth = v2.Token(auth_url='http://127.0.0.1:5000',
+                        token=self.TEST_TOKEN)
+        session_ = session.Session(auth=auth)
+        self.client = client.Client(session=session_,
+                                    include_metadata='True')._adapter
+
+        self.mgr = base.Manager(self.client)
+        self.mgr.resource_class = base.Resource
+
+    def mock_request_method(self, request_method, body):
+        return self.useFixture(fixtures.MockPatchObject(
+            self.client, request_method, autospec=True,
+            return_value=(self.resp, body))
+        ).mock
+
+    def test_get(self):
+        body = {"hello": {"hi": 1}}
+        get_mock = self.mock_request_method('get', body)
+        rsrc = self.mgr._get(self.url, "hello")
+        get_mock.assert_called_once_with(self.url)
+        self.assertEqual(rsrc.data.hi, 1)
+        self.assertEqual(rsrc.request_ids[0], TEST_REQUEST_ID)
+
+    def test_list(self):
+        body = {"hello": [{"name": "admin"}, {"name": "admin"}]}
+        get_mock = self.mock_request_method('get', body)
+
+        returned_list = self.mgr._list(self.url, "hello")
+        self.assertEqual(returned_list.request_ids[0], TEST_REQUEST_ID)
+        get_mock.assert_called_once_with(self.url)
+
+    def test_list_with_multiple_response_objects(self):
+        body = {"hello": [{"name": "admin"}, {"name": "admin"}]}
+        resp_1 = requests.Response()
+        resp_1.headers['x-openstack-request-id'] = TEST_REQUEST_ID
+        resp_2 = requests.Response()
+        resp_2.headers['x-openstack-request-id'] = TEST_REQUEST_ID_1
+
+        resp_result = [resp_1, resp_2]
+        get_mock = self.useFixture(fixtures.MockPatchObject(
+            self.client, 'get', autospec=True,
+            return_value=(resp_result, body))
+        ).mock
+
+        returned_list = self.mgr._list(self.url, "hello")
+        self.assertIn(returned_list.request_ids[0], [
+            TEST_REQUEST_ID, TEST_REQUEST_ID_1])
+        self.assertIn(returned_list.request_ids[1], [
+            TEST_REQUEST_ID, TEST_REQUEST_ID_1])
+        get_mock.assert_called_once_with(self.url)
+
+    def test_post(self):
+        body = {"hello": {"hi": 1}}
+        post_mock = self.mock_request_method('post', body)
+        rsrc = self.mgr._post(self.url, body, "hello")
+        post_mock.assert_called_once_with(self.url, body=body)
+        self.assertEqual(rsrc.data.hi, 1)
+
+        post_mock.reset_mock()
+
+        rsrc = self.mgr._post(self.url, body, "hello", return_raw=True)
+        post_mock.assert_called_once_with(self.url, body=body)
+        self.assertNotIsInstance(rsrc, base.Response)
+        self.assertEqual(rsrc["hi"], 1)
+
+    def test_put(self):
+        body = {"hello": {"hi": 1}}
+        put_mock = self.mock_request_method('put', body)
+        rsrc = self.mgr._put(self.url, body, "hello")
+        put_mock.assert_called_once_with(self.url, body=body)
+        self.assertEqual(rsrc.data.hi, 1)
+
+        put_mock.reset_mock()
+
+        rsrc = self.mgr._put(self.url, body)
+        put_mock.assert_called_once_with(self.url, body=body)
+        self.assertEqual(rsrc.data.hello["hi"], 1)
+        self.assertEqual(rsrc.request_ids[0], TEST_REQUEST_ID)
+
+    def test_head(self):
+        get_mock = self.mock_request_method('head', None)
+        rsrc = self.mgr._head(self.url)
+        get_mock.assert_called_once_with(self.url)
+        self.assertFalse(rsrc.data)
+        self.assertEqual(rsrc.request_ids[0], TEST_REQUEST_ID)
+
+    def test_delete(self):
+        delete_mock = self.mock_request_method('delete', None)
+        resp, base_resp = self.mgr._delete(self.url, name="hello")
+
+        delete_mock.assert_called_once_with('/test-url', name='hello')
+        self.assertEqual(base_resp.request_ids[0], TEST_REQUEST_ID)
+        self.assertEqual(base_resp.data, None)
+        self.assertTrue(isinstance(resp, requests.Response))
+
+    def test_patch(self):
+        body = {"hello": {"hi": 1}}
+        patch_mock = self.mock_request_method('patch', body)
+        rsrc = self.mgr._patch(self.url, body, "hello")
+        patch_mock.assert_called_once_with(self.url, body=body)
+        self.assertEqual(rsrc.data.hi, 1)
+
+        patch_mock.reset_mock()
+
+        rsrc = self.mgr._patch(self.url, body)
+        patch_mock.assert_called_once_with(self.url, body=body)
+        self.assertEqual(rsrc.data.hello["hi"], 1)
+        self.assertEqual(rsrc.request_ids[0], TEST_REQUEST_ID)
+
+    def test_update(self):
+        body = {"hello": {"hi": 1}}
+        patch_mock = self.mock_request_method('patch', body)
+        put_mock = self.mock_request_method('put', body)
+
+        rsrc = self.mgr._update(
+            self.url, body=body, response_key="hello", method="PATCH",
+            management=False)
+        patch_mock.assert_called_once_with(
+            self.url, management=False, body=body)
+        self.assertEqual(rsrc.data.hi, 1)
+
+        rsrc = self.mgr._update(
+            self.url, body=None, response_key="hello", method="PUT",
+            management=True)
+        put_mock.assert_called_once_with(self.url, management=True, body=None)
+        self.assertEqual(rsrc.data.hi, 1)
+        self.assertEqual(rsrc.request_ids[0], TEST_REQUEST_ID)
+
+
+class ManagerWithFindRequestIdTest(utils.TestCase):
+    url = "/fakes"
+    resp = create_response_with_request_id_header()
+
+    def setUp(self):
+        super(ManagerWithFindRequestIdTest, self).setUp()
+
+        auth = v2.Token(auth_url='http://127.0.0.1:5000',
+                        token=self.TEST_TOKEN)
+        session_ = session.Session(auth=auth)
+        self.client = client.Client(session=session_,
+                                    include_metadata='True')._adapter
+
+    def test_find_resource(self):
+        body = {"roles": [{"name": 'entity_one'}, {"name": 'entity_one_1'}]}
+        request_resp = requests.Response()
+        request_resp.headers['x-openstack-request-id'] = TEST_REQUEST_ID
+
+        get_mock = self.useFixture(fixtures.MockPatchObject(
+            self.client, 'get', autospec=True,
+            side_effect=[exceptions.NotFound, (request_resp, body)])
+        ).mock
+
+        mgr = roles.RoleManager(self.client)
+        mgr.resource_class = roles.Role
+        response = base_utils.find_resource(mgr, 'entity_one')
+        get_mock.assert_called_with('/OS-KSADM/roles')
+        self.assertEqual(response.request_ids[0], TEST_REQUEST_ID)
+
+
+class CrudManagerRequestIdTest(utils.TestCase):
+    resp = create_response_with_request_id_header()
+    request_resp = requests.Response()
+    request_resp.headers['x-openstack-request-id'] = TEST_REQUEST_ID
+
+    def setUp(self):
+        super(CrudManagerRequestIdTest, self).setUp()
+
+        auth = v2.Token(auth_url='http://127.0.0.1:5000',
+                        token=self.TEST_TOKEN)
+        session_ = session.Session(auth=auth)
+        self.client = client.Client(session=session_,
+                                    include_metadata='True')._adapter
+
+    def test_find_resource(self):
+        body = {"users": [{"name": 'entity_one'}]}
+        get_mock = self.useFixture(fixtures.MockPatchObject(
+            self.client, 'get', autospec=True,
+            side_effect=[exceptions.NotFound, (self.request_resp, body)])
+        ).mock
+        mgr = users.UserManager(self.client)
+        mgr.resource_class = users.User
+        response = base_utils.find_resource(mgr, 'entity_one')
+        get_mock.assert_called_with('/users?name=entity_one')
+        self.assertEqual(response.request_ids[0], TEST_REQUEST_ID)
+
+    def test_list(self):
+        body = {"users": [{"name": "admin"}, {"name": "admin"}]}
+
+        get_mock = self.useFixture(fixtures.MockPatchObject(
+            self.client, 'get', autospec=True,
+            return_value=(self.request_resp, body))
+        ).mock
+        mgr = users.UserManager(self.client)
+        mgr.resource_class = users.User
+        returned_list = mgr.list()
+        self.assertEqual(returned_list.request_ids[0], TEST_REQUEST_ID)
+        get_mock.assert_called_once_with('/users?')