From 5050027610cb4c8f20c85b7c60c1cc7f2c44121c Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 8 Jan 2016 11:31:32 -0800 Subject: _RetryBody doesn't need to take explicit etag/content-length Also, don't try to do int(None) for chunk-encoded responses (like DLOs that are longer than a single container listing). Change-Id: Ibacd75d5ee46135d62388786903c895fda8ed3ba --- swiftclient/client.py | 18 ++++++--------- tests/unit/test_swiftclient.py | 52 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/swiftclient/client.py b/swiftclient/client.py index 406149f..a5c4dc3 100644 --- a/swiftclient/client.py +++ b/swiftclient/client.py @@ -202,15 +202,13 @@ class _RetryBody(_ObjectBody): (from offset) if the connection is dropped after partially downloading the object. """ - def __init__(self, resp, expected_length, etag, connection, container, obj, + def __init__(self, resp, connection, container, obj, resp_chunk_size=None, query_string=None, response_dict=None, headers=None): """ Wrap the underlying response :param resp: the response to wrap - :param expected_length: the object size in bytes - :param etag: the object's etag :param connection: Connection class instance :param container: the name of the container the object is in :param obj: the name of object we are downloading @@ -222,8 +220,7 @@ class _RetryBody(_ObjectBody): include in the request """ super(_RetryBody, self).__init__(resp, resp_chunk_size) - self.expected_length = expected_length - self.expected_etag = etag + self.expected_length = int(self.resp.getheader('Content-Length')) self.conn = connection self.container = container self.obj = obj @@ -244,7 +241,7 @@ class _RetryBody(_ObjectBody): if (not buf and self.bytes_read < self.expected_length and self.conn.attempts <= self.conn.retries): self.headers['Range'] = 'bytes=%d-' % self.bytes_read - self.headers['If-Match'] = self.expected_etag + self.headers['If-Match'] = self.resp.getheader('ETag') hdrs, body = self.conn._retry(None, get_object, self.container, self.obj, resp_chunk_size=self.chunk_size, @@ -252,7 +249,7 @@ class _RetryBody(_ObjectBody): response_dict=self.response_dict, headers=self.headers, attempts=self.conn.attempts) - self.resp = body + self.resp = body.resp buf = self.read(length) return buf @@ -1593,11 +1590,10 @@ class Connection(object): not headers or 'range' not in (k.lower() for k in headers)) retry_is_possible = ( is_not_range_request and resp_chunk_size and - self.attempts <= self.retries) + self.attempts <= self.retries and + rheaders.get('transfer-encoding') is None) if retry_is_possible: - body = _RetryBody(body.resp, int(rheaders['content-length']), - rheaders['etag'], - self, container, obj, + body = _RetryBody(body.resp, self, container, obj, resp_chunk_size=resp_chunk_size, query_string=query_string, response_dict=response_dict, diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py index 56d3ff8..b2453b0 100644 --- a/tests/unit/test_swiftclient.py +++ b/tests/unit/test_swiftclient.py @@ -835,6 +835,58 @@ class TestGetObject(MockHttpTest): self.assertRaises(StopIteration, next, resp) self.assertEqual(resp.read(), '') + def test_chunk_size_iter_chunked_no_retry(self): + conn = c.Connection('http://auth.url/', 'some_user', 'some_key') + with mock.patch('swiftclient.client.get_auth_1_0') as mock_get_auth: + mock_get_auth.return_value = ('http://auth.url/', 'tToken') + c.http_connection = self.fake_http_connection( + 200, body='abcdef', headers={'Transfer-Encoding': 'chunked'}) + __, resp = conn.get_object('asdf', 'asdf', resp_chunk_size=2) + self.assertEqual(next(resp), 'ab') + # simulate a dropped connection + resp.resp.read() + self.assertRaises(StopIteration, next, resp) + + def test_chunk_size_iter_retry(self): + conn = c.Connection('http://auth.url/', 'some_user', 'some_key') + with mock.patch('swiftclient.client.get_auth_1_0') as mock_get_auth: + mock_get_auth.return_value = ('http://auth.url', 'tToken') + c.http_connection = self.fake_http_connection( + StubResponse(200, 'abcdef', {'etag': 'some etag', + 'content-length': '6'}), + StubResponse(206, 'cdef', {'etag': 'some etag', + 'content-length': '4'}), + StubResponse(206, 'ef', {'etag': 'some etag', + 'content-length': '2'}), + ) + __, resp = conn.get_object('asdf', 'asdf', resp_chunk_size=2) + self.assertEqual(next(resp), 'ab') + self.assertEqual(1, conn.attempts) + # simulate a dropped connection + resp.resp.read() + self.assertEqual(next(resp), 'cd') + self.assertEqual(2, conn.attempts) + # simulate a dropped connection + resp.resp.read() + self.assertEqual(next(resp), 'ef') + self.assertEqual(3, conn.attempts) + self.assertRaises(StopIteration, next, resp) + self.assertRequests([ + ('GET', '/asdf/asdf', '', { + 'x-auth-token': 'tToken', + }), + ('GET', '/asdf/asdf', '', { + 'range': 'bytes=2-', + 'if-match': 'some etag', + 'x-auth-token': 'tToken', + }), + ('GET', '/asdf/asdf', '', { + 'range': 'bytes=4-', + 'if-match': 'some etag', + 'x-auth-token': 'tToken', + }), + ]) + def test_get_object_with_resp_chunk_size_zero(self): def get_connection(self): def get_auth(): -- cgit v1.2.1