Merge "Fix delete versioning objects when previous is expired"

swift/proxy/controllers/obj.py

@@ -789,11 +789,11 @@ class ObjectController(Controller):
             lcontainer = object_versions.split('/')[0]
             prefix_len = '%03x' % len(self.object_name)
             lprefix = prefix_len + self.object_name + '/'
-            last_item = None
+            item_list = []
             try:
-                for last_item in self._listing_iter(lcontainer, lprefix,
+                for _item in self._listing_iter(lcontainer, lprefix,
                                                 req.environ):
-                    pass
+                    item_list.append(_item)
             except ListingIterNotFound:
                 # no worries, last_item is None
                 pass
@@ -801,15 +801,19 @@ class ObjectController(Controller):
                 return err.aresp
             except ListingIterError:
                 return HTTPServerError(request=req)
-            if last_item:
+
+            while len(item_list) > 0:
+                previous_version = item_list.pop()
                 # there are older versions so copy the previous version to the
                 # current object and delete the previous version
                 orig_container = self.container_name
                 orig_obj = self.object_name
                 self.container_name = lcontainer
-                self.object_name = last_item['name'].encode('utf-8')
+                self.object_name = previous_version['name'].encode('utf-8')
+
                 copy_path = '/v1/' + self.account_name + '/' + \
                             self.container_name + '/' + self.object_name
+
                 copy_headers = {'X-Newest': 'True',
                                 'Destination': orig_container + '/' + orig_obj
                                 }
@@ -819,6 +823,11 @@ class ObjectController(Controller):
                 creq = Request.blank(copy_path, headers=copy_headers,
                                      environ=copy_environ)
                 copy_resp = self.COPY(creq)
+                if copy_resp.status_int == HTTP_NOT_FOUND:
+                    # the version isn't there so we'll try with previous
+                    self.container_name = orig_container
+                    self.object_name = orig_obj
+                    continue
                 if is_client_error(copy_resp.status_int):
                     # some user error, maybe permissions
                     return HTTPPreconditionFailed(request=req)
@@ -827,7 +836,7 @@ class ObjectController(Controller):
                     return HTTPServiceUnavailable(request=req)
                 # reset these because the COPY changed them
                 self.container_name = lcontainer
-                self.object_name = last_item['name'].encode('utf-8')
+                self.object_name = previous_version['name'].encode('utf-8')
                 new_del_req = Request.blank(copy_path, environ=req.environ)
                 container_info = self.container_info(
                     self.account_name, self.container_name, req)
@@ -844,6 +853,7 @@ class ObjectController(Controller):
                 # remove 'X-If-Delete-At', since it is not for the older copy
                 if 'X-If-Delete-At' in req.headers:
                     del req.headers['X-If-Delete-At']
+                break
         if 'swift.authorize' in req.environ:
             aresp = req.environ['swift.authorize'](req)
             if aresp:

test/unit/proxy/test_server.py

@@ -1548,7 +1548,7 @@ class TestObjectController(unittest.TestCase):
             #                HEAD HEAD GET  GET  HEAD GET  GET  GET  PUT  PUT
             #                PUT  DEL  DEL  DEL
             set_http_connect(200, 200, 200, 200, 200, 200, 200, 200, 201, 201,
-                             201, 200, 200, 200,
+                             201, 204, 204, 204,
                              give_connect=test_connect,
                              body_iter=body_iter,
                              headers={'x-versions-location': 'foo'})
@@ -1560,6 +1560,59 @@ class TestObjectController(unittest.TestCase):
             controller.DELETE(req)
             self.assertEquals(test_errors, [])
 
+    @patch_policies([
+        StoragePolicy(0, 'zero', False, object_ring=FakeRing()),
+        StoragePolicy(1, 'one', True, object_ring=FakeRing())
+    ])
+    def test_DELETE_on_expired_versioned_object(self):
+        methods = set()
+
+        def test_connect(ipaddr, port, device, partition, method, path,
+                         headers=None, query_string=None):
+            methods.add((method, path))
+
+        def fake_container_info(account, container, req):
+            return {'status': 200, 'sync_key': None,
+                    'meta': {}, 'cors': {'allow_origin': None,
+                                         'expose_headers': None,
+                                         'max_age': None},
+                    'sysmeta': {}, 'read_acl': None, 'object_count': None,
+                    'write_acl': None, 'versions': 'foo',
+                    'partition': 1, 'bytes': None, 'storage_policy': '1',
+                    'nodes': [{'zone': 0, 'ip': '10.0.0.0', 'region': 0,
+                               'id': 0, 'device': 'sda', 'port': 1000},
+                              {'zone': 1, 'ip': '10.0.0.1', 'region': 1,
+                               'id': 1, 'device': 'sdb', 'port': 1001},
+                              {'zone': 2, 'ip': '10.0.0.2', 'region': 0,
+                               'id': 2, 'device': 'sdc', 'port': 1002}]}
+
+        def fake_list_iter(container, prefix, env):
+            object_list = [{'name': '1'}, {'name': '2'}, {'name': '3'}]
+            for obj in object_list:
+                yield obj
+
+        with save_globals():
+            controller = proxy_server.ObjectController(self.app,
+                                                       'a', 'c', 'o')
+            controller.container_info = fake_container_info
+            controller._listing_iter = fake_list_iter
+            set_http_connect(404, 404, 404,  # get for the previous version
+                             200, 200, 200,  # get for the pre-previous
+                             201, 201, 201,  # put move the pre-previous
+                             204, 204, 204,  # delete for the pre-previous
+                             give_connect=test_connect)
+            req = Request.blank('/v1/a/c/o',
+                                environ={'REQUEST_METHOD': 'DELETE'})
+
+            self.app.memcache.store = {}
+            self.app.update_request(req)
+            controller.DELETE(req)
+            exp_methods = [('GET', '/a/foo/3'),
+                           ('GET', '/a/foo/2'),
+                           ('PUT', '/a/c/o'),
+                           ('DELETE', '/a/foo/2')]
+            self.assertEquals(set(exp_methods), (methods))
+
     def test_PUT_auto_content_type(self):
         with save_globals():
             controller = proxy_server.ObjectController(self.app, 'account',