From 6a7e7cce4dd82df96ce5141c3c690d82e15bbb93 Mon Sep 17 00:00:00 2001 From: Brian Curtin Date: Tue, 24 Mar 2015 10:21:43 -0500 Subject: [PATCH] Proxy delete method This change introduces a common delete API that can be applied to every proxy method that revolves around deleting a resource. In particular, the change applies that base method to deleting a server. The method takes either a resource or an ID and calls delete on the corresponding resource, after obtaining the canonical ID for that value out of Resource.get_id. This value is checked before making the request to see if we can determine it's of the wrong resource type by the _check_resource decorator. Additionally, in the case of a 404, by default this is ignored, but if ignore_missing is set to False, we handle a 404 and raise exceptions.ResourceNotFound as a more descriptive exception. Change-Id: I755c57e9bd9cec597a5bcf84c527e7bd6d710fb3 --- openstack/compute/v2/_proxy.py | 16 ++++- openstack/proxy.py | 41 +++++++++++++ openstack/tests/unit/compute/v2/test_proxy.py | 16 ++++- openstack/tests/unit/test_proxy.py | 61 +++++++++++++++++++ openstack/tests/unit/test_proxy_base.py | 3 + 5 files changed, 133 insertions(+), 4 deletions(-) diff --git a/openstack/compute/v2/_proxy.py b/openstack/compute/v2/_proxy.py index 49f92b8b..422982bb 100644 --- a/openstack/compute/v2/_proxy.py +++ b/openstack/compute/v2/_proxy.py @@ -110,8 +110,20 @@ class Proxy(proxy.BaseProxy): def create_server(self, **data): return server.Server(data).create(self.session) - def delete_server(self, **data): - server.Server(data).delete(self.session) + def delete_server(self, value, ignore_missing=True): + """Delete a server + + :param value: The value can be either the ID of a server or a + :class:`~openstack.compute.v2.server.Server` instance. + :param bool ignore_missing: When set to ``False`` + :class:`~openstack.exceptions.ResourceNotFound` will be + raised when the server does not exist. + When set to ``True``, no exception will be set when + attempting to delete a nonexistent server. + + :returns: ``None`` + """ + self._delete(server.Server, value, ignore_missing) def find_server(self, name_or_id): return server.Server.find(self.session, name_or_id) diff --git a/openstack/proxy.py b/openstack/proxy.py index 11f3f054..eea47113 100644 --- a/openstack/proxy.py +++ b/openstack/proxy.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +from openstack import exceptions from openstack import resource @@ -41,3 +42,43 @@ class BaseProxy(object): def __init__(self, session): self.session = session + + @_check_resource(strict=False) + def _delete(self, resource_type, value, ignore_missing=True): + """Delete a resource + + :param resource_type: The type of resource to delete. This should + be a :class:`~openstack.resource.Resource` + subclass with a ``from_id`` method. + :param value: The value to delete. Can be either the ID of a + resource or a :class:`~openstack.resource.Resource` + subclass. + :param bool ignore_missing: When set to ``False`` + :class:`~openstack.exceptions.ResourceNotFound` will be + raised when the resource does not exist. + When set to ``True``, no exception will be set when + attempting to delete a nonexistent server. + + :returns: The result of the ``delete`` + :raises: ``ValueError`` if ``value`` is a + :class:`~openstack.resource.Resource` that doesn't match + the ``resource_type``. + :class:`~openstack.exceptions.ResourceNotFound` when + ignore_missing if ``False`` and a nonexistent resource + is attempted to be deleted. + + """ + res = resource_type.existing(id=resource.Resource.get_id(value)) + + try: + rv = res.delete(self.session) + except exceptions.NotFoundException as exc: + if ignore_missing: + return None + else: + # Reraise with a more specific type and message + raise exceptions.ResourceNotFound( + "No %s found for %s" % (resource_type.__name__, value), + details=exc.details, status_code=exc.status_code) + + return rv diff --git a/openstack/tests/unit/compute/v2/test_proxy.py b/openstack/tests/unit/compute/v2/test_proxy.py index 22936c59..dafcc215 100644 --- a/openstack/tests/unit/compute/v2/test_proxy.py +++ b/openstack/tests/unit/compute/v2/test_proxy.py @@ -11,6 +11,7 @@ # under the License. from openstack.compute.v2 import _proxy +from openstack.compute.v2 import server from openstack.tests.unit import test_proxy_base @@ -153,8 +154,19 @@ class TestComputeProxy(test_proxy_base.TestProxyBase): self.proxy.create_server) def test_server_delete(self): - self.verify_delete('openstack.compute.v2.server.Server.delete', - self.proxy.delete_server) + self.verify_delete2('openstack.proxy.BaseProxy._delete', + self.proxy.delete_server, + method_args=["resource_or_id"], + expected_args=[server.Server, "resource_or_id", + True]) + + def test_server_delete_ignore(self): + self.verify_delete2('openstack.proxy.BaseProxy._delete', + self.proxy.delete_server, + method_args=["resource_or_id"], + method_kwargs={"ignore_missing": False}, + expected_args=[server.Server, + "resource_or_id", False]) def test_server_find(self): self.verify_find('openstack.compute.v2.server.Server.find', diff --git a/openstack/tests/unit/test_proxy.py b/openstack/tests/unit/test_proxy.py index d3138eb5..ea1d81ed 100644 --- a/openstack/tests/unit/test_proxy.py +++ b/openstack/tests/unit/test_proxy.py @@ -13,10 +13,15 @@ import mock import testtools +from openstack import exceptions from openstack import proxy from openstack import resource +class DeleteableResource(resource.Resource): + allow_delete = True + + class Test_check_resource(testtools.TestCase): def setUp(self): @@ -66,3 +71,59 @@ class Test_check_resource(testtools.TestCase): self.assertRaisesRegexp(ValueError, "Expected OneType but received AnotherType", decorated, self.sot, OneType, value) + + +class TestProxyDelete(testtools.TestCase): + + def setUp(self): + super(TestProxyDelete, self).setUp() + + self.session = mock.Mock() + + self.fake_id = 1 + self.res = mock.Mock(spec=DeleteableResource) + self.res.id = self.fake_id + self.res.delete = mock.Mock() + + self.sot = proxy.BaseProxy(self.session) + DeleteableResource.existing = mock.Mock(return_value=self.res) + + def test_delete(self): + self.sot._delete(DeleteableResource, self.res) + DeleteableResource.existing.assert_called_with(id=self.res.id) + self.res.delete.assert_called_with(self.session) + + self.sot._delete(DeleteableResource, self.fake_id) + DeleteableResource.existing.assert_called_with(id=self.fake_id) + self.res.delete.assert_called_with(self.session) + + # Delete generally doesn't return anything, so we will normally + # swallow any return from within a service's proxy, but make sure + # we can still return for any cases where values are returned. + self.res.delete.return_value = self.fake_id + rv = self.sot._delete(DeleteableResource, self.fake_id) + self.assertEqual(rv, self.fake_id) + + def test_delete_ignore_missing(self): + self.res.delete.side_effect = exceptions.NotFoundException( + message="test", status_code=404) + + rv = self.sot._delete(DeleteableResource, self.fake_id) + self.assertIsNone(rv) + + def test_delete_ResourceNotFound(self): + self.res.delete.side_effect = exceptions.NotFoundException( + message="test", status_code=404) + + self.assertRaisesRegexp( + exceptions.ResourceNotFound, + "No %s found for %s" % (DeleteableResource.__name__, self.res), + self.sot._delete, DeleteableResource, self.res, + ignore_missing=False) + + def test_delete_HttpException(self): + self.res.delete.side_effect = exceptions.HttpException( + message="test", status_code=500) + + self.assertRaises(exceptions.HttpException, self.sot._delete, + DeleteableResource, self.res, ignore_missing=False) diff --git a/openstack/tests/unit/test_proxy_base.py b/openstack/tests/unit/test_proxy_base.py index e57a92c9..c7d43e7e 100644 --- a/openstack/tests/unit/test_proxy_base.py +++ b/openstack/tests/unit/test_proxy_base.py @@ -76,6 +76,9 @@ class TestProxyBase(base.TestCase): def verify_delete(self, mock_method, test_method, **kwargs): self._verify(mock_method, test_method, **kwargs) + def verify_delete2(self, mock_method, test_method, **kwargs): + self._verify2(mock_method, test_method, **kwargs) + def verify_get(self, mock_method, test_method, **kwargs): self._verify(mock_method, test_method, expected_result="result", **kwargs)