openstack/deb-python-oauth2client
Code Issues Proposed changes

Fix resumable upload protocol. Reviewed in http://codereview.appspot.com/5572075/

Browse Source
This commit is contained in:
Joe Gregorio
2012-01-27 17:01:06 -05:00
parent dd81382e0e
commit 945be3e0ae
4 changed files with 47 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
apiclient/discovery.py
View File

@@ -461,7 +461,7 @@ def createResource(http, baseUrl, model, requestBuilder,
multipart_boundary = ''
if media_filename:
# Convert a simple filename into a MediaUpload object.
# Ensure we end up with a valid MediaUpload object.
if isinstance(media_filename, basestring):
(media_mime_type, encoding) = mimetypes.guess_type(media_filename)
if media_mime_type is None:
@@ -474,9 +474,6 @@ def createResource(http, baseUrl, model, requestBuilder,
else:
raise TypeError('media_filename must be str or MediaUpload.')
if media_upload.resumable():
resumable = media_upload
# Check the maxSize
if maxSize > 0 and media_upload.size() > maxSize:
raise MediaUploadSizeError("Media larger than: %s" % maxSize)
@@ -488,11 +485,15 @@ def createResource(http, baseUrl, model, requestBuilder,
expanded_url = uritemplate.expand(mediaPathUrl, params)
url = urlparse.urljoin(self._baseUrl, expanded_url + query)
if media_upload.resumable():
# This is all we need to do for resumable, if the body exists it gets
# sent in the first request, otherwise an empty body is sent.
resumable = media_upload
else:
# A non-resumable upload
if body is None:
# This is a simple media upload
headers['content-type'] = media_upload.mimetype()
expanded_url = uritemplate.expand(mediaResumablePathUrl, params)
if not media_upload.resumable():
body = media_upload.getbytes(0, media_upload.size())
else:
# This is a multipart/related upload.
@@ -509,40 +510,6 @@ def createResource(http, baseUrl, model, requestBuilder,
msg = MIMENonMultipart(*media_upload.mimetype().split('/'))
msg['Content-Transfer-Encoding'] = 'binary'
if media_upload.resumable():
# This is a multipart resumable upload, where a multipart payload
# looks like this:
#
# --===============1678050750164843052==
# Content-Type: application/json
# MIME-Version: 1.0
#
# {'foo': 'bar'}
# --===============1678050750164843052==
# Content-Type: image/png
# MIME-Version: 1.0
# Content-Transfer-Encoding: binary
#
# <BINARY STUFF>
# --===============1678050750164843052==--
#
# In the case of resumable multipart media uploads, the <BINARY
# STUFF> is large and will be spread across multiple PUTs. What we
# do here is compose the multipart message with a random payload in
# place of <BINARY STUFF> and then split the resulting content into
# two pieces, text before <BINARY STUFF> and text after <BINARY
# STUFF>. The text after <BINARY STUFF> is the multipart boundary.
# In apiclient.http the HttpRequest will send the text before
# <BINARY STUFF>, then send the actual binary media in chunks, and
# then will send the multipart delimeter.
payload = hex(random.getrandbits(300))
msg.set_payload(payload)
msgRoot.attach(msg)
body = msgRoot.as_string()
body, _ = body.split(payload)
resumable = media_upload
else:
payload = media_upload.getbytes(0, media_upload.size())
msg.set_payload(payload)
msgRoot.attach(msg)

45
apiclient/http.py
View File

@@ -151,7 +151,7 @@ class MediaFileUpload(MediaUpload):
media_body=media).execute()
"""
def __init__(self, filename, mimetype=None, chunksize=150000, resumable=False):
def __init__(self, filename, mimetype=None, chunksize=256*1024, resumable=False):
"""Constructor.
Args:
@@ -252,11 +252,8 @@ class HttpRequest(object):
major, minor, params = mimeparse.parse_mime_type(
headers.get('content-type', 'application/json'))
# Terminating multipart boundary get a trailing '--' appended.
self.multipart_boundary = params.get('boundary', '').strip('"') + '--'
# If this was a multipart resumable, the size of the non-media part.
self.multipart_size = 0
# The size of the non-media part of the request.
self.body_size = len(self.body or '')
# The resumable URI to send chunks to.
self.resumable_uri = None
@@ -264,18 +261,6 @@ class HttpRequest(object):
# The bytes that have been uploaded.
self.resumable_progress = 0
self.total_size = 0
if resumable is not None:
if self.body is not None:
self.multipart_size = len(self.body)
else:
self.multipart_size = 0
self.total_size = (
self.resumable.size() +
self.multipart_size +
len(self.multipart_boundary))
def execute(self, http=None):
"""Execute the request.
@@ -340,28 +325,23 @@ class HttpRequest(object):
start_headers = copy.copy(self.headers)
start_headers['X-Upload-Content-Type'] = self.resumable.mimetype()
start_headers['X-Upload-Content-Length'] = str(self.resumable.size())
start_headers['Content-Length'] = '0'
start_headers['content-length'] = str(self.body_size)
resp, content = http.request(self.uri, self.method,
body="",
body=self.body,
headers=start_headers)
if resp.status == 200 and 'location' in resp:
self.resumable_uri = resp['location']
else:
raise ResumableUploadError("Failed to retrieve starting URI.")
if self.body:
begin = 0
data = self.body
else:
begin = self.resumable_progress - self.multipart_size
data = self.resumable.getbytes(begin, self.resumable.chunksize())
# Tack on the multipart/related boundary if we are at the end of the file.
if begin + self.resumable.chunksize() >= self.resumable.size():
data += self.multipart_boundary
data = self.resumable.getbytes(self.resumable_progress,
self.resumable.chunksize())
headers = {
'Content-Range': 'bytes %d-%d/%d' % (
self.resumable_progress, self.resumable_progress + len(data) - 1,
self.total_size),
self.resumable.size()),
}
resp, content = http.request(self.resumable_uri, 'PUT',
body=data,
@@ -371,14 +351,13 @@ class HttpRequest(object):
elif resp.status == 308:
# A "308 Resume Incomplete" indicates we are not done.
self.resumable_progress = int(resp['range'].split('-')[1]) + 1
if self.resumable_progress >= self.multipart_size:
self.body = None
if 'location' in resp:
self.resumable_uri = resp['location']
else:
raise HttpError(resp, content, self.uri)
return MediaUploadProgress(self.resumable_progress, self.total_size), None
return (MediaUploadProgress(self.resumable_progress, self.resumable.size()),
None)
def to_json(self):
"""Returns a JSON representation of the HttpRequest."""

24
tests/test_discovery.py
View File

@@ -347,13 +347,12 @@ class Discovery(unittest.TestCase):
media_upload = MediaFileUpload(datafile('small.png'), resumable=True)
request = zoo.animals().insert(media_body=media_upload, body={})
self.assertTrue(request.headers['content-type'].startswith(
'multipart/related'))
self.assertEquals('--==', request.body[0:4])
'application/json'))
self.assertEquals('{"data": {}}', request.body)
self.assertEquals(media_upload, request.resumable)
self.assertEquals('image/png', request.resumable.mimetype())
self.assertTrue(len(request.multipart_boundary) > 0)
self.assertNotEquals(request.body, None)
self.assertEquals(request.resumable_uri, None)
@@ -365,7 +364,7 @@ class Discovery(unittest.TestCase):
'range': '0-12'}, ''),
({'status': '308',
'location': 'http://upload.example.com/3',
'range': '0-%d' % (request.total_size - 2)}, ''),
'range': '0-%d' % (media_upload.size() - 2)}, ''),
({'status': '200'}, '{"foo": "bar"}'),
])
@@ -374,10 +373,6 @@ class Discovery(unittest.TestCase):
self.assertTrue(isinstance(status, MediaUploadProgress))
self.assertEquals(13, status.resumable_progress)
# request.body is not None because the server only acknowledged 12 bytes,
# which is less than the size of the body, so we need to send it again.
self.assertNotEquals(request.body, None)
# Two requests should have been made and the resumable_uri should have been
# updated for each one.
self.assertEquals(request.resumable_uri, 'http://upload.example.com/2')
@@ -387,8 +382,8 @@ class Discovery(unittest.TestCase):
status, body = request.next_chunk(http)
self.assertEquals(request.resumable_uri, 'http://upload.example.com/3')
self.assertEquals(request.total_size-1, request.resumable_progress)
self.assertEquals(request.body, None)
self.assertEquals(media_upload.size()-1, request.resumable_progress)
self.assertEquals('{"data": {}}', request.body)
# Final call to next_chunk should complete the upload.
status, body = request.next_chunk(http)
@@ -403,13 +398,10 @@ class Discovery(unittest.TestCase):
media_upload = MediaFileUpload(datafile('small.png'), resumable=True)
request = zoo.animals().insert(media_body=media_upload, body=None)
self.assertTrue(request.headers['content-type'].startswith(
'image/png'))
self.assertEquals(media_upload, request.resumable)
self.assertEquals('image/png', request.resumable.mimetype())
self.assertEquals(request.multipart_boundary, '--')
self.assertEquals(request.body, None)
self.assertEquals(request.resumable_uri, None)
@@ -421,7 +413,7 @@ class Discovery(unittest.TestCase):
'range': '0-12'}, ''),
({'status': '308',
'location': 'http://upload.example.com/3',
'range': '0-%d' % (request.total_size - 2)}, ''),
'range': '0-%d' % (media_upload.size() - 2)}, ''),
({'status': '200'}, '{"foo": "bar"}'),
])
@@ -439,7 +431,7 @@ class Discovery(unittest.TestCase):
status, body = request.next_chunk(http)
self.assertEquals(request.resumable_uri, 'http://upload.example.com/3')
self.assertEquals(request.total_size-1, request.resumable_progress)
self.assertEquals(media_upload.size()-1, request.resumable_progress)
self.assertEquals(request.body, None)
# Final call to next_chunk should complete the upload.
@@ -464,7 +456,7 @@ class Discovery(unittest.TestCase):
'range': '0-12'}, ''),
({'status': '308',
'location': 'http://upload.example.com/3',
'range': '0-%d' % (request.total_size - 2)}, ''),
'range': '0-%d' % media_upload.size()}, ''),
({'status': '200'}, '{"foo": "bar"}'),
])

1
tests/test_http.py
View File

@@ -109,7 +109,6 @@ class TestUserAgent(unittest.TestCase):
self.assertEquals(new_req.body, '{}')
self.assertEquals(new_req.http, http)
self.assertEquals(new_req.resumable.to_json(), media_upload.to_json())
self.assertEquals(new_req.multipart_boundary, '---flubber--')
EXPECTED = """POST /someapi/v1/collection/?foo=bar HTTP/1.1
Content-Type: application/json