Skip to content

Commit 4adbe4e

Browse files
Enhance file upload functionality in clients and tests
- Simplified `UploadFiles` types and updated handling for single and sequence-based file specifications. - Replaced mapping-based file upload support with streamlined normalization logic, because the target field is always "file". - Introduced `_normalize_path_inputs` for consistent path handling in `create_from_paths`. - Updated file creation flows to use the refined file upload logic. - Added and extended test cases to cover updated behaviors and edge cases. Assisted-by: Codex
1 parent a2cd781 commit 4adbe4e

2 files changed

Lines changed: 137 additions & 79 deletions

File tree

src/pdfrest/client.py

Lines changed: 56 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,13 @@
4444
FileTuple3 = tuple[str | None, FileContent, str | None]
4545
FileTuple4 = tuple[str | None, FileContent, str | None, Mapping[str, str]]
4646
FileTypes = FileContent | FileTuple2 | FileTuple3 | FileTuple4
47-
UploadFiles = Mapping[str, FileTypes] | Sequence[tuple[str, FileTypes]]
47+
UploadFiles = Sequence[FileTypes] | FileTypes
4848

4949
FilePath = str | PathLike[str]
5050
FilePathTuple2 = tuple[FilePath, str | None]
5151
FilePathTuple3 = tuple[FilePath, str | None, Mapping[str, str]]
5252
FilePathTypes = FilePath | FilePathTuple2 | FilePathTuple3
53+
FilePathInput = FilePathTypes | Sequence[FilePathTypes]
5354
NormalizedFileTypes: TypeAlias = FileContent | FileTuple2 | FileTuple3 | FileTuple4
5455

5556

@@ -128,26 +129,24 @@ def _normalize_file_type(file_value: FileTypes) -> NormalizedFileTypes:
128129
def _normalize_upload_files(
129130
files: UploadFiles,
130131
) -> list[tuple[str, NormalizedFileTypes]]:
131-
is_mapping = isinstance(files, Mapping)
132-
if is_mapping:
133-
mapping_files = cast(Mapping[str, FileTypes], files)
134-
items: list[tuple[str, FileTypes]] = list(mapping_files.items())
132+
if isinstance(files, Mapping):
133+
msg = "Upload files must be provided as a sequence or a single file specification."
134+
raise TypeError(msg)
135+
136+
if isinstance(files, Sequence) and not isinstance(files, (str, bytes, bytearray)):
137+
items = list(files)
135138
else:
136-
sequence_files = cast(Sequence[tuple[str, FileTypes]], files)
137-
items = list(sequence_files)
139+
# Treat single file specification as a one-element sequence.
140+
items = [cast(FileTypes, files)]
141+
138142
if not items:
139143
msg = "At least one file must be provided."
140144
raise ValueError(msg)
141145
normalized_items: list[tuple[str, NormalizedFileTypes]] = []
142-
for entry in items:
143-
if is_mapping:
144-
field_name, file_value = entry
145-
else:
146-
if not isinstance(entry, tuple) or len(entry) != 2:
147-
msg = "Files sequence entries must be (field_name, file_value) tuples."
148-
raise TypeError(msg)
149-
field_name, file_value = entry
150-
normalized_items.append((str(field_name), _normalize_file_type(file_value)))
146+
for file_value in items:
147+
normalized_items.append(
148+
(FILE_UPLOAD_FIELD_NAME, _normalize_file_type(cast(FileTypes, file_value)))
149+
)
151150
return normalized_items
152151

153152

@@ -175,6 +174,22 @@ def _parse_path_spec(spec: FilePathTypes) -> tuple[Path, str | None, Mapping[str
175174
return path, None, {}
176175

177176

177+
def _normalize_path_inputs(
178+
file_paths: FilePathInput,
179+
) -> list[FilePathTypes]:
180+
if isinstance(file_paths, Sequence) and not isinstance(
181+
file_paths, (str, bytes, bytearray)
182+
):
183+
sequence_paths = cast(Sequence[FilePathTypes], file_paths)
184+
items: list[FilePathTypes] = list(sequence_paths)
185+
else:
186+
items = [cast(FilePathTypes, file_paths)]
187+
if not items:
188+
msg = "At least one file path must be provided."
189+
raise ValueError(msg)
190+
return items
191+
192+
178193
ClientType = TypeVar("ClientType", httpx.Client, httpx.AsyncClient)
179194

180195

@@ -621,13 +636,11 @@ def __init__(self, client: _SyncApiClient) -> None:
621636
self._client = client
622637

623638
def create(self, files: UploadFiles) -> list[PdfRestFile]:
624-
"""Upload one or more files by content, in the same style accepted by
625-
the `files` parameter of `httpx.Client.post`.
639+
"""Upload one or more files by content.
626640
627-
Provide either a mapping of field names to file specifications, or a
628-
sequence of `(field_name, file_spec)` tuples. File specifications may be
629-
raw file-like objects, bytes, str, or the tuple forms documented by
630-
httpx.
641+
Provide either a single file specification or a sequence of file
642+
specifications (each matching the shapes accepted by httpx). Every
643+
uploaded part is sent using the field name ``file``.
631644
"""
632645
normalized_files = _normalize_upload_files(files)
633646
request = self._client.prepare_request(
@@ -637,42 +650,29 @@ def create(self, files: UploadFiles) -> list[PdfRestFile]:
637650
file_ids = _extract_uploaded_file_ids(payload)
638651
return [self._client.fetch_file_info(file_id) for file_id in file_ids]
639652

640-
def create_from_paths(
641-
self, file_paths: Sequence[FilePathTypes]
642-
) -> list[PdfRestFile]:
653+
def create_from_paths(self, file_paths: FilePathInput) -> list[PdfRestFile]:
643654
"""Upload one or more files by their path.
644655
645656
Each entry may be a bare path-like object or a tuple of
646657
`(path, content_type)` / `(path, content_type, headers)` where headers
647658
mirrors the httpx multipart header mapping. All opened file handles are
648659
closed once the request completes.
649660
"""
650-
if not file_paths:
651-
msg = "At least one file path must be provided."
652-
raise ValueError(msg)
661+
normalized_paths = _normalize_path_inputs(file_paths)
653662

654663
with ExitStack() as stack:
655-
upload_entries: list[tuple[str, FileTypes]] = []
656-
for spec in file_paths:
664+
upload_specs: list[FileTypes] = []
665+
for spec in normalized_paths:
657666
path, content_type, headers = _parse_path_spec(spec)
658667
file_obj = stack.enter_context(path.open("rb"))
659668
filename = path.name
660669
if headers:
661-
upload_entries.append(
662-
(
663-
FILE_UPLOAD_FIELD_NAME,
664-
(filename, file_obj, content_type, headers),
665-
)
666-
)
670+
upload_specs.append((filename, file_obj, content_type, headers))
667671
elif content_type is not None:
668-
upload_entries.append(
669-
(FILE_UPLOAD_FIELD_NAME, (filename, file_obj, content_type))
670-
)
672+
upload_specs.append((filename, file_obj, content_type))
671673
else:
672-
upload_entries.append(
673-
(FILE_UPLOAD_FIELD_NAME, (filename, file_obj))
674-
)
675-
return self.create(upload_entries)
674+
upload_specs.append((filename, file_obj))
675+
return self.create(upload_specs)
676676

677677

678678
class _AsyncFilesClient:
@@ -688,13 +688,12 @@ def __init__(
688688
self._concurrency_limit = concurrency_limit
689689

690690
async def create(self, files: UploadFiles) -> list[PdfRestFile]:
691-
"""Upload one or more files by content, in the same style accepted by
692-
the `files` parameter of `httpx.AsyncClient.post`.
691+
"""Upload one or more files by content.
693692
694-
Provide either a mapping of field names to file specifications, or a
695-
sequence of `(field_name, file_spec)` tuples. File specifications may be
696-
raw file-like objects, bytes, str, or the tuple forms documented by
697-
httpx."""
693+
Provide either a single file specification or a sequence of file
694+
specifications (each matching the shapes accepted by httpx). Every
695+
uploaded part is sent using the field name ``file``.
696+
"""
698697
normalized_files = _normalize_upload_files(files)
699698
request = self._client.prepare_request(
700699
"POST", "/upload", files=normalized_files
@@ -709,42 +708,29 @@ async def fetch(file_id: str) -> PdfRestFile:
709708

710709
return await asyncio.gather(*(fetch(file_id) for file_id in file_ids))
711710

712-
async def create_from_paths(
713-
self, file_paths: Sequence[FilePathTypes]
714-
) -> list[PdfRestFile]:
711+
async def create_from_paths(self, file_paths: FilePathInput) -> list[PdfRestFile]:
715712
"""Upload one or more files by their path.
716713
717714
Each entry may be a bare path-like object or a tuple of
718715
`(path, content_type)` / `(path, content_type, headers)` where headers
719716
mirrors the httpx multipart header mapping. All opened file handles are
720717
closed once the request completes.
721718
"""
722-
if not file_paths:
723-
msg = "At least one file path must be provided."
724-
raise ValueError(msg)
719+
normalized_paths = _normalize_path_inputs(file_paths)
725720

726721
with ExitStack() as stack:
727-
upload_entries: list[tuple[str, FileTypes]] = []
728-
for spec in file_paths:
722+
upload_specs: list[FileTypes] = []
723+
for spec in normalized_paths:
729724
path, content_type, headers = _parse_path_spec(spec)
730725
file_obj = stack.enter_context(path.open("rb"))
731726
filename = path.name
732727
if headers:
733-
upload_entries.append(
734-
(
735-
FILE_UPLOAD_FIELD_NAME,
736-
(filename, file_obj, content_type, headers),
737-
)
738-
)
728+
upload_specs.append((filename, file_obj, content_type, headers))
739729
elif content_type is not None:
740-
upload_entries.append(
741-
(FILE_UPLOAD_FIELD_NAME, (filename, file_obj, content_type))
742-
)
730+
upload_specs.append((filename, file_obj, content_type))
743731
else:
744-
upload_entries.append(
745-
(FILE_UPLOAD_FIELD_NAME, (filename, file_obj))
746-
)
747-
return await self.create(upload_entries)
732+
upload_specs.append((filename, file_obj))
733+
return await self.create(upload_specs)
748734

749735

750736
class PdfRestClient(_SyncApiClient):

tests/test_files.py

Lines changed: 81 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import uuid
44
from datetime import datetime, timezone
5-
from typing import Any
5+
from typing import Any, cast
66

77
import httpx
88
import pytest
@@ -76,7 +76,7 @@ def handler(request: httpx.Request) -> httpx.Response:
7676
report_pdf = get_test_resource_path("report.pdf")
7777
try:
7878
with report_pdf.open("rb") as pdf_file:
79-
response = client.files.create({"file": ("report.pdf", pdf_file)})
79+
response = client.files.create([("report.pdf", pdf_file)])
8080
finally:
8181
client.close()
8282

@@ -133,6 +133,41 @@ def handler(request: httpx.Request) -> httpx.Response:
133133
_assert_file_matches_payload(file_repr, payload)
134134

135135

136+
def test_files_create_from_paths_single_path() -> None:
137+
uploaded_file_id = str(uuid.uuid4())
138+
info_payload = _build_file_info_payload(uploaded_file_id, "report.pdf")
139+
140+
def handler(request: httpx.Request) -> httpx.Response:
141+
if request.method == "POST" and request.url.path == "/upload":
142+
body = request.content
143+
assert body.count(b'name="file"') == 1
144+
assert b'filename="report.pdf"' in body
145+
return httpx.Response(
146+
200,
147+
json={
148+
"files": [
149+
{"name": "report.pdf", "id": uploaded_file_id},
150+
]
151+
},
152+
)
153+
if request.method == "GET" and request.url.path.startswith("/resource/"):
154+
assert request.url.params["format"] == "info"
155+
return httpx.Response(200, json=info_payload)
156+
msg = f"Unexpected request: {request.method} {request.url}"
157+
raise AssertionError(msg)
158+
159+
transport = httpx.MockTransport(handler)
160+
client = PdfRestClient(api_key=VALID_API_KEY, transport=transport)
161+
report_pdf = get_test_resource_path("report.pdf")
162+
try:
163+
response = client.files.create_from_paths(report_pdf)
164+
finally:
165+
client.close()
166+
167+
assert len(response) == 1
168+
_assert_file_matches_payload(response[0], info_payload)
169+
170+
136171
def test_files_create_from_paths_supports_metadata() -> None:
137172
uploaded_file_id = str(uuid.uuid4())
138173
info_payload = _build_file_info_payload(uploaded_file_id, "report.pdf")
@@ -183,8 +218,11 @@ def test_files_create_rejects_empty_input() -> None:
183218
transport=httpx.MockTransport(lambda _: httpx.Response(200)),
184219
)
185220
try:
186-
with pytest.raises(ValueError, match=r"At least one file must be provided\."):
187-
client.files.create({})
221+
with pytest.raises(
222+
TypeError,
223+
match=r"Upload files must be provided as a sequence or a single file specification\.",
224+
):
225+
client.files.create(cast(Any, {}))
188226
with pytest.raises(ValueError, match=r"At least one file must be provided\."):
189227
client.files.create([])
190228
with pytest.raises(
@@ -231,8 +269,8 @@ def handler(request: httpx.Request) -> httpx.Response:
231269
with report_pdf.open("rb") as pdf_file, report_docx.open("rb") as docx_file:
232270
response = await client.files.create(
233271
[
234-
("file", ("report.pdf", pdf_file)),
235-
("file", ("report.docx", docx_file)),
272+
("report.pdf", pdf_file),
273+
("report.docx", docx_file),
236274
]
237275
)
238276

@@ -288,13 +326,47 @@ def handler(request: httpx.Request) -> httpx.Response:
288326
_assert_file_matches_payload(file_repr, payload)
289327

290328

329+
@pytest.mark.asyncio
330+
async def test_async_files_create_from_paths_single_path() -> None:
331+
uploaded_file_id = str(uuid.uuid4())
332+
info_payload = _build_file_info_payload(uploaded_file_id, "report.pdf")
333+
334+
def handler(request: httpx.Request) -> httpx.Response:
335+
if request.method == "POST" and request.url.path == "/upload":
336+
body = request.content
337+
assert body.count(b'name="file"') == 1
338+
assert b'filename="report.pdf"' in body
339+
return httpx.Response(
340+
200,
341+
json={
342+
"files": [
343+
{"name": "report.pdf", "id": uploaded_file_id},
344+
]
345+
},
346+
)
347+
if request.method == "GET" and request.url.path.startswith("/resource/"):
348+
assert request.url.params["format"] == "info"
349+
return httpx.Response(200, json=info_payload)
350+
msg = f"Unexpected request: {request.method} {request.url}"
351+
raise AssertionError(msg)
352+
353+
transport = httpx.MockTransport(handler)
354+
client = AsyncPdfRestClient(api_key=VALID_API_KEY, transport=transport)
355+
report_pdf = get_test_resource_path("report.pdf")
356+
async with client:
357+
response = await client.files.create_from_paths(report_pdf)
358+
359+
assert len(response) == 1
360+
_assert_file_matches_payload(response[0], info_payload)
361+
362+
291363
def test_live_file_create(pdfrest_api_key: str, pdfrest_live_base_url: str) -> None:
292364
with PdfRestClient(
293365
api_key=pdfrest_api_key, base_url=pdfrest_live_base_url
294366
) as client:
295367
report_pdf = get_test_resource_path("report.pdf")
296368
with report_pdf.open("rb") as pdf_file:
297-
response = client.files.create({"file": pdf_file})
369+
response = client.files.create([pdf_file])
298370
assert isinstance(response, list)
299371
assert len(response) == 1
300372
file_repr = response[0]
@@ -312,7 +384,7 @@ def test_live_file_create_two_files(
312384
report_pdf = get_test_resource_path("report.pdf")
313385
report_docx = get_test_resource_path("report.docx")
314386
with report_pdf.open("rb") as pdf_file, report_docx.open("rb") as docx_file:
315-
response = client.files.create([("file", pdf_file), ("file", docx_file)])
387+
response = client.files.create([pdf_file, docx_file])
316388
assert isinstance(response, list)
317389
assert len(response) == 2
318390
names = {file_repr.name for file_repr in response}
@@ -342,7 +414,7 @@ async def test_live_async_file_create(
342414
report_pdf = get_test_resource_path("report.pdf")
343415
async with client:
344416
with report_pdf.open("rb") as pdf_file:
345-
response = await client.files.create({"file": pdf_file})
417+
response = await client.files.create([pdf_file])
346418
assert isinstance(response, list)
347419
assert len(response) == 1
348420
file_repr = response[0]

0 commit comments

Comments
 (0)