Merge "versioning: Only list versions container if it seems to exist"

swift/common/middleware/versioned_writes/object_versioning.py

@@ -1165,12 +1165,22 @@ class ContainerContext(ObjectVersioningContext):
             params['prefix'] = get_reserved_name(params['prefix'])
 
         # NB: no end_marker support (yet)
-        versions_req.params = {
-            k: params.get(k, '')
-            for k in ('prefix', 'marker', 'limit', 'delimiter', 'reverse')}
-        versions_resp = versions_req.get_response(self.app)
+        if get_container_info(versions_req.environ, self.app,
+                              swift_source='OV')['status'] == 404:
+            # we don't usually like to LBYL like this, but 404s tend to be
+            # expensive (since we check all primaries and a bunch of handoffs)
+            # and we expect this to be a reasonably common way to listing
+            # objects since it's more complete from the user's perspective
+            # (see also: s3api and that client ecosystem)
+            versions_resp = None
+        else:
+            versions_req.params = {
+                k: params.get(k, '') for k in (
+                    'prefix', 'marker', 'limit', 'delimiter', 'reverse')}
+            versions_resp = versions_req.get_response(self.app)
 
-        if versions_resp.status_int == HTTP_NOT_FOUND:
+        if versions_resp is None \
+                or versions_resp.status_int == HTTP_NOT_FOUND:
             subdir_listing = [{'subdir': s} for s in subdir_set]
             broken_listing = []
             for item in current_versions.values():

test/unit/common/middleware/test_object_versioning.py

@@ -101,6 +101,11 @@ class ObjectVersioningBaseTestCase(unittest.TestCase):
         self.cache_version_off.set(
             get_cache_key('a', self.build_container_name('c')),
             {'status': 200})
+
+        self.cache_version_never_on = FakeMemcache()
+        self.cache_version_never_on.set(get_cache_key('a'), {'status': 200})
+        self.cache_version_never_on.set(get_cache_key('a', 'c'),
+                                        {'status': 200})
         self.expected_unread_requests = {}
 
     def tearDown(self):
@@ -2999,6 +3004,73 @@ class ObjectVersioningTestContainerOperations(ObjectVersioningBaseTestCase):
         }]
         self.assertEqual(expected, json.loads(body))
 
+    def test_list_versions_never_enabled(self):
+        listing_body = [{
+            'bytes': 8,
+            'name': 'my-other-object',
+            'hash': 'ebdd8d46ecb4a07f6c433d67eb05d5f3',
+            'last_modified': '1970-01-01T00:00:05.000000',
+            'content_type': 'application/bar',
+        }, {
+            'bytes': 9,
+            'name': 'obj',
+            'hash': 'e55cedc11adb39c404b7365f7d6291fa',
+            'last_modified': '1970-01-01T00:00:20.000000',
+            'content_type': 'text/plain',
+        }]
+
+        self.app.register(
+            'GET', self.build_versions_path(), swob.HTTPNotFound, {}, '')
+        self.app.register(
+            'GET', '/v1/a/c', swob.HTTPOk, {},
+            json.dumps(listing_body).encode('utf8'))
+        req = Request.blank(
+            '/v1/a/c?versions',
+            environ={'REQUEST_METHOD': 'GET',
+                     'swift.cache': self.cache_version_never_on})
+        status, headers, body = self.call_ov(req)
+        self.assertEqual(status, '200 OK')
+        self.assertNotIn('x-versions-enabled', [h.lower() for h, _ in headers])
+        self.assertEqual(len(self.authorized), 1)
+        self.assertRequestEqual(req, self.authorized[0])
+        expected = [{
+            'bytes': 8,
+            'name': 'my-other-object',
+            'version_id': 'null',
+            'hash': 'ebdd8d46ecb4a07f6c433d67eb05d5f3',
+            'last_modified': '1970-01-01T00:00:05.000000',
+            'content_type': 'application/bar',
+            'is_latest': True,
+        }, {
+            'bytes': 9,
+            'name': 'obj',
+            'version_id': 'null',
+            'hash': 'e55cedc11adb39c404b7365f7d6291fa',
+            'last_modified': '1970-01-01T00:00:20.000000',
+            'content_type': 'text/plain',
+            'is_latest': True,
+        }]
+        self.assertEqual(expected, json.loads(body))
+        self.assertEqual(self.app.calls, [
+            ('GET', '/v1/a/c?format=json'),
+            ('HEAD', self.build_versions_path()),
+        ])
+
+        # if it's in cache, we won't even get the HEAD
+        self.app._calls = []
+        self.cache_version_never_on.set(
+            get_cache_key('a', self.build_container_name('c')),
+            {'status': 404})
+        req = Request.blank(
+            '/v1/a/c?versions',
+            environ={'REQUEST_METHOD': 'GET',
+                     'swift.cache': self.cache_version_never_on})
+        status, headers, body = self.call_ov(req)
+        self.assertEqual(status, '200 OK')
+        self.assertNotIn('x-versions-enabled', [h.lower() for h, _ in headers])
+        self.assertEqual(expected, json.loads(body))
+        self.assertEqual(self.app.calls, [('GET', '/v1/a/c?format=json')])
+
     def test_bytes_count(self):
         self.app.register(
             'HEAD', self.build_versions_path(), swob.HTTPOk,