Skip to content

Commit dcf6f68

Browse files
authored
Merge pull request #425
MPT-25117 Declare the journal upload content type
2 parents a5f948a + d8bd042 commit dcf6f68

9 files changed

Lines changed: 335 additions & 64 deletions

File tree

‎docs/architecture.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ mpt_api_client/
3232
│ ├── async_service.py # AsyncService — async service (extends ServiceBase)
3333
│ ├── query_state.py # Query parameter accumulation
3434
│ ├── client_utils.py # URL validation helpers
35+
│ ├── file_utils.py # Upload content-type declaration
3536
│ ├── types.py # Type aliases (Response, HeaderTypes, etc.)
3637
│ ├── json_envelope_parser.py # Incremental {$meta, data} envelope parsing
3738
│ ├── jsonl_lines.py # JSONL record-line splitting (newlines only)
@@ -218,7 +219,9 @@ class ProductsService(
218219
- base URL resolution
219220
- retry transport (configurable)
220221
- error transformation into `MPTHttpError` / `MPTAPIError`
221-
- multipart file upload support
222+
- multipart file upload support; `http/file_utils.py` declares a content type for an
223+
upload whose extension `mimetypes` does not recognise, such as the `.jsonl` a billing
224+
journal carries, which httpx would otherwise send as `application/octet-stream`
222225

223226
Transport-level settings (`base_url`, `timeout`, `retries`) are grouped in the
224227
`TransportSettings` dataclass (`http/transport_settings.py`), passed to the client

‎mpt_api_client/constants.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
APPLICATION_JSON = "application/json"
22
APPLICATION_JSONL = "application/jsonl"
3+
APPLICATION_X_NDJSON = "application/x-ndjson"
34
MIMETYPE_EXCEL_XLSX = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"
45

56
MPT_STREAMING_HEADER = "MPT-Streaming"

‎mpt_api_client/http/file_utils.py‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
from pathlib import Path
2+
3+
from mpt_api_client.http.types import FileContent, FileTypes
4+
5+
DEFAULT_UPLOAD_FILENAME = "upload"
6+
7+
8+
def _upload_filename(payload: FileContent) -> str:
9+
"""Derive the multipart filename the way httpx does for content passed without one.
10+
11+
Only a file object opened from a path carries a usable ``name``; ``bytes``, ``str`` and
12+
``io.BytesIO`` have none, and a file opened from a descriptor names an ``int``. httpx
13+
falls back to ``upload`` in each of those cases, so this does too. The basename keeps an
14+
absolute path out of the multipart part.
15+
"""
16+
name = getattr(payload, "name", DEFAULT_UPLOAD_FILENAME)
17+
18+
return Path(str(name)).name or DEFAULT_UPLOAD_FILENAME
19+
20+
21+
def declare_content_type(file_part: FileTypes, content_type: str) -> FileTypes:
22+
"""Declare a content type on a file part unless the caller already declared one.
23+
24+
httpx derives the content type of an upload from the filename extension and falls back
25+
to ``application/octet-stream`` whenever :mod:`mimetypes` does not recognise it. The
26+
API rejects that fallback, so endpoints whose payload extension is unknown to
27+
:mod:`mimetypes`, such as ``.jsonl``, have to name their own type.
28+
29+
Args:
30+
file_part: File part in any of the shapes httpx accepts, which fix the content type
31+
at the third position: bare content, ``(filename, content)``,
32+
``(filename, content, content_type)`` or
33+
``(filename, content, content_type, headers)``.
34+
content_type: Content type to declare when the file part carries none.
35+
36+
Returns:
37+
FileTypes: File part with a content type declared.
38+
"""
39+
if not isinstance(file_part, tuple):
40+
return _upload_filename(file_part), file_part, content_type
41+
42+
filename, payload, *type_and_headers = file_part
43+
declared_type = type_and_headers[0] if type_and_headers else None
44+
headers = type_and_headers[1:]
45+
46+
return (filename, payload, declared_type or content_type, *headers)

‎mpt_api_client/resources/billing/journals.py‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
from mpt_api_client.constants import APPLICATION_X_NDJSON
12
from mpt_api_client.http import AsyncService, Service
3+
from mpt_api_client.http.file_utils import declare_content_type
24
from mpt_api_client.http.mixins import (
35
AsyncCollectionMixin,
46
AsyncManagedResourceMixin,
@@ -52,6 +54,9 @@ def upload(
5254
) -> Journal | ModelCollection[Journal]: # noqa: WPS110
5355
"""Upload journal file.
5456
57+
The journal file holds JSON Lines. Unless the caller declares a content type of its
58+
own, the file is sent as ``application/x-ndjson``, which the endpoint accepts.
59+
5560
Args:
5661
journal_id: Journal ID.
5762
file: journal file.
@@ -60,24 +65,22 @@ def upload(
6065
Journal | ModelCollection[Journal]: The uploaded resource as a single Journal
6166
instance, or a ModelCollection[Journal] when the response contains multiple records.
6267
"""
63-
files = {}
68+
files: dict[str, FileTypes] = {}
6469

6570
if file:
66-
files[self._upload_file_key] = file # UNUSED type: ignore[attr-defined]
67-
files[self._upload_data_key] = journal_id # UNUSED type: ignore
71+
files[self._upload_file_key] = declare_content_type(file, APPLICATION_X_NDJSON)
72+
files[self._upload_data_key] = (None, journal_id)
6873

6974
path = join_url_path(self.path, journal_id, "upload")
7075

71-
response = self.http_client.request( # UNUSED type: ignore[attr-defined]
76+
response = self.http_client.request(
7277
"post",
73-
path, # UNUSED type: ignore[attr-defined]
78+
path,
7479
files=files,
7580
force_multipart=True,
7681
)
7782

78-
return self._model_class.from_response(
79-
response
80-
) # UNUSED type: ignore[attr-defined, no-any-return]
83+
return self._model_class.from_response(response)
8184

8285
def attachments(self, journal_id: str) -> JournalAttachmentsService:
8386
"""Return journal attachments service."""
@@ -113,6 +116,9 @@ async def upload(
113116
) -> Journal | ModelCollection[Journal]: # noqa: WPS110
114117
"""Upload journal file.
115118
119+
The journal file holds JSON Lines. Unless the caller declares a content type of its
120+
own, the file is sent as ``application/x-ndjson``, which the endpoint accepts.
121+
116122
Args:
117123
journal_id: Journal ID.
118124
file: journal file.
@@ -121,24 +127,22 @@ async def upload(
121127
Journal | ModelCollection[Journal]: The uploaded resource as a single Journal
122128
instance, or a ModelCollection[Journal] when the response contains multiple records.
123129
"""
124-
files = {}
130+
files: dict[str, FileTypes] = {}
125131

126132
if file:
127-
files[self._upload_file_key] = file # UNUSED type: ignore[attr-defined]
128-
files[self._upload_data_key] = journal_id # UNUSED type: ignore
133+
files[self._upload_file_key] = declare_content_type(file, APPLICATION_X_NDJSON)
134+
files[self._upload_data_key] = (None, journal_id)
129135

130136
path = join_url_path(self.path, journal_id, "upload")
131137

132-
response = await self.http_client.request( # UNUSED type: ignore[attr-defined]
138+
response = await self.http_client.request(
133139
"post",
134-
path, # UNUSED type: ignore[attr-defined]
140+
path,
135141
files=files,
136142
force_multipart=True,
137143
)
138144

139-
return self._model_class.from_response(
140-
response
141-
) # UNUSED type: ignore[attr-defined, no-any-return]
145+
return self._model_class.from_response(response)
142146

143147
def attachments(self, journal_id: str) -> AsyncJournalAttachmentsService:
144148
"""Return journal attachments service."""

‎tests/e2e/billing/conftest.py‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,95 @@
1+
import asyncio
12
import pathlib
3+
import time
24

35
import pytest
46

7+
from mpt_api_client.exceptions import MPTError
8+
9+
# A journal accepts DELETE only in these states; the API answers 400 for any other.
10+
DELETABLE_STATUSES = frozenset(("Draft", "Validated", "Error", "Enquiring"))
11+
# Uploading charges moves a journal through Validating for a second or so before it
12+
# settles. A journal that has been accepted and completed never becomes deletable.
13+
TRANSIENT_STATUSES = frozenset(("Validating", "Generating"))
14+
# A test that deletes the journal itself leaves nothing for the fixture to clean up.
15+
ALREADY_GONE_STATUSES = frozenset(("Deleted",))
16+
SETTLE_TIMEOUT_SECONDS = 10
17+
POLL_INTERVAL_SECONDS = 0.5
18+
19+
20+
def _report_teardown(message):
21+
print(f"TEARDOWN - {message}") # noqa: WPS421
22+
23+
24+
def _settled_status(journals_service, journal_id):
25+
deadline = time.monotonic() + SETTLE_TIMEOUT_SECONDS
26+
status = journals_service.get(journal_id).status
27+
while status in TRANSIENT_STATUSES and time.monotonic() < deadline:
28+
time.sleep(POLL_INTERVAL_SECONDS)
29+
status = journals_service.get(journal_id).status
30+
31+
return status
32+
33+
34+
def _delete_journal(journals_service, journal_id):
35+
try:
36+
status = _settled_status(journals_service, journal_id)
37+
except MPTError as error:
38+
_report_teardown(f"could not read billing journal {journal_id}: {error}")
39+
return
40+
41+
if status in ALREADY_GONE_STATUSES:
42+
return
43+
44+
if status not in DELETABLE_STATUSES:
45+
_report_teardown(f"billing journal {journal_id} not deletable in status {status}")
46+
return
47+
48+
try:
49+
journals_service.delete(journal_id)
50+
except MPTError as error:
51+
_report_teardown(f"could not delete billing journal {journal_id}: {error}")
52+
53+
54+
async def _async_settled_status(journals_service, journal_id):
55+
deadline = time.monotonic() + SETTLE_TIMEOUT_SECONDS
56+
journal = await journals_service.get(journal_id)
57+
while journal.status in TRANSIENT_STATUSES and time.monotonic() < deadline:
58+
await asyncio.sleep(POLL_INTERVAL_SECONDS)
59+
journal = await journals_service.get(journal_id)
60+
61+
return journal.status
62+
63+
64+
async def _async_delete_journal(journals_service, journal_id):
65+
try:
66+
status = await _async_settled_status(journals_service, journal_id)
67+
except MPTError as error:
68+
_report_teardown(f"could not read billing journal {journal_id}: {error}")
69+
return
70+
71+
if status in ALREADY_GONE_STATUSES:
72+
return
73+
74+
if status not in DELETABLE_STATUSES:
75+
_report_teardown(f"billing journal {journal_id} not deletable in status {status}")
76+
return
77+
78+
try:
79+
await journals_service.delete(journal_id)
80+
except MPTError as error:
81+
_report_teardown(f"could not delete billing journal {journal_id}: {error}")
82+
83+
84+
@pytest.fixture
85+
def delete_billing_journal():
86+
return _delete_journal
87+
88+
89+
@pytest.fixture
90+
def async_delete_billing_journal():
91+
return _async_delete_journal
92+
593

694
@pytest.fixture
795
def billing_journal_fd():

‎tests/e2e/billing/journal/test_async_journal.py‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77

88

99
@pytest.fixture
10-
async def created_billing_journal(async_mpt_vendor, billing_journal_factory):
10+
async def created_billing_journal(
11+
async_mpt_vendor, billing_journal_factory, async_delete_billing_journal
12+
):
1113
new_billing_journal_request_data = billing_journal_factory(
1214
name="E2E Created Billing Journal",
1315
)
@@ -18,10 +20,9 @@ async def created_billing_journal(async_mpt_vendor, billing_journal_factory):
1820

1921
yield created_billing_journal
2022

21-
try:
22-
await async_mpt_vendor.billing.journals.delete(created_billing_journal.id)
23-
except MPTAPIError as error:
24-
print(f"TEARDOWN - Unable to delete billing journal: {error.title}") # noqa: WPS421
23+
await async_delete_billing_journal(
24+
async_mpt_vendor.billing.journals, created_billing_journal.id
25+
)
2526

2627

2728
@pytest.fixture

‎tests/e2e/billing/journal/test_sync_journal.py‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88

99
@pytest.fixture
10-
def created_billing_journal(mpt_vendor, billing_journal_factory):
10+
def created_billing_journal(mpt_vendor, billing_journal_factory, delete_billing_journal):
1111
new_billing_journal_request_data = billing_journal_factory(
1212
name="E2E Created Billing Journal",
1313
)
@@ -16,10 +16,7 @@ def created_billing_journal(mpt_vendor, billing_journal_factory):
1616

1717
yield created_billing_journal
1818

19-
try:
20-
mpt_vendor.billing.journals.delete(created_billing_journal.id)
21-
except MPTAPIError as error:
22-
print(f"TEARDOWN - Unable to delete billing journal: {error.title}") # noqa: WPS421
19+
delete_billing_journal(mpt_vendor.billing.journals, created_billing_journal.id)
2320

2421

2522
@pytest.fixture

‎tests/unit/http/test_file_utils.py‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import io
2+
import os
3+
4+
import pytest
5+
6+
from mpt_api_client.http.file_utils import declare_content_type
7+
8+
NDJSON = "application/x-ndjson"
9+
CSV = "text/csv"
10+
11+
12+
@pytest.mark.parametrize(
13+
("file_part", "expected"),
14+
[
15+
pytest.param(
16+
b'{"id": 1}\n',
17+
("upload", b'{"id": 1}\n', NDJSON),
18+
id="bytes without a name",
19+
),
20+
pytest.param(
21+
("journal.jsonl", b'{"id": 1}\n'),
22+
("journal.jsonl", b'{"id": 1}\n', NDJSON),
23+
id="filename and content",
24+
),
25+
pytest.param(
26+
("journal.jsonl", b'{"id": 1}\n', None),
27+
("journal.jsonl", b'{"id": 1}\n', NDJSON),
28+
id="content type left unset",
29+
),
30+
pytest.param(
31+
("journal.jsonl", b'{"id": 1}\n', CSV),
32+
("journal.jsonl", b'{"id": 1}\n', CSV),
33+
id="content type declared by the caller",
34+
),
35+
pytest.param(
36+
("journal.jsonl", b'{"id": 1}\n', ""),
37+
("journal.jsonl", b'{"id": 1}\n', NDJSON),
38+
id="blank content type treated as absent",
39+
),
40+
pytest.param(
41+
("journal.jsonl", b'{"id": 1}\n', None, {"X-Trace": "1"}),
42+
("journal.jsonl", b'{"id": 1}\n', NDJSON, {"X-Trace": "1"}),
43+
id="extra headers preserved",
44+
),
45+
pytest.param(
46+
(None, b'{"id": 1}\n'),
47+
(None, b'{"id": 1}\n', NDJSON),
48+
id="form field without a filename",
49+
),
50+
],
51+
)
52+
def test_declare_content_type(file_part, expected):
53+
result = declare_content_type(file_part, NDJSON)
54+
55+
assert result == expected
56+
57+
58+
def test_filename_taken_from_file_object(tmp_path):
59+
file_path = tmp_path / "journal.jsonl"
60+
file_path.write_bytes(b'{"id": 1}\n')
61+
62+
with file_path.open("rb") as file_obj:
63+
result = declare_content_type(file_obj, NDJSON)
64+
65+
assert result == ("journal.jsonl", file_obj, NDJSON)
66+
67+
68+
def test_filename_falls_back_without_name():
69+
file_obj = io.BytesIO(b'{"id": 1}\n')
70+
71+
result = declare_content_type(file_obj, NDJSON)
72+
73+
assert result == ("upload", file_obj, NDJSON)
74+
75+
76+
def test_filename_from_file_descriptor(tmp_path):
77+
file_path = tmp_path / "journal.jsonl"
78+
file_path.write_bytes(b'{"id": 1}\n')
79+
descriptor = os.open(file_path, os.O_RDONLY)
80+
81+
with os.fdopen(descriptor, "rb") as file_obj:
82+
result = declare_content_type(file_obj, NDJSON)
83+
84+
assert result == (str(descriptor), file_obj, NDJSON)

0 commit comments

Comments
 (0)