From b2e9caee38ca66147552a8f677468becf812e16e Mon Sep 17 00:00:00 2001 From: David Stanek Date: Wed, 15 Jun 2016 12:51:04 +0000 Subject: [PATCH] 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 Partially Implements: blueprint return-request-id-to-caller Change-Id: Ibefaa484158ff08bfcacc1e2802d87fc26fd76a5 --- doc/source/using-api-v3.rst | 25 +++ keystoneclient/base.py | 122 ++++++++++---- keystoneclient/httpclient.py | 5 + keystoneclient/tests/unit/test_base.py | 221 +++++++++++++++++++++++++ 4 files changed, 343 insertions(+), 30 deletions(-) diff --git a/doc/source/using-api-v3.rst b/doc/source/using-api-v3.rst index f0c82c5f2..4f305e81f 100644 --- a/doc/source/using-api-v3.rst +++ b/doc/source/using-api-v3.rst @@ -102,6 +102,31 @@ For more information on Sessions refer to: `Using Sessions`_. .. _`Using Sessions`: using-sessions.html +Getting Metadata Responses +========================== + +Instantiating :py:class:`keystoneclient.v3.client.Client` using +`include_metadata=True` will cause manager response to return +:py:class:`keystoneclient.base.Response` instead of just the data. +The metadata property will be available directly to the +:py:class:`keystoneclient.base.Response` and the response data will +be available as property `data` to it. + + >>> from keystoneauth1.identity import v3 + >>> from keystoneauth1 import session + >>> from keystoneclient.v3 import client + >>> auth = v3.Password(auth_url='https://my.keystone.com:5000/v3', + ... user_id='myuserid', + ... password='mypassword', + ... project_id='myprojectid') + >>> sess = session.Session(auth=auth) + >>> keystone = client.Client(session=sess, include_metadata=True) + >>> resp = keystone.projects.list() + >>> resp.request_ids[0] + req-1234-5678-... + >>> resp.data + [, , ...] + Non-Session Authentication (deprecated) ======================================= diff --git a/keystoneclient/base.py b/keystoneclient/base.py index c466b1b9c..5824e6de2 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 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 - return found + 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 _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 50d393ad0..8d157ce9e 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 0a0fde1c3..bd5deb7c5 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?')