From 04236214acc99cc5576a928c55e2eea9c13ab35a Mon Sep 17 00:00:00 2001 From: tengqm Date: Tue, 29 Dec 2015 22:39:17 -0500 Subject: [PATCH] Allow resource get to carry query string This patch augments the resource and proxy class to do resource get with optional queries, i.e. an 'args' parameter. We need this for some parameterized resource retrieval. For example: GET /?with_details=True Change-Id: I2b89c1b16b31db92845640bfc22f485b4f3ca00c --- openstack/cluster/v1/_proxy.py | 8 +++++--- openstack/compute/v2/limits.py | 4 +++- openstack/compute/v2/server_metadata.py | 2 +- openstack/object_store/v1/obj.py | 2 +- openstack/proxy.py | 7 +++++-- openstack/resource.py | 14 ++++++++++---- openstack/tests/unit/cluster/v1/test_proxy.py | 8 +++++++- openstack/tests/unit/test_proxy.py | 10 ++++++++-- openstack/tests/unit/test_proxy_base.py | 11 ++++++++--- 9 files changed, 48 insertions(+), 18 deletions(-) diff --git a/openstack/cluster/v1/_proxy.py b/openstack/cluster/v1/_proxy.py index 0651ece7..747ecd72 100644 --- a/openstack/cluster/v1/_proxy.py +++ b/openstack/cluster/v1/_proxy.py @@ -436,17 +436,19 @@ class Proxy(proxy.BaseProxy): return self._find(_node.Node, name_or_id, ignore_missing=ignore_missing) - def get_node(self, node): + def get_node(self, node, args=None): """Get a single node. - :param value: The value can be the name or ID of a node or a + :param node: The value can be the name or ID of a node or a :class:`~openstack.cluster.v1.node.Node` instance. + :param args: An optional argument that will be translated into query + strings when retrieving the node. :returns: One :class:`~openstack.cluster.v1.node.Node` :raises: :class:`~openstack.exceptions.ResourceNotFound` when no node matching the name or ID could be found. """ - return self._get(_node.Node, node) + return self._get(_node.Node, node, args=args) def nodes(self, **query): """Retrieve a generator of nodes. diff --git a/openstack/compute/v2/limits.py b/openstack/compute/v2/limits.py index 8578cc4d..a0f1610b 100644 --- a/openstack/compute/v2/limits.py +++ b/openstack/compute/v2/limits.py @@ -76,11 +76,13 @@ class Limits(resource.Resource): absolute = resource.prop("absolute", type=AbsoluteLimits) rate = resource.prop("rate", type=list) - def get(self, session, include_headers=False): + def get(self, session, args=None, include_headers=False): """Get the Limits resource. :param session: The session to use for making this request. :type session: :class:`~openstack.session.Session` + :param dict args: An optional dict that will be translated into query + strings for retrieving the object when specified. :returns: A Limits instance :rtype: :class:`~openstack.compute.v2.limits.Limits` diff --git a/openstack/compute/v2/server_metadata.py b/openstack/compute/v2/server_metadata.py index bc75a5c1..c243e18c 100644 --- a/openstack/compute/v2/server_metadata.py +++ b/openstack/compute/v2/server_metadata.py @@ -42,7 +42,7 @@ class ServerMetadata(resource.Resource): return attrs @classmethod - def get_data_by_id(cls, session, resource_id, path_args=None, + def get_data_by_id(cls, session, resource_id, path_args=None, args=None, include_headers=False): url = cls._get_url(path_args) resp = session.get(url, endpoint_filter=cls.service) diff --git a/openstack/object_store/v1/obj.py b/openstack/object_store/v1/obj.py index 635edbd2..1de2a7fa 100644 --- a/openstack/object_store/v1/obj.py +++ b/openstack/object_store/v1/obj.py @@ -146,7 +146,7 @@ class Object(resource.Resource): #: value in the X-Delete-At metadata item. delete_after = resource.header("x-delete-after", type=int) - def get(self, session): + def get(self, session, args=None): url = self._get_url(self, self.id) # TODO(thowe): Add filter header support bug #1488269 headers = {'Accept': 'bytes'} diff --git a/openstack/proxy.py b/openstack/proxy.py index 5527145b..e56d3020 100644 --- a/openstack/proxy.py +++ b/openstack/proxy.py @@ -177,7 +177,7 @@ class BaseProxy(object): return res.create(self.session) @_check_resource(strict=False) - def _get(self, resource_type, value=None, path_args=None): + def _get(self, resource_type, value=None, path_args=None, args=None): """Get a resource :param resource_type: The type of resource to get. @@ -187,13 +187,16 @@ class BaseProxy(object): subclass. :param path_args: A dict containing arguments for forming the request URL, if needed. + :param args: A optional dict containing arguments that will be + translated into query strings when forming the request URL. + :returns: The result of the ``get`` :rtype: :class:`~openstack.resource.Resource` """ res = self._get_resource(resource_type, value, path_args) try: - return res.get(self.session) + return res.get(self.session, args=args) except ksa_exc.NotFound as exc: raise exceptions.ResourceNotFound( "No %s found for %s" % (resource_type.__name__, value), diff --git a/openstack/resource.py b/openstack/resource.py index cf249d3c..867c7435 100644 --- a/openstack/resource.py +++ b/openstack/resource.py @@ -37,6 +37,7 @@ import time from keystoneauth1 import exceptions as ksa_exc import six +from six.moves.urllib import parse as url_parse from openstack import exceptions from openstack import utils @@ -570,7 +571,7 @@ class Resource(collections.MutableMapping): return self @classmethod - def get_data_by_id(cls, session, resource_id, path_args=None, + def get_data_by_id(cls, session, resource_id, path_args=None, args=None, include_headers=False): """Get the attributes of a remote resource from an id. @@ -581,6 +582,8 @@ class Resource(collections.MutableMapping): :param dict path_args: A dictionary of arguments to construct a compound URL. See `How path_args are used`_ for details. + :param dict args: A dictionary of query parameters to be appended to + the compound URL. :param bool include_headers: ``True`` if header data should be included in the response body, ``False`` if not. @@ -593,6 +596,8 @@ class Resource(collections.MutableMapping): raise exceptions.MethodNotSupported(cls, 'retrieve') url = cls._get_url(path_args, resource_id) + if args: + url = '?'.join([url, url_parse.urlencode(args)]) response = session.get(url, endpoint_filter=cls.service) body = response.json() @@ -629,7 +634,7 @@ class Resource(collections.MutableMapping): include_headers=include_headers) return cls.existing(**body) - def get(self, session, include_headers=False): + def get(self, session, include_headers=False, args=None): """Get the remote resource associated with this instance. :param session: The session to use for making this request. @@ -637,12 +642,13 @@ class Resource(collections.MutableMapping): :param bool include_headers: ``True`` if header data should be included in the response body, ``False`` if not. - + :param dict args: A dictionary of query parameters to be appended to + the compound URL. :return: This :class:`Resource` instance. :raises: :exc:`~openstack.exceptions.MethodNotSupported` if :data:`Resource.allow_retrieve` is not set to ``True``. """ - body = self.get_data_by_id(session, self.id, path_args=self, + body = self.get_data_by_id(session, self.id, path_args=self, args=args, include_headers=include_headers) self._attrs.update(body) self._loaded = True diff --git a/openstack/tests/unit/cluster/v1/test_proxy.py b/openstack/tests/unit/cluster/v1/test_proxy.py index 89d7e8d0..88cf7bda 100644 --- a/openstack/tests/unit/cluster/v1/test_proxy.py +++ b/openstack/tests/unit/cluster/v1/test_proxy.py @@ -304,7 +304,13 @@ class TestClusterProxy(test_proxy_base.TestProxyBase): self.verify_find(self.proxy.find_node, node.Node) def test_node_get(self): - self.verify_get(self.proxy.get_node, node.Node) + self.verify_get(self.proxy.get_node, node.Node, args=None, + expected_kwargs={'args': None}) + + def test_node_get_with_args(self): + self.verify_get(self.proxy.get_node, node.Node, args={'details': True}, + method_kwargs={'args': {'details': True}}, + expected_kwargs={'args': {'details': True}}) def test_nodes(self): self.verify_list(self.proxy.nodes, node.Node, diff --git a/openstack/tests/unit/test_proxy.py b/openstack/tests/unit/test_proxy.py index deb02c11..a016c6de 100644 --- a/openstack/tests/unit/test_proxy.py +++ b/openstack/tests/unit/test_proxy.py @@ -221,14 +221,20 @@ class TestProxyGet(testtools.TestCase): def test_get_resource(self): rv = self.sot._get(RetrieveableResource, self.res) - self.res.get.assert_called_with(self.session) + self.res.get.assert_called_with(self.session, args=None) + self.assertEqual(rv, self.fake_result) + + def test_get_resource_with_args(self): + rv = self.sot._get(RetrieveableResource, self.res, args={'K': 'V'}) + + self.res.get.assert_called_with(self.session, args={'K': 'V'}) self.assertEqual(rv, self.fake_result) def test_get_id(self): rv = self.sot._get(RetrieveableResource, self.fake_id) RetrieveableResource.existing.assert_called_with(id=self.fake_id) - self.res.get.assert_called_with(self.session) + self.res.get.assert_called_with(self.session, args=None) self.assertEqual(rv, self.fake_result) def test_get_not_found(self): diff --git a/openstack/tests/unit/test_proxy_base.py b/openstack/tests/unit/test_proxy_base.py index f6ddb469..30696e18 100644 --- a/openstack/tests/unit/test_proxy_base.py +++ b/openstack/tests/unit/test_proxy_base.py @@ -100,16 +100,21 @@ class TestProxyBase(base.TestCase): expected_args=[resource_type, "resource_or_id"], expected_kwargs=expected_kwargs) - def verify_get(self, test_method, resource_type, value=None, + def verify_get(self, test_method, resource_type, value=None, args=None, mock_method="openstack.proxy.BaseProxy._get", ignore_value=False, **kwargs): the_value = value if value is None: the_value = [] if ignore_value else ["value"] - expected_kwargs = {"path_args": kwargs} if kwargs else {} + expected_kwargs = kwargs.pop("expected_kwargs", {}) + method_kwargs = kwargs.pop("method_kwargs", kwargs) + if args: + expected_kwargs["args"] = args + if kwargs: + expected_kwargs["path_args"] = kwargs self._verify2(mock_method, test_method, method_args=the_value, - method_kwargs=kwargs, + method_kwargs=method_kwargs or {}, expected_args=[resource_type] + the_value, expected_kwargs=expected_kwargs)