Merge "Fix misnamed dictionary key."

4 files changed, 328 insertions, 72 deletions

diff --git a/swiftclient/service.py b/swiftclient/service.py
index 326a041..55e6daa 100644
--- a/swiftclient/service.py
+++ b/swiftclient/service.py
@@ -1530,8 +1530,8 @@ class SwiftService(object):
             old_manifest = None
             old_slo_manifest_paths = []
             new_slo_manifest_paths = set()
-            if options['changed'] or options['skip_identical'] \
-                    or not options['leave_segments']:
+            if (options['changed'] or options['skip_identical']
+                    or not options['leave_segments']):
                 checksum = None
                 if options['skip_identical']:
                     try:
@@ -1556,11 +1556,12 @@ class SwiftService(object):
                                 'status': 'skipped-identical'
                             })
                             return res
+
                     cl = int(headers.get('content-length'))
                     mt = headers.get('x-object-meta-mtime')
-                    if path is not None and options['changed']\
-                            and cl == getsize(path) and \
-                            mt == put_headers['x-object-meta-mtime']:
+                    if (path is not None and options['changed']
+                            and cl == getsize(path)
+                            and mt == put_headers['x-object-meta-mtime']):
                         res.update({
                             'success': True,
                             'status': 'skipped-changed'
@@ -1594,8 +1595,8 @@ class SwiftService(object):
             # a segment job if we're reading from a stream - we may fail if we
             # go over the single object limit, but this gives us a nice way
             # to create objects from memory
-            if path is not None and options['segment_size'] and \
-                    getsize(path) > int(options['segment_size']):
+            if (path is not None and options['segment_size']
+                    and getsize(path) > int(options['segment_size'])):
                 res['large_object'] = True
                 seg_container = container + '_segments'
                 if options['segment_container']:
@@ -1851,9 +1852,8 @@ class SwiftService(object):
 
                             # Cancel the remaining container deletes, but yield
                             # any pending results
-                            if not cancelled and \
-                                    options['fail_fast'] and \
-                                    not res['success']:
+                            if (not cancelled and options['fail_fast']
+                                    and not res['success']):
                                 cancelled = True
 
     @staticmethod
@@ -1861,24 +1861,17 @@ class SwiftService(object):
         results_dict = {}
         try:
             conn.delete_object(container, obj, response_dict=results_dict)
-            res = {
-                'action': 'delete_segment',
-                'container': container,
-                'object': obj,
-                'success': True,
-                'attempts': conn.attempts,
-                'response_dict': results_dict
-            }
+            res = {'success': True}
         except Exception as e:
-            res = {
-                'action': 'delete_segment',
-                'container': container,
-                'object': obj,
-                'success': False,
-                'attempts': conn.attempts,
-                'response_dict': results_dict,
-                'exception': e
-            }
+            res = {'success': False, 'error': e}
+
+        res.update({
+            'action': 'delete_segment',
+            'container': container,
+            'object': obj,
+            'attempts': conn.attempts,
+            'response_dict': results_dict
+        })
 
         if results_queue is not None:
             results_queue.put(res)
@@ -1899,8 +1892,7 @@ class SwiftService(object):
                 try:
                     headers = conn.head_object(container, obj)
                     old_manifest = headers.get('x-object-manifest')
-                    if config_true_value(
-                            headers.get('x-static-large-object')):
+                    if config_true_value(headers.get('x-static-large-object')):
                         query_string = 'multipart-manifest=delete'
                 except ClientException as err:
                     if err.http_status != 404:
@@ -1958,23 +1950,17 @@ class SwiftService(object):
         results_dict = {}
         try:
             conn.delete_container(container, response_dict=results_dict)
-            res = {
-                'action': 'delete_container',
-                'container': container,
-                'object': None,
-                'success': True,
-                'attempts': conn.attempts,
-                'response_dict': results_dict
-            }
+            res = {'success': True}
         except Exception as e:
-            res = {
-                'action': 'delete_container',
-                'container': container,
-                'object': None,
-                'success': False,
-                'response_dict': results_dict,
-                'error': e
-            }
+            res = {'success': False, 'error': e}
+
+        res.update({
+            'action': 'delete_container',
+            'container': container,
+            'object': None,
+            'attempts': conn.attempts,
+            'response_dict': results_dict
+        })
         return res
 
     def _delete_container(self, container, options):
@@ -1982,9 +1968,7 @@ class SwiftService(object):
             objs = []
             for part in self.list(container=container):
                 if part["success"]:
-                    objs.extend([
-                        o['name'] for o in part['listing']
-                    ])
+                    objs.extend([o['name'] for o in part['listing']])
                 else:
                     raise part["error"]
 
diff --git a/swiftclient/shell.py b/swiftclient/shell.py
index 2c627ad..8d4da67 100755
--- a/swiftclient/shell.py
+++ b/swiftclient/shell.py
@@ -118,34 +118,29 @@ def st_delete(parser, args, output_manager):
                     del_iter = swift.delete(container=container)
 
             for r in del_iter:
+                c = r.get('container', '')
+                o = r.get('object', '')
+                a = r.get('attempts')
+
                 if r['success']:
                     if options.verbose:
+                        a = ' [after {0} attempts]'.format(a) if a > 1 else ''
+
                         if r['action'] == 'delete_object':
-                            c = r['container']
-                            o = r['object']
-                            p = '%s/%s' % (c, o) if options.yes_all else o
-                            a = r['attempts']
-                            if a > 1:
-                                output_manager.print_msg(
-                                    '%s [after %d attempts]', p, a)
+                            if options.yes_all:
+                                p = '{0}/{1}'.format(c, o)
                             else:
-                                output_manager.print_msg(p)
-
+                                p = o
                         elif r['action'] == 'delete_segment':
-                            c = r['container']
-                            o = r['object']
-                            p = '%s/%s' % (c, o)
-                            a = r['attempts']
-                            if a > 1:
-                                output_manager.print_msg(
-                                    '%s [after %d attempts]', p, a)
-                            else:
-                                output_manager.print_msg(p)
+                            p = '{0}/{1}'.format(c, o)
+                        elif r['action'] == 'delete_container':
+                            p = c
 
+                        output_manager.print_msg('{0}{1}'.format(p, a))
                 else:
-                    # Special case error prints
-                    output_manager.error("An unexpected error occurred whilst "
-                                         "deleting: %s" % r['error'])
+                    p = '{0}/{1}'.format(c, o) if o else c
+                    output_manager.error('Error Deleting: {0}: {1}'
+                                         .format(p, r['error']))
         except SwiftError as err:
             output_manager.error(err.value)
 
diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py
index 5211456..e10c065 100644
--- a/tests/unit/test_service.py
+++ b/tests/unit/test_service.py
@@ -12,12 +12,15 @@
 # implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
+import mock
 import testtools
+from mock import Mock, PropertyMock
+from six.moves.queue import Queue, Empty as QueueEmptyError
 from hashlib import md5
 
-from swiftclient.service import SwiftService, SwiftError
 import swiftclient
+from swiftclient.service import SwiftService, SwiftError
+from swiftclient.client import Connection
 
 
 class TestSwiftPostObject(testtools.TestCase):
@@ -59,7 +62,7 @@ class TestSwiftReader(testtools.TestCase):
         self.assertTrue(isinstance(sr._actual_md5, self.md5_type))
 
     def test_create_with_large_object_headers(self):
-        # md5 should not be initalized if large object headers are present
+        # md5 should not be initialized if large object headers are present
         sr = self.sr('path', 'body', {'x-object-manifest': 'test'})
         self.assertEqual(sr._path, 'path')
         self.assertEqual(sr._body, 'body')
@@ -129,6 +132,225 @@ class TestSwiftReader(testtools.TestCase):
                          '97ac82a5b825239e782d0339e2d7b910')
 
 
+class TestServiceDelete(testtools.TestCase):
+    def setUp(self):
+        super(TestServiceDelete, self).setUp()
+        self.opts = {'leave_segments': False, 'yes_all': False}
+        self.exc = Exception('test_exc')
+        # Base response to be copied and updated to matched the expected
+        # response for each test
+        self.expected = {
+            'action': None,   # Should be string in the form delete_XX
+            'container': 'test_c',
+            'object': 'test_o',
+            'attempts': 2,
+            'response_dict': {},
+            'success': None   # Should be a bool
+        }
+
+    def _get_mock_connection(self, attempts=2):
+        m = Mock(spec=Connection)
+        type(m).attempts = PropertyMock(return_value=attempts)
+        return m
+
+    def _get_queue(self, q):
+        # Instead of blocking pull items straight from the queue.
+        # expects at least one item otherwise the test will fail.
+        try:
+            return q.get_nowait()
+        except QueueEmptyError:
+            self.fail('Expected item in queue but found none')
+
+    def _get_expected(self, update=None):
+        expected = self.expected.copy()
+        if update:
+            expected.update(update)
+
+        return expected
+
+    def _assertDictEqual(self, a, b, m=None):
+        # assertDictEqual is not available in py2.6 so use a shallow check
+        # instead
+        if hasattr(self, 'assertDictEqual'):
+            self.assertDictEqual(a, b, m)
+        else:
+            self.assertTrue(isinstance(a, dict))
+            self.assertTrue(isinstance(b, dict))
+            self.assertEqual(len(a), len(b), m)
+            for k, v in a.items():
+                self.assertTrue(k in b, m)
+                self.assertEqual(b[k], v, m)
+
+    def test_delete_segment(self):
+        mock_q = Queue()
+        mock_conn = self._get_mock_connection()
+        expected_r = self._get_expected({
+            'action': 'delete_segment',
+            'object': 'test_s',
+            'success': True,
+        })
+
+        r = SwiftService._delete_segment(mock_conn, 'test_c', 'test_s', mock_q)
+
+        mock_conn.delete_object.assert_called_once_with(
+            'test_c', 'test_s', response_dict={}
+        )
+        self._assertDictEqual(expected_r, r)
+        self._assertDictEqual(expected_r, self._get_queue(mock_q))
+
+    def test_delete_segment_exception(self):
+        mock_q = Queue()
+        mock_conn = self._get_mock_connection()
+        mock_conn.delete_object = Mock(side_effect=self.exc)
+        expected_r = self._get_expected({
+            'action': 'delete_segment',
+            'object': 'test_s',
+            'success': False,
+            'error': self.exc
+        })
+
+        r = SwiftService._delete_segment(mock_conn, 'test_c', 'test_s', mock_q)
+
+        mock_conn.delete_object.assert_called_once_with(
+            'test_c', 'test_s', response_dict={}
+        )
+        self._assertDictEqual(expected_r, r)
+        self._assertDictEqual(expected_r, self._get_queue(mock_q))
+
+    def test_delete_object(self):
+        mock_q = Queue()
+        mock_conn = self._get_mock_connection()
+        mock_conn.head_object = Mock(return_value={})
+        expected_r = self._get_expected({
+            'action': 'delete_object',
+            'success': True
+        })
+
+        s = SwiftService()
+        r = s._delete_object(mock_conn, 'test_c', 'test_o', self.opts, mock_q)
+
+        mock_conn.head_object.assert_called_once_with('test_c', 'test_o')
+        mock_conn.delete_object.assert_called_once_with(
+            'test_c', 'test_o', query_string=None, response_dict={}
+        )
+        self._assertDictEqual(expected_r, r)
+
+    def test_delete_object_exception(self):
+        mock_q = Queue()
+        mock_conn = self._get_mock_connection()
+        mock_conn.delete_object = Mock(side_effect=self.exc)
+        expected_r = self._get_expected({
+            'action': 'delete_object',
+            'success': False,
+            'error': self.exc
+        })
+        # _delete_object doesnt populate attempts or response dict if it hits
+        # an error. This may not be the correct behaviour.
+        del expected_r['response_dict'], expected_r['attempts']
+
+        s = SwiftService()
+        r = s._delete_object(mock_conn, 'test_c', 'test_o', self.opts, mock_q)
+
+        mock_conn.head_object.assert_called_once_with('test_c', 'test_o')
+        mock_conn.delete_object.assert_called_once_with(
+            'test_c', 'test_o', query_string=None, response_dict={}
+        )
+        self._assertDictEqual(expected_r, r)
+
+    def test_delete_object_slo_support(self):
+        # If SLO headers are present the delete call should include an
+        # additional query string to cause the right delete server side
+        mock_q = Queue()
+        mock_conn = self._get_mock_connection()
+        mock_conn.head_object = Mock(
+            return_value={'x-static-large-object': True}
+        )
+        expected_r = self._get_expected({
+            'action': 'delete_object',
+            'success': True
+        })
+
+        s = SwiftService()
+        r = s._delete_object(mock_conn, 'test_c', 'test_o', self.opts, mock_q)
+
+        mock_conn.head_object.assert_called_once_with('test_c', 'test_o')
+        mock_conn.delete_object.assert_called_once_with(
+            'test_c', 'test_o',
+            query_string='multipart-manifest=delete',
+            response_dict={}
+        )
+        self._assertDictEqual(expected_r, r)
+
+    def test_delete_object_dlo_support(self):
+        mock_q = Queue()
+        s = SwiftService()
+        mock_conn = self._get_mock_connection()
+        expected_r = self._get_expected({
+            'action': 'delete_object',
+            'success': True,
+            'dlo_segments_deleted': True
+        })
+        # A DLO object is determined in _delete_object by heading the object
+        # and checking for the existence of a x-object-manifest header.
+        # Mock that here.
+        mock_conn.head_object = Mock(
+            return_value={'x-object-manifest': 'manifest_c/manifest_p'}
+        )
+        mock_conn.get_container = Mock(
+            side_effect=[(None, [{'name': 'test_seg_1'},
+                                 {'name': 'test_seg_2'}]),
+                         (None, {})]
+        )
+
+        def get_mock_list_conn(options):
+            return mock_conn
+
+        with mock.patch('swiftclient.service.get_conn', get_mock_list_conn):
+            r = s._delete_object(
+                mock_conn, 'test_c', 'test_o', self.opts, mock_q
+            )
+
+        self._assertDictEqual(expected_r, r)
+        expected = [
+            mock.call('test_c', 'test_o', query_string=None, response_dict={}),
+            mock.call('manifest_c', 'test_seg_1', response_dict={}),
+            mock.call('manifest_c', 'test_seg_2', response_dict={})]
+        mock_conn.delete_object.assert_has_calls(expected, any_order=True)
+
+    def test_delete_empty_container(self):
+        mock_conn = self._get_mock_connection()
+        expected_r = self._get_expected({
+            'action': 'delete_container',
+            'success': True,
+            'object': None
+        })
+
+        r = SwiftService._delete_empty_container(mock_conn, 'test_c')
+
+        mock_conn.delete_container.assert_called_once_with(
+            'test_c', response_dict={}
+        )
+        self._assertDictEqual(expected_r, r)
+
+    def test_delete_empty_container_excpetion(self):
+        mock_conn = self._get_mock_connection()
+        mock_conn.delete_container = Mock(side_effect=self.exc)
+        expected_r = self._get_expected({
+            'action': 'delete_container',
+            'success': False,
+            'object': None,
+            'error': self.exc
+        })
+
+        s = SwiftService()
+        r = s._delete_empty_container(mock_conn, 'test_c')
+
+        mock_conn.delete_container.assert_called_once_with(
+            'test_c', response_dict={}
+        )
+        self._assertDictEqual(expected_r, r)
+
+
 class TestService(testtools.TestCase):
 
     def test_upload_with_bad_segment_size(self):
diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py
index 9d44248..617fba2 100644
--- a/tests/unit/test_shell.py
+++ b/tests/unit/test_shell.py
@@ -387,6 +387,61 @@ class TestShell(unittest.TestCase):
         connection.return_value.delete_object.assert_called_with(
             'container', 'object', query_string=None, response_dict={})
 
+    def test_delete_verbose_output(self):
+        del_obj_res = {'success': True, 'response_dict': {}, 'attempts': 2,
+                       'container': 'test_c', 'action': 'delete_object',
+                       'object': 'test_o'}
+
+        del_seg_res = del_obj_res.copy()
+        del_seg_res.update({'action': 'delete_segment'})
+
+        del_con_res = del_obj_res.copy()
+        del_con_res.update({'action': 'delete_container', 'object': None})
+
+        test_exc = Exception('test_exc')
+        error_res = del_obj_res.copy()
+        error_res.update({'success': False, 'error': test_exc, 'object': None})
+
+        mock_delete = mock.Mock()
+        base_argv = ['', '--verbose', 'delete']
+
+        with mock.patch('swiftclient.shell.SwiftService.delete', mock_delete):
+            with CaptureOutput() as out:
+                mock_delete.return_value = [del_obj_res]
+                swiftclient.shell.main(base_argv + ['test_c', 'test_o'])
+
+                mock_delete.assert_called_once_with(container='test_c',
+                                                    objects=['test_o'])
+                self.assertTrue(out.out.find(
+                    'test_o [after 2 attempts]') >= 0)
+
+            with CaptureOutput() as out:
+                mock_delete.return_value = [del_seg_res]
+                swiftclient.shell.main(base_argv + ['test_c', 'test_o'])
+
+                mock_delete.assert_called_with(container='test_c',
+                                               objects=['test_o'])
+                self.assertTrue(out.out.find(
+                    'test_c/test_o [after 2 attempts]') >= 0)
+
+            with CaptureOutput() as out:
+                mock_delete.return_value = [del_con_res]
+                swiftclient.shell.main(base_argv + ['test_c'])
+
+                mock_delete.assert_called_with(container='test_c')
+                self.assertTrue(out.out.find(
+                    'test_c [after 2 attempts]') >= 0)
+
+            with CaptureOutput() as out:
+                mock_delete.return_value = [error_res]
+                self.assertRaises(SystemExit,
+                                  swiftclient.shell.main,
+                                  base_argv + ['test_c'])
+
+                mock_delete.assert_called_with(container='test_c')
+                self.assertTrue(out.err.find(
+                    'Error Deleting: test_c: test_exc') >= 0)
+
     @mock.patch('swiftclient.service.Connection')
     def test_post_account(self, connection):
         argv = ["", "post"]