Merge "Have versioned_writes use reverse listings"

iloveyou416068 · Nov 26, 2015 · 1a28ccd · 1a28ccd
2 parents ea3a0d3 + 0e5cc89
commit 1a28ccd
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 66 deletions.
diff --git a/swift/common/middleware/versioned_writes.py b/swift/common/middleware/versioned_writes.py
@@ -139,11 +139,18 @@ def __init__(self, wsgi_app, logger):
         WSGIContext.__init__(self, wsgi_app)
         self.logger = logger
 
-    def _listing_iter(self, account_name, lcontainer, lprefix, env):
-        for page in self._listing_pages_iter(account_name,
-                                             lcontainer, lprefix, env):
-            for item in page:
-                yield item
+    def _listing_iter(self, account_name, lcontainer, lprefix, req):
+        try:
+            for page in self._listing_pages_iter(account_name, lcontainer,
+                                                 lprefix, req.environ):
+                for item in page:
+                    yield item
+        except ListingIterNotFound:
+            pass
+        except HTTPPreconditionFailed:
+            raise HTTPPreconditionFailed(request=req)
+        except ListingIterError:
+            raise HTTPServerError(request=req)
 
     def _listing_pages_iter(self, account_name, lcontainer, lprefix, env):
         marker = ''
@@ -152,8 +159,8 @@ def _listing_pages_iter(self, account_name, lcontainer, lprefix, env):
                 env, method='GET', swift_source='VW',
                 path='/v1/%s/%s' % (account_name, lcontainer))
             lreq.environ['QUERY_STRING'] = \
-                'format=json&prefix=%s&marker=%s' % (quote(lprefix),
-                                                     quote(marker))
+                'format=json&prefix=%s&reverse=on&marker=%s' % (
+                    quote(lprefix), quote(marker))
             lresp = lreq.get_response(self.app)
             if not is_success(lresp.status_int):
                 if lresp.status_int == HTTP_NOT_FOUND:
@@ -245,31 +252,22 @@ def handle_obj_versions_delete(self, req, object_versions,
         lcontainer = object_versions.split('/')[0]
         prefix_len = '%03x' % len(object_name)
         lprefix = prefix_len + object_name + '/'
-        item_list = []
-        try:
-            for _item in self._listing_iter(account_name, lcontainer, lprefix,
-                                            req.environ):
-                item_list.append(_item)
-        except ListingIterNotFound:
-            pass
-        except HTTPPreconditionFailed:
-            return HTTPPreconditionFailed(request=req)
-        except ListingIterError:
-            return HTTPServerError(request=req)
-
-        if item_list:
-            # we're about to start making COPY requests - need to validate the
-            # write access to the versioned container
-            if 'swift.authorize' in req.environ:
-                container_info = get_container_info(
-                    req.environ, self.app)
-                req.acl = container_info.get('write_acl')
-                aresp = req.environ['swift.authorize'](req)
-                if aresp:
-                    return aresp
-
-        while len(item_list) > 0:
-            previous_version = item_list.pop()
+
+        item_iter = self._listing_iter(account_name, lcontainer, lprefix, req)
+
+        authed = False
+        for previous_version in item_iter:
+            if not authed:
+                # we're about to start making COPY requests - need to
+                # validate the write access to the versioned container
+                if 'swift.authorize' in req.environ:
+                    container_info = get_container_info(
+                        req.environ, self.app)
+                    req.acl = container_info.get('write_acl')
+                    aresp = req.environ['swift.authorize'](req)
+                    if aresp:
+                        return aresp
+                    authed = True
 
             # there are older versions so copy the previous version to the
             # current object and delete the previous version

diff --git a/test/unit/common/middleware/test_versioned_writes.py b/test/unit/common/middleware/test_versioned_writes.py
@@ -426,7 +426,8 @@ def test_delete_first_object_success(self):
         self.app.register(
             'DELETE', '/v1/a/c/o', swob.HTTPOk, {}, 'passed')
         self.app.register(
-            'GET', '/v1/a/ver_cont?format=json&prefix=001o/&marker=',
+            'GET',
+            '/v1/a/ver_cont?format=json&prefix=001o/&reverse=on&marker=',
             swob.HTTPNotFound, {}, None)
 
         cache = FakeCache({'sysmeta': {'versions-location': 'ver_cont'}})
@@ -443,22 +444,19 @@ def test_delete_latest_version_success(self):
         self.app.register(
             'DELETE', '/v1/a/c/o', swob.HTTPOk, {}, 'passed')
         self.app.register(
-            'GET', '/v1/a/ver_cont?format=json&prefix=001o/&marker=',
+            'GET',
+            '/v1/a/ver_cont?format=json&prefix=001o/&reverse=on&marker=',
             swob.HTTPOk, {},
-            '[{"hash": "x", '
-            '"last_modified": "2014-11-21T14:14:27.409100", '
-            '"bytes": 3, '
-            '"name": "001o/1", '
-            '"content_type": "text/plain"}, '
-            '{"hash": "y", '
+            '[{"hash": "y", '
             '"last_modified": "2014-11-21T14:23:02.206740", '
             '"bytes": 3, '
             '"name": "001o/2", '
+            '"content_type": "text/plain"}, '
+            '{"hash": "x", '
+            '"last_modified": "2014-11-21T14:14:27.409100", '
+            '"bytes": 3, '
+            '"name": "001o/1", '
             '"content_type": "text/plain"}]')
-        self.app.register(
-            'GET', '/v1/a/ver_cont?format=json&prefix=001o/'
-            '&marker=001o/2',
-            swob.HTTPNotFound, {}, None)
         self.app.register(
             'COPY', '/v1/a/ver_cont/001o/2', swob.HTTPCreated,
             {}, None)
@@ -487,22 +485,19 @@ def test_delete_latest_version_success(self):
 
     def test_DELETE_on_expired_versioned_object(self):
         self.app.register(
-            'GET', '/v1/a/ver_cont?format=json&prefix=001o/&marker=',
+            'GET',
+            '/v1/a/ver_cont?format=json&prefix=001o/&reverse=on&marker=',
             swob.HTTPOk, {},
-            '[{"hash": "x", '
-            '"last_modified": "2014-11-21T14:14:27.409100", '
-            '"bytes": 3, '
-            '"name": "001o/1", '
-            '"content_type": "text/plain"}, '
-            '{"hash": "y", '
+            '[{"hash": "y", '
             '"last_modified": "2014-11-21T14:23:02.206740", '
             '"bytes": 3, '
             '"name": "001o/2", '
+            '"content_type": "text/plain"}, '
+            '{"hash": "x", '
+            '"last_modified": "2014-11-21T14:14:27.409100", '
+            '"bytes": 3, '
+            '"name": "001o/1", '
             '"content_type": "text/plain"}]')
-        self.app.register(
-            'GET', '/v1/a/ver_cont?format=json&prefix=001o/'
-            '&marker=001o/2',
-            swob.HTTPNotFound, {}, None)
 
         # expired object
         self.app.register(
@@ -530,22 +525,19 @@ def test_denied_DELETE_of_versioned_object(self):
         self.app.register(
             'DELETE', '/v1/a/c/o', swob.HTTPOk, {}, 'passed')
         self.app.register(
-            'GET', '/v1/a/ver_cont?format=json&prefix=001o/&marker=',
+            'GET',
+            '/v1/a/ver_cont?format=json&prefix=001o/&reverse=on&marker=',
             swob.HTTPOk, {},
-            '[{"hash": "x", '
-            '"last_modified": "2014-11-21T14:14:27.409100", '
-            '"bytes": 3, '
-            '"name": "001o/1", '
-            '"content_type": "text/plain"}, '
-            '{"hash": "y", '
+            '[{"hash": "y", '
             '"last_modified": "2014-11-21T14:23:02.206740", '
             '"bytes": 3, '
             '"name": "001o/2", '
+            '"content_type": "text/plain"}, '
+            '{"hash": "x", '
+            '"last_modified": "2014-11-21T14:14:27.409100", '
+            '"bytes": 3, '
+            '"name": "001o/1", '
             '"content_type": "text/plain"}]')
-        self.app.register(
-            'GET', '/v1/a/ver_cont?format=json&prefix=001o/'
-            '&marker=001o/2',
-            swob.HTTPNotFound, {}, None)
         self.app.register(
             'DELETE', '/v1/a/c/o', swob.HTTPForbidden,
             {}, None)