Centralize session profile-update parameter resolution

Co-authored-by: Shri Sukhani <shrisukhani@users.noreply.github.com>

Author: cursoragent

CONTRIBUTING.md

@@ -101,6 +101,7 @@ This runs lint, format checks, compile checks, tests, and package build.
   - `tests/test_polling_loop_usage.py` (`while True` polling-loop centralization in `hyperbrowser/client/polling.py`),
   - `tests/test_pyproject_architecture_marker.py` (pytest marker registration enforcement),
   - `tests/test_readme_examples_listing.py` (README example-listing consistency enforcement),
+  - `tests/test_session_profile_update_helper_usage.py` (session profile-update parameter helper usage enforcement),
   - `tests/test_session_upload_helper_usage.py` (session upload-input normalization helper usage enforcement),
   - `tests/test_tool_mapping_reader_usage.py` (tools mapping-helper usage),
   - `tests/test_type_utils_usage.py` (type `__mro__` boundary centralization in `hyperbrowser/type_utils.py`),

hyperbrowser/client/managers/async_manager/session.py

@@ -7,6 +7,7 @@
     serialize_model_dump_to_dict,
     serialize_optional_model_dump_to_dict,
 )
+from ..session_profile_update_utils import resolve_update_profile_params
 from ..session_upload_utils import normalize_upload_file_input
 from ..session_utils import (
     parse_session_recordings_response_data,
@@ -225,36 +226,11 @@ async def update_profile_params(
         *,
         persist_changes: Optional[bool] = None,
     ) -> BasicResponse:
-        params_obj: UpdateSessionProfileParams
-
-        if type(params) is UpdateSessionProfileParams:
-            if persist_changes is not None:
-                raise HyperbrowserError(
-                    "Pass either UpdateSessionProfileParams as the second argument or persist_changes=bool, not both."
-                )
-            params_obj = params
-        elif isinstance(params, UpdateSessionProfileParams):
-            raise HyperbrowserError(
-                "update_profile_params() requires a plain UpdateSessionProfileParams object."
-            )
-        elif isinstance(params, bool):
-            if persist_changes is not None:
-                raise HyperbrowserError(
-                    "Pass either a boolean as the second argument or persist_changes=bool, not both."
-                )
-            self._warn_update_profile_params_boolean_deprecated()
-            params_obj = UpdateSessionProfileParams(persist_changes=params)
-        elif params is None:
-            if persist_changes is None:
-                raise HyperbrowserError(
-                    "update_profile_params() requires either UpdateSessionProfileParams or persist_changes=bool."
-                )
-            self._warn_update_profile_params_boolean_deprecated()
-            params_obj = UpdateSessionProfileParams(persist_changes=persist_changes)
-        else:
-            raise HyperbrowserError(
-                "update_profile_params() requires either UpdateSessionProfileParams or a boolean persist_changes."
-            )
+        params_obj = resolve_update_profile_params(
+            params,
+            persist_changes=persist_changes,
+            on_deprecated_bool_usage=self._warn_update_profile_params_boolean_deprecated,
+        )
 
         serialized_params = serialize_model_dump_to_dict(
             params_obj,

hyperbrowser/client/managers/session_profile_update_utils.py

@@ -0,0 +1,39 @@
+from typing import Callable, Optional, Union
+
+from hyperbrowser.exceptions import HyperbrowserError
+from hyperbrowser.models.session import UpdateSessionProfileParams
+
+
+def resolve_update_profile_params(
+    params: Union[UpdateSessionProfileParams, bool, None],
+    *,
+    persist_changes: Optional[bool],
+    on_deprecated_bool_usage: Callable[[], None],
+) -> UpdateSessionProfileParams:
+    if type(params) is UpdateSessionProfileParams:
+        if persist_changes is not None:
+            raise HyperbrowserError(
+                "Pass either UpdateSessionProfileParams as the second argument or persist_changes=bool, not both."
+            )
+        return params
+    if isinstance(params, UpdateSessionProfileParams):
+        raise HyperbrowserError(
+            "update_profile_params() requires a plain UpdateSessionProfileParams object."
+        )
+    if isinstance(params, bool):
+        if persist_changes is not None:
+            raise HyperbrowserError(
+                "Pass either a boolean as the second argument or persist_changes=bool, not both."
+            )
+        on_deprecated_bool_usage()
+        return UpdateSessionProfileParams(persist_changes=params)
+    if params is None:
+        if persist_changes is None:
+            raise HyperbrowserError(
+                "update_profile_params() requires either UpdateSessionProfileParams or persist_changes=bool."
+            )
+        on_deprecated_bool_usage()
+        return UpdateSessionProfileParams(persist_changes=persist_changes)
+    raise HyperbrowserError(
+        "update_profile_params() requires either UpdateSessionProfileParams or a boolean persist_changes."
+    )

hyperbrowser/client/managers/sync_manager/session.py

@@ -7,6 +7,7 @@
     serialize_model_dump_to_dict,
     serialize_optional_model_dump_to_dict,
 )
+from ..session_profile_update_utils import resolve_update_profile_params
 from ..session_upload_utils import normalize_upload_file_input
 from ..session_utils import (
     parse_session_recordings_response_data,
@@ -217,36 +218,11 @@ def update_profile_params(
         *,
         persist_changes: Optional[bool] = None,
     ) -> BasicResponse:
-        params_obj: UpdateSessionProfileParams
-
-        if type(params) is UpdateSessionProfileParams:
-            if persist_changes is not None:
-                raise HyperbrowserError(
-                    "Pass either UpdateSessionProfileParams as the second argument or persist_changes=bool, not both."
-                )
-            params_obj = params
-        elif isinstance(params, UpdateSessionProfileParams):
-            raise HyperbrowserError(
-                "update_profile_params() requires a plain UpdateSessionProfileParams object."
-            )
-        elif isinstance(params, bool):
-            if persist_changes is not None:
-                raise HyperbrowserError(
-                    "Pass either a boolean as the second argument or persist_changes=bool, not both."
-                )
-            self._warn_update_profile_params_boolean_deprecated()
-            params_obj = UpdateSessionProfileParams(persist_changes=params)
-        elif params is None:
-            if persist_changes is None:
-                raise HyperbrowserError(
-                    "update_profile_params() requires either UpdateSessionProfileParams or persist_changes=bool."
-                )
-            self._warn_update_profile_params_boolean_deprecated()
-            params_obj = UpdateSessionProfileParams(persist_changes=persist_changes)
-        else:
-            raise HyperbrowserError(
-                "update_profile_params() requires either UpdateSessionProfileParams or a boolean persist_changes."
-            )
+        params_obj = resolve_update_profile_params(
+            params,
+            persist_changes=persist_changes,
+            on_deprecated_bool_usage=self._warn_update_profile_params_boolean_deprecated,
+        )
 
         serialized_params = serialize_model_dump_to_dict(
             params_obj,

tests/test_architecture_marker_usage.py

@@ -33,6 +33,7 @@
     "tests/test_example_sync_async_parity.py",
     "tests/test_example_run_instructions.py",
     "tests/test_computer_action_endpoint_helper_usage.py",
+    "tests/test_session_profile_update_helper_usage.py",
     "tests/test_session_upload_helper_usage.py",
     "tests/test_web_payload_helper_usage.py",
 )

tests/test_session_profile_update_helper_usage.py

@@ -0,0 +1,20 @@
+from pathlib import Path
+
+import pytest
+
+pytestmark = pytest.mark.architecture
+
+
+SESSION_MANAGER_MODULES = (
+    "hyperbrowser/client/managers/sync_manager/session.py",
+    "hyperbrowser/client/managers/async_manager/session.py",
+)
+
+
+def test_session_managers_use_shared_profile_update_param_helper():
+    for module_path in SESSION_MANAGER_MODULES:
+        module_text = Path(module_path).read_text(encoding="utf-8")
+        assert "resolve_update_profile_params(" in module_text
+        assert "requires a plain UpdateSessionProfileParams object." not in module_text
+        assert "Pass either UpdateSessionProfileParams as the second argument" not in module_text
+        assert "Pass either a boolean as the second argument" not in module_text

tests/test_session_profile_update_utils.py

@@ -0,0 +1,98 @@
+import pytest
+
+from hyperbrowser.client.managers.session_profile_update_utils import (
+    resolve_update_profile_params,
+)
+from hyperbrowser.exceptions import HyperbrowserError
+from hyperbrowser.models.session import UpdateSessionProfileParams
+
+
+def test_resolve_update_profile_params_returns_plain_params_without_warning():
+    warnings_triggered = 0
+
+    def _on_deprecated_bool_usage() -> None:
+        nonlocal warnings_triggered
+        warnings_triggered += 1
+
+    params = UpdateSessionProfileParams(persist_changes=True)
+    result = resolve_update_profile_params(
+        params,
+        persist_changes=None,
+        on_deprecated_bool_usage=_on_deprecated_bool_usage,
+    )
+
+    assert result is params
+    assert warnings_triggered == 0
+
+
+def test_resolve_update_profile_params_rejects_plain_params_with_keyword_argument():
+    with pytest.raises(HyperbrowserError, match="not both"):
+        resolve_update_profile_params(
+            UpdateSessionProfileParams(persist_changes=True),
+            persist_changes=True,
+            on_deprecated_bool_usage=lambda: None,
+        )
+
+
+def test_resolve_update_profile_params_rejects_params_subclass():
+    class _Params(UpdateSessionProfileParams):
+        pass
+
+    with pytest.raises(HyperbrowserError, match="plain UpdateSessionProfileParams"):
+        resolve_update_profile_params(
+            _Params(persist_changes=True),
+            persist_changes=None,
+            on_deprecated_bool_usage=lambda: None,
+        )
+
+
+def test_resolve_update_profile_params_builds_from_bool_and_warns():
+    warnings_triggered = 0
+
+    def _on_deprecated_bool_usage() -> None:
+        nonlocal warnings_triggered
+        warnings_triggered += 1
+
+    result = resolve_update_profile_params(
+        True,
+        persist_changes=None,
+        on_deprecated_bool_usage=_on_deprecated_bool_usage,
+    )
+
+    assert result.persist_changes is True
+    assert warnings_triggered == 1
+
+
+def test_resolve_update_profile_params_builds_from_keyword_bool_and_warns():
+    warnings_triggered = 0
+
+    def _on_deprecated_bool_usage() -> None:
+        nonlocal warnings_triggered
+        warnings_triggered += 1
+
+    result = resolve_update_profile_params(
+        None,
+        persist_changes=False,
+        on_deprecated_bool_usage=_on_deprecated_bool_usage,
+    )
+
+    assert result.persist_changes is False
+    assert warnings_triggered == 1
+
+
+def test_resolve_update_profile_params_requires_argument_or_keyword():
+    with pytest.raises(HyperbrowserError, match="requires either"):
+        resolve_update_profile_params(
+            None,
+            persist_changes=None,
+            on_deprecated_bool_usage=lambda: None,
+        )
+
+
+def test_resolve_update_profile_params_rejects_unexpected_param_value():
+    with pytest.raises(HyperbrowserError, match="requires either"):
+        resolve_update_profile_params(  # type: ignore[arg-type]
+            "true",
+            persist_changes=None,
+            on_deprecated_bool_usage=lambda: None,
+        )