diff --git a/mpt_api_client/http/async_client.py b/mpt_api_client/http/async_client.py index d0f74b71..281c55e5 100644 --- a/mpt_api_client/http/async_client.py +++ b/mpt_api_client/http/async_client.py @@ -8,7 +8,9 @@ HTTPStatusError, ) +from mpt_api_client.constants import APPLICATION_JSON from mpt_api_client.exceptions import MPTError, transform_http_status_exception +from mpt_api_client.http.client import json_to_file_payload from mpt_api_client.http.types import ( HeaderTypes, QueryParam, @@ -65,6 +67,8 @@ async def request( # noqa: WPS211 json: Any | None = None, query_params: QueryParam | None = None, headers: HeaderTypes | None = None, + json_file_key: str = "_attachment_data", + force_multipart: bool = False, ) -> Response: """Perform an HTTP request. @@ -75,6 +79,8 @@ async def request( # noqa: WPS211 json: Request JSON data. query_params: Query parameters. headers: Request headers. + json_file_key: json file name for data when sending a multipart request. + force_multipart: force multipart request even if file is not provided. Returns: Response object. @@ -84,6 +90,10 @@ async def request( # noqa: WPS211 MPTApiError: If the response contains an error. MPTHttpError: If the response contains an HTTP error. """ + files = dict(files or {}) + if force_multipart or (files and json): + files[json_file_key] = (None, json_to_file_payload(json), APPLICATION_JSON) + json = None try: response = await self.httpx_client.request( method, diff --git a/mpt_api_client/http/client.py b/mpt_api_client/http/client.py index f623a613..96a49fe5 100644 --- a/mpt_api_client/http/client.py +++ b/mpt_api_client/http/client.py @@ -1,3 +1,4 @@ +import json as json_package import os from typing import Any @@ -8,6 +9,7 @@ HTTPTransport, ) +from mpt_api_client.constants import APPLICATION_JSON from mpt_api_client.exceptions import ( MPTError, transform_http_status_exception, @@ -18,6 +20,16 @@ RequestFiles, Response, ) +from mpt_api_client.models import ResourceData + + +def json_to_file_payload(resource_data: ResourceData | None) -> bytes: + """Convert resource data to file payload.""" + if resource_data is None: + resource_data = {} + return json_package.dumps( + resource_data, ensure_ascii=False, separators=(",", ":"), allow_nan=False + ).encode("utf-8") class HTTPClient: @@ -67,6 +79,8 @@ def request( # noqa: WPS211 json: Any | None = None, query_params: QueryParam | None = None, headers: HeaderTypes | None = None, + json_file_key: str = "_attachment_data", + force_multipart: bool = False, ) -> Response: """Perform an HTTP request. @@ -77,6 +91,8 @@ def request( # noqa: WPS211 json: Request JSON data. query_params: Query parameters. headers: Request headers. + json_file_key: json file name for data when sending a multipart request. + force_multipart: force multipart request even if file is not provided. Returns: Response object. @@ -86,6 +102,10 @@ def request( # noqa: WPS211 MPTApiError: If the response contains an error. MPTHttpError: If the response contains an HTTP error. """ + files = dict(files or {}) + if force_multipart or (files and json): + files[json_file_key] = (None, json_to_file_payload(json), APPLICATION_JSON) + json = None try: response = self.httpx_client.request( method, diff --git a/mpt_api_client/http/mixins.py b/mpt_api_client/http/mixins.py index 66d22dad..eb45c19f 100644 --- a/mpt_api_client/http/mixins.py +++ b/mpt_api_client/http/mixins.py @@ -5,6 +5,7 @@ from mpt_api_client.constants import APPLICATION_JSON from mpt_api_client.exceptions import MPTError +from mpt_api_client.http.client import json_to_file_payload from mpt_api_client.http.query_state import QueryState from mpt_api_client.http.types import FileTypes, Response from mpt_api_client.models import Collection, FileModel, ResourceData @@ -12,12 +13,6 @@ from mpt_api_client.rql import RQLQuery -def _json_to_file_payload(resource_data: ResourceData) -> bytes: - return json.dumps( - resource_data, ensure_ascii=False, separators=(",", ":"), allow_nan=False - ).encode("utf-8") - - class CreateMixin[Model]: """Create resource mixin.""" @@ -110,7 +105,7 @@ def create( if resource_data: files[data_key] = ( None, - _json_to_file_payload(resource_data), + json_to_file_payload(resource_data), APPLICATION_JSON, ) response = self.http_client.request("post", self.path, files=files) # type: ignore[attr-defined] @@ -282,7 +277,7 @@ async def create( if resource_data: files[data_key] = ( None, - _json_to_file_payload(resource_data), + json_to_file_payload(resource_data), APPLICATION_JSON, ) diff --git a/mpt_api_client/resources/catalog/mixins.py b/mpt_api_client/resources/catalog/mixins.py index 5e5bb789..cc4a96c0 100644 --- a/mpt_api_client/resources/catalog/mixins.py +++ b/mpt_api_client/resources/catalog/mixins.py @@ -1,8 +1,6 @@ -from mpt_api_client.constants import APPLICATION_JSON from mpt_api_client.http.mixins import ( AsyncDownloadFileMixin, DownloadFileMixin, - _json_to_file_payload, ) from mpt_api_client.http.types import FileTypes from mpt_api_client.models import ResourceData @@ -83,14 +81,14 @@ async def unpublish(self, resource_id: str, resource_data: ResourceData | None = ) -class AsyncDocumentMixin[Model]( - AsyncDownloadFileMixin[Model], - AsyncPublishableMixin[Model], -): - """Async document mixin.""" +class AsyncCreateFileMixin[Model]: + """Create file mixin.""" + + _upload_file_key = "file" + _upload_data_key = "document" async def create(self, resource_data: ResourceData, file: FileTypes | None = None) -> Model: - """Creates document resource. + """Create document. Creates a document resource by specifying a `file` or an `url`. @@ -100,27 +98,34 @@ async def create(self, resource_data: ResourceData, file: FileTypes | None = Non Returns: Created resource. - """ files = {} - - if resource_data: - files["document"] = ( - None, - _json_to_file_payload(resource_data), - APPLICATION_JSON, - ) if file: - files["file"] = file # type: ignore[assignment] - response = await self.http_client.request("post", self.path, files=files) # type: ignore[attr-defined] + files[self._upload_file_key] = file + response = await self.http_client.request( # type: ignore[attr-defined] + "post", + self.path, # type: ignore[attr-defined] + json=resource_data, + files=files, + json_file_key=self._upload_data_key, + force_multipart=True, + ) return self._model_class.from_response(response) # type: ignore[attr-defined, no-any-return] -class DocumentMixin[Model]( - DownloadFileMixin[Model], - PublishableMixin[Model], +class AsyncDocumentMixin[Model]( + AsyncCreateFileMixin[Model], + AsyncDownloadFileMixin[Model], + AsyncPublishableMixin[Model], ): - """Document mixin.""" + """Async document mixin.""" + + +class CreateFileMixin[Model]: + """Create file mixin.""" + + _upload_file_key = "file" + _upload_data_key = "document" def create(self, resource_data: ResourceData, file: FileTypes | None = None) -> Model: """Create document. @@ -135,19 +140,41 @@ def create(self, resource_data: ResourceData, file: FileTypes | None = None) -> Created resource. """ files = {} - - if resource_data: - files["document"] = ( - None, - _json_to_file_payload(resource_data), - APPLICATION_JSON, - ) if file: - files["file"] = file # type: ignore[assignment] - response = self.http_client.request("post", self.path, files=files) # type: ignore[attr-defined] + files[self._upload_file_key] = file + response = self.http_client.request( # type: ignore[attr-defined] + "post", + self.path, # type: ignore[attr-defined] + json=resource_data, + files=files, + json_file_key=self._upload_data_key, + force_multipart=True, + ) return self._model_class.from_response(response) # type: ignore[attr-defined, no-any-return] +class DocumentMixin[Model]( + CreateFileMixin[Model], + DownloadFileMixin[Model], + PublishableMixin[Model], +): + """Document mixin.""" + + _upload_file_key = "file" + _upload_data_key = "document" + + +class MediaMixin[Model]( + CreateFileMixin[Model], + DownloadFileMixin[Model], + PublishableMixin[Model], +): + """Document mixin.""" + + _upload_file_key = "file" + _upload_data_key = "media" + + class ActivatableMixin[Model]: """Activatable mixin adds the ability to activate and deactivate.""" diff --git a/mpt_api_client/resources/catalog/products_media.py b/mpt_api_client/resources/catalog/products_media.py index 36cb7b27..431f6830 100644 --- a/mpt_api_client/resources/catalog/products_media.py +++ b/mpt_api_client/resources/catalog/products_media.py @@ -8,13 +8,12 @@ AsyncFilesOperationsMixin, AsyncModifiableResourceMixin, CollectionMixin, - FilesOperationsMixin, ModifiableResourceMixin, ) from mpt_api_client.models import Model, ResourceData from mpt_api_client.resources.catalog.mixins import ( AsyncPublishableMixin, - PublishableMixin, + MediaMixin, ) @@ -31,8 +30,7 @@ class MediaServiceConfig: class MediaService( - FilesOperationsMixin[Media], - PublishableMixin[Media], + MediaMixin[Media], ModifiableResourceMixin[Media], CollectionMixin[Media], Service[Media], @@ -40,48 +38,6 @@ class MediaService( ): """Media service.""" - @override - def create( - self, - resource_data: ResourceData | None = None, - files: dict[str, FileTypes] | None = None, - data_key: str = "_media_data", - ) -> Media: - """Create Media resource. - - Currently are two types of media resources available image and video. - - Video: - resource_data: - { - "name": "SomeMediaFile", - "description":"Some media description", - "mediaType": "Video", - "url": http://www.somemedia.com/somevideo.avi, - "displayOrder": 1 - } - files: Add an image with the video thumbnail - - Image: - resource_data: - { - "name": "SomeMediaFile", - "description":"Some media description", - "mediaType": "Video", - "displayOrder": 1 - } - files: The image itself - - Args: - resource_data: Resource data. - files: Files data. - data_key: Key to use for the JSON data in the multipart form. - - Returns: - Media resource. - """ - return super().create(resource_data=resource_data, files=files, data_key=data_key) - class AsyncMediaService( AsyncFilesOperationsMixin[Media], diff --git a/mpt_api_client/resources/notifications/batches.py b/mpt_api_client/resources/notifications/batches.py index 4c3ec1ab..a1f63b4d 100644 --- a/mpt_api_client/resources/notifications/batches.py +++ b/mpt_api_client/resources/notifications/batches.py @@ -1,12 +1,12 @@ from httpx._types import FileTypes from mpt_api_client.http import AsyncService, Service +from mpt_api_client.http.client import json_to_file_payload from mpt_api_client.http.mixins import ( AsyncCollectionMixin, AsyncGetMixin, CollectionMixin, GetMixin, - _json_to_file_payload, ) from mpt_api_client.models import FileModel, Model, ResourceData @@ -52,7 +52,7 @@ def create( if resource_data: files[data_key] = ( None, - _json_to_file_payload(resource_data), + json_to_file_payload(resource_data), "application/json", ) @@ -105,7 +105,7 @@ async def create( if resource_data: files[data_key] = ( None, - _json_to_file_payload(resource_data), + json_to_file_payload(resource_data), "application/json", ) diff --git a/setup.cfg b/setup.cfg index 190eaed5..3d376c36 100644 --- a/setup.cfg +++ b/setup.cfg @@ -39,6 +39,7 @@ per-file-ignores = mpt_api_client/resources/accounts/*.py: WPS202 WPS215 WPS214 WPS235 mpt_api_client/resources/billing/*.py: WPS202 WPS204 WPS214 WPS215 mpt_api_client/resources/catalog/*.py: WPS110 WPS214 WPS215 WPS235 + mpt_api_client/resources/catalog/mixins.py: WPS110 WPS202 WPS214 WPS215 WPS235 mpt_api_client/resources/catalog/products.py: WPS204 WPS214 WPS215 WPS235 mpt_api_client/rql/query_builder.py: WPS110 WPS115 WPS210 WPS214 tests/unit/http/test_async_service.py: WPS204 WPS202 diff --git a/tests/e2e/accounts/conftest.py b/tests/e2e/accounts/conftest.py index 31dbcf61..df2d3ad7 100644 --- a/tests/e2e/accounts/conftest.py +++ b/tests/e2e/accounts/conftest.py @@ -1,5 +1,4 @@ import datetime as dt -import pathlib import pytest @@ -10,9 +9,8 @@ def timestamp(): @pytest.fixture -def account_icon(): - icon_path = pathlib.Path(__file__).parents[1] / "logo.png" - return pathlib.Path.open(icon_path, "rb") +def account_icon(logo_fd): + return logo_fd @pytest.fixture diff --git a/tests/e2e/catalog/product/conftest.py b/tests/e2e/catalog/product/conftest.py index 3350ada9..52383b34 100644 --- a/tests/e2e/catalog/product/conftest.py +++ b/tests/e2e/catalog/product/conftest.py @@ -1,14 +1,6 @@ -import pathlib - import pytest -@pytest.fixture -def product_icon(): - icon_path = pathlib.Path(__file__).parents[2] / "logo.png" - return pathlib.Path.open(icon_path, "rb") - - @pytest.fixture def product_data(): return {"name": "Test Product", "website": "https://www.example.com"} diff --git a/tests/e2e/catalog/product/documents/conftest.py b/tests/e2e/catalog/product/documents/conftest.py index dd6f1bc8..f4badb30 100644 --- a/tests/e2e/catalog/product/documents/conftest.py +++ b/tests/e2e/catalog/product/documents/conftest.py @@ -1,5 +1,3 @@ -import pathlib - import pytest @@ -17,12 +15,6 @@ def document_data(): } -@pytest.fixture -def test_file(): - file_path = pathlib.Path(__file__).parents[3] / "logo.png" - return pathlib.Path.open(file_path, "rb") - - @pytest.fixture def vendor_document_service(mpt_vendor, product_id): return mpt_vendor.catalog.products.documents(product_id) diff --git a/tests/e2e/catalog/product/test_async_product.py b/tests/e2e/catalog/product/test_async_product.py index 0a4f226f..9909910c 100644 --- a/tests/e2e/catalog/product/test_async_product.py +++ b/tests/e2e/catalog/product/test_async_product.py @@ -5,8 +5,8 @@ @pytest.fixture -async def async_created_product(logger, async_mpt_vendor, product_data, product_icon): - product = await async_mpt_vendor.catalog.products.create(product_data, icon=product_icon) +async def async_created_product(logger, async_mpt_vendor, product_data, logo_fd): + product = await async_mpt_vendor.catalog.products.create(product_data, icon=logo_fd) yield product @@ -22,13 +22,13 @@ def test_create_product(async_created_product, product_data): @pytest.mark.flaky -async def test_update_product(async_mpt_vendor, async_created_product, product_icon): +async def test_update_product(async_mpt_vendor, async_created_product, logo_fd): update_data = {"name": "Updated Product"} product = await async_mpt_vendor.catalog.products.update( async_created_product.id, update_data, - icon=product_icon, + icon=logo_fd, ) assert product.name == update_data["name"] diff --git a/tests/e2e/catalog/product/test_sync_product.py b/tests/e2e/catalog/product/test_sync_product.py index d9d0483c..dd5f166e 100644 --- a/tests/e2e/catalog/product/test_sync_product.py +++ b/tests/e2e/catalog/product/test_sync_product.py @@ -5,8 +5,8 @@ @pytest.fixture -def created_product(logger, mpt_vendor, product_data, product_icon): - product = mpt_vendor.catalog.products.create(product_data, icon=product_icon) +def created_product(logger, mpt_vendor, product_data, logo_fd): + product = mpt_vendor.catalog.products.create(product_data, icon=logo_fd) yield product @@ -22,10 +22,10 @@ def test_create_product(created_product, product_data): @pytest.mark.flaky -def test_update_product(mpt_vendor, created_product, product_icon): +def test_update_product(mpt_vendor, created_product, logo_fd): update_data = {"name": "Updated Product"} - product = mpt_vendor.catalog.products.update(created_product.id, update_data, icon=product_icon) + product = mpt_vendor.catalog.products.update(created_product.id, update_data, icon=logo_fd) assert product.name == update_data["name"] diff --git a/tests/e2e/conftest.py b/tests/e2e/conftest.py index 074b6088..b70d3b14 100644 --- a/tests/e2e/conftest.py +++ b/tests/e2e/conftest.py @@ -165,3 +165,9 @@ def api_token_id(e2e_config): @pytest.fixture def invalid_api_token_id(): return "TKN-0000-0000" + + +@pytest.fixture +def logo_fd(): + file_path = pathlib.Path(__file__).parent / "logo.png" + return pathlib.Path.open(file_path, "rb") diff --git a/tests/unit/http/conftest.py b/tests/unit/http/conftest.py index 821eadce..df452bd6 100644 --- a/tests/unit/http/conftest.py +++ b/tests/unit/http/conftest.py @@ -171,3 +171,12 @@ def multiple_results_response(): @pytest.fixture def filter_status_active(): return RQLQuery(status="active") + + +@pytest.fixture +def mock_httpx_response(mocker): + response = mocker.Mock(httpx.Response, autospec=True) + response.headers = {} + response.status_code = 200 + response.content = "" + return response diff --git a/tests/unit/http/test_async_client.py b/tests/unit/http/test_async_client.py index f37f9ba7..995a7b6d 100644 --- a/tests/unit/http/test_async_client.py +++ b/tests/unit/http/test_async_client.py @@ -1,3 +1,4 @@ +import io import json import pytest @@ -86,3 +87,43 @@ async def test_async_http_call_failure(async_http_client): await async_http_client.request("GET", "/timeout") assert timeout_route.called + + +async def test_http_call_with_json_and_files(mocker, async_http_client, mock_httpx_response): # noqa: WPS210 + json_data = {"foo": "bar"} + files = {"file": ("test.txt", io.StringIO("file content"), "text/plain")} + + parent_request = mocker.patch.object( + async_http_client.httpx_client, "request", autospec=True, return_value=mock_httpx_response + ) + await async_http_client.request("POST", "/upload", files=files, json=json_data) + + called_kwargs = parent_request.call_args[1] + assert called_kwargs["json"] is None + sent_files = called_kwargs["files"] + assert "file" in sent_files + assert "_attachment_data" in sent_files + + payload_tuple = sent_files["_attachment_data"] + assert payload_tuple[2] == "application/json" + assert payload_tuple[1].decode() == '{"foo":"bar"}' + + +async def test_http_call_force_multipart(mocker, async_http_client, mock_httpx_response): # noqa: WPS210 + json_data = {"foo": "bar"} + + parent_request = mocker.patch.object( + async_http_client.httpx_client, "request", autospec=True, return_value=mock_httpx_response + ) + + await async_http_client.request("POST", "/upload", json=json_data, force_multipart=True) + + called_kwargs = parent_request.call_args[1] + sent_files = called_kwargs["files"] + + assert called_kwargs["json"] is None + assert "_attachment_data" in sent_files + + payload_tuple = sent_files["_attachment_data"] + assert payload_tuple[2] == "application/json" + assert payload_tuple[1].decode() == '{"foo":"bar"}' diff --git a/tests/unit/http/test_client.py b/tests/unit/http/test_client.py index 6b4613f0..a2e54d4c 100644 --- a/tests/unit/http/test_client.py +++ b/tests/unit/http/test_client.py @@ -1,3 +1,4 @@ +import io import json import pytest @@ -76,3 +77,39 @@ def test_http_call_failure(http_client): http_client.request("GET", "/timeout") assert timeout_route.called + + +def test_http_call_with_json_and_files(mocker, http_client, mock_httpx_response): + files = {"file": ("test.txt", io.StringIO("file content"), "text/plain")} + parent_request = mocker.patch.object( + http_client.httpx_client, "request", autospec=True, return_value=mock_httpx_response + ) + + http_client.request("POST", "/upload", files=files, json={"foo": "bar"}) + + called_kwargs = parent_request.call_args[1] + assert called_kwargs["json"] is None + sent_files = called_kwargs["files"] + assert "file" in sent_files + assert "_attachment_data" in sent_files + + payload_tuple = sent_files["_attachment_data"] + assert payload_tuple[2] == "application/json" + assert payload_tuple[1].decode() == '{"foo":"bar"}' + + +def test_http_call_force_multipart(mocker, http_client): + json_data = {"foo": "bar"} + parent_request = mocker.patch.object(http_client.httpx_client, "request", autospec=True) + + http_client.request("POST", "/upload", json=json_data, force_multipart=True) + + called_kwargs = parent_request.call_args[1] + sent_files = called_kwargs["files"] + + assert called_kwargs["json"] is None + assert "_attachment_data" in sent_files + + payload_tuple = sent_files["_attachment_data"] + assert payload_tuple[2] == "application/json" + assert payload_tuple[1].decode() == '{"foo":"bar"}' diff --git a/tests/unit/http/test_mixins.py b/tests/unit/http/test_mixins.py index 5e801dbd..6916195c 100644 --- a/tests/unit/http/test_mixins.py +++ b/tests/unit/http/test_mixins.py @@ -374,18 +374,18 @@ def test_sync_file_create_with_data(media_service): json=media_data, ) ) - files = {"media": ("test.jpg", io.BytesIO(b"Image content"), "image/jpeg")} - new_media = media_service.create({"name": "Product image"}, files=files) + image_file = ("test.jpg", io.BytesIO(b"Image content"), "image/jpeg") + new_media = media_service.create({"name": "Product image"}, image_file) request: httpx.Request = mock_route.calls[0].request assert ( - b'Content-Disposition: form-data; name="_media_data"\r\n' + b'Content-Disposition: form-data; name="media"\r\n' b"Content-Type: application/json\r\n\r\n" b'{"name":"Product image"}\r\n' in request.content ) assert ( - b'Content-Disposition: form-data; name="media"; filename="test.jpg"\r\n' + b'Content-Disposition: form-data; name="file"; filename="test.jpg"\r\n' b"Content-Type: image/jpeg\r\n\r\n" b"Image content\r\n" in request.content ) @@ -404,13 +404,13 @@ def test_sync_file_create_no_data(media_service): json=media_data, ) ) - files = {"media": ("test.jpg", io.BytesIO(b"Image content"), "image/jpeg")} - new_media = media_service.create(files=files) + image_file = ("test.jpg", io.BytesIO(b"Image content"), "image/jpeg") + new_media = media_service.create({}, image_file) request: httpx.Request = mock_route.calls[0].request assert ( - b'Content-Disposition: form-data; name="media"; filename="test.jpg"\r\n' + b'Content-Disposition: form-data; name="file"; filename="test.jpg"\r\n' b"Content-Type: image/jpeg\r\n\r\n" b"Image content\r\n" in request.content )