Skip to content

Commit d8bd042

Browse files
jentykclaude
andcommitted
fix(billing): declare the journal upload content type
JournalsService.upload and its async twin passed the caller's file straight to httpx without declaring a content type. httpx derives one from the filename extension, and mimetypes does not know .jsonl, so the part went out as application/octet-stream and the API answered 400 "Unsupported file type". Journal upload was therefore broken for every caller that passes a plain file object, which is exactly what the client's own e2e tests do. Probing the endpoint on TEST with both a single-record and a multi-record .jsonl file: application/octet-stream and text/plain are rejected, while application/x-ndjson, application/jsonl and application/json are accepted. The upload now declares application/x-ndjson unless the caller declares a type of its own. x-ndjson is the name the server's own sniffer detects for this format and the type MPT-24953 settled on for the AWS extension. The selection lives in mpt_api_client/http/file_utils.py, which fills in a content type across every shape FileTypes admits, so other endpoints whose payload extension is unknown to mimetypes can reuse it. The same request also placed journal_id in files= as a bare string, so httpx encoded it as a file part with the synthetic filename "upload" instead of a plain form field. Passing (None, journal_id) emits a real form field. The server tolerated the old shape, so that part is a correctness fix rather than a bug fix. Dead "UNUSED type: ignore" comments on the touched lines are removed; they suppressed nothing, because mypy only reads an ignore that starts the comment. Fixing the upload exposed a leak in the e2e fixtures. While the upload failed, the journal stayed in Draft and the teardown delete succeeded; now that it succeeds, the journal moves to Validating and the API rejects the delete with "Journal must be in Draft or Validated or Error or Enquiring status", so every run stranded journals on TEST. The teardown now waits for validation to settle - measured at about 1.5s to reach Validated - before deleting, skips a journal a test already deleted, and reports one that can never be deleted instead of retrying. The helpers are factory fixtures in tests/e2e/billing/conftest.py because that package has no __init__.py to import them through. tests/unit/http/test_file_utils.py covers the content-type selection over every FileTypes shape, and the journals tests assert the emitted multipart headers for the file part and for the id field on both the sync and async services. Against TEST, reverting only journals.py reproduces 2 failed / 42 passed in tests/e2e/billing/journal, and the fix gives 44 passed; the full tests/e2e/billing suite is 158 passed with no journal left behind. docs/architecture.md lists the new module and notes why an upload declares its own content type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent a5f948a commit d8bd042

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)