Fix delete versioning objects when previous is expired

When deleteing versioned objects proxy will try to restore the previous
copy. The COPY request will fail if the previous version is expired but
not handled by object-expirer.

This patch checks COPY respones on the previous copy, if it's
HTTP_NOT_FOUND(mostly because it's expired) proxy will try to restore
with the version before previous.

Closes-Bug #1308446
Change-Id: I17f049ea3ef62723effae8086ec427f6e151cd9c

swift/proxy/controllers/obj.py

@@ -799,11 +799,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):
+                                                req.environ):
-                    pass
+                    item_list.append(_item)
             except ListingIterNotFound:
                 # no worries, last_item is None
                 pass
@@ -811,15 +811,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
                                 }
@@ -829,6 +833,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)
@@ -837,7 +846,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)
@@ -854,6 +863,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

@@ -1511,7 +1511,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'})
@@ -1523,6 +1523,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',