diff --git a/boxsdk/util/multipart_stream.py b/boxsdk/util/multipart_stream.py index 10c53bf24..1fe5e23d4 100644 --- a/boxsdk/util/multipart_stream.py +++ b/boxsdk/util/multipart_stream.py @@ -1,19 +1,34 @@ -from collections import OrderedDict +from io import BytesIO +from typing import Any -from requests_toolbelt.multipart.encoder import MultipartEncoder +from box_sdk_gen.networking.multipart_stream import ( + MultipartField, + MultipartStream as _MultipartStream, +) -class MultipartStream(MultipartEncoder): +class MultipartStream(_MultipartStream): """ - Subclass of the requests_toolbelt's :class:`MultipartEncoder` that ensures that data - is encoded before files. This allows a server to process information in the data before - receiving the file bytes. + Streaming multipart/form-data body that ensures that data is encoded before files. + This allows a server to process information in the data before receiving the file bytes. + File streams are read lazily, so uploads are sent without loading whole files in memory. + + Field values are either a string, bytes or a binary stream, or a + (file_name, value) or (file_name, value, content_type) tuple. """ - def __init__(self, data, files): - fields = OrderedDict() - for k in data: - fields[k] = data[k] - for k in files: - fields[k] = files[k] - super().__init__(fields) + def __init__(self, data: dict, files: dict): + super().__init__( + [self._to_field(name, value) for name, value in data.items()] + + [self._to_field(name, value) for name, value in files.items()] + ) + + @staticmethod + def _to_field(name: str, value: Any) -> MultipartField: + file_name, content_type = None, None + if isinstance(value, tuple): + file_name, value, *rest = value + content_type = rest[0] if rest else None + if isinstance(value, bytes): + value = BytesIO(value) + return name, file_name, value, content_type diff --git a/docs/boxsdk/usage/files.md b/docs/boxsdk/usage/files.md index ea96c3eff..7a5f89ff3 100644 --- a/docs/boxsdk/usage/files.md +++ b/docs/boxsdk/usage/files.md @@ -197,7 +197,7 @@ which controls how the file content is uploaded. If you are uploading a large file, you may want to stream the request to avoid excessive memory usage. According to `requests'` library [docs][request_docs], by default, the `requests` library does not support streaming uploads, and all the data must be read into memory before being sent to the server. -However, the `requests-toolbelt` package includes a `MultipartEncoder` class, which enables file uploads without +However, the Box Python SDK includes a streaming multipart encoder, which enables file uploads without loading the entire file into memory. This approach is the default in the Box Python SDK. That said, handling 307 Temporary Redirects presents a challenge with streamed file uploads. diff --git a/setup.py b/setup.py index 88cc8843d..d5313e66b 100644 --- a/setup.py +++ b/setup.py @@ -56,7 +56,6 @@ def main(): 'urllib3', 'dataclasses', 'requests<3', - 'requests-toolbelt<2', 'python-dateutil', ] redis_requires = ['redis>=2.10.3'] diff --git a/test/boxsdk/unit/session/test_session.py b/test/boxsdk/unit/session/test_session.py index 932b42650..c865ac36c 100644 --- a/test/boxsdk/unit/session/test_session.py +++ b/test/boxsdk/unit/session/test_session.py @@ -8,8 +8,6 @@ SSLError, ConnectionError as RequestsConnectionError, ) -from requests_toolbelt import MultipartEncoder - import pytest from boxsdk import CCGAuth @@ -19,6 +17,7 @@ from boxsdk.network.default_network import DefaultNetwork, DefaultNetworkResponse from boxsdk.session.box_response import BoxResponse from boxsdk.session.session import Session, Translator, AuthorizedSession +from boxsdk.util.multipart_stream import MultipartStream @pytest.fixture(scope='function', params=[False, True]) @@ -265,7 +264,9 @@ def test_box_session_seeks_file_after_retry( mock_file_2.tell.return_value = 3 files = {'file': ('unused', mock_file_1), 'f2': ('unused', mock_file_2)} - box_response = box_session.post(url=test_url, files=files) + # the multipart encoder sizes the streams itself, so only count the session's seeks + with patch('boxsdk.session.session.MultipartStream'): + box_response = box_session.post(url=test_url, files=files) assert box_response.status_code == 200 assert box_response.json() == generic_successful_response.json() assert box_response.ok == generic_successful_response.ok @@ -645,7 +646,14 @@ def test_multipart_request_with_enabled_streaming_file_content( assert call_args[1] == test_url assert call_kwargs['access_token'] == 'fake_access_token' assert call_kwargs['log_response_content'] is True - assert isinstance(call_kwargs['data'], MultipartEncoder) - assert call_kwargs['data'].fields['attributes'] == '{"name": "test_file"}' - assert call_kwargs['data'].fields['file'][0] == 'unused' - assert isinstance(call_kwargs['data'].fields['file'][1], BytesIO) + multipart_stream = call_kwargs['data'] + assert isinstance(multipart_stream, MultipartStream) + assert call_kwargs['headers']['Content-Type'] == multipart_stream.content_type + body = multipart_stream.read() + assert ( + b'name="attributes"\r\n\r\n{"name": "test_file"}\r\n' + + f'--{multipart_stream.boundary}\r\n'.encode() + + b'Content-Disposition: form-data; name="file"; filename="unused"\r\n\r\n' + + file_bytes + + b'\r\n' + ) in body diff --git a/test/boxsdk/unit/util/test_multipart_stream.py b/test/boxsdk/unit/util/test_multipart_stream.py index d872fb1c5..83be5de99 100644 --- a/test/boxsdk/unit/util/test_multipart_stream.py +++ b/test/boxsdk/unit/util/test_multipart_stream.py @@ -1,3 +1,5 @@ +from io import BytesIO + import pytest from boxsdk.util.multipart_stream import MultipartStream @@ -17,10 +19,8 @@ def test_multipart_stream_orders_data_before_files( multipart_stream_data, multipart_stream_files ): # pylint:disable=redefined-outer-name - if not multipart_stream_data and not multipart_stream_files: - pytest.xfail('Encoder does not support empty fields.') stream = MultipartStream(multipart_stream_data, multipart_stream_files) - encoded_stream = stream.to_string() + encoded_stream = stream.read() data_indices = [ encoded_stream.find(value) for value in multipart_stream_data.values() ] @@ -30,3 +30,42 @@ def test_multipart_stream_orders_data_before_files( assert -1 not in data_indices assert -1 not in file_indices assert all(all(data_index < f for f in file_indices) for data_index in data_indices) + assert len(encoded_stream) == stream.len + + +def test_multipart_stream_encodes_data_and_file_tuples(): + stream = MultipartStream( + {'attributes': '{"name": "test_file"}'}, + { + 'file': ('unused', BytesIO(b'file content')), + 'pic': ('avatar.png', BytesIO(b'png bytes'), 'image/png'), + }, + ) + + assert stream.content_type == f'multipart/form-data; boundary={stream.boundary}' + assert ( + stream.read() + == ( + f'--{stream.boundary}\r\n' + 'Content-Disposition: form-data; name="attributes"\r\n\r\n' + '{"name": "test_file"}\r\n' + f'--{stream.boundary}\r\n' + 'Content-Disposition: form-data; name="file"; filename="unused"\r\n\r\n' + 'file content\r\n' + f'--{stream.boundary}\r\n' + 'Content-Disposition: form-data; name="pic"; filename="avatar.png"\r\n' + 'Content-Type: image/png\r\n\r\n' + 'png bytes\r\n' + f'--{stream.boundary}--\r\n' + ).encode() + ) + + +def test_multipart_stream_reads_file_lazily(): + file_stream = BytesIO(b'file content') + file_stream.read(5) + + stream = MultipartStream({}, {'file': ('unused', file_stream)}) + + assert file_stream.tell() == 5 + assert b'\r\n\r\ncontent\r\n' in stream.read()