diff --git a/apiclient/discovery.py b/apiclient/discovery.py index a2c81c6..a0ca930 100644 --- a/apiclient/discovery.py +++ b/apiclient/discovery.py @@ -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,69 +485,39 @@ def createResource(http, baseUrl, model, requestBuilder, expanded_url = uritemplate.expand(mediaPathUrl, params) url = urlparse.urljoin(self._baseUrl, expanded_url + query) - 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()) + 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: - # This is a multipart/related upload. - msgRoot = MIMEMultipart('related') - # msgRoot should not write out it's own headers - setattr(msgRoot, '_write_headers', lambda self: None) - - # attach the body as one part - msg = MIMENonMultipart(*headers['content-type'].split('/')) - msg.set_payload(body) - msgRoot.attach(msg) - - # attach the media as the second part - 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 - # - # - # --===============1678050750164843052==-- - # - # In the case of resumable multipart media uploads, the 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 and then split the resulting content into - # two pieces, text before and text after . The text after is the multipart boundary. - # In apiclient.http the HttpRequest will send the text before - # , 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 + # A non-resumable upload + if body is None: + # This is a simple media upload + headers['content-type'] = media_upload.mimetype() + body = media_upload.getbytes(0, media_upload.size()) else: + # This is a multipart/related upload. + msgRoot = MIMEMultipart('related') + # msgRoot should not write out it's own headers + setattr(msgRoot, '_write_headers', lambda self: None) + + # attach the body as one part + msg = MIMENonMultipart(*headers['content-type'].split('/')) + msg.set_payload(body) + msgRoot.attach(msg) + + # attach the media as the second part + msg = MIMENonMultipart(*media_upload.mimetype().split('/')) + msg['Content-Transfer-Encoding'] = 'binary' + payload = media_upload.getbytes(0, media_upload.size()) msg.set_payload(payload) msgRoot.attach(msg) body = msgRoot.as_string() - multipart_boundary = msgRoot.get_boundary() - headers['content-type'] = ('multipart/related; ' - 'boundary="%s"') % multipart_boundary + multipart_boundary = msgRoot.get_boundary() + headers['content-type'] = ('multipart/related; ' + 'boundary="%s"') % multipart_boundary logging.info('URL being requested: %s' % url) return self._requestBuilder(self._http, diff --git a/apiclient/http.py b/apiclient/http.py index c8a8cd4..8e15e11 100644 --- a/apiclient/http.py +++ b/apiclient/http.py @@ -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.""" diff --git a/tests/test_discovery.py b/tests/test_discovery.py index e1ef94f..f15ec62 100644 --- a/tests/test_discovery.py +++ b/tests/test_discovery.py @@ -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"}'), ]) diff --git a/tests/test_http.py b/tests/test_http.py index 4154310..2744d9e 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -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