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

@@ -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)

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

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',

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)

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)