Skip to content

Commit 812d28d

Browse files
Centralize session upload input normalization
Co-authored-by: Shri Sukhani <shrisukhani@users.noreply.github.com>
1 parent 698606f commit 812d28d

7 files changed

Lines changed: 213 additions & 101 deletions

File tree

CONTRIBUTING.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ This runs lint, format checks, compile checks, tests, and package build.
9797
- `tests/test_docs_python3_commands.py` (`README`/`CONTRIBUTING`/examples python3 command consistency enforcement),
9898
- `tests/test_example_sync_async_parity.py` (sync/async example parity enforcement),
9999
- `tests/test_example_run_instructions.py` (example run-instruction consistency enforcement),
100-
- `tests/test_computer_action_endpoint_helper_usage.py` (computer-action endpoint-normalization helper usage enforcement).
100+
- `tests/test_computer_action_endpoint_helper_usage.py` (computer-action endpoint-normalization helper usage enforcement),
101+
- `tests/test_session_upload_helper_usage.py` (session upload-input normalization helper usage enforcement).
101102

102103
## Code quality conventions
103104

hyperbrowser/client/managers/async_manager/session.py

Lines changed: 12 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
import os
21
from os import PathLike
32
from typing import IO, List, Optional, Union, overload
43
import warnings
54
from hyperbrowser.exceptions import HyperbrowserError
6-
from hyperbrowser.type_utils import is_plain_string, is_string_subclass_instance
7-
from ...file_utils import ensure_existing_file_path
85
from ..serialization_utils import serialize_model_dump_to_dict
6+
from ..session_upload_utils import normalize_upload_file_input
97
from ..session_utils import (
108
parse_session_recordings_response_data,
119
parse_session_response_model,
@@ -166,21 +164,12 @@ async def get_downloads_url(self, id: str) -> GetSessionDownloadsUrlResponse:
166164
async def upload_file(
167165
self, id: str, file_input: Union[str, PathLike[str], IO]
168166
) -> UploadFileResponse:
169-
if is_plain_string(file_input) or isinstance(file_input, PathLike):
170-
try:
171-
raw_file_path = os.fspath(file_input)
172-
except HyperbrowserError:
173-
raise
174-
except Exception as exc:
175-
raise HyperbrowserError(
176-
"file_input path is invalid",
177-
original_error=exc,
178-
) from exc
179-
file_path = ensure_existing_file_path(
180-
raw_file_path,
181-
missing_file_message=f"Upload file not found at path: {raw_file_path}",
182-
not_file_message=f"Upload file path must point to a file: {raw_file_path}",
183-
)
167+
file_path, file_obj = normalize_upload_file_input(
168+
file_input,
169+
missing_file_message=f"Upload file not found at path: {file_input}",
170+
not_file_message=f"Upload file path must point to a file: {file_input}",
171+
)
172+
if file_path is not None:
184173
try:
185174
with open(file_path, "rb") as file_obj:
186175
files = {"file": file_obj}
@@ -193,39 +182,12 @@ async def upload_file(
193182
f"Failed to open upload file at path: {file_path}",
194183
original_error=exc,
195184
) from exc
196-
elif is_string_subclass_instance(file_input):
197-
raise HyperbrowserError("file_input path must be a plain string path")
198185
else:
199-
try:
200-
read_method = getattr(file_input, "read", None)
201-
except HyperbrowserError:
202-
raise
203-
except Exception as exc:
204-
raise HyperbrowserError(
205-
"file_input file-like object state is invalid",
206-
original_error=exc,
207-
) from exc
208-
if callable(read_method):
209-
try:
210-
is_closed = bool(getattr(file_input, "closed", False))
211-
except HyperbrowserError:
212-
raise
213-
except Exception as exc:
214-
raise HyperbrowserError(
215-
"file_input file-like object state is invalid",
216-
original_error=exc,
217-
) from exc
218-
if is_closed:
219-
raise HyperbrowserError("file_input file-like object must be open")
220-
files = {"file": file_input}
221-
response = await self._client.transport.post(
222-
self._client._build_url(f"/session/{id}/uploads"),
223-
files=files,
224-
)
225-
else:
226-
raise HyperbrowserError(
227-
"file_input must be a file path or file-like object"
228-
)
186+
files = {"file": file_obj}
187+
response = await self._client.transport.post(
188+
self._client._build_url(f"/session/{id}/uploads"),
189+
files=files,
190+
)
229191

230192
return parse_session_response_model(
231193
response.data,
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import os
2+
from os import PathLike
3+
from typing import IO, Optional, Tuple, Union
4+
5+
from hyperbrowser.exceptions import HyperbrowserError
6+
from hyperbrowser.type_utils import is_plain_string, is_string_subclass_instance
7+
8+
from ..file_utils import ensure_existing_file_path
9+
10+
11+
def normalize_upload_file_input(
12+
file_input: Union[str, PathLike[str], IO],
13+
*,
14+
missing_file_message: str,
15+
not_file_message: str,
16+
) -> Tuple[Optional[str], Optional[IO]]:
17+
if is_plain_string(file_input) or isinstance(file_input, PathLike):
18+
try:
19+
raw_file_path = os.fspath(file_input)
20+
except HyperbrowserError:
21+
raise
22+
except Exception as exc:
23+
raise HyperbrowserError(
24+
"file_input path is invalid",
25+
original_error=exc,
26+
) from exc
27+
file_path = ensure_existing_file_path(
28+
raw_file_path,
29+
missing_file_message=missing_file_message,
30+
not_file_message=not_file_message,
31+
)
32+
return file_path, None
33+
34+
if is_string_subclass_instance(file_input):
35+
raise HyperbrowserError("file_input path must be a plain string path")
36+
37+
try:
38+
read_method = getattr(file_input, "read", None)
39+
except HyperbrowserError:
40+
raise
41+
except Exception as exc:
42+
raise HyperbrowserError(
43+
"file_input file-like object state is invalid",
44+
original_error=exc,
45+
) from exc
46+
if not callable(read_method):
47+
raise HyperbrowserError("file_input must be a file path or file-like object")
48+
49+
try:
50+
is_closed = bool(getattr(file_input, "closed", False))
51+
except HyperbrowserError:
52+
raise
53+
except Exception as exc:
54+
raise HyperbrowserError(
55+
"file_input file-like object state is invalid",
56+
original_error=exc,
57+
) from exc
58+
if is_closed:
59+
raise HyperbrowserError("file_input file-like object must be open")
60+
61+
return None, file_input

hyperbrowser/client/managers/sync_manager/session.py

Lines changed: 12 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
import os
21
from os import PathLike
32
from typing import IO, List, Optional, Union, overload
43
import warnings
54
from hyperbrowser.exceptions import HyperbrowserError
6-
from hyperbrowser.type_utils import is_plain_string, is_string_subclass_instance
7-
from ...file_utils import ensure_existing_file_path
85
from ..serialization_utils import serialize_model_dump_to_dict
6+
from ..session_upload_utils import normalize_upload_file_input
97
from ..session_utils import (
108
parse_session_recordings_response_data,
119
parse_session_response_model,
@@ -158,21 +156,12 @@ def get_downloads_url(self, id: str) -> GetSessionDownloadsUrlResponse:
158156
def upload_file(
159157
self, id: str, file_input: Union[str, PathLike[str], IO]
160158
) -> UploadFileResponse:
161-
if is_plain_string(file_input) or isinstance(file_input, PathLike):
162-
try:
163-
raw_file_path = os.fspath(file_input)
164-
except HyperbrowserError:
165-
raise
166-
except Exception as exc:
167-
raise HyperbrowserError(
168-
"file_input path is invalid",
169-
original_error=exc,
170-
) from exc
171-
file_path = ensure_existing_file_path(
172-
raw_file_path,
173-
missing_file_message=f"Upload file not found at path: {raw_file_path}",
174-
not_file_message=f"Upload file path must point to a file: {raw_file_path}",
175-
)
159+
file_path, file_obj = normalize_upload_file_input(
160+
file_input,
161+
missing_file_message=f"Upload file not found at path: {file_input}",
162+
not_file_message=f"Upload file path must point to a file: {file_input}",
163+
)
164+
if file_path is not None:
176165
try:
177166
with open(file_path, "rb") as file_obj:
178167
files = {"file": file_obj}
@@ -185,39 +174,12 @@ def upload_file(
185174
f"Failed to open upload file at path: {file_path}",
186175
original_error=exc,
187176
) from exc
188-
elif is_string_subclass_instance(file_input):
189-
raise HyperbrowserError("file_input path must be a plain string path")
190177
else:
191-
try:
192-
read_method = getattr(file_input, "read", None)
193-
except HyperbrowserError:
194-
raise
195-
except Exception as exc:
196-
raise HyperbrowserError(
197-
"file_input file-like object state is invalid",
198-
original_error=exc,
199-
) from exc
200-
if callable(read_method):
201-
try:
202-
is_closed = bool(getattr(file_input, "closed", False))
203-
except HyperbrowserError:
204-
raise
205-
except Exception as exc:
206-
raise HyperbrowserError(
207-
"file_input file-like object state is invalid",
208-
original_error=exc,
209-
) from exc
210-
if is_closed:
211-
raise HyperbrowserError("file_input file-like object must be open")
212-
files = {"file": file_input}
213-
response = self._client.transport.post(
214-
self._client._build_url(f"/session/{id}/uploads"),
215-
files=files,
216-
)
217-
else:
218-
raise HyperbrowserError(
219-
"file_input must be a file path or file-like object"
220-
)
178+
files = {"file": file_obj}
179+
response = self._client.transport.post(
180+
self._client._build_url(f"/session/{id}/uploads"),
181+
files=files,
182+
)
221183

222184
return parse_session_response_model(
223185
response.data,

tests/test_architecture_marker_usage.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"tests/test_example_sync_async_parity.py",
2828
"tests/test_example_run_instructions.py",
2929
"tests/test_computer_action_endpoint_helper_usage.py",
30+
"tests/test_session_upload_helper_usage.py",
3031
)
3132

3233

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
from pathlib import Path
2+
3+
import pytest
4+
5+
pytestmark = pytest.mark.architecture
6+
7+
8+
SESSION_MANAGER_MODULES = (
9+
"hyperbrowser/client/managers/sync_manager/session.py",
10+
"hyperbrowser/client/managers/async_manager/session.py",
11+
)
12+
13+
14+
def test_session_managers_use_shared_upload_input_normalizer():
15+
for module_path in SESSION_MANAGER_MODULES:
16+
module_text = Path(module_path).read_text(encoding="utf-8")
17+
assert "normalize_upload_file_input(" in module_text
18+
assert "os.fspath(" not in module_text

tests/test_session_upload_utils.py

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import io
2+
from os import PathLike
3+
from pathlib import Path
4+
5+
import pytest
6+
7+
from hyperbrowser.client.managers.session_upload_utils import (
8+
normalize_upload_file_input,
9+
)
10+
from hyperbrowser.exceptions import HyperbrowserError
11+
12+
13+
def test_normalize_upload_file_input_returns_path_for_plain_string(tmp_path: Path):
14+
file_path = tmp_path / "file.txt"
15+
file_path.write_text("content")
16+
17+
normalized_path, file_obj = normalize_upload_file_input(
18+
str(file_path),
19+
missing_file_message=f"Upload file not found at path: {file_path}",
20+
not_file_message=f"Upload file path must point to a file: {file_path}",
21+
)
22+
23+
assert normalized_path == str(file_path)
24+
assert file_obj is None
25+
26+
27+
def test_normalize_upload_file_input_returns_path_for_pathlike(tmp_path: Path):
28+
file_path = tmp_path / "file.txt"
29+
file_path.write_text("content")
30+
31+
normalized_path, file_obj = normalize_upload_file_input(
32+
file_path,
33+
missing_file_message=f"Upload file not found at path: {file_path}",
34+
not_file_message=f"Upload file path must point to a file: {file_path}",
35+
)
36+
37+
assert normalized_path == str(file_path)
38+
assert file_obj is None
39+
40+
41+
def test_normalize_upload_file_input_rejects_string_subclass():
42+
class _PathString(str):
43+
pass
44+
45+
with pytest.raises(
46+
HyperbrowserError, match="file_input path must be a plain string path"
47+
) as exc_info:
48+
normalize_upload_file_input(
49+
_PathString("/tmp/file.txt"), # type: ignore[arg-type]
50+
missing_file_message="Upload file not found at path: /tmp/file.txt",
51+
not_file_message="Upload file path must point to a file: /tmp/file.txt",
52+
)
53+
54+
assert exc_info.value.original_error is None
55+
56+
57+
def test_normalize_upload_file_input_wraps_invalid_pathlike_state_errors():
58+
class _BrokenPathLike(PathLike[str]):
59+
def __fspath__(self) -> str:
60+
raise RuntimeError("broken fspath")
61+
62+
with pytest.raises(
63+
HyperbrowserError, match="file_input path is invalid"
64+
) as exc_info:
65+
normalize_upload_file_input(
66+
_BrokenPathLike(),
67+
missing_file_message="Upload file not found",
68+
not_file_message="Upload file path must point to a file",
69+
)
70+
71+
assert isinstance(exc_info.value.original_error, RuntimeError)
72+
73+
74+
def test_normalize_upload_file_input_returns_open_file_like_object():
75+
file_obj = io.BytesIO(b"content")
76+
77+
normalized_path, normalized_file_obj = normalize_upload_file_input(
78+
file_obj,
79+
missing_file_message="Upload file not found",
80+
not_file_message="Upload file path must point to a file",
81+
)
82+
83+
assert normalized_path is None
84+
assert normalized_file_obj is file_obj
85+
86+
87+
def test_normalize_upload_file_input_rejects_closed_file_like_object():
88+
file_obj = io.BytesIO(b"content")
89+
file_obj.close()
90+
91+
with pytest.raises(HyperbrowserError, match="file-like object must be open"):
92+
normalize_upload_file_input(
93+
file_obj,
94+
missing_file_message="Upload file not found",
95+
not_file_message="Upload file path must point to a file",
96+
)
97+
98+
99+
def test_normalize_upload_file_input_rejects_non_callable_read_attribute():
100+
fake_file = type("FakeFile", (), {"read": "not-callable"})()
101+
102+
with pytest.raises(HyperbrowserError, match="file_input must be a file path"):
103+
normalize_upload_file_input(
104+
fake_file, # type: ignore[arg-type]
105+
missing_file_message="Upload file not found",
106+
not_file_message="Upload file path must point to a file",
107+
)

0 commit comments

Comments
 (0)