[corehttp] add support for multipart file tuples and for simultaneous data and file input (#34082)

Author: iscai-msft

sdk/core/corehttp/CHANGELOG.md

@@ -1,5 +1,12 @@
 # Release History
 
+## 1.0.0b3 (2024-02-01)
+
+### Features Added
+
+- Support tuple input for `files` values to `corehttp.rest.HttpRequest` #34082
+- Support simultaneous `files` and `data` field entry into `corehttp.rest.HttpRequest` #34082
+
 ## 1.0.0b2 (2023-11-14)
 
 ### Features Added

sdk/core/corehttp/corehttp/_version.py

@@ -9,4 +9,4 @@
 # regenerated.
 # --------------------------------------------------------------------------
 
-VERSION = "1.0.0b2"
+VERSION = "1.0.0b3"

sdk/core/corehttp/corehttp/rest/_helpers.py

@@ -44,6 +44,7 @@
 )
 import xml.etree.ElementTree as ET
 from ..serialization import CoreJSONEncoder
+from ..utils._utils import get_file_items
 
 
 ################################### TYPES SECTION #########################
@@ -54,7 +55,14 @@
 ParamsType = Mapping[str, Union[PrimitiveData, Sequence[PrimitiveData]]]
 
 FileContent = Union[str, bytes, IO[str], IO[bytes]]
-FileType = Tuple[Optional[str], FileContent]
+FileType = Union[
+    # file (or bytes)
+    FileContent,
+    # (filename, file (or bytes))
+    Tuple[Optional[str], FileContent],
+    # (filename, file (or bytes), content_type)
+    Tuple[Optional[str], FileContent, Optional[str]],
+]
 
 FilesType = Union[Mapping[str, FileType], Sequence[Tuple[str, FileType]]]
 
@@ -73,7 +81,7 @@ def _verify_data_object(name, value):
         raise TypeError("Invalid type for data value. Expected primitive type, got {}: {}".format(type(name), name))
 
 
-def _format_data_helper(data: Union[str, IO]) -> Union[Tuple[None, str], Tuple[Optional[str], IO, str]]:
+def _format_data_helper(data: FileType) -> Union[Tuple[Optional[str], str], Tuple[Optional[str], FileContent, str]]:
     """Helper for _format_data.
 
     Format field data according to whether it is a stream or
@@ -84,16 +92,34 @@ def _format_data_helper(data: Union[str, IO]) -> Union[Tuple[None, str], Tuple[O
     :rtype: tuple[str, IO, str] or tuple[None, str]
     :return: A tuple of (data name, data IO, "application/octet-stream") or (None, data str)
     """
-    if hasattr(data, "read"):
-        data = cast(IO, data)
-        data_name = None
-        try:
-            if data.name[0] != "<" and data.name[-1] != ">":
-                data_name = os.path.basename(data.name)
-        except (AttributeError, TypeError):
-            pass
-        return (data_name, data, "application/octet-stream")
-    return (None, cast(str, data))
+    content_type: Optional[str] = None
+    filename: Optional[str] = None
+    if isinstance(data, tuple):
+        if len(data) == 2:
+            # Filename and file bytes are included
+            filename, file_bytes = cast(Tuple[Optional[str], FileContent], data)
+        elif len(data) == 3:
+            # Filename, file object, and content_type are included
+            filename, file_bytes, content_type = cast(Tuple[Optional[str], FileContent, str], data)
+        else:
+            raise ValueError(
+                "Unexpected data format. Expected file, or tuple of (filename, file_bytes) or "
+                "(filename, file_bytes, content_type)."
+            )
+    else:
+        # here we just get the file content
+        if hasattr(data, "read"):
+            data = cast(IO, data)
+            try:
+                if data.name[0] != "<" and data.name[-1] != ">":
+                    filename = os.path.basename(data.name)
+            except (AttributeError, TypeError):
+                pass
+            content_type = "application/octet-stream"
+        file_bytes = data
+    if content_type:
+        return (filename, file_bytes, content_type)
+    return (filename, cast(str, file_bytes))
 
 
 def set_urlencoded_body(data, has_files):
@@ -115,9 +141,9 @@ def set_urlencoded_body(data, has_files):
     return default_headers, body
 
 
-def set_multipart_body(files):
-    formatted_files = {f: _format_data_helper(d) for f, d in files.items() if d is not None}
-    return {}, formatted_files
+def set_multipart_body(files: FilesType):
+    formatted_files = [(f, _format_data_helper(d)) for f, d in get_file_items(files) if d is not None]
+    return {}, dict(formatted_files) if isinstance(files, Mapping) else formatted_files
 
 
 def set_xml_body(content):

sdk/core/corehttp/corehttp/transport/aiohttp/_aiohttp.py

@@ -39,6 +39,7 @@
 from .._base_async import AsyncHttpTransport, _handle_non_stream_rest_response
 from .._base import _create_connection_config
 from ...rest._aiohttp import RestAioHttpTransportResponse
+from ...utils._utils import get_file_items
 
 
 if TYPE_CHECKING:
@@ -136,7 +137,7 @@ def _get_request_data(self, request: RestHttpRequest):
         """
         if request._files:  # pylint: disable=protected-access
             form_data = aiohttp.FormData(request._data or {})  # pylint: disable=protected-access
-            for form_file, data in request._files.items():  # pylint: disable=protected-access
+            for form_file, data in get_file_items(request._files):  # pylint: disable=protected-access
                 content_type = data[2] if len(data) > 2 else None
                 try:
                     form_data.add_field(form_file, data[1], filename=data[0], content_type=content_type)

sdk/core/corehttp/corehttp/utils/_utils.py

@@ -15,9 +15,15 @@
     Tuple,
     Union,
     Dict,
+    cast,
+    Sequence,
+    TYPE_CHECKING,
 )
 from urllib.parse import urlparse
 
+if TYPE_CHECKING:
+    from corehttp.rest._helpers import FileType, FilesType
+
 
 class _FixedOffset(datetime.tzinfo):
     """Fixed offset in minutes east from UTC.
@@ -150,3 +156,11 @@ def __eq__(self, other: Any) -> bool:
 
     def __repr__(self) -> str:
         return str(dict(self.items()))
+
+
+def get_file_items(files: "FilesType") -> Sequence[Tuple[str, "FileType"]]:
+    if isinstance(files, Mapping):
+        # casting because ItemsView technically isn't a Sequence, even
+        # though realistically it is ordered python 3.7 and after
+        return cast(Sequence[Tuple[str, "FileType"]], files.items())
+    return files

sdk/core/corehttp/tests/async_tests/test_rest_http_request_async.py

@@ -10,6 +10,7 @@
 
 import pytest
 from corehttp.rest import HttpRequest
+from utils import NamedIo
 
 
 @pytest.fixture
@@ -95,3 +96,40 @@ async def content():
     await assert_aiterator_body(request, b"test 123")
     # in this case, request._data is what we end up passing to the requests transport
     assert isinstance(request._data, collections.abc.AsyncIterable)
+
+
+@pytest.mark.asyncio
+async def test_multipart_tuple_input_multiple_same_name(client):
+    request = HttpRequest(
+        "POST",
+        url="/multipart/tuple-input-multiple-same-name",
+        files=[
+            ("file", ("firstFileName", NamedIo("firstFile"), "image/pdf")),
+            ("file", ("secondFileName", NamedIo("secondFile"), "image/png")),
+        ],
+    )
+    (await client.send_request(request)).raise_for_status()
+
+
+@pytest.mark.asyncio
+async def test_multipart_tuple_input_multiple_same_name_with_tuple_file_value(client):
+    request = HttpRequest(
+        "POST",
+        url="/multipart/tuple-input-multiple-same-name-with-tuple-file-value",
+        files=[("images", ("foo.png", NamedIo("notMyName.pdf"), "image/png")), ("images", NamedIo("foo.png"))],
+    )
+    (await client.send_request(request)).raise_for_status()
+
+
+@pytest.mark.asyncio
+async def test_data_and_file_input_same_name(client):
+    request = HttpRequest(
+        "POST",
+        url="/multipart/data-and-file-input-same-name",
+        data={"message": "Hello, world!"},
+        files=[
+            ("file", ("firstFileName", NamedIo("firstFile"), "image/pdf")),
+            ("file", ("secondFileName", NamedIo("secondFile"), "image/png")),
+        ],
+    )
+    (await client.send_request(request)).raise_for_status()

sdk/core/corehttp/tests/test_rest_http_request.py

@@ -18,6 +18,7 @@
 from corehttp.rest import HttpRequest
 from corehttp.runtime.policies import SansIOHTTPPolicy
 from rest_client import MockRestClient
+from utils import NamedIo
 
 
 @pytest.fixture
@@ -448,3 +449,97 @@ def remaining(self):
 def test_stream_input():
     data_stream = Stream(length=4)
     HttpRequest(method="PUT", url="http://www.example.com", content=data_stream)  # ensure we can make this HttpRequest
+
+
+@pytest.fixture
+def filebytes():
+    file_path = os.path.join(os.path.dirname(__file__), "./conftest.py")
+    return open(file_path, "rb")
+
+
+def test_multipart_bytes(filebytes):
+    request = HttpRequest("POST", url="http://example.org", files={"file": filebytes})
+
+    assert request.content == {"file": ("conftest.py", filebytes, "application/octet-stream")}
+
+
+def test_multipart_filename_and_bytes(filebytes):
+    files = ("specifiedFileName", filebytes)
+    request = HttpRequest("POST", url="http://example.org", files={"file": files})
+
+    assert request.content == {"file": ("specifiedFileName", filebytes)}
+
+
+def test_multipart_filename_and_bytes_and_content_type(filebytes):
+    files = ("specifiedFileName", filebytes, "application/json")
+    request = HttpRequest("POST", url="http://example.org", files={"file": files})
+
+    assert request.content == {"file": ("specifiedFileName", filebytes, "application/json")}
+
+
+def test_multipart_incorrect_tuple_entry(filebytes):
+    files = ("specifiedFileName", filebytes, "application/json", "extra")
+    with pytest.raises(ValueError):
+        HttpRequest("POST", url="http://example.org", files={"file": files})
+
+
+def test_multipart_tuple_input_single(filebytes):
+    request = HttpRequest("POST", url="http://example.org", files=[("file", filebytes)])
+
+    assert request.content == [("file", ("conftest.py", filebytes, "application/octet-stream"))]
+
+
+def test_multipart_tuple_input_multiple(filebytes):
+    request = HttpRequest("POST", url="http://example.org", files=[("file", filebytes), ("file2", filebytes)])
+
+    assert request.content == [
+        ("file", ("conftest.py", filebytes, "application/octet-stream")),
+        ("file2", ("conftest.py", filebytes, "application/octet-stream")),
+    ]
+
+
+def test_multipart_tuple_input_multiple_with_filename_and_content_type(filebytes):
+    request = HttpRequest(
+        "POST",
+        url="http://example.org",
+        files=[("file", ("first file", filebytes, "image/pdf")), ("file2", ("second file", filebytes, "image/png"))],
+    )
+
+    assert request.content == [
+        ("file", ("first file", filebytes, "image/pdf")),
+        ("file2", ("second file", filebytes, "image/png")),
+    ]
+
+
+def test_multipart_tuple_input_multiple_same_name(client):
+    request = HttpRequest(
+        "POST",
+        url="/multipart/tuple-input-multiple-same-name",
+        files=[
+            ("file", ("firstFileName", NamedIo("firstFile"), "image/pdf")),
+            ("file", ("secondFileName", NamedIo("secondFile"), "image/png")),
+        ],
+    )
+    client.send_request(request).raise_for_status()
+
+
+def test_multipart_tuple_input_multiple_same_name_with_tuple_file_value(client):
+    request = HttpRequest(
+        "POST",
+        url="/multipart/tuple-input-multiple-same-name-with-tuple-file-value",
+        files=[("images", ("foo.png", NamedIo("notMyName.pdf"), "image/png")), ("images", NamedIo("foo.png"))],
+    )
+    client.send_request(request).raise_for_status()
+
+
+def test_data_and_file_input_same_name(client):
+    request = HttpRequest(
+        "POST",
+        url="/multipart/data-and-file-input-same-name",
+        data={"message": "Hello, world!"},
+        files=[
+            ("file", ("firstFileName", NamedIo("firstFile"), "image/pdf")),
+            ("file", ("secondFileName", NamedIo("secondFile"), "image/png")),
+        ],
+    )
+    client.send_request(request).raise_for_status()

sdk/core/corehttp/tests/testserver_tests/coretestserver/coretestserver/test_routes/multipart.py

@@ -146,3 +146,50 @@ def multipart_request():
         body_as_str.encode("ascii"),
         content_type="multipart/mixed; boundary=batchresponse_66925647-d0cb-4109-b6d3-28efe3e1e5ed",
     )
+
+
+@multipart_api.route("/tuple-input-multiple-same-name", methods=["POST"])
+def tuple_input_multiple_same_name():
+    assert_with_message("content type", multipart_header_start, request.content_type[: len(multipart_header_start)])
+
+    files = request.files.getlist("file")
+    assert_with_message("num files", 2, len(files))
+
+    file1 = files[0]
+    assert_with_message("file content type", "image/pdf", file1.content_type)
+    assert_with_message("filename", "firstFileName", file1.filename)
+
+    file2 = files[1]
+    assert_with_message("file content type", "image/png", file2.content_type)
+    assert_with_message("filename", "secondFileName", file2.filename)
+    return Response(status=200)
+
+
+@multipart_api.route("/tuple-input-multiple-same-name-with-tuple-file-value", methods=["POST"])
+def test_input_multiple_same_name_with_tuple_file_value():
+    assert_with_message("content type", multipart_header_start, request.content_type[: len(multipart_header_start)])
+
+    images = request.files.getlist("images")
+    assert_with_message("num images", 2, len(images))
+
+    tuple_image = images[0]
+    assert_with_message("image content type", "image/png", tuple_image.content_type)
+    assert_with_message("filename", "foo.png", tuple_image.filename)
+
+    single_image = images[1]
+    assert_with_message("file content type", "application/octet-stream", single_image.content_type)
+    assert_with_message("filename", "foo.png", single_image.filename)
+    return Response(status=200)
+
+
+@multipart_api.route("/data-and-file-input-same-name", methods=["POST"])
+def data_and_file_input_same_name():
+    assert_with_message("content type", multipart_header_start, request.content_type[: len(multipart_header_start)])
+
+    # use call to this function to check files
+    tuple_input_multiple_same_name()
+
+    assert_with_message("data items num", 1, len(request.form.keys()))
+    assert_with_message("message", "Hello, world!", request.form["message"])
+
+    return Response(status=200)

sdk/core/corehttp/tests/utils.py

@@ -5,6 +5,7 @@
 # -------------------------------------------------------------------------
 import pytest
 import types
+import io
 
 ############################## LISTS USED TO PARAMETERIZE TESTS ##############################
 from corehttp.rest import HttpRequest
@@ -119,3 +120,9 @@ def readonly_checks(response):
         if attr == "encoding":
             # encoding is the only settable new attr
             continue
+
+
+class NamedIo(io.BytesIO):
+    def __init__(self, name: str, *args, **kwargs):
+        super(NamedIo, self).__init__(*args, **kwargs)
+        self.name = name