Skip to content

Commit b0c9a73

Browse files
fix: address confirmed review findings on submit
1 parent 6cddd04 commit b0c9a73

4 files changed

Lines changed: 58 additions & 15 deletions

File tree

src/photon/_transport.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
Classification reads the response **body** before the status code, because the API
88
puts the real outcome in the body's ``message``: a document that is still
99
processing comes back as an HTTP 200, and several genuine failures come back as
10-
403. See ``plans/API-REFERENCE.md`` for the observed responses.
10+
403. See the official API docs (apidocs.photoncommerce.com) for the responses.
1111
"""
1212

1313
from __future__ import annotations

src/photon/client.py

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,13 @@
4343
# actual file type.
4444
_FILE_FIELD = "pdf"
4545

46+
# Filename sent for raw bytes and unnamed file objects. The API recognises file
47+
# types by extension, so the upload must carry one; callers whose content is not
48+
# a PDF should pass a path or a named file object instead.
49+
_DEFAULT_FILENAME = "upload.pdf"
50+
4651
_SUBACCOUNT_MAX_LEN = 50
47-
_SUBACCOUNT_RE = re.compile(r"^[A-Za-z0-9-]+$")
52+
_SUBACCOUNT_RE = re.compile(r"[A-Za-z0-9-]+")
4853

4954

5055
class PhotonClient:
@@ -142,7 +147,10 @@ def submit(
142147
Args:
143148
document: The document to upload — a path, an open binary file
144149
object, or raw bytes. A path is opened and closed by the
145-
client; a file object is read as-is and left open.
150+
client; a file object is read as-is and left open. The API
151+
recognises file types by filename extension, so raw bytes and
152+
unnamed file objects are uploaded as ``upload.pdf``; for other
153+
file types, pass a path or a file object with a ``.name``.
146154
doctype: What kind of document this is; the API defaults to
147155
invoice. Any string is passed through, so doctypes newer than
148156
this SDK still work.
@@ -190,12 +198,12 @@ def submit(
190198
with contextlib.ExitStack() as cleanup:
191199
files: Any = None
192200
if isinstance(document, bytes):
193-
files = {_FILE_FIELD: document}
201+
files = {_FILE_FIELD: (_DEFAULT_FILENAME, document)}
194202
elif isinstance(document, (str, os.PathLike)):
195203
handle = cleanup.enter_context(open(document, "rb"))
196204
files = {_FILE_FIELD: (os.path.basename(os.fspath(document)), handle)}
197205
elif document is not None:
198-
files = {_FILE_FIELD: document}
206+
files = {_FILE_FIELD: (_filename_for(document), document)}
199207

200208
body = self._transport.request_json(
201209
"POST", SUBMIT_PATH, params=params, files=files
@@ -216,8 +224,7 @@ def retrieve(self, photon_key: str) -> dict[str, Any]:
216224
217225
Raises:
218226
ValueError: ``photon_key`` is empty — checked before any I/O.
219-
NotReadyError: The document is still being processed; retry later,
220-
or let ``extract()`` (Week 3) poll for you.
227+
NotReadyError: The document is still being processed; retry later.
221228
APIError: The response reported success but carried no ``data``
222229
object.
223230
PhotonError: See :meth:`Transport.request_json` for the rest of
@@ -265,5 +272,21 @@ def __repr__(self) -> str:
265272
)
266273

267274

275+
def _filename_for(document: IO[bytes]) -> str:
276+
"""The filename to upload a file object under.
277+
278+
The API recognises file types by the uploaded filename's extension, so an
279+
unnamed stream (``BytesIO``, a pipe, a fd-opened file) gets the default
280+
rather than httpx's extensionless fallback.
281+
"""
282+
name = getattr(document, "name", None)
283+
if isinstance(name, str) and os.path.basename(name):
284+
return os.path.basename(name)
285+
return _DEFAULT_FILENAME
286+
287+
268288
def _is_valid_subaccount(subaccount: str) -> bool:
269-
return len(subaccount) <= _SUBACCOUNT_MAX_LEN and bool(_SUBACCOUNT_RE.match(subaccount))
289+
# fullmatch, not match: with match, "$" would accept a trailing newline.
290+
return len(subaccount) <= _SUBACCOUNT_MAX_LEN and bool(
291+
_SUBACCOUNT_RE.fullmatch(subaccount)
292+
)

tests/test_submit.py

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Tests for PhotonClient.submit: input modes, validation, and the Submission model.
22
3-
The API is stubbed with respx; the submit response shape comes from
4-
``plans/API-REFERENCE.md``.
3+
The API is stubbed with respx; the submit response shape comes from the
4+
official API docs (apidocs.photoncommerce.com).
55
"""
66

77
from __future__ import annotations
@@ -78,7 +78,9 @@ def test_path_input_uploads_multipart_pdf_field(
7878
assert submission.doc_path == "uploads/2026/invoice-abc.pdf"
7979

8080

81-
def test_file_object_input_is_uploaded_and_left_open(client: PhotonClient) -> None:
81+
def test_unnamed_file_object_is_uploaded_with_an_extension_and_left_open(
82+
client: PhotonClient,
83+
) -> None:
8284
handle = io.BytesIO(PDF_BYTES)
8385

8486
with respx.mock(base_url=BASE_URL) as mock:
@@ -87,18 +89,36 @@ def test_file_object_input_is_uploaded_and_left_open(client: PhotonClient) -> No
8789

8890
request = route.calls.last.request
8991
assert b'name="pdf"' in request.content
92+
# The API recognises file types by extension, so an unnamed stream must not
93+
# go out under httpx's extensionless fallback name.
94+
assert b'filename="upload.pdf"' in request.content
9095
assert PDF_BYTES in request.content
9196
assert not handle.closed
9297

9398

94-
def test_bytes_input_is_uploaded(client: PhotonClient) -> None:
99+
def test_named_file_object_keeps_its_own_filename(
100+
tmp_path: Path, client: PhotonClient
101+
) -> None:
102+
document = tmp_path / "receipt.png"
103+
document.write_bytes(PDF_BYTES)
104+
105+
with respx.mock(base_url=BASE_URL) as mock:
106+
route = mock_submit(mock)
107+
with document.open("rb") as handle:
108+
client.submit(handle)
109+
110+
assert b'filename="receipt.png"' in route.calls.last.request.content
111+
112+
113+
def test_bytes_input_is_uploaded_with_an_extension(client: PhotonClient) -> None:
95114
with respx.mock(base_url=BASE_URL) as mock:
96115
route = mock_submit(mock)
97116
client.submit(PDF_BYTES)
98117

99118
request = route.calls.last.request
100119
assert request.headers["content-type"].startswith("multipart/form-data")
101120
assert b'name="pdf"' in request.content
121+
assert b'filename="upload.pdf"' in request.content
102122
assert PDF_BYTES in request.content
103123

104124

@@ -178,7 +198,7 @@ def test_both_document_and_url_is_a_value_error(client: PhotonClient) -> None:
178198

179199
@pytest.mark.parametrize(
180200
"subaccount",
181-
["a" * 51, "under_score", "has space", "", "email@nope", "slash/nope"],
201+
["a" * 51, "under_score", "has space", "", "email@nope", "slash/nope", "team-1\n"],
182202
)
183203
def test_invalid_subaccount_is_a_value_error(client: PhotonClient, subaccount: str) -> None:
184204
with respx.mock(base_url=BASE_URL) as mock, pytest.raises(ValueError, match="subaccount"):

tests/test_transport.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
"""Tests for the HTTP transport: headers, response classification, lifecycle.
22
33
The API is stubbed with respx, so these assert the SDK's behaviour against the
4-
responses recorded in ``plans/API-REFERENCE.md`` — including the ones that report
5-
failure with an HTTP 200.
4+
responses shown in the official API docs (apidocs.photoncommerce.com) — including
5+
the ones that report failure with an HTTP 200.
66
"""
77

88
from __future__ import annotations

0 commit comments

Comments
 (0)