openstack/swift
Code Issues Proposed changes

slo: Send ETag header in 206 responses

Browse Source 
Why weren't we doing that before?? The etag should be the same as for
GET/HEAD, and by sending it, we can assure resuming clients that they're
downlading the same object even if they didn't include an If-Match
header.

Change-Id: I4ccbd1ae3a909ecb4606ef18211d1b868f5cad86
Related-Change: Ic11662eb5c7176fbf422a6fc87a569928d6f85a1
This commit is contained in:
Tim Burke 2018-01-12 13:17:45 -08:00
parent d707fc7b6d
commit d1656e3349
3 changed files with 26 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
swift/common/middleware/slo.py
View File

@ -876,8 +876,6 @@ class SloGetContext(WSGIContext):
headers=response_headers,
conditional_response=True,
app_iter=segmented_iter)
if req.range:
response.headers.pop('Etag')
return response

28
test/functional/test_slo.py
View File

@ -219,9 +219,21 @@ class TestSlo(Base):
"Expected slo_enabled to be True/False, got %r" %
(self.env.slo_enabled,))
manifest_abcde_hash = hashlib.md5()
manifest_abcde_hash.update(hashlib.md5('a' * 1024 * 1024).hexdigest())
manifest_abcde_hash.update(hashlib.md5('b' * 1024 * 1024).hexdigest())
manifest_abcde_hash.update(hashlib.md5('c' * 1024 * 1024).hexdigest())
manifest_abcde_hash.update(hashlib.md5('d' * 1024 * 1024).hexdigest())
manifest_abcde_hash.update(hashlib.md5('e').hexdigest())
self.manifest_abcde_etag = manifest_abcde_hash.hexdigest()
def test_slo_get_simple_manifest(self):
file_item = self.env.container.file('manifest-abcde')
file_contents = file_item.read()
self.assertEqual(file_item.conn.response.status, 200)
headers = dict(file_item.conn.response.getheaders())
self.assertIn('etag', headers)
self.assertEqual(headers['etag'], '"%s"' % self.manifest_abcde_etag)
self.assertEqual(4 * 1024 * 1024 + 1, len(file_contents))
self.assertEqual('a', file_contents[0])
self.assertEqual('a', file_contents[1024 * 1024 - 1])
@ -348,6 +360,10 @@ class TestSlo(Base):
file_item = self.env.container.file('manifest-abcde')
file_contents = file_item.read(size=1024 * 1024 + 2,
offset=1024 * 1024 - 1)
self.assertEqual(file_item.conn.response.status, 206)
headers = dict(file_item.conn.response.getheaders())
self.assertIn('etag', headers)
self.assertEqual(headers['etag'], '"%s"' % self.manifest_abcde_etag)
self.assertEqual('a', file_contents[0])
self.assertEqual('b', file_contents[1])
self.assertEqual('b', file_contents[-2])
@ -418,16 +434,10 @@ class TestSlo(Base):
self.assertEqual('d', file_contents[-1])
def test_slo_etag_is_hash_of_etags(self):
expected_hash = hashlib.md5()
expected_hash.update(hashlib.md5('a' * 1024 * 1024).hexdigest())
expected_hash.update(hashlib.md5('b' * 1024 * 1024).hexdigest())
expected_hash.update(hashlib.md5('c' * 1024 * 1024).hexdigest())
expected_hash.update(hashlib.md5('d' * 1024 * 1024).hexdigest())
expected_hash.update(hashlib.md5('e').hexdigest())
expected_etag = expected_hash.hexdigest()
# we have this check in test_slo_get_simple_manifest, too,
# but verify that it holds for HEAD requests
file_item = self.env.container.file('manifest-abcde')
self.assertEqual(expected_etag, file_item.info()['etag'])
self.assertEqual(self.manifest_abcde_etag, file_item.info()['etag'])
def test_slo_etag_is_hash_of_etags_submanifests(self):

17
test/unit/common/middleware/test_slo.py
View File

@ -59,6 +59,9 @@ class SloTestCase(unittest.TestCase):
slo_conf = {'rate_limit_under_size': '0'}
self.slo = slo.filter_factory(slo_conf)(self.app)
self.slo.logger = self.app.logger
self.manifest_abcd_etag = md5hex(
md5hex("a" * 5) + md5hex(md5hex("b" * 10) + md5hex("c" * 15)) +
md5hex("d" * 20))
def call_app(self, req, app=None):
if app is None:
@ -1683,10 +1686,6 @@ class TestSloGetManifest(SloTestCase):
'Etag': md5hex(_abcdefghijkl_manifest_json)},
_abcdefghijkl_manifest_json)
self.manifest_abcd_etag = md5hex(
md5hex("a" * 5) + md5hex(md5hex("b" * 10) + md5hex("c" * 15)) +
md5hex("d" * 20))
_bc_ranges_manifest_json = json.dumps(
[{'name': '/gettest/b_10', 'hash': md5hex('b' * 10),
'content_type': 'text/plain', 'bytes': '10',
@ -1986,7 +1985,7 @@ class TestSloGetManifest(SloTestCase):
self.assertEqual(status, '206 Partial Content')
self.assertEqual(headers['Content-Length'], '15')
self.assertNotIn('Etag', headers)
self.assertEqual(headers['Etag'], '"%s"' % self.manifest_abcd_etag)
self.assertEqual(body, 'aabbbbbbbbbbccc')
self.assertEqual(
@ -2231,7 +2230,7 @@ class TestSloGetManifest(SloTestCase):
self.assertEqual(status, '206 Partial Content')
self.assertEqual(headers['Content-Length'], '25')
self.assertNotIn('Etag', headers)
self.assertEqual(headers['Etag'], '"%s"' % self.manifest_abcd_etag)
self.assertEqual(body, 'bbbbbbbbbbccccccccccccccc')
self.assertEqual(
@ -2434,7 +2433,7 @@ class TestSloGetManifest(SloTestCase):
self.assertEqual(status, '206 Partial Content')
self.assertEqual(headers['Content-Length'], '20')
self.assertEqual(headers['Content-Type'], 'application/json')
self.assertNotIn('Etag', headers)
self.assertIn('Etag', headers)
self.assertEqual(body, 'accccccccbbbbbbbbddd')
self.assertEqual(
@ -3235,9 +3234,7 @@ class TestSloConditionalGetOldManifest(SloTestCase):
self.assertEqual(status, '206 Partial Content')
self.assertIn(('Content-Length', '4'), headers)
# We intentionally drop Etag for ranged requests.
# Presumably because of broken clients?
self.assertNotIn('etag', [h.lower() for h, v in headers])
self.assertIn(('Etag', '"%s"' % self.manifest_abcd_etag), headers)
self.assertEqual(body, 'aabb')
expected_app_calls = [