From 8d5abfbf56a5620d1326d44dee8ffdc9dfba1c62 Mon Sep 17 00:00:00 2001
From: David Shrewsbury <shrewsbury.dave@gmail.com>
Date: Fri, 11 Dec 2015 15:45:42 -0500
Subject: [PATCH] Bug fix: delete_object() returns True/False

Our delete APIs return True if the delete succeeded, or False if
the thing being deleted was not found. delete_object() was not doing
this, so this makes it consistent with the other delete API calls.

Also adds missing unit tests for this method.

Change-Id: I0951765193459300f08b0ab804e6ca327c6fa57d
---
 .../delete-obj-return-a3ecf0415b7a2989.yaml   |  5 +++
 shade/openstackcloud.py                       | 12 ++++++-
 shade/tests/unit/test_object.py               | 34 +++++++++++++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/delete-obj-return-a3ecf0415b7a2989.yaml

diff --git a/releasenotes/notes/delete-obj-return-a3ecf0415b7a2989.yaml b/releasenotes/notes/delete-obj-return-a3ecf0415b7a2989.yaml
new file mode 100644
index 000000000..381bcb99a
--- /dev/null
+++ b/releasenotes/notes/delete-obj-return-a3ecf0415b7a2989.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - The delete_object() method was not returning True/False,
+    similar to other delete methods. It is now consistent with
+    the other delete APIs.
diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py
index 1ffa483f2..930b7597e 100644
--- a/shade/openstackcloud.py
+++ b/shade/openstackcloud.py
@@ -3646,8 +3646,17 @@ class OpenStackCloud(object):
                     e.http_reason, e.http_host, e.http_path))
 
     def delete_object(self, container, name):
+        """Delete an object from a container.
+
+        :param string container: Name of the container holding the object.
+        :param string name: Name of the object to delete.
+
+        :returns: True if delete succeeded, False if the object was not found.
+
+        :raises: OpenStackCloudException on operation error.
+        """
         if not self.get_object_metadata(container, name):
-            return
+            return False
         try:
             self.manager.submitTask(_tasks.ObjectDelete(
                 container=container, obj=name))
@@ -3655,6 +3664,7 @@ class OpenStackCloud(object):
             raise OpenStackCloudException(
                 "Object deletion failed: %s (%s/%s)" % (
                     e.http_reason, e.http_host, e.http_path))
+        return True
 
     def get_object_metadata(self, container, name):
         try:
diff --git a/shade/tests/unit/test_object.py b/shade/tests/unit/test_object.py
index 15c72b33c..b27edb32e 100644
--- a/shade/tests/unit/test_object.py
+++ b/shade/tests/unit/test_object.py
@@ -288,3 +288,37 @@ class TestObject(base.TestCase):
             "ERROR")
         self.assertRaises(exc.OpenStackCloudException,
                           self.cloud.list_objects, 'container_name')
+
+    @mock.patch.object(shade.OpenStackCloud, 'get_object_metadata')
+    @mock.patch.object(shade.OpenStackCloud, 'swift_client')
+    def test_delete_object(self, mock_swift, mock_get_meta):
+        container_name = 'container_name'
+        object_name = 'object_name'
+        mock_get_meta.return_value = {'object': object_name}
+        self.assertTrue(self.cloud.delete_object(container_name, object_name))
+        mock_get_meta.assert_called_once_with(container_name, object_name)
+        mock_swift.delete_object.assert_called_once_with(
+            container=container_name, obj=object_name
+        )
+
+    @mock.patch.object(shade.OpenStackCloud, 'get_object_metadata')
+    @mock.patch.object(shade.OpenStackCloud, 'swift_client')
+    def test_delete_object_not_found(self, mock_swift, mock_get_meta):
+        container_name = 'container_name'
+        object_name = 'object_name'
+        mock_get_meta.return_value = None
+        self.assertFalse(self.cloud.delete_object(container_name, object_name))
+        mock_get_meta.assert_called_once_with(container_name, object_name)
+        self.assertFalse(mock_swift.delete_object.called)
+
+    @mock.patch.object(shade.OpenStackCloud, 'get_object_metadata')
+    @mock.patch.object(shade.OpenStackCloud, 'swift_client')
+    def test_delete_object_exception(self, mock_swift, mock_get_meta):
+        container_name = 'container_name'
+        object_name = 'object_name'
+        mock_get_meta.return_value = {'object': object_name}
+        mock_swift.delete_object.side_effect = swift_exc.ClientException(
+            "ERROR")
+        self.assertRaises(shade.OpenStackCloudException,
+                          self.cloud.delete_object,
+                          container_name, object_name)