Repository navigation
Commit f2b5e4f
chore: use a dataclass to return values from prepare_send_data
I found the tuple of three values confusing. So instead use a
dataclass to return the three values. It is still confusing but a
little bit less so.
Also add some unit tests1 parent d387d91 commit f2b5e4f
4 files changed
Lines changed: 62 additions & 15 deletions
File tree
- gitlab
- _backends
- tests/unit/_backends
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,6 +1,7 @@ | |||
| 1 | 1 | from __future__ import annotations | |
| 2 | 2 | ||
| 3 | - from typing import Any, Dict, Optional, Tuple, TYPE_CHECKING, Union | ||
| 3 | + import dataclasses | ||
| 4 | + from typing import Any, Dict, Optional, TYPE_CHECKING, Union | ||
| 4 | 5 | ||
| 5 | 6 | import requests | |
| 6 | 7 | from requests.structures import CaseInsensitiveDict | |
@@ -9,6 +10,20 @@ | |||
| 9 | 10 | from . import protocol | |
| 10 | 11 | ||
| 11 | 12 | ||
| 13 | + @dataclasses.dataclass | ||
| 14 | + class SendData: | ||
| 15 | + content_type: str | ||
| 16 | + data: Optional[Union[Dict[str, Any], MultipartEncoder]] = None | ||
| 17 | + json: Optional[Union[Dict[str, Any], bytes]] = None | ||
| 18 | + | ||
| 19 | + def __post_init__(self) -> None: | ||
| 20 | + if self.json is not None and self.data is not None: | ||
| 21 | + raise ValueError( | ||
| 22 | + f"`json` and `data` are mutually exclusive. Only one can be set. " | ||
| 23 | + f"json={self.json!r} data={self.data!r}" | ||
| 24 | + ) | ||
| 25 | + | ||
| 26 | + | ||
| 12 | 27 | class RequestsResponse(protocol.BackendResponse): | |
| 13 | 28 | def __init__(self, response: requests.Response) -> None: | |
| 14 | 29 | self._response: requests.Response = response | |
@@ -50,11 +65,7 @@ def prepare_send_data( | |||
| 50 | 65 | files: Optional[Dict[str, Any]] = None, | |
| 51 | 66 | post_data: Optional[Union[Dict[str, Any], bytes]] = None, | |
| 52 | 67 | raw: bool = False, | |
| 53 | - ) -> Tuple[ | ||
| 54 | - Optional[Union[Dict[str, Any], bytes]], | ||
| 55 | - Optional[Union[Dict[str, Any], MultipartEncoder]], | ||
| 56 | - str, | ||
| 57 | - ]: | ||
| 68 | + ) -> SendData: | ||
| 58 | 69 | if files: | |
| 59 | 70 | if post_data is None: | |
| 60 | 71 | post_data = {} | |
@@ -70,12 +81,12 @@ def prepare_send_data( | |||
| 70 | 81 | post_data["avatar"] = files.get("avatar") | |
| 71 | 82 | ||
| 72 | 83 | data = MultipartEncoder(post_data) | |
| 73 | - return (None, data, data.content_type) | ||
| 84 | + return SendData(data=data, content_type=data.content_type) | ||
| 74 | 85 | ||
| 75 | 86 | if raw and post_data: | |
| 76 | - return (None, post_data, "application/octet-stream") | ||
| 87 | + return SendData(data=post_data, content_type="application/octet-stream") | ||
| 77 | 88 | ||
| 78 | - return (post_data, None, "application/json") | ||
| 89 | + return SendData(json=post_data, content_type="application/json") | ||
| 79 | 90 | ||
| 80 | 91 | def http_request( | |
| 81 | 92 | self, | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -716,19 +716,17 @@ def http_request( | |||
| 716 | 716 | retry_transient_errors = self.retry_transient_errors | |
| 717 | 717 | ||
| 718 | 718 | # We need to deal with json vs. data when uploading files | |
| 719 | - json, data, content_type = self._backend.prepare_send_data( | ||
| 720 | - files, post_data, raw | ||
| 721 | - ) | ||
| 722 | - opts["headers"]["Content-type"] = content_type | ||
| 719 | + send_data = self._backend.prepare_send_data(files, post_data, raw) | ||
| 720 | + opts["headers"]["Content-type"] = send_data.content_type | ||
| 723 | 721 | ||
| 724 | 722 | cur_retries = 0 | |
| 725 | 723 | while True: | |
| 726 | 724 | try: | |
| 727 | 725 | result = self._backend.http_request( | |
| 728 | 726 | method=verb, | |
| 729 | 727 | url=url, | |
| 730 | - json=json, | ||
| 731 | - data=data, | ||
| 728 | + json=send_data.json, | ||
| 729 | + data=send_data.data, | ||
| 732 | 730 | params=params, | |
| 733 | 731 | timeout=timeout, | |
| 734 | 732 | verify=verify, | |
Whitespace-only changes.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,38 @@ | |||
| 1 | + import pytest | ||
| 2 | + from requests_toolbelt.multipart.encoder import MultipartEncoder # type: ignore | ||
| 3 | + | ||
| 4 | + from gitlab._backends import requests_backend | ||
| 5 | + | ||
| 6 | + | ||
| 7 | + class TestSendData: | ||
| 8 | + def test_senddata_json(self) -> None: | ||
| 9 | + result = requests_backend.SendData( | ||
| 10 | + json={"a": 1}, content_type="application/json" | ||
| 11 | + ) | ||
| 12 | + assert result.data is None | ||
| 13 | + | ||
| 14 | + def test_senddata_data(self) -> None: | ||
| 15 | + result = requests_backend.SendData( | ||
| 16 | + data={"b": 2}, content_type="application/octet-stream" | ||
| 17 | + ) | ||
| 18 | + assert result.json is None | ||
| 19 | + | ||
| 20 | + def test_senddata_json_and_data(self) -> None: | ||
| 21 | + with pytest.raises(ValueError, match=r"json={'a': 1} data={'b': 2}"): | ||
| 22 | + requests_backend.SendData( | ||
| 23 | + json={"a": 1}, data={"b": 2}, content_type="application/json" | ||
| 24 | + ) | ||
| 25 | + | ||
| 26 | + | ||
| 27 | + class TestRequestsBackend: | ||
| 28 | + def test_prepare_send_data_str_parentid(self) -> None: | ||
| 29 | + file = "12345" | ||
| 30 | + files = {"file": ("file.tar.gz", file, "application/octet-stream")} | ||
| 31 | + post_data = {"parent_id": "12"} | ||
| 32 | + | ||
| 33 | + result = requests_backend.RequestsBackend.prepare_send_data( | ||
| 34 | + files=files, post_data=post_data, raw=False | ||
| 35 | + ) | ||
| 36 | + assert result.json is None | ||
| 37 | + assert result.content_type.startswith("multipart/form-data") | ||
| 38 | + assert isinstance(result.data, MultipartEncoder) | ||
0 commit comments