Fix deleting SLO segments on overwrite

This patch fixes a bug that results in SLO segments being left
behind regardless of the setting of --leave-segments.

Change-Id: I2b3c8ab4817df6facb4b5961dbe570645441a552
Closes-Bug: 1418007

2 files changed, 52 insertions, 10 deletions

diff --git a/swiftclient/service.py b/swiftclient/service.py
index d7a5795..2980fd8 100644
--- a/swiftclient/service.py
+++ b/swiftclient/service.py
@@ -1590,7 +1590,7 @@ class SwiftService(object):
             # 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'])):
+                    and (getsize(path) > int(options['segment_size']))):
                 res['large_object'] = True
                 seg_container = container + '_segments'
                 if options['segment_container']:
@@ -1711,6 +1711,7 @@ class SwiftService(object):
                     )
                     res['response_dict'] = obr
             if old_manifest or old_slo_manifest_paths:
+                drs = []
                 if old_manifest:
                     scontainer, sprefix = old_manifest.split('/', 1)
                     scontainer = unquote(scontainer)
@@ -1719,26 +1720,24 @@ class SwiftService(object):
                     for delobj in conn.get_container(scontainer,
                                                      prefix=sprefix)[1]:
                         delobjs.append(delobj['name'])
-                    drs = []
                     for dr in self.delete(container=scontainer,
                                           objects=delobjs):
                         drs.append(dr)
-                    res['segment_delete_results'] = drs
                 if old_slo_manifest_paths:
                     delobjsmap = {}
                     for seg_to_delete in old_slo_manifest_paths:
                         if seg_to_delete in new_slo_manifest_paths:
                             continue
                         scont, sobj = \
-                            seg_to_delete.split('/', 1)
+                            seg_to_delete.split(b'/', 1)
                         delobjs_cont = delobjsmap.get(scont, [])
                         delobjs_cont.append(sobj)
-                        drs = []
-                        for (dscont, dsobjs) in delobjsmap.items():
-                            for dr in self.delete(container=dscont,
-                                                  objects=dsobjs):
-                                drs.append(dr)
-                        res['segment_delete_results'] = drs
+                        delobjsmap[scont] = delobjs_cont
+                    for (dscont, dsobjs) in delobjsmap.items():
+                        for dr in self.delete(container=dscont,
+                                              objects=dsobjs):
+                            drs.append(dr)
+                res['segment_delete_results'] = drs
 
             # return dict for printing
             res.update({
diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py
index 34473bf..c5a47b8 100644
--- a/tests/unit/test_shell.py
+++ b/tests/unit/test_shell.py
@@ -15,6 +15,7 @@
 from genericpath import getmtime
 
 import hashlib
+import json
 import mock
 import os
 import tempfile
@@ -455,6 +456,48 @@ class TestShell(unittest.TestCase):
                      'x-object-meta-mtime': mock.ANY},
             response_dict={})
 
+    @mock.patch('swiftclient.shell.walk')
+    @mock.patch('swiftclient.service.Connection')
+    def test_upload_delete(self, connection, walk):
+        # Upload delete existing segments
+        connection.return_value.head_container.return_value = {
+            'x-storage-policy': 'one'}
+        connection.return_value.attempts = 0
+        argv = ["", "upload", "container", self.tmpfile]
+        connection.return_value.head_object.side_effect = [
+            {'x-static-large-object': 'true',  # For the upload call
+             'content-length': '2'},
+            {'x-static-large-object': 'false',  # For the 1st delete call
+             'content-length': '2'},
+            {'x-static-large-object': 'false',  # For the 2nd delete call
+             'content-length': '2'}
+        ]
+        connection.return_value.get_object.return_value = ({}, json.dumps(
+            [{'name': 'container1/old_seg1'}, {'name': 'container2/old_seg2'}]
+        ))
+        swiftclient.shell.main(argv)
+        connection.return_value.put_object.assert_called_with(
+            'container',
+            self.tmpfile.lstrip('/'),
+            mock.ANY,
+            content_length=0,
+            headers={'x-object-meta-mtime': mock.ANY},
+            response_dict={})
+        expected_delete_calls = [
+            mock.call(
+                b'container1', b'old_seg1',
+                query_string=None, response_dict={}
+            ),
+            mock.call(
+                b'container2', b'old_seg2',
+                query_string=None, response_dict={}
+            )
+        ]
+        self.assertEqual(
+            sorted(expected_delete_calls),
+            sorted(connection.return_value.delete_object.mock_calls)
+        )
+
     @mock.patch('swiftclient.service.Connection')
     def test_upload_segments_to_same_container(self, connection):
         # Upload in segments to same container