From 440b08129a0c6fec5bb8eb1436a21490c17e9e35 Mon Sep 17 00:00:00 2001 From: Ankit Agrawal Date: Wed, 2 Mar 2016 11:45:21 +0000 Subject: [PATCH] Fix image member apis Fixed following image member APIs to accept required image_id attribute. 1. members() 2. get_member() 3. find_member() 4. delete_member() 5. create_member() 6. update_member() Updated these methods to accept the required parameter image and pass it to the respective base class methods in the required format. Closes-Bug: 1555955 Change-Id: I4bee69957cc080f04b91cc4dbc299ce4abfda630 --- openstack/image/v2/_proxy.py | 68 ++++++++++++--- openstack/tests/unit/image/v2/test_proxy.py | 96 +++++++++++++++++++-- openstack/tests/unit/test_proxy_base.py | 43 +++++---- 3 files changed, 172 insertions(+), 35 deletions(-) diff --git a/openstack/image/v2/_proxy.py b/openstack/image/v2/_proxy.py index db4d51f1f..22789b6c7 100644 --- a/openstack/image/v2/_proxy.py +++ b/openstack/image/v2/_proxy.py @@ -14,6 +14,7 @@ from openstack.image.v2 import image as _image from openstack.image.v2 import member as _member from openstack.image.v2 import tag as _tag from openstack import proxy +from openstack import resource class Proxy(proxy.BaseProxy): @@ -102,9 +103,12 @@ class Proxy(proxy.BaseProxy): """ return self._update(_image.Image, image, **attrs) - def create_member(self, **attrs): + def create_member(self, image, **attrs): """Create a new member from attributes + :param image: The value can be the ID of a image or a + :class:`~openstack.image.v2.image.Image` instance + that the member will be created for. :param dict attrs: Keyword arguments which will be used to create a :class:`~openstack.image.v2.member.Member`, comprised of the properties on the Member class. @@ -112,13 +116,19 @@ class Proxy(proxy.BaseProxy): :returns: The results of member creation :rtype: :class:`~openstack.image.v2.member.Member` """ - return self._create(_member.Member, **attrs) + image_id = resource.Resource.get_id(image) + return self._create(_member.Member, + path_args={'image_id': image_id}, **attrs) - def delete_member(self, member, ignore_missing=True): + def delete_member(self, member, image=None, ignore_missing=True): """Delete a member :param member: The value can be either the ID of a member or a :class:`~openstack.image.v2.member.Member` instance. + :param image: This is the image that the member belongs to, + this parameter need to be specified when member ID is + given as value. The value can be the ID of a image or a + :class:`~openstack.image.v2.image.Image` instance. :param bool ignore_missing: When set to ``False`` :class:`~openstack.exceptions.ResourceNotFound` will be raised when the member does not exist. @@ -127,12 +137,21 @@ class Proxy(proxy.BaseProxy): :returns: ``None`` """ - self._delete(_member.Member, member, ignore_missing=ignore_missing) + if isinstance(member, _member.Member): + image_id = member.image_id + else: + image_id = resource.Resource.get_id(image) + self._delete(_member.Member, member, + path_args={'image_id': image_id}, + ignore_missing=ignore_missing) - def find_member(self, name_or_id, ignore_missing=True): + def find_member(self, name_or_id, image, ignore_missing=True): """Find a single member :param name_or_id: The name or ID of a member. + :param image: This is the image that the member belongs to, + the value can be the ID of a image or a + :class:`~openstack.image.v2.image.Image` instance. :param bool ignore_missing: When set to ``False`` :class:`~openstack.exceptions.ResourceNotFound` will be raised when the resource does not exist. @@ -140,44 +159,69 @@ class Proxy(proxy.BaseProxy): attempting to find a nonexistent resource. :returns: One :class:`~openstack.image.v2.member.Member` or None """ + image_id = resource.Resource.get_id(image) return self._find(_member.Member, name_or_id, + path_args={'image_id': image_id}, ignore_missing=ignore_missing) - def get_member(self, member): + def get_member(self, member, image=None): """Get a single member :param member: The value can be the ID of a member or a :class:`~openstack.image.v2.member.Member` instance. - + :param image: This is the image that the member belongs to, + this parameter need to be specified when member ID is + given as value. The value can be the ID of a image or a + :class:`~openstack.image.v2.image.Image` instance. :returns: One :class:`~openstack.image.v2.member.Member` :raises: :class:`~openstack.exceptions.ResourceNotFound` when no resource can be found. """ - return self._get(_member.Member, member) + if isinstance(member, _member.Member): + image_id = member.image_id + else: + image_id = resource.Resource.get_id(image) + return self._get(_member.Member, member, + path_args={'image_id': image_id}) - def members(self, **query): + def members(self, image, **query): """Return a generator of members + :param image: This is the image that the member belongs to, + the value can be the ID of a image or a + :class:`~openstack.image.v2.image.Image` instance. :param kwargs \*\*query: Optional query parameters to be sent to limit the resources being returned. :returns: A generator of member objects :rtype: :class:`~openstack.image.v2.member.Member` """ - return self._list(_member.Member, paginated=False, **query) + image_id = resource.Resource.get_id(image) + return self._list(_member.Member, paginated=False, + path_args={'image_id': image_id}, + **query) - def update_member(self, member, **attrs): + def update_member(self, member, image=None, **attrs): """Update a member :param member: Either the ID of a member or a :class:`~openstack.image.v2.member.Member` instance. + :param image: This is the image that the member belongs to, + this parameter need to be specified when member ID is + given as value. The value can be the ID of a image or a + :class:`~openstack.image.v2.image.Image` instance. :attrs kwargs: The attributes to update on the member represented by ``value``. :returns: The updated member :rtype: :class:`~openstack.image.v2.member.Member` """ - return self._update(_member.Member, member, **attrs) + if isinstance(member, _member.Member): + image_id = member.image_id + else: + image_id = resource.Resource.get_id(image) + return self._update(_member.Member, member, + path_args={'image_id': image_id}, **attrs) def create_tag(self, **attrs): """Create a new tag from attributes diff --git a/openstack/tests/unit/image/v2/test_proxy.py b/openstack/tests/unit/image/v2/test_proxy.py index cf61857da..780079020 100644 --- a/openstack/tests/unit/image/v2/test_proxy.py +++ b/openstack/tests/unit/image/v2/test_proxy.py @@ -44,22 +44,106 @@ class TestImageProxy(test_proxy_base.TestProxyBase): self.verify_list(self.proxy.images, image.Image, paginated=True) def test_member_create_attrs(self): - self.verify_create(self.proxy.create_member, member.Member) + self.verify_create(self.proxy.create_member, member.Member, + method_kwargs={'image': 'image_1'}, + expected_kwargs={'path_args': { + 'image_id': 'image_1'}}) + + def test_member_create_attrs_with_image_instance(self): + image_1 = image.Image.from_id('image_1') + self.verify_create(self.proxy.create_member, member.Member, + method_kwargs={'image': image_1}, + expected_kwargs={'path_args': { + 'image_id': 'image_1'}}) def test_member_delete(self): - self.verify_delete(self.proxy.delete_member, member.Member, False) + self.verify_delete(self.proxy.delete_member, member.Member, False, + input_path_args=['resource_or_id', 'image_1'], + expected_path_args={'image_id': 'image_1'}) def test_member_delete_ignore(self): - self.verify_delete(self.proxy.delete_member, member.Member, True) + self.verify_delete(self.proxy.delete_member, member.Member, True, + input_path_args=['resource_or_id', 'image_1'], + expected_path_args={'image_id': 'image_1'}) + + def test_member_delete_with_image_instance(self): + image_1 = image.Image.from_id('image_1') + self.verify_delete(self.proxy.delete_member, member.Member, True, + input_path_args=['resource_or_id', image_1], + expected_path_args={'image_id': 'image_1'}) + + def test_member_delete_with_member_instance(self): + member_1 = member.Member.from_id('member_1') + member_1.image_id = 'image_1' + self._verify2('openstack.proxy.BaseProxy._delete', + self.proxy.delete_member, + method_args=[member_1], + expected_args=[member.Member, member_1], + expected_kwargs={'path_args': { + 'image_id': 'image_1'}, + 'ignore_missing': True}) def test_member_update(self): - self.verify_update(self.proxy.update_member, member.Member) + self.verify_update(self.proxy.update_member, member.Member, + ['resource_or_id', 'image_1'], + path_args={'image_id': 'image_1'}) + + def test_member_update_with_image_instance(self): + image_1 = image.Image.from_id('image_1') + self.verify_update(self.proxy.update_member, member.Member, + ['resource_or_id', image_1], + path_args={'image_id': 'image_1'}) + + def test_member_update_with_member_instance(self): + member_1 = member.Member.from_id('member_1') + member_1.image_id = 'image_1' + self.verify_update(self.proxy.update_member, member.Member, + [member_1], path_args={'image_id': 'image_1'}, + expected_args=[member_1]) def test_member_get(self): - self.verify_get(self.proxy.get_member, member.Member) + self.verify_get(self.proxy.get_member, member.Member, + ['member_1', 'image_1'], + expected_args=[member.Member, 'member_1'], + expected_kwargs={'path_args': {'image_id': 'image_1'}}) + + def test_member_get_with_image_instance(self): + image_1 = image.Image.from_id('image_1') + self.verify_get(self.proxy.get_member, member.Member, + ['member_1', image_1], + expected_args=[member.Member, 'member_1'], + expected_kwargs={'path_args': {'image_id': 'image_1'}}) + + def test_member_get_with_member_instance(self): + member_1 = member.Member.from_id('member_1') + member_1.image_id = 'image_1' + self.verify_get(self.proxy.get_member, member.Member, + [member_1], expected_args=[member.Member, member_1], + expected_kwargs={'path_args': {'image_id': 'image_1'}}) + + def test_member_find(self): + self.verify_find(self.proxy.find_member, member.Member, + ['name_or_id', 'image_1'], + path_args={'image_id': 'image_1'}) + + def test_member_find_with_image_instance(self): + image_1 = image.Image.from_id('image_1') + self.verify_find(self.proxy.find_member, member.Member, + ['name_or_id', image_1], + path_args={'image_id': 'image_1'}) def test_members(self): - self.verify_list(self.proxy.members, member.Member, paginated=False) + self.verify_list(self.proxy.members, member.Member, paginated=False, + method_args=('image_1',), + expected_kwargs={ + 'path_args': {'image_id': 'image_1'}}) + + def test_members_with_image_instance(self): + image_1 = image.Image.from_id('image_1') + self.verify_list(self.proxy.members, member.Member, paginated=False, + method_args=(image_1,), + expected_kwargs={ + 'path_args': {'image_id': 'image_1'}}) def test_tag_create_attrs(self): self.verify_create(self.proxy.create_tag, tag.Tag) diff --git a/openstack/tests/unit/test_proxy_base.py b/openstack/tests/unit/test_proxy_base.py index 51d946e88..348422d82 100644 --- a/openstack/tests/unit/test_proxy_base.py +++ b/openstack/tests/unit/test_proxy_base.py @@ -21,10 +21,11 @@ class TestProxyBase(base.TestCase): self.session = mock.Mock() def _add_path_args_for_verify(self, path_args, method_args, - expected_kwargs): + expected_kwargs, value=None): if path_args is not None: - for key in path_args: - method_args.append(path_args[key]) + if value is None: + for key in path_args: + method_args.append(path_args[key]) expected_kwargs['path_args'] = path_args def _verify(self, mock_method, test_method, @@ -98,15 +99,18 @@ class TestProxyBase(base.TestCase): def verify_delete(self, test_method, resource_type, ignore, input_path_args=None, expected_path_args=None, mock_method="openstack.proxy.BaseProxy._delete"): + method_args = ["resource_or_id"] method_kwargs = {"ignore_missing": ignore} if isinstance(input_path_args, dict): for key in input_path_args: method_kwargs[key] = input_path_args[key] + elif isinstance(input_path_args, list): + method_args = input_path_args expected_kwargs = {"ignore_missing": ignore} if expected_path_args: expected_kwargs["path_args"] = expected_path_args self._verify2(mock_method, test_method, - method_args=["resource_or_id"], + method_args=method_args, method_kwargs=method_kwargs, expected_args=[resource_type, "resource_or_id"], expected_kwargs=expected_kwargs) @@ -117,16 +121,19 @@ class TestProxyBase(base.TestCase): the_value = value if value is None: the_value = [] if ignore_value else ["value"] + expected_args = kwargs.pop("expected_args", []) 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 + if not expected_args: + expected_args = [resource_type] + the_value self._verify2(mock_method, test_method, method_args=the_value, method_kwargs=method_kwargs or {}, - expected_args=[resource_type] + the_value, + expected_args=expected_args, expected_kwargs=expected_kwargs) def verify_head(self, test_method, resource_type, @@ -140,12 +147,14 @@ class TestProxyBase(base.TestCase): expected_args=[resource_type] + the_value, expected_kwargs=expected_kwargs) - def verify_find(self, test_method, resource_type, path_args=None, - mock_method="openstack.proxy.BaseProxy._find", **kwargs): - method_args = ["name_or_id"] + def verify_find(self, test_method, resource_type, value=None, + mock_method="openstack.proxy.BaseProxy._find", + path_args=None, **kwargs): + method_args = value or ["name_or_id"] expected_kwargs = {} - self._add_path_args_for_verify(path_args, method_args, expected_kwargs) + self._add_path_args_for_verify(path_args, method_args, expected_kwargs, + value=value) # TODO(briancurtin): if sub-tests worked in this mess of # test dependencies, the following would be a lot easier to work with. @@ -189,22 +198,22 @@ class TestProxyBase(base.TestCase): expected_kwargs={"paginated": paginated}, expected_result=["result"]) - def verify_update(self, test_method, resource_type, + def verify_update(self, test_method, resource_type, value=None, mock_method="openstack.proxy.BaseProxy._update", expected_result="result", path_args=None, **kwargs): - the_kwargs = {"x": 1, "y": 2, "z": 3} - method_args = ["resource_or_id"] - method_kwargs = the_kwargs.copy() - expected_args = [resource_type, "resource_or_id"] - expected_kwargs = the_kwargs.copy() + method_args = value or ["resource_or_id"] + method_kwargs = {"x": 1, "y": 2, "z": 3} + expected_args = kwargs.pop("expected_args", ["resource_or_id"]) + expected_kwargs = method_kwargs.copy() - self._add_path_args_for_verify(path_args, method_args, expected_kwargs) + self._add_path_args_for_verify(path_args, method_args, expected_kwargs, + value=value) self._verify2(mock_method, test_method, expected_result=expected_result, method_args=method_args, method_kwargs=method_kwargs, - expected_args=expected_args, + expected_args=[resource_type] + expected_args, expected_kwargs=expected_kwargs, **kwargs)