From 37e4121874c6f669f212c6bb2445f12a128248c3 Mon Sep 17 00:00:00 2001 From: Casper van der Wel Date: Tue, 10 Oct 2023 16:28:30 +0200 Subject: [PATCH 1/5] Disable multipart encoding --- CHANGES.md | 4 ++-- clean_python/api_client/sync_api_provider.py | 4 +++- .../fastapi_example/presentation.py | 20 +++++++++++++++---- tests/api_client/test_sync_api_provider.py | 2 ++ 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 1671191..34c346c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +4,7 @@ 0.6.9 (unreleased) ------------------ -- Nothing changed yet. +- Disable multpart encoding in `SyncApiProvider`. 0.6.8 (2023-10-10) @@ -16,7 +16,7 @@ 0.6.7 (2023-10-09) ------------------ -- Adapt call signature of the `fetch_token` callable in `ApiProvicer`. +- Adapt call signature of the `fetch_token` callable in `ApiProvider`. - Add `clean_python.oauth.client_credentials`. diff --git a/clean_python/api_client/sync_api_provider.py b/clean_python/api_client/sync_api_provider.py index 88db4f9..7c754e4 100644 --- a/clean_python/api_client/sync_api_provider.py +++ b/clean_python/api_client/sync_api_provider.py @@ -74,7 +74,9 @@ def _request( elif fields is not None: request_kwargs["fields"] = fields headers.update(self._fetch_token()) - return self._pool.request(headers=headers, **request_kwargs) + return self._pool.request( + headers=headers, encode_multipart=False, **request_kwargs + ) def request( self, diff --git a/integration_tests/fastapi_example/presentation.py b/integration_tests/fastapi_example/presentation.py index ad466a1..7e96034 100644 --- a/integration_tests/fastapi_example/presentation.py +++ b/integration_tests/fastapi_example/presentation.py @@ -6,6 +6,7 @@ from fastapi import Depends from fastapi import Form +from fastapi import Request from fastapi import Response from fastapi import UploadFile from fastapi.responses import JSONResponse @@ -86,19 +87,30 @@ async def urlencode(self, name: str): @post("/token") def token( self, + request: Request, grant_type: str = Form(), scope: str = Form(), credentials: HTTPBasicCredentials = Depends(basic), ): """For testing client credentials grant""" + if request.headers["Content-Type"] != "application/x-www-form-urlencoded": + return Response(status_code=HTTPStatus.METHOD_NOT_ALLOWED) if grant_type != "client_credentials": - return JSONResponse({"error": "invalid_grant"}) + return JSONResponse( + {"error": "invalid_grant"}, status_code=HTTPStatus.BAD_REQUEST + ) if credentials.username != "testclient": - return JSONResponse({"error": "invalid_client"}) + return JSONResponse( + {"error": "invalid_client"}, status_code=HTTPStatus.BAD_REQUEST + ) if credentials.password != "supersecret": - return JSONResponse({"error": "invalid_client"}) + return JSONResponse( + {"error": "invalid_client"}, status_code=HTTPStatus.BAD_REQUEST + ) if scope != "all": - return JSONResponse({"error": "invalid_grant"}) + return JSONResponse( + {"error": "invalid_grant"}, status_code=HTTPStatus.BAD_REQUEST + ) claims = {"user": "foo", "exp": int(time.time()) + 3600} payload = base64.b64encode(json.dumps(claims).encode()).decode() return { diff --git a/tests/api_client/test_sync_api_provider.py b/tests/api_client/test_sync_api_provider.py index d3669f3..a19125e 100644 --- a/tests/api_client/test_sync_api_provider.py +++ b/tests/api_client/test_sync_api_provider.py @@ -49,6 +49,7 @@ def test_get(api_provider: SyncApiProvider, response): url="http://testserver/foo", headers={"Authorization": "Bearer tenant-2"}, timeout=5.0, + encode_multipart=False, ) assert actual == {"foo": 2} @@ -69,6 +70,7 @@ def test_post_json(api_provider: SyncApiProvider, response): "Authorization": "Bearer tenant-2", }, timeout=5.0, + encode_multipart=False, ) assert actual == {"foo": 2} From f00cbc64961e2fccd14ee9dee4de72b79e8f2280 Mon Sep 17 00:00:00 2001 From: Casper van der Wel Date: Wed, 11 Oct 2023 11:00:13 +0200 Subject: [PATCH 2/5] Add file upload to SyncApiProvider --- clean_python/api_client/api_provider.py | 21 +++++++++++++- clean_python/api_client/sync_api_provider.py | 28 +++++++++++++++---- .../fastapi_example/presentation.py | 4 +-- .../test_int_sync_api_provider.py | 10 +++++-- 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/clean_python/api_client/api_provider.py b/clean_python/api_client/api_provider.py index 8acc6f7..04e9e73 100644 --- a/clean_python/api_client/api_provider.py +++ b/clean_python/api_client/api_provider.py @@ -1,6 +1,8 @@ import asyncio import re from http import HTTPStatus +from io import BytesIO +from typing import Any from typing import Awaitable from typing import Callable from typing import Dict @@ -13,13 +15,15 @@ from aiohttp import ClientResponse from aiohttp import ClientSession from pydantic import AnyHttpUrl +from pydantic import field_validator from clean_python import Json +from clean_python import ValueObject from .exceptions import ApiException from .response import Response -__all__ = ["ApiProvider"] +__all__ = ["ApiProvider", "FileFormPost"] RETRY_STATUSES = frozenset({413, 429, 503}) # like in urllib3 @@ -57,6 +61,21 @@ def add_query_params(url: str, params: Optional[Json]) -> str: return url + "?" + urlencode(params, doseq=True) +class FileFormPost(ValueObject): + file_name: str + file: Any # typing of BinaryIO / BytesIO is hard! + field_name: str = "file" + content_type: str = "application/octet-stream" + + @field_validator("file") + @classmethod + def validate_file(cls, v): + if isinstance(v, bytes): + return BytesIO(v) + assert hasattr(v, "read") # poor-mans BinaryIO validation + return v + + class ApiProvider: """Basic JSON API provider with retry policy and bearer tokens. diff --git a/clean_python/api_client/sync_api_provider.py b/clean_python/api_client/sync_api_provider.py index 7c754e4..59920dc 100644 --- a/clean_python/api_client/sync_api_provider.py +++ b/clean_python/api_client/sync_api_provider.py @@ -12,6 +12,7 @@ from clean_python import Json from .api_provider import add_query_params +from .api_provider import FileFormPost from .api_provider import is_json_content_type from .api_provider import is_success from .api_provider import join @@ -55,6 +56,7 @@ def _request( params: Optional[Json], json: Optional[Json], fields: Optional[Json], + file: Optional[FileFormPost], timeout: float, ): headers = {} @@ -68,15 +70,27 @@ def _request( # for urllib3<2, we dump json ourselves if json is not None and fields is not None: raise ValueError("Cannot both specify 'json' and 'fields'") + elif json is not None and file is not None: + raise ValueError("Cannot both specify 'json' and 'file'") elif json is not None: request_kwargs["body"] = json_lib.dumps(json).encode() headers["Content-Type"] = "application/json" - elif fields is not None: + elif fields is not None and file is None: request_kwargs["fields"] = fields + request_kwargs["encode_multipart"] = False + elif file is not None: + request_kwargs["fields"] = { + file.field_name: ( + file.file_name, + file.file.read(), + file.content_type, + ), + **(fields or {}), + } + request_kwargs["encode_multipart"] = True + headers.update(self._fetch_token()) - return self._pool.request( - headers=headers, encode_multipart=False, **request_kwargs - ) + return self._pool.request(headers=headers, **request_kwargs) def request( self, @@ -85,9 +99,10 @@ def request( params: Optional[Json] = None, json: Optional[Json] = None, fields: Optional[Json] = None, + file: Optional[FileFormPost] = None, timeout: float = 5.0, ) -> Optional[Json]: - response = self._request(method, path, params, json, fields, timeout) + response = self._request(method, path, params, json, fields, file, timeout) status = HTTPStatus(response.status) content_type = response.headers.get("Content-Type") if status is HTTPStatus.NO_CONTENT: @@ -109,9 +124,10 @@ def request_raw( params: Optional[Json] = None, json: Optional[Json] = None, fields: Optional[Json] = None, + file: Optional[FileFormPost] = None, timeout: float = 5.0, ) -> Response: - response = self._request(method, path, params, json, fields, timeout) + response = self._request(method, path, params, json, fields, file, timeout) return Response( status=response.status, data=response.data, diff --git a/integration_tests/fastapi_example/presentation.py b/integration_tests/fastapi_example/presentation.py index 7e96034..8793d81 100644 --- a/integration_tests/fastapi_example/presentation.py +++ b/integration_tests/fastapi_example/presentation.py @@ -77,8 +77,8 @@ async def form(self, name: str = Form()): return {"name": name} @post("/file") - async def file(self, file: UploadFile): - return {file.filename: (await file.read()).decode()} + async def file(self, file: UploadFile, description: str = Form()): + return {file.filename: (await file.read()).decode(), "description": description} @put("/urlencode/{name}", response_model=Author) async def urlencode(self, name: str): diff --git a/integration_tests/test_int_sync_api_provider.py b/integration_tests/test_int_sync_api_provider.py index 7ce50a6..3bdbeef 100644 --- a/integration_tests/test_int_sync_api_provider.py +++ b/integration_tests/test_int_sync_api_provider.py @@ -7,6 +7,7 @@ from clean_python import ctx from clean_python import Tenant from clean_python.api_client import ApiException +from clean_python.api_client import FileFormPost from clean_python.api_client import SyncApiProvider @@ -46,10 +47,15 @@ def test_request_form_body(provider: SyncApiProvider): def test_request_form_file(provider: SyncApiProvider): - response = provider.request("POST", "v1/file", fields={"file": ("x.txt", b"foo")}) + response = provider.request( + "POST", + "v1/file", + fields={"description": "bla"}, + file=FileFormPost(file_name="x.txt", file=b"foo"), + ) assert isinstance(response, dict) - assert response["x.txt"] == "foo" + assert response == {"x.txt": "foo", "description": "bla"} @pytest.fixture From 030076bd095687ea7b4142f888a76ad28dddc207 Mon Sep 17 00:00:00 2001 From: Casper van der Wel Date: Wed, 11 Oct 2023 11:04:27 +0200 Subject: [PATCH 3/5] Unittests --- tests/api_client/test_sync_api_provider.py | 46 +++++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/tests/api_client/test_sync_api_provider.py b/tests/api_client/test_sync_api_provider.py index a19125e..92b3ba8 100644 --- a/tests/api_client/test_sync_api_provider.py +++ b/tests/api_client/test_sync_api_provider.py @@ -8,6 +8,7 @@ from clean_python import ctx from clean_python import Tenant from clean_python.api_client import ApiException +from clean_python.api_client import FileFormPost from clean_python.api_client import SyncApiProvider MODULE = "clean_python.api_client.sync_api_provider" @@ -49,7 +50,6 @@ def test_get(api_provider: SyncApiProvider, response): url="http://testserver/foo", headers={"Authorization": "Bearer tenant-2"}, timeout=5.0, - encode_multipart=False, ) assert actual == {"foo": 2} @@ -70,7 +70,6 @@ def test_post_json(api_provider: SyncApiProvider, response): "Authorization": "Bearer tenant-2", }, timeout=5.0, - encode_multipart=False, ) assert actual == {"foo": 2} @@ -161,3 +160,46 @@ def test_trailing_slash(api_provider: SyncApiProvider, path, trailing_slash, exp api_provider._pool.request.call_args[1]["url"] == "http://testserver/foo/" + expected ) + + +def test_post_file(api_provider: SyncApiProvider): + api_provider.request( + "POST", + "bar", + file=FileFormPost(file_name="test.zip", file=b"foo", field_name="x"), + ) + + assert api_provider._pool.request.call_count == 1 + + assert api_provider._pool.request.call_args[1] == dict( + method="POST", + url="http://testserver/foo/bar", + fields={"x": ("test.zip", b"foo", "application/octet-stream")}, + headers={ + "Authorization": "Bearer tenant-2", + }, + timeout=5.0, + encode_multipart=True, + ) + + +def test_post_file_with_fields(api_provider: SyncApiProvider): + api_provider.request( + "POST", + "bar", + fields={"a": "b"}, + file=FileFormPost(file_name="test.zip", file=b"foo", field_name="x"), + ) + + assert api_provider._pool.request.call_count == 1 + + assert api_provider._pool.request.call_args[1] == dict( + method="POST", + url="http://testserver/foo/bar", + fields={"a": "b", "x": ("test.zip", b"foo", "application/octet-stream")}, + headers={ + "Authorization": "Bearer tenant-2", + }, + timeout=5.0, + encode_multipart=True, + ) From 6d3e04540d9b3368fcbe8cce2cfe22fb6108b5d8 Mon Sep 17 00:00:00 2001 From: Casper van der Wel Date: Wed, 11 Oct 2023 11:05:37 +0200 Subject: [PATCH 4/5] Fix ApiProvider signature --- clean_python/api_client/api_provider.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/clean_python/api_client/api_provider.py b/clean_python/api_client/api_provider.py index 04e9e73..707836e 100644 --- a/clean_python/api_client/api_provider.py +++ b/clean_python/api_client/api_provider.py @@ -113,8 +113,11 @@ async def _request_with_retry( params: Optional[Json], json: Optional[Json], fields: Optional[Json], + file: Optional[FileFormPost], timeout: float, ) -> ClientResponse: + if file is not None: + raise NotImplementedError("ApiProvider doesn't yet support file uploads") request_kwargs = { "method": method, "url": add_query_params( @@ -149,10 +152,11 @@ async def request( params: Optional[Json] = None, json: Optional[Json] = None, fields: Optional[Json] = None, + file: Optional[FileFormPost] = None, timeout: float = 5.0, ) -> Optional[Json]: response = await self._request_with_retry( - method, path, params, json, fields, timeout + method, path, params, json, fields, file, timeout ) status = HTTPStatus(response.status) content_type = response.headers.get("Content-Type") @@ -175,10 +179,11 @@ async def request_raw( params: Optional[Json] = None, json: Optional[Json] = None, fields: Optional[Json] = None, + file: Optional[FileFormPost] = None, timeout: float = 5.0, ) -> Response: response = await self._request_with_retry( - method, path, params, json, fields, timeout + method, path, params, json, fields, file, timeout ) return Response( status=response.status, From 05aa51f84ed65235dfcf693d285b39954c9b78ed Mon Sep 17 00:00:00 2001 From: Casper van der Wel Date: Wed, 11 Oct 2023 11:07:36 +0200 Subject: [PATCH 5/5] Changes [ci skip] --- CHANGES.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 34c346c..2c976f0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +4,9 @@ 0.6.9 (unreleased) ------------------ -- Disable multpart encoding in `SyncApiProvider`. +- Disable the default multipart encoding in `SyncApiProvider`. + +- Added `file` parameter to `ApiProvider` to upload files (async is a TODO). 0.6.8 (2023-10-10)