diff --git a/docs/public-api.md b/docs/public-api.md index 12ee217..923fe08 100644 --- a/docs/public-api.md +++ b/docs/public-api.md @@ -78,22 +78,37 @@ from mlbstatsapi import ( ) ``` +The symbols above are available in every install. `AsyncMlbDataAdapter` is +equally public, but it resolves only when the optional `async` extra is +installed; see [Optional async support](#optional-async-support). + ### Classification of package-root symbols -| Symbol | Status | -| --- | --- | -| `Mlb` | Public and stable in 1.x | -| `MlbDataAdapter` | Public and stable in 1.x | -| `MlbResult` | Public and stable in 1.x | -| `create_retry_policy` | Public and stable in 1.x | -| `TheMlbStatsApiException` | Public and stable in 1.x | -| `MlbTransportError` | Public and stable in 1.x | -| `MlbTimeoutError` | Public and stable in 1.x | -| `MlbHttpError` | Public and stable in 1.x | -| `MlbDecodeError` | Public and stable in 1.x | -| `MlbHttpCompatibilityWarning` | Public and stable in 1.x | -| `return_splits` | Public legacy helper, stable in 1.x but not preferred for new code | -| `get_stat_attributes` | Public legacy helper, stable in 1.x but not preferred for new code | +Status and availability are separate questions. Every symbol below is public and +covered by the stability policy above; the availability column records whether +resolving it needs an optional dependency. + +| Symbol | Status | Availability | +| --- | --- | --- | +| `Mlb` | Public and stable in 1.x | Always available | +| `MlbDataAdapter` | Public and stable in 1.x | Always available | +| `AsyncMlbDataAdapter` | Public and stable in 1.x | Requires the optional `async` extra | +| `MlbResult` | Public and stable in 1.x | Always available | +| `create_retry_policy` | Public and stable in 1.x | Always available | +| `TheMlbStatsApiException` | Public and stable in 1.x | Always available | +| `MlbTransportError` | Public and stable in 1.x | Always available | +| `MlbTimeoutError` | Public and stable in 1.x | Always available | +| `MlbHttpError` | Public and stable in 1.x | Always available | +| `MlbDecodeError` | Public and stable in 1.x | Always available | +| `MlbHttpCompatibilityWarning` | Public and stable in 1.x | Always available | +| `return_splits` | Public legacy helper, stable in 1.x but not preferred for new code | Always available | +| `get_stat_attributes` | Public legacy helper, stable in 1.x but not preferred for new code | Always available | + +`AsyncMlbDataAdapter` is supported 1.x API on the same terms as the synchronous +symbols: it will not be removed or renamed during the series, and its documented +behavior stays compatible. Only its availability is conditional, because its +HTTP dependency ships with the `async` extra. See +[Optional async support](#optional-async-support). No package-root symbol is marked deprecated in version 1.0. Deprecation requires a documented replacement, a warning strategy, a removal timeline, and a @@ -133,6 +148,37 @@ surface. A future focused issue may introduce `__all__` after deciding how to treat the accidental submodule names (for example, a documented deprecation period). +## Optional async support + +`AsyncMlbDataAdapter` is a public package-root symbol, like `MlbDataAdapter`, +and appears in the classification table above. Its HTTP dependency is optional +and installed with the `async` extra: + +```bash +pip install "python-mlb-statsapi[async]" +``` + +With the extra installed: + +```python +from mlbstatsapi import AsyncMlbDataAdapter +``` + +Async symbols are resolved on first access, so the optional dependency is not +imported by `import mlbstatsapi`. A synchronous-only install is unaffected: + +* `import mlbstatsapi` succeeds without the `async` extra +* every package-root symbol marked "Always available" above stays importable +* nothing in the synchronous surface changes + +Requesting async functionality without the extra raises `ImportError` naming +the install command above. That failure happens only when async functionality +is requested — importing the package, or any supported synchronous symbol, +never triggers it. + +The async HTTP library is an implementation detail. It is not re-exported from +the package root, and its types are not part of the public API. + ## Primary client `Mlb` is the primary synchronous client. diff --git a/mlbstatsapi/__init__.py b/mlbstatsapi/__init__.py index bb3c21c..e5a6cf7 100644 --- a/mlbstatsapi/__init__.py +++ b/mlbstatsapi/__init__.py @@ -25,3 +25,27 @@ return_splits, get_stat_attributes ) + +# Async symbols are resolved lazily. HTTPX is an optional dependency installed +# with the ``async`` extra, so importing the async adapter eagerly here would +# make ``import mlbstatsapi`` fail for every sync-only install. Resolving on +# first access keeps async functionality discoverable from the package root +# while the missing-dependency error surfaces only when async is actually +# requested. See docs/public-api.md. +_LAZY_ASYNC_EXPORTS = ("AsyncMlbDataAdapter",) + + +def __getattr__(name: str): + if name in _LAZY_ASYNC_EXPORTS: + from .async_mlb_dataadapter import AsyncMlbDataAdapter + + # Cache on the module so later attribute access is an ordinary lookup. + globals()["AsyncMlbDataAdapter"] = AsyncMlbDataAdapter + return AsyncMlbDataAdapter + + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + +def __dir__() -> list[str]: + # Keeps the lazy async names discoverable without importing HTTPX. + return sorted(set(globals()) | set(_LAZY_ASYNC_EXPORTS)) diff --git a/mlbstatsapi/_async_support.py b/mlbstatsapi/_async_support.py new file mode 100644 index 0000000..88b7fb4 --- /dev/null +++ b/mlbstatsapi/_async_support.py @@ -0,0 +1,43 @@ +"""Private optional-dependency boundary for async support. + +HTTPX ships only with the ``async`` extra, so a sync-only install must be able +to ``import mlbstatsapi`` and use ``Mlb`` / ``MlbDataAdapter`` without it. Every +async entry point routes its HTTPX import through :func:`import_httpx`, so a +missing optional dependency produces one actionable install message instead of a +bare ``ModuleNotFoundError`` naming a library the user never asked for. Import +failures that are not a missing ``httpx`` are left alone. + +HTTPX itself stays an implementation detail: nothing here re-exports it. +""" + +from types import ModuleType + +ASYNC_EXTRA_REQUIREMENT = 'python-mlb-statsapi[async]' + +MISSING_HTTPX_MESSAGE = ( + "Async support requires the optional HTTPX dependency, which is not " + "installed. Install it with:\n\n" + f' pip install "{ASYNC_EXTRA_REQUIREMENT}"\n' +) + + +def import_httpx() -> ModuleType: + """Return the ``httpx`` module, or raise an actionable ``ImportError``. + + Only a genuinely missing top-level ``httpx`` is translated into the install + message. An installed-but-broken HTTPX fails on some other module (a + missing transitive dependency, for example), and telling that user to + install the extra would send them chasing the wrong problem, so those + failures propagate unchanged. + + The original failure is preserved as the exception cause so a broken async + install stays diagnosable. + """ + try: + import httpx + except ModuleNotFoundError as exc: + if exc.name != "httpx": + raise + raise ImportError(MISSING_HTTPX_MESSAGE) from exc + + return httpx diff --git a/mlbstatsapi/_http.py b/mlbstatsapi/_http.py new file mode 100644 index 0000000..c8f3feb --- /dev/null +++ b/mlbstatsapi/_http.py @@ -0,0 +1,127 @@ +import inspect +import warnings +from typing import Protocol + +from .exceptions import MlbHttpError +from .warnings import MlbHttpCompatibilityWarning + + +HTTP_ERROR_BODY_EXCERPT_LIMIT = 500 + + +class _ResponseLike(Protocol): + content: bytes + text: str + + def json(self) -> object: + ... + + +def _is_mlbstatsapi_module(module_name: str) -> bool: + """Return True when module_name belongs to this package.""" + return module_name == "mlbstatsapi" or module_name.startswith("mlbstatsapi.") + + +def _compatibility_warning_stacklevel() -> int: + """Return a warnings.warn stacklevel for the first non-package caller.""" + frame = inspect.currentframe() + stacklevel = 1 + + try: + frame = frame.f_back + + while frame is not None: + module_name = frame.f_globals.get("__name__", "") + + if not _is_mlbstatsapi_module(module_name): + return stacklevel + + stacklevel += 1 + frame = frame.f_back + finally: + del frame + + return 1 + + +def _warn_http_compatibility( + *, + status_code: int, + url: str, +) -> None: + warnings.warn( + ( + f"HTTP {status_code} for {url} was suppressed because " + "strict_http=False explicitly selected compatibility mode, so the " + "historical empty result was returned. Strict HTTP behavior is the " + "default in version 1.0. Remove strict_http=False or pass " + "strict_http=True to raise MlbHttpError." + ), + MlbHttpCompatibilityWarning, + stacklevel=_compatibility_warning_stacklevel(), + ) + + +def _extract_error_response_data( + response: _ResponseLike, +) -> dict | list | None: + """Best-effort JSON extraction from an error response.""" + try: + if not response.content: + return None + + data = response.json() + except Exception: + return None + + if isinstance(data, (dict, list)): + return data + + return None + + +def _extract_error_body_excerpt( + response: _ResponseLike, +) -> str | None: + """Best-effort bounded text excerpt from an error response.""" + try: + if not response.content: + return None + + text = response.text + except Exception: + return None + + if not text: + return None + + return text[:HTTP_ERROR_BODY_EXCERPT_LIMIT] + + +def _build_http_error( + response: _ResponseLike, + *, + status_code: int, + reason: str, + url: str | None, + method: str, +) -> MlbHttpError: + """Build MlbHttpError from transport-neutral response context.""" + try: + response_data = _extract_error_response_data(response) + except Exception: + response_data = None + + try: + body_excerpt = _extract_error_body_excerpt(response) + except Exception: + body_excerpt = None + + return MlbHttpError( + status_code=status_code, + reason=reason, + url=url, + method=method, + response_data=response_data, + body_excerpt=body_excerpt, + ) \ No newline at end of file diff --git a/mlbstatsapi/async_mlb_dataadapter.py b/mlbstatsapi/async_mlb_dataadapter.py new file mode 100644 index 0000000..65f2974 --- /dev/null +++ b/mlbstatsapi/async_mlb_dataadapter.py @@ -0,0 +1,316 @@ +import asyncio +import logging + +from typing import Dict + +from ._async_support import import_httpx +from .exceptions import ( + MlbDecodeError, + MlbTimeoutError, + MlbTransportError, +) +from .mlb_dataadapter import ( + DEFAULT_TIMEOUT, + MlbResult, + TimeoutType, + _build_user_agent, + create_retry_policy, +) + +from ._http import ( + _build_http_error, + _warn_http_compatibility, +) + +# HTTPX is optional; it ships with the ``async`` extra. Importing it through +# the shared boundary means a sync-only install that reaches for async +# functionality gets install guidance instead of a bare ModuleNotFoundError +# naming a library it never asked for. Binding the module here keeps every +# ``httpx.`` reference below unchanged. +httpx = import_httpx() + + +class AsyncMlbDataAdapter: + """Async data adapter for MLB API.""" + + + def __init__( + self, + hostname: str = "statsapi.mlb.com", + ver: str = "v1", + logger: logging.Logger | None = None, + timeout: TimeoutType = DEFAULT_TIMEOUT, + client: httpx.AsyncClient | None = None, + *, + strict_http: bool = True, + ): + self.url = f"https://{hostname}/api/{ver}/" + self._logger = logger or logging.getLogger(__name__) + self._timeout = timeout + self._strict_http = strict_http + self._owns_client = client is None + self._retry_policy = create_retry_policy() + + if client is None: + # Only a library-owned client gets the package User-Agent. Passing + # it to the constructor replaces just that header, so httpx's other + # default headers (Accept, Accept-Encoding, Connection) survive. + self._client = httpx.AsyncClient( + headers={"User-Agent": _build_user_agent()}, + ) + else: + # An injected client stays exactly as the caller configured it. + self._client = client + + self._closed = False + + async def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> MlbResult: + """Get data from the MLB API.""" + """ + return a MlbResult from endpoint + + Parameters + ---------- + endpoint : str + rest api endpoint + ep_params : dict + params + data : dict + data to send with requests (we aren't using this) + + Returns + ------- + MlbResult + """ + + full_url = self.url + endpoint + logline_pre = f'url={full_url}' + logline_post = " ,".join( + ( + logline_pre, + 'success={}, status_code={}, message={}, url={}' + ) + ) + + self._logger.debug(logline_post) + response = await self._request_with_retries(full_url, ep_params) + + status_code = response.status_code + + if 400 <= status_code <= 499: + self._logger.error(msg=logline_post.format( + 'Invalid Request', + status_code, + response.reason_phrase, + str(response.url), + )) + # Strict mode raises for final non-404 4xx after retries are exhausted. + # 404 stays an empty MlbResult so endpoints keep None / [] / {} behavior. + if self._strict_http and status_code != 404: + raise _build_http_error( + response, + status_code=response.status_code, + reason=response.reason_phrase, + url=str(response.url) if response.url else full_url, + method="GET", + ) + if status_code != 404: + _warn_http_compatibility( + status_code=status_code, + url=str(response.url) if response.url else full_url, + ) + return MlbResult( + status_code=status_code, + message=response.reason_phrase, + data={}, + ) + + if 500 <= status_code <= 599: + self._logger.error(msg=logline_post.format( + 'Internal error occurred', + status_code, + response.reason_phrase, + str(response.url), + )) + raise _build_http_error( + response, + status_code=response.status_code, + reason=response.reason_phrase, + url=str(response.url) if response.url else full_url, + method="GET", + ) + + if not 200 <= status_code <= 299: + raise _build_http_error( + response, + status_code=response.status_code, + reason=response.reason_phrase, + url=str(response.url) if response.url else full_url, + method="GET", + ) + + self._logger.debug(msg=logline_post.format( + 'success', + status_code, + response.reason_phrase, + str(response.url), + )) + + if not response.content: + response_data = {} + else: + try: + response_data = response.json() + except ValueError as exc: + self._logger.error(msg=(str(exc))) + raise MlbDecodeError( + "Bad JSON in response" + ) from exc + + return MlbResult( + status_code, + message=response.reason_phrase, + data=response_data, + ) + + async def _request_with_retries( + self, + full_url: str, + ep_params: Dict, + ) -> httpx.Response: + """Issue the GET call, retrying with bounded backoff when this + adapter owns its httpx.AsyncClient. + + An injected client is called exactly once; its retry behavior stays + under caller control, matching the sync adapter's session-ownership + rule. + + Failures spend the retry budget the sync policy would spend, and + surface the public exception the sync adapter raises: + + ReadTimeout -> read budget -> MlbTimeoutError + ConnectTimeout -> connect budget -> MlbTimeoutError + ConnectError -> connect budget -> MlbTransportError + other TimeoutException -> total budget -> MlbTimeoutError + other RequestError -> total budget -> MlbTransportError + retryable HTTP status -> status budget + """ + policy = self._retry_policy + + attempt = 0 + while True: + attempt += 1 + try: + response = await self._client.get( + url=full_url, + params=ep_params, + timeout=self._translate_timeout(self._timeout), + ) + + except httpx.ReadTimeout as exc: + max_attempts = policy.read + 1 if self._owns_client else 1 + + if attempt >= max_attempts: + self._logger.error(msg=str(exc)) + raise MlbTimeoutError("Request failed") from exc + + await self._sleep_before_retry(attempt=attempt, response=None) + continue + + except httpx.ConnectTimeout as exc: + # Caught before httpx.TimeoutException: a connect timeout is a + # timeout for the caller, but it spends the connect budget so + # the retry accounting matches the sync policy. + max_attempts = policy.connect + 1 if self._owns_client else 1 + + if attempt >= max_attempts: + self._logger.error(msg=str(exc)) + raise MlbTimeoutError("Request failed") from exc + + await self._sleep_before_retry(attempt=attempt, response=None) + continue + + except httpx.ConnectError as exc: + max_attempts = policy.connect + 1 if self._owns_client else 1 + + if attempt >= max_attempts: + self._logger.error(msg=str(exc)) + raise MlbTransportError("Request failed") from exc + + await self._sleep_before_retry( + attempt=attempt, + response=None, + ) + continue + + except httpx.TimeoutException as exc: + max_attempts = policy.total + 1 if self._owns_client else 1 + + if attempt >= max_attempts: + raise MlbTimeoutError("Request failed") from exc + + await self._sleep_before_retry( + attempt=attempt, + response=None, + ) + continue + + except httpx.RequestError as exc: + max_attempts = policy.total + 1 if self._owns_client else 1 + + if attempt >= max_attempts: + self._logger.error(msg=str(exc)) + raise MlbTransportError("Request failed") from exc + + await self._sleep_before_retry(attempt=attempt, response=None) + continue + + max_attempts = policy.status + 1 if self._owns_client else 1 + + if response.status_code not in policy.status_forcelist or attempt >= max_attempts: + return response + + await self._sleep_before_retry(attempt=attempt, response=response) + + async def _sleep_before_retry( + self, + *, + attempt: int, + response: httpx.Response | None, + ) -> None: + policy = self._retry_policy + + if policy.respect_retry_after_header and response is not None: + retry_after = policy.get_retry_after(response) + if retry_after: + await asyncio.sleep(retry_after) + return + + # Mirrors urllib3's Retry.get_backoff_time(): no delay before the + # first retry, exponential thereafter, capped at backoff_max. + delay = 0.0 if attempt <= 1 else min( + policy.backoff_factor * (2 ** (attempt - 1)), + policy.backoff_max, + ) + + if delay > 0: + await asyncio.sleep(delay) + + @staticmethod + def _translate_timeout(timeout: TimeoutType) -> httpx.Timeout: + if isinstance(timeout, tuple): + connect_timeout, read_timeout = timeout + + return httpx.Timeout( + connect=connect_timeout, + read=read_timeout, + write=read_timeout, + pool=connect_timeout, + ) + + return httpx.Timeout(timeout) + + async def aclose(self) -> None: + if self._owns_client and not self._closed: + await self._client.aclose() + self._closed = True diff --git a/mlbstatsapi/mlb_dataadapter.py b/mlbstatsapi/mlb_dataadapter.py index 8082896..ae10ba6 100644 --- a/mlbstatsapi/mlb_dataadapter.py +++ b/mlbstatsapi/mlb_dataadapter.py @@ -3,19 +3,19 @@ from .exceptions import ( MlbDecodeError, - MlbHttpError, MlbTimeoutError, MlbTransportError, ) -from .warnings import MlbHttpCompatibilityWarning -import inspect import logging -import warnings import requests from requests.adapters import HTTPAdapter from urllib3.util.retry import Retry +from ._http import ( + _build_http_error, + _warn_http_compatibility, +) # Connect timeout, then read timeout. Callers may override with a scalar or tuple. DEFAULT_TIMEOUT = (3.05, 30.0) @@ -26,131 +26,6 @@ PACKAGE_DISTRIBUTION_NAME = "python-mlb-statsapi" UNKNOWN_PACKAGE_VERSION = "unknown" -# Bounded excerpt for error response bodies attached to MlbHttpError. -HTTP_ERROR_BODY_EXCERPT_LIMIT = 500 - - -def _is_mlbstatsapi_module(module_name: str) -> bool: - """Return True when *module_name* belongs to this package.""" - return module_name == "mlbstatsapi" or module_name.startswith("mlbstatsapi.") - - -def _compatibility_warning_stacklevel() -> int: - """Return a warnings.warn stacklevel for the first non-package caller. - - A fixed stack level cannot serve both direct MlbDataAdapter.get() calls and - public Mlb endpoint methods that wrap the adapter. Walk frames from the - caller of this helper outward and stop at the first module outside the - mlbstatsapi package namespace. - """ - frame = inspect.currentframe() - stacklevel = 1 - try: - frame = frame.f_back - while frame is not None: - module_name = frame.f_globals.get("__name__", "") - if not _is_mlbstatsapi_module(module_name): - return stacklevel - stacklevel += 1 - frame = frame.f_back - finally: - del frame - return 1 - - -def _warn_http_compatibility( - *, - status_code: int, - url: str, -) -> None: - """Warn that compatibility mode suppressed an error strict mode would raise. - - Only the status code and URL are reported; response bodies, headers, and - credentials must never reach a warning message. - """ - warnings.warn( - ( - f"HTTP {status_code} for {url} was suppressed because " - "strict_http=False explicitly selected compatibility mode, so the " - "historical empty result was returned. Strict HTTP behavior is the " - "default in version 1.0. Remove strict_http=False or pass " - "strict_http=True to raise MlbHttpError." - ), - MlbHttpCompatibilityWarning, - stacklevel=_compatibility_warning_stacklevel(), - ) - - -def _extract_error_response_data( - response: requests.Response, -) -> dict | list | None: - """Best-effort JSON object/list extraction from an error response. - - Returns None for empty bodies, invalid JSON, scalars, or unexpected failures. - Must not raise; context extraction cannot replace the original HTTP error. - """ - try: - if not response.content: - return None - data = response.json() - except Exception: - return None - - if isinstance(data, (dict, list)): - return data - return None - - -def _extract_error_body_excerpt( - response: requests.Response, -) -> str | None: - """Best-effort bounded text excerpt from an error response body. - - Returns None for empty bodies or unexpected text-decoding failures. - Must not raise; context extraction cannot replace the original HTTP error. - """ - try: - if not response.content: - return None - text = response.text - except Exception: - return None - - if not text: - return None - return text[:HTTP_ERROR_BODY_EXCERPT_LIMIT] - - -def _build_http_error( - response: requests.Response, - *, - method: str, - fallback_url: str, -) -> MlbHttpError: - """Build an MlbHttpError with best-effort response context. - - Extraction failures must not prevent raising MlbHttpError with status, - reason, URL, and method. - """ - try: - response_data = _extract_error_response_data(response) - except Exception: - response_data = None - - try: - body_excerpt = _extract_error_body_excerpt(response) - except Exception: - body_excerpt = None - - return MlbHttpError( - status_code=response.status_code, - reason=response.reason, - url=response.url or fallback_url, - method=method, - response_data=response_data, - body_excerpt=body_excerpt, - ) - def create_retry_policy() -> Retry: """Create a new instance of the default MLB HTTP retry policy.""" @@ -340,8 +215,10 @@ def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> MlbRe if self._strict_http and status_code != 404: raise _build_http_error( response, + status_code=response.status_code, + reason=response.reason, + url=response.url or full_url, method="GET", - fallback_url=full_url, ) if status_code != 404: _warn_http_compatibility( @@ -363,15 +240,19 @@ def get(self, endpoint: str, ep_params: Dict = None, data: Dict = None) -> MlbRe )) raise _build_http_error( response, + status_code=response.status_code, + reason=response.reason, + url=response.url or full_url, method="GET", - fallback_url=full_url, ) if not 200 <= status_code <= 299: raise _build_http_error( response, + status_code=response.status_code, + reason=response.reason, + url=response.url or full_url, method="GET", - fallback_url=full_url, ) self._logger.debug(msg=logline_post.format( diff --git a/tests/test_async_mlb_dataadapter.py b/tests/test_async_mlb_dataadapter.py new file mode 100644 index 0000000..a5ab3b8 --- /dev/null +++ b/tests/test_async_mlb_dataadapter.py @@ -0,0 +1,771 @@ +"""Focused offline tests for the AsyncMlbDataAdapter implementation. + +Covers the behavior delivered in issue #301: successful GETs, the HTTP status +contract, exception mapping, lifecycle and ownership, timeout translation, +User-Agent, bounded retry-with-backoff, cancellation, and concurrency. The +exhaustive async transport-contract matrix belongs to #302. + +The retry assertions mirror the contract asserted for the sync adapter in +tests/test_mlb_retries.py, adapted to httpx.MockTransport instead of a real +threaded HTTP server, since the async retry loop here is hand-rolled Python +rather than logic buried inside urllib3/requests internals. + +HTTPX ships only with the ``async`` extra, so the whole module skips when it is +absent. See the import section below. + +These tests must not contact the live MLB API. +""" + +from __future__ import annotations + +import asyncio +import contextlib +from importlib.metadata import PackageNotFoundError +from unittest.mock import AsyncMock, patch + +import pytest + +# Every test below drives the real HTTPX-backed adapter, so a sync-only install +# has nothing here to run. Skipping at collection keeps ``pytest tests/`` +# working without the ``async`` extra instead of erroring on the import. The +# optional-dependency contract itself is asserted in +# tests/test_async_optional_dependency.py, which runs with or without HTTPX. +httpx = pytest.importorskip("httpx", reason="requires the async extra (HTTPX)") + +from mlbstatsapi import ( # noqa: E402 + MlbDecodeError, + MlbHttpCompatibilityWarning, + MlbHttpError, + MlbTimeoutError, + MlbTransportError, +) +from mlbstatsapi.async_mlb_dataadapter import AsyncMlbDataAdapter # noqa: E402 +from mlbstatsapi.mlb_dataadapter import PACKAGE_DISTRIBUTION_NAME # noqa: E402 + +from http_contract_support import ( # noqa: E402 + RETRYABLE_STATUS_CODES, + SERVER_ERRORS, + assert_library_retry_policy, +) + + +BASE_URL = "https://statsapi.mlb.com/api/v1/" + +SLEEP_TARGET = "mlbstatsapi.async_mlb_dataadapter.asyncio.sleep" + +# Patched only while a test adapter is constructed, so the adapter creates its +# own library-owned client the way production does, over a MockTransport. +CLIENT_TARGET = "mlbstatsapi.async_mlb_dataadapter.httpx.AsyncClient" + +# Matches tests/test_mlb_session.py, so both adapters assert the same contract. +MOCKED_PACKAGE_VERSION = "9.8.7" +MOCKED_USER_AGENT = f"python-mlb-statsapi/{MOCKED_PACKAGE_VERSION}" + + +# Adapters built by _owned_adapter(); run_async() closes them inside the same +# event loop that used them, so no AsyncClient is left open by a test. +_ADAPTERS_TO_CLOSE: list[AsyncMlbDataAdapter] = [] + + +def run_async(coro): + async def runner(): + try: + return await coro + finally: + while _ADAPTERS_TO_CLOSE: + await _ADAPTERS_TO_CLOSE.pop().aclose() + + return asyncio.run(runner()) + + +class _ScriptedHandler: + """Serve a scripted sequence of httpx Responses/exceptions. + + The last entry repeats for any call beyond the script's length, so a + single-item script models a persistent failure. + """ + + def __init__(self, *script: httpx.Response | Exception): + self._script = list(script) + self.call_count = 0 + + def __call__(self, request: httpx.Request) -> httpx.Response: + self.call_count += 1 + index = min(self.call_count - 1, len(self._script) - 1) + item = self._script[index] + if isinstance(item, Exception): + raise item + return item + + +def _response(status_code: int, *, headers: dict | None = None, text: str | None = None) -> httpx.Response: + return httpx.Response(status_code, headers=headers or {}, text=text) + + +def _owned_adapter(handler, **kwargs) -> AsyncMlbDataAdapter: + """Build an adapter that owns its client, so retries are active. + + The adapter still builds its own client through the production path — only + the transport is swapped for a MockTransport — so ownership, headers, and + retry behavior are exactly what the library does at runtime, and no client + is constructed and then discarded. Call this from inside a run_async() + scenario; run_async() closes what it creates. + """ + real_async_client = httpx.AsyncClient + + def mock_transport_client(**client_kwargs) -> httpx.AsyncClient: + return real_async_client( + transport=httpx.MockTransport(handler), + **client_kwargs, + ) + + with patch(CLIENT_TARGET, mock_transport_client): + adapter = AsyncMlbDataAdapter(**kwargs) + + _ADAPTERS_TO_CLOSE.append(adapter) + return adapter + + +def _injected_adapter(handler, **kwargs) -> AsyncMlbDataAdapter: + """Build an adapter with a caller-supplied client, so retries are bypassed.""" + client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + return AsyncMlbDataAdapter(client=client, **kwargs) + + +def test_retry_policy_matches_library_default(): + adapter = AsyncMlbDataAdapter() + assert_library_retry_policy(adapter._retry_policy) + + +def test_200_succeeds_with_no_retry(): + handler = _ScriptedHandler(_response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock) as sleep_mock: + result = await adapter.get(endpoint="sports") + return result, sleep_mock + + result, sleep_mock = run_async(scenario()) + assert result.status_code == 200 + assert handler.call_count == 1 + sleep_mock.assert_not_awaited() + + +def test_200_response_returns_actual_json_data(): + payload = {"sports": [{"id": 1, "name": "Major League Baseball"}]} + handler = _ScriptedHandler(httpx.Response(200, json=payload)) + + async def scenario(): + adapter = _owned_adapter(handler) + return await adapter.get(endpoint="sports") + + result = run_async(scenario()) + assert result.status_code == 200 + assert result.data == payload + + +def test_explicit_empty_successful_response_returns_empty_data(): + handler = _ScriptedHandler(_response(204, text="")) + + async def scenario(): + adapter = _owned_adapter(handler) + return await adapter.get(endpoint="sports") + + result = run_async(scenario()) + assert result.status_code == 204 + assert result.data == {} + + +def test_mlb_http_error_has_structured_context(): + payload = {"messageNumber": 1, "message": "Internal error occurred"} + handler = _ScriptedHandler(httpx.Response(500, json=payload)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.raises(MlbHttpError) as exc_info: + await adapter.get(endpoint="sports") + return exc_info.value + + error = run_async(scenario()) + assert error.status_code == 500 + assert error.reason == "Internal Server Error" + assert error.method == "GET" + assert error.url == f"{BASE_URL}sports" + assert error.response_data == payload + assert error.body_excerpt is not None + assert "Internal error occurred" in error.body_excerpt + + +def test_library_owned_client_closes(): + async def scenario(): + adapter = AsyncMlbDataAdapter() + was_open = not adapter._client.is_closed + await adapter.aclose() + return was_open, adapter._client.is_closed + + was_open, is_closed = run_async(scenario()) + assert was_open is True + assert is_closed is True + + +def test_aclose_is_idempotent(): + async def scenario(): + adapter = AsyncMlbDataAdapter() + await adapter.aclose() + with patch.object(adapter._client, "aclose", new_callable=AsyncMock) as aclose_mock: + await adapter.aclose() + return aclose_mock + + aclose_mock = run_async(scenario()) + aclose_mock.assert_not_awaited() + + +def test_injected_client_is_not_closed(): + async def scenario(): + client = httpx.AsyncClient() + adapter = AsyncMlbDataAdapter(client=client) + await adapter.aclose() + was_closed = client.is_closed + await client.aclose() + return was_closed + + was_closed = run_async(scenario()) + assert was_closed is False + + +def test_injected_client_timeout_configuration_is_not_mutated(): + """The library's timeout is applied per request, not written to the client.""" + handler = _ScriptedHandler(_response(200)) + + async def scenario(): + client = httpx.AsyncClient( + transport=httpx.MockTransport(handler), + timeout=httpx.Timeout(11.0), + ) + try: + adapter = AsyncMlbDataAdapter(client=client, timeout=(1.0, 2.0)) + await adapter.get(endpoint="sports") + return client.timeout + finally: + await client.aclose() + + timeout = run_async(scenario()) + assert timeout.connect == 11.0 + assert timeout.read == 11.0 + assert timeout.write == 11.0 + assert timeout.pool == 11.0 + + +def test_scalar_timeout_translation(): + result = AsyncMlbDataAdapter._translate_timeout(5) + assert result.connect == 5 + assert result.read == 5 + assert result.write == 5 + assert result.pool == 5 + + +def test_tuple_timeout_translation(): + result = AsyncMlbDataAdapter._translate_timeout((3.05, 30.0)) + assert result.connect == 3.05 + assert result.pool == 3.05 + assert result.read == 30.0 + assert result.write == 30.0 + + +def test_multiple_concurrent_requests_on_one_adapter(): + responses = { + "sports": httpx.Response(200, json={"id": "sports"}), + "teams": httpx.Response(200, json={"id": "teams"}), + } + + def handler(request: httpx.Request) -> httpx.Response: + endpoint = request.url.path.rsplit("/", 1)[-1] + return responses[endpoint] + + async def scenario(): + adapter = _owned_adapter(handler) + return await asyncio.gather( + adapter.get(endpoint="sports"), + adapter.get(endpoint="teams"), + ) + + sports_result, teams_result = run_async(scenario()) + assert sports_result.data == {"id": "sports"} + assert teams_result.data == {"id": "teams"} + + +def test_cancelling_one_request_does_not_cancel_another(): + async def handler(request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("hang"): + await asyncio.sleep(10) + raise AssertionError("handler should have been cancelled before returning") + return _response(200) + + async def scenario(): + adapter = _owned_adapter(handler) + + hanging_task = asyncio.ensure_future(adapter.get(endpoint="hang")) + await asyncio.sleep(0) + + other_task = asyncio.ensure_future(adapter.get(endpoint="sports")) + + hanging_task.cancel() + with pytest.raises(asyncio.CancelledError): + await hanging_task + + return await other_task + + result = run_async(scenario()) + assert result.status_code == 200 + + +def test_injected_client_persistent_server_error_is_not_retried(): + handler = _ScriptedHandler(_response(500)) + + async def scenario(): + adapter = _injected_adapter(handler) + with pytest.raises(MlbHttpError) as exc_info: + await adapter.get(endpoint="sports") + return exc_info.value.status_code + + status_code = run_async(scenario()) + assert status_code == 500 + assert handler.call_count == 1 + + +def test_injected_client_does_not_consume_a_second_scripted_response(): + handler = _ScriptedHandler(_response(500), _response(200)) + + async def scenario(): + adapter = _injected_adapter(handler) + with pytest.raises(MlbHttpError): + await adapter.get(endpoint="sports") + + run_async(scenario()) + assert handler.call_count == 1 + + +@pytest.mark.parametrize("status_code", RETRYABLE_STATUS_CODES) +def test_owned_client_retries_retryable_status_then_succeeds(status_code): + handler = _ScriptedHandler(_response(status_code), _response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + return await adapter.get(endpoint="sports") + + result = run_async(scenario()) + assert result.status_code == 200 + assert handler.call_count == 2 + + +@pytest.mark.parametrize("status_code", SERVER_ERRORS) +def test_owned_client_exhausts_retries_on_persistent_server_error(status_code): + handler = _ScriptedHandler(_response(status_code)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.raises(MlbHttpError) as exc_info: + await adapter.get(endpoint="sports") + return exc_info.value.status_code + + returned_status = run_async(scenario()) + assert returned_status == status_code + assert handler.call_count == 4 + + +def test_owned_client_final_429_raises_under_strict_http(): + handler = _ScriptedHandler(_response(429)) + + async def scenario(): + adapter = _owned_adapter(handler, strict_http=True) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.raises(MlbHttpError) as exc_info: + await adapter.get(endpoint="sports") + return exc_info.value.status_code + + status_code = run_async(scenario()) + assert status_code == 429 + assert handler.call_count == 4 + + +def test_owned_client_final_429_returns_empty_result_under_compatibility_mode(): + handler = _ScriptedHandler(_response(429)) + + async def scenario(): + adapter = _owned_adapter(handler, strict_http=False) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.warns(MlbHttpCompatibilityWarning) as warning_info: + result = await adapter.get(endpoint="sports") + return result, warning_info + + result, warning_info = run_async(scenario()) + assert result.status_code == 429 + assert result.data == {} + assert len(warning_info) == 1 + assert handler.call_count == 4 + + +def test_400_is_not_retried(): + handler = _ScriptedHandler(_response(400), _response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + with pytest.raises(MlbHttpError): + await adapter.get(endpoint="sports") + + run_async(scenario()) + assert handler.call_count == 1 + + +def test_404_is_not_retried(): + handler = _ScriptedHandler(_response(404), _response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + return await adapter.get(endpoint="sports") + + result = run_async(scenario()) + assert result.status_code == 404 + assert result.data == {} + assert handler.call_count == 1 + + +def test_other_non_2xx_status_raises_http_error(): + """A final non-2xx outside the 4xx/5xx ranges still raises MlbHttpError.""" + handler = _ScriptedHandler( + _response(302, headers={"Location": "https://example.test/moved"}), + ) + + async def scenario(): + # Redirects are not followed, so the 302 reaches the status contract. + adapter = _owned_adapter(handler) + with pytest.raises(MlbHttpError) as exc_info: + await adapter.get(endpoint="sports") + return exc_info.value + + error = run_async(scenario()) + assert error.status_code == 302 + assert error.method == "GET" + assert handler.call_count == 1 + + +def test_timeout_retried_then_succeeds(): + handler = _ScriptedHandler(httpx.ReadTimeout("timed out"), _response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + return await adapter.get(endpoint="sports") + + result = run_async(scenario()) + assert result.status_code == 200 + assert handler.call_count == 2 + + +def test_timeout_exhausts_retries_and_raises_mlb_timeout_error(): + handler = _ScriptedHandler(httpx.ReadTimeout("timed out")) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.raises(MlbTimeoutError): + await adapter.get(endpoint="sports") + + run_async(scenario()) + assert handler.call_count == 3 + + +def test_connect_timeout_exhausts_retries_and_raises_mlb_timeout_error(): + """A connect timeout stays a timeout for the caller. + + httpx.ConnectTimeout subclasses httpx.TimeoutException, so it needs its + own branch to spend the connect budget while still raising + MlbTimeoutError rather than MlbTransportError. + """ + handler = _ScriptedHandler(httpx.ConnectTimeout("connect timed out")) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.raises(MlbTimeoutError) as exc_info: + await adapter.get(endpoint="sports") + return exc_info.value + + error = run_async(scenario()) + assert handler.call_count == 4 + # MlbTimeoutError subclasses MlbTransportError, so only the exact type + # distinguishes a timeout from a plain transport failure. + assert type(error) is MlbTimeoutError + assert isinstance(error.__cause__, httpx.ConnectTimeout) + + +def test_connect_timeout_spends_the_connect_retry_budget(): + """The connect budget bounds a connect timeout, not the total or read one. + + The default policy uses total=3 and connect=3, so attempt counts alone + cannot tell those two budgets apart. Narrowing connect makes the + difference observable: falling through to the generic timeout branch + would still allow four attempts here. + """ + handler = _ScriptedHandler(httpx.ConnectTimeout("connect timed out")) + + async def scenario(): + adapter = _owned_adapter(handler) + adapter._retry_policy.connect = 1 + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.raises(MlbTimeoutError): + await adapter.get(endpoint="sports") + + run_async(scenario()) + assert handler.call_count == 2 + + +def test_transport_error_retried_then_succeeds(): + handler = _ScriptedHandler(httpx.ConnectError("connection refused"), _response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + return await adapter.get(endpoint="sports") + + result = run_async(scenario()) + assert result.status_code == 200 + assert handler.call_count == 2 + + +def test_transport_error_exhausts_retries_and_raises_mlb_transport_error(): + handler = _ScriptedHandler(httpx.ConnectError("connection refused")) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock): + with pytest.raises(MlbTransportError): + await adapter.get(endpoint="sports") + + run_async(scenario()) + assert handler.call_count == 4 + + +def test_retry_after_header_drives_sleep_duration(): + handler = _ScriptedHandler( + _response(429, headers={"Retry-After": "7"}), + _response(200), + ) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock) as sleep_mock: + await adapter.get(endpoint="sports") + return sleep_mock + + sleep_mock = run_async(scenario()) + sleep_mock.assert_awaited_once_with(7) + + +def test_no_delay_before_first_retry(): + handler = _ScriptedHandler(_response(500), _response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock) as sleep_mock: + await adapter.get(endpoint="sports") + return sleep_mock + + sleep_mock = run_async(scenario()) + sleep_mock.assert_not_awaited() + + +def test_backoff_grows_exponentially_between_retries(): + handler = _ScriptedHandler(_response(500)) + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, new_callable=AsyncMock) as sleep_mock: + with pytest.raises(MlbHttpError): + await adapter.get(endpoint="sports") + return sleep_mock + + sleep_mock = run_async(scenario()) + assert [call.args[0] for call in sleep_mock.await_args_list] == [1.0, 2.0] + + +def test_retry_sleep_is_async_and_non_blocking(): + """A real (unmocked) backoff wait must yield the event loop. + + If _sleep_before_retry ever used a blocking call (e.g. time.sleep) + instead of `await asyncio.sleep(...)`, the whole event loop would + freeze for the wait's duration and the concurrently running marker + task below would make zero progress during it. + """ + handler = _ScriptedHandler(_response(500), _response(500), _response(200)) + + async def scenario(): + adapter = _owned_adapter(handler) + # Small but real backoff so the test stays fast without mocking sleep. + adapter._retry_policy.backoff_factor = 0.05 + + marker_ticks = 0 + + async def marker(): + nonlocal marker_ticks + for _ in range(50): + await asyncio.sleep(0.005) + marker_ticks += 1 + + marker_task = asyncio.ensure_future(marker()) + try: + result = await adapter.get(endpoint="sports") + finally: + marker_task.cancel() + with contextlib.suppress(asyncio.CancelledError): + await marker_task + + return result, marker_ticks + + result, marker_ticks = run_async(scenario()) + assert result.status_code == 200 + assert marker_ticks > 0 + + +def test_cancelled_error_propagates_without_retry_during_network_call(): + call_count = 0 + + async def hanging_handler(request: httpx.Request) -> httpx.Response: + nonlocal call_count + call_count += 1 + await asyncio.sleep(10) + raise AssertionError("handler should have been cancelled before returning") + + async def scenario(): + adapter = _owned_adapter(hanging_handler) + task = asyncio.ensure_future(adapter.get(endpoint="sports")) + await asyncio.sleep(0) + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + + run_async(scenario()) + assert call_count == 1 + + +def test_cancelled_error_propagates_without_retry_during_backoff_sleep(): + handler = _ScriptedHandler(_response(500), _response(500), _response(200)) + + async def cancelling_sleep(delay): + raise asyncio.CancelledError() + + async def scenario(): + adapter = _owned_adapter(handler) + with patch(SLEEP_TARGET, side_effect=cancelling_sleep): + with pytest.raises(asyncio.CancelledError): + await adapter.get(endpoint="sports") + + run_async(scenario()) + assert handler.call_count == 2 + + +def test_json_decode_failure_is_not_retried(): + handler = _ScriptedHandler(_response(200, text="not json")) + + async def scenario(): + adapter = _owned_adapter(handler) + with pytest.raises(MlbDecodeError) as exc_info: + await adapter.get(endpoint="sports") + return exc_info.value + + error = run_async(scenario()) + # Matches the sync adapter: the underlying decode failure stays the cause. + assert isinstance(error.__cause__, ValueError) + assert handler.call_count == 1 + + +# --- Versioned User-Agent --- + + +def test_library_owned_client_has_versioned_user_agent(): + """A library-created AsyncClient sends the package and version User-Agent.""" + with patch( + "mlbstatsapi.mlb_dataadapter.package_version", + return_value=MOCKED_PACKAGE_VERSION, + ) as lookup: + adapter = AsyncMlbDataAdapter() + try: + assert adapter._client.headers["User-Agent"] == MOCKED_USER_AGENT + finally: + run_async(adapter.aclose()) + + lookup.assert_called_with(PACKAGE_DISTRIBUTION_NAME) + + +def test_library_owned_client_user_agent_uses_installed_version(): + """Without patching, the User-Agent still names this package.""" + adapter = AsyncMlbDataAdapter() + try: + assert adapter._client.headers["User-Agent"].startswith( + f"{PACKAGE_DISTRIBUTION_NAME}/", + ) + finally: + run_async(adapter.aclose()) + + +def test_library_owned_client_user_agent_falls_back_when_metadata_missing(): + """Missing distribution metadata yields the "unknown" fallback, not an error.""" + with patch( + "mlbstatsapi.mlb_dataadapter.package_version", + side_effect=PackageNotFoundError(PACKAGE_DISTRIBUTION_NAME), + ): + adapter = AsyncMlbDataAdapter() + try: + assert adapter._client.headers["User-Agent"] == "python-mlb-statsapi/unknown" + finally: + run_async(adapter.aclose()) + + +def test_library_owned_client_preserves_httpx_default_headers(): + """Only User-Agent changes; HTTPX's other default headers are untouched. + + Mirrors test_mlb_session.test_library_created_session_preserves_requests_ + default_headers for the async client. + """ + baseline = httpx.AsyncClient() + adapter = AsyncMlbDataAdapter() + try: + for header, value in baseline.headers.items(): + if header.lower() == "user-agent": + continue + assert adapter._client.headers[header] == value + + for header in ("Accept", "Accept-Encoding", "Connection"): + assert adapter._client.headers[header] == baseline.headers[header] + + assert adapter._client.headers["User-Agent"] != baseline.headers["User-Agent"] + finally: + run_async(adapter.aclose()) + run_async(baseline.aclose()) + + +def test_injected_client_headers_are_unchanged(): + """Headers on a caller-supplied client survive adapter construction.""" + async def scenario(): + client = httpx.AsyncClient( + headers={ + "User-Agent": "my-baseball-project/1.0", + "X-Application": "scoreboard", + }, + ) + headers_before = dict(client.headers) + try: + adapter = AsyncMlbDataAdapter(client=client) + + assert adapter._client is client + assert dict(client.headers) == headers_before + assert client.headers["User-Agent"] == "my-baseball-project/1.0" + assert client.headers["X-Application"] == "scoreboard" + finally: + await client.aclose() + + run_async(scenario()) diff --git a/tests/test_async_optional_dependency.py b/tests/test_async_optional_dependency.py new file mode 100644 index 0000000..b6d8cca --- /dev/null +++ b/tests/test_async_optional_dependency.py @@ -0,0 +1,427 @@ +"""Offline tests for the async optional-dependency boundary (issue #301). + +HTTPX ships only with the ``python-mlb-statsapi[async]`` extra, so three things +have to hold at once: + +* ``from mlbstatsapi import AsyncMlbDataAdapter`` works when the extra is + installed +* ``import mlbstatsapi`` and the whole sync surface keep working when it is not +* reaching for async functionality without it produces actionable install + guidance instead of a bare ``ModuleNotFoundError`` + +That guidance is reserved for a genuinely missing HTTPX: an installed but broken +HTTPX must keep reporting its own failure. + +Optional-import behavior is easy to test misleadingly, because ``httpx`` and +``mlbstatsapi`` are already in ``sys.modules`` by the time this file runs. Every +"HTTPX is missing" case therefore runs in a child interpreter that blocks the +import at ``sys.meta_path`` before ``mlbstatsapi`` is imported at all, which +also means the developer environment never has to uninstall anything. + +Unlike tests/test_async_mlb_dataadapter.py, this module must never skip as a +whole: most of what it asserts is exactly the behavior of an install that has no +HTTPX, so it has to keep running in one. Nothing that needs HTTPX is imported at +module scope; the few cases that do require it skip individually. + +These tests must not contact the live MLB API. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest + +import mlbstatsapi + +from test_public_api import ( + OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS, + SUPPORTED_PACKAGE_ROOT_SYMBOLS, +) + + +PROJECT_ROOT = Path(__file__).resolve().parent.parent + +# The guidance callers must be able to act on. Asserted as a substring so the +# surrounding sentence can be reworded without breaking these tests. +ASYNC_EXTRA_REQUIREMENT = "python-mlb-statsapi[async]" + +# Prepended to a child program to simulate a sync-only install. The finder +# rejects httpx before any path-based finder can satisfy it, so an installed +# HTTPX in this environment is invisible to the child. +BLOCK_HTTPX = """ +import sys + + +class _HttpxBlocker: + # Makes httpx look uninstalled, exactly as ModuleNotFoundError would. + def find_spec(self, fullname, path=None, target=None): + if fullname == "httpx" or fullname.startswith("httpx."): + raise ModuleNotFoundError( + f"No module named {fullname!r}", name=fullname + ) + return None + + +sys.meta_path.insert(0, _HttpxBlocker()) +assert "httpx" not in sys.modules, "child started with httpx already imported" +assert "mlbstatsapi" not in sys.modules, "child started with mlbstatsapi imported" +""" + +# Prepended to a child program to simulate an installed but broken HTTPX: the +# httpx import fails, yet httpx itself is present. The user's problem is a +# broken dependency tree, not a missing extra, so the boundary must not rewrite +# it into install guidance. +BREAK_HTTPX_DEPENDENCY = """ +import sys + + +class _BrokenHttpxDependency: + def find_spec(self, fullname, path=None, target=None): + if fullname == "httpx": + raise ModuleNotFoundError( + "No module named 'httpcore'", name="httpcore" + ) + return None + + +sys.meta_path.insert(0, _BrokenHttpxDependency()) +assert "httpx" not in sys.modules, "child started with httpx already imported" +""" + + +def _run_child( + body: str, + *, + block_httpx: bool = False, + break_httpx: bool = False, +) -> str: + """Run ``body`` in a fresh interpreter against this working tree. + + ``block_httpx`` makes HTTPX look uninstalled; ``break_httpx`` makes it look + installed but unimportable. They describe different environments, so a test + picks exactly one. + """ + assert not (block_httpx and break_httpx), "pick one HTTPX environment" + + program = textwrap.dedent(body) + if block_httpx: + program = BLOCK_HTTPX + program + elif break_httpx: + program = BREAK_HTTPX_DEPENDENCY + program + + completed = subprocess.run( + [sys.executable, "-c", program], + cwd=PROJECT_ROOT, + # Import the working tree rather than any installed copy of the package. + env={**os.environ, "PYTHONPATH": str(PROJECT_ROOT)}, + capture_output=True, + text=True, + timeout=120, + ) + + assert completed.returncode == 0, ( + "child interpreter failed\n" + f"--- stdout ---\n{completed.stdout}\n" + f"--- stderr ---\n{completed.stderr}" + ) + return completed.stdout + + +# --------------------------------------------------------------------------- +# Package-root boundary +# +# These run in any environment. The two cases that need a real HTTPX to prove +# anything skip individually rather than taking the module with them. +# --------------------------------------------------------------------------- + + +def test_async_adapter_is_exported_from_the_package_root() -> None: + pytest.importorskip("httpx", reason="requires the async extra (HTTPX)") + from mlbstatsapi.async_mlb_dataadapter import AsyncMlbDataAdapter + + from mlbstatsapi import AsyncMlbDataAdapter as exported + + assert exported is AsyncMlbDataAdapter + assert mlbstatsapi.AsyncMlbDataAdapter is AsyncMlbDataAdapter + assert exported.__module__ == "mlbstatsapi.async_mlb_dataadapter" + + +def test_async_adapter_is_discoverable_from_the_package_root() -> None: + assert "AsyncMlbDataAdapter" in dir(mlbstatsapi) + + +def test_package_root_does_not_expose_httpx() -> None: + """HTTPX stays an implementation detail of the async adapter.""" + assert not hasattr(mlbstatsapi, "httpx") + + +def test_unknown_package_root_attribute_still_raises_attribute_error() -> None: + with pytest.raises(AttributeError): + mlbstatsapi.NotARealPublicSymbol # noqa: B018 + + +def test_importing_the_package_does_not_import_httpx() -> None: + """The boundary is lazy: a sync-only caller never pays for HTTPX.""" + _run_child( + """ + import sys + + import mlbstatsapi + from mlbstatsapi import Mlb, MlbDataAdapter + + imported = sorted(name for name in sys.modules if name.startswith("httpx")) + assert not imported, imported + assert "mlbstatsapi.async_mlb_dataadapter" not in sys.modules + """, + block_httpx=False, + ) + + +def test_async_access_imports_httpx_on_demand() -> None: + pytest.importorskip("httpx", reason="requires the async extra (HTTPX)") + _run_child( + """ + import sys + + import mlbstatsapi + + assert "httpx" not in sys.modules + adapter_class = mlbstatsapi.AsyncMlbDataAdapter + assert "httpx" in sys.modules + assert adapter_class.__module__ == "mlbstatsapi.async_mlb_dataadapter" + + # Resolved once, then cached as an ordinary module attribute. + assert mlbstatsapi.AsyncMlbDataAdapter is adapter_class + """, + block_httpx=False, + ) + + +# --------------------------------------------------------------------------- +# Without HTTPX installed +# --------------------------------------------------------------------------- + + +def test_sync_only_install_can_import_the_package() -> None: + _run_child( + """ + import sys + + import mlbstatsapi + from mlbstatsapi import Mlb + from mlbstatsapi import MlbDataAdapter + + assert "httpx" not in sys.modules + """, + block_httpx=True, + ) + + +def test_sync_only_install_keeps_every_supported_package_root_symbol() -> None: + """Every always-available public symbol must resolve without the extra. + + ``SUPPORTED_PACKAGE_ROOT_SYMBOLS`` is the always-available half of the 1.x + package-root API. The async half is covered separately below; both halves + are public API. + """ + _run_child( + f""" + import mlbstatsapi + + for name in {list(SUPPORTED_PACKAGE_ROOT_SYMBOLS)!r}: + assert getattr(mlbstatsapi, name) is not None, name + """, + block_httpx=True, + ) + + +def test_sync_only_install_reports_the_extra_for_every_async_symbol() -> None: + """The optional async manifest is exactly what the extra unlocks.""" + _run_child( + f""" + import mlbstatsapi + + for name in {list(OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS)!r}: + try: + getattr(mlbstatsapi, name) + except ImportError as exc: + assert "python-mlb-statsapi[async]" in str(exc), str(exc) + else: + raise AssertionError(f"expected an ImportError for {{name}}") + """, + block_httpx=True, + ) + + +def test_sync_only_install_can_still_use_the_sync_adapter() -> None: + """The boundary changes no sync behavior, including Session ownership.""" + _run_child( + """ + import sys + + from mlbstatsapi import Mlb, MlbDataAdapter, MlbResult + + adapter = MlbDataAdapter() + try: + assert adapter.url == "https://statsapi.mlb.com/api/v1/" + assert adapter._owns_session is True + assert "python-mlb-statsapi/" in adapter._session.headers["User-Agent"] + finally: + adapter.close() + assert adapter._closed is True + + with Mlb() as mlb: + assert mlb._owns_session is True + + result = MlbResult(404, "Not Found") + assert result.data == {} + + assert "httpx" not in sys.modules + """, + block_httpx=True, + ) + + +def test_missing_httpx_reports_the_async_extra_from_the_package_root() -> None: + stdout = _run_child( + """ + import mlbstatsapi + + try: + from mlbstatsapi import AsyncMlbDataAdapter + except ImportError as exc: + message = str(exc) + cause = exc.__cause__ + else: + raise AssertionError("expected an ImportError without httpx") + + assert "python-mlb-statsapi[async]" in message, message + assert "pip install" in message, message + # The real failure stays diagnosable behind the friendly message. + assert isinstance(cause, ModuleNotFoundError), cause + assert cause.name == "httpx", cause.name + + print(message) + """, + block_httpx=True, + ) + + assert ASYNC_EXTRA_REQUIREMENT in stdout + + +def test_missing_httpx_reports_the_async_extra_from_attribute_access() -> None: + _run_child( + """ + import mlbstatsapi + + try: + mlbstatsapi.AsyncMlbDataAdapter + except ImportError as exc: + message = str(exc) + else: + raise AssertionError("expected an ImportError without httpx") + + assert "python-mlb-statsapi[async]" in message, message + """, + block_httpx=True, + ) + + +def test_missing_httpx_reports_the_async_extra_from_the_async_module() -> None: + """Importing the module directly hits the same boundary, not a raw httpx error.""" + _run_child( + """ + try: + import mlbstatsapi.async_mlb_dataadapter # noqa: F401 + except ImportError as exc: + message = str(exc) + else: + raise AssertionError("expected an ImportError without httpx") + + assert "python-mlb-statsapi[async]" in message, message + assert "pip install" in message, message + """, + block_httpx=True, + ) + + +def test_async_name_stays_discoverable_without_httpx() -> None: + """Discoverability must not require the optional dependency.""" + _run_child( + f""" + import sys + + import mlbstatsapi + + for name in {list(OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS)!r}: + assert name in dir(mlbstatsapi), name + assert "httpx" not in sys.modules + """, + block_httpx=True, + ) + + +def test_failed_async_access_leaves_the_sync_api_usable() -> None: + _run_child( + """ + import mlbstatsapi + + for _ in range(2): + try: + mlbstatsapi.AsyncMlbDataAdapter + except ImportError as exc: + assert "python-mlb-statsapi[async]" in str(exc), str(exc) + else: + raise AssertionError("expected an ImportError without httpx") + + adapter = mlbstatsapi.MlbDataAdapter() + try: + assert adapter.url == "https://statsapi.mlb.com/api/v1/" + finally: + adapter.close() + """, + block_httpx=True, + ) + + +# --------------------------------------------------------------------------- +# With HTTPX installed but broken +# --------------------------------------------------------------------------- + + +def test_broken_httpx_install_is_not_reported_as_a_missing_extra() -> None: + """Installing the extra would not fix a broken HTTPX, so do not suggest it.""" + _run_child( + """ + try: + import mlbstatsapi.async_mlb_dataadapter # noqa: F401 + except ModuleNotFoundError as exc: + assert exc.name == "httpcore", exc.name + assert "python-mlb-statsapi[async]" not in str(exc), str(exc) + else: + raise AssertionError("expected the underlying import failure") + """, + break_httpx=True, + ) + + +def test_broken_httpx_install_surfaces_from_the_package_root_too() -> None: + _run_child( + """ + import mlbstatsapi + + try: + mlbstatsapi.AsyncMlbDataAdapter + except ModuleNotFoundError as exc: + assert exc.name == "httpcore", exc.name + assert "python-mlb-statsapi[async]" not in str(exc), str(exc) + else: + raise AssertionError("expected the underlying import failure") + """, + break_httpx=True, + ) diff --git a/tests/test_mlb_exceptions.py b/tests/test_mlb_exceptions.py index effbff3..1ba92ac 100644 --- a/tests/test_mlb_exceptions.py +++ b/tests/test_mlb_exceptions.py @@ -14,9 +14,9 @@ MlbTransportError, TheMlbStatsApiException, ) -from mlbstatsapi.mlb_dataadapter import ( - HTTP_ERROR_BODY_EXCERPT_LIMIT, +from mlbstatsapi._http import ( _build_http_error, + HTTP_ERROR_BODY_EXCERPT_LIMIT, ) @@ -404,11 +404,15 @@ def test_url_fallback_when_response_url_missing(): response.json.return_value = {"message": "boom"} response.text = '{"message": "boom"}' - exc = _build_http_error( - response, - method="GET", - fallback_url=f"{BASE_URL}sports", - ) + session = MagicMock() + session.get.return_value = response + + adapter = MlbDataAdapter(session=session) + + with pytest.raises(MlbHttpError) as exc_info: + adapter.get(endpoint="sports") + + exc = exc_info.value assert exc.url == f"{BASE_URL}sports" assert exc.method == "GET" @@ -426,11 +430,11 @@ def test_best_effort_extraction_failure_still_raises_mlb_http_error(requests_moc with ( patch( - "mlbstatsapi.mlb_dataadapter._extract_error_response_data", + "mlbstatsapi._http._extract_error_response_data", side_effect=RuntimeError("unexpected json failure"), ), patch( - "mlbstatsapi.mlb_dataadapter._extract_error_body_excerpt", + "mlbstatsapi._http._extract_error_body_excerpt", side_effect=RuntimeError("unexpected text failure"), ), pytest.raises(MlbHttpError) as exc_info, diff --git a/tests/test_public_api.py b/tests/test_public_api.py index 6d2dc85..2575c3a 100644 --- a/tests/test_public_api.py +++ b/tests/test_public_api.py @@ -4,13 +4,21 @@ exception and warning inheritance, Session ownership guarantees, and the explicit ``Mlb`` public-method manifest documented in ``docs/public-api.md``. +The package-root surface is split across two manifests because "public API" and +"available without optional dependencies" are different questions. Everything in +either manifest is public and stable in 1.x; only the async manifest needs the +optional ``async`` extra to resolve. + They must not contact the live MLB API. """ from __future__ import annotations +import importlib.util import inspect +import re import warnings +from pathlib import Path from typing import Any import pytest @@ -36,11 +44,18 @@ from http_contract_support import assert_library_retry_policy +PROJECT_ROOT = Path(__file__).resolve().parent.parent +PUBLIC_API_DOC = PROJECT_ROOT / "docs" / "public-api.md" + + # --------------------------------------------------------------------------- # Package-root manifests # --------------------------------------------------------------------------- -# Intentionally supported package-root symbols for the 1.x series. +# Supported package-root symbols for the 1.x series that are always available, +# including in a sync-only install without the ``async`` extra. Sync-only +# environments freeze their surface against this manifest, so a symbol that +# needs an optional dependency must not be added here. SUPPORTED_PACKAGE_ROOT_SYMBOLS: tuple[str, ...] = ( "Mlb", "MlbDataAdapter", @@ -56,6 +71,18 @@ "return_splits", ) +# Supported package-root symbols for the 1.x series that require the optional +# ``async`` extra (HTTPX). These are public and stable exactly like the symbols +# above; only their availability is conditional. See docs/public-api.md. +OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS: tuple[str, ...] = ( + "AsyncMlbDataAdapter", +) + +# The complete supported package-root API for the 1.x series. +SUPPORTED_PACKAGE_ROOT_API: tuple[str, ...] = ( + SUPPORTED_PACKAGE_ROOT_SYMBOLS + OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS +) + # Legacy helpers remain supported but are not preferred for new code. LEGACY_PACKAGE_ROOT_HELPERS: tuple[str, ...] = ( "get_stat_attributes", @@ -74,6 +101,26 @@ ) +# HTTPX ships only with the ``async`` extra, so this module must stay runnable +# in a sync-only environment. Cases that assert async availability are skipped +# there; tests/test_async_optional_dependency.py covers the sync-only half of +# the contract in child interpreters that block HTTPX outright. +def _async_extra_installed() -> bool: + """Report whether HTTPX is available, without importing it here.""" + try: + return importlib.util.find_spec("httpx") is not None + except ImportError: + # An environment may also make httpx unavailable by raising from a meta + # path finder instead of reporting no spec. + return False + + +requires_async_extra = pytest.mark.skipif( + not _async_extra_installed(), + reason="requires the optional async extra (HTTPX)", +) + + # Python 3.14 renders typing.Union[a, b] as "a | b" while Python 3.10-3.13 # render "Union[a, b]". The annotation object itself is unchanged, so the legacy # spelling is rewritten here and one manifest stays valid across the whole @@ -187,6 +234,32 @@ def test_supported_package_root_symbols_are_unique() -> None: assert len(SUPPORTED_PACKAGE_ROOT_SYMBOLS) == len(set(SUPPORTED_PACKAGE_ROOT_SYMBOLS)) +def test_optional_async_package_root_symbols_are_unique() -> None: + assert len(OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS) == len( + set(OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS) + ) + + +def test_package_root_manifests_are_disjoint() -> None: + """A symbol is either always available or gated behind the async extra.""" + assert not set(SUPPORTED_PACKAGE_ROOT_SYMBOLS) & set( + OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS + ) + + +def test_supported_package_root_api_is_the_union_of_both_manifests() -> None: + assert set(SUPPORTED_PACKAGE_ROOT_API) == set(SUPPORTED_PACKAGE_ROOT_SYMBOLS) | set( + OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS + ) + assert len(SUPPORTED_PACKAGE_ROOT_API) == len(set(SUPPORTED_PACKAGE_ROOT_API)) + + +def test_async_data_adapter_is_part_of_the_supported_api() -> None: + """The async adapter is supported 1.x API, not merely an optional add-on.""" + assert "AsyncMlbDataAdapter" in OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS + assert "AsyncMlbDataAdapter" in SUPPORTED_PACKAGE_ROOT_API + + def test_supported_package_root_symbols_are_importable_from_package() -> None: for name in SUPPORTED_PACKAGE_ROOT_SYMBOLS: assert hasattr(mlbstatsapi, name), name @@ -201,12 +274,40 @@ def test_supported_symbols_are_importable_by_name(name: str) -> None: assert namespace[name] is getattr(mlbstatsapi, name) +@pytest.mark.parametrize("name", OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS) +def test_optional_async_symbols_are_discoverable_without_the_extra(name: str) -> None: + """Discoverability is unconditional; only resolution needs HTTPX.""" + assert name in dir(mlbstatsapi) + + +@requires_async_extra +@pytest.mark.parametrize("name", OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS) +def test_optional_async_symbols_are_importable_with_the_extra(name: str) -> None: + namespace: dict[str, Any] = {} + exec(f"from mlbstatsapi import {name}", namespace) + assert name in namespace + assert namespace[name] is getattr(mlbstatsapi, name) + + +@requires_async_extra +def test_async_data_adapter_resolves_to_the_async_module() -> None: + adapter_class = mlbstatsapi.AsyncMlbDataAdapter + assert adapter_class.__module__ == "mlbstatsapi.async_mlb_dataadapter" + assert adapter_class.__name__ == "AsyncMlbDataAdapter" + + def test_package_does_not_define_all_in_version_1_0() -> None: """``__all__`` is omitted so star-import behavior is not silently narrowed.""" assert getattr(mlbstatsapi, "__all__", None) is None def test_star_import_includes_supported_symbols() -> None: + """Only the always-available manifest is asserted here. + + Async symbols resolve lazily, so whether a wildcard import sees them depends + on whether something already touched them in this interpreter. Their + documented access path is an explicit import, not ``import *``. + """ namespace: dict[str, Any] = {} exec("from mlbstatsapi import *", namespace) for name in SUPPORTED_PACKAGE_ROOT_SYMBOLS: @@ -230,6 +331,40 @@ def test_legacy_helpers_remain_package_root_importable() -> None: assert name in SUPPORTED_PACKAGE_ROOT_SYMBOLS +# --------------------------------------------------------------------------- +# Documented classification +# --------------------------------------------------------------------------- + + +def _documented_package_root_classifications() -> dict[str, str]: + """Return the symbol/status rows of the classification table in the docs.""" + text = PUBLIC_API_DOC.read_text(encoding="utf-8") + section = text.split("### Classification of package-root symbols", 1)[1] + section = re.split(r"\n#{2,} ", section, maxsplit=1)[0] + + rows: dict[str, str] = {} + for line in section.splitlines(): + match = re.match(r"^\|\s*`([A-Za-z_][A-Za-z0-9_]*)`\s*\|(.+?)\|\s*$", line) + if match: + rows[match.group(1)] = match.group(2).strip() + return rows + + +def test_documentation_classifies_every_supported_package_root_symbol() -> None: + documented = _documented_package_root_classifications() + for name in SUPPORTED_PACKAGE_ROOT_API: + assert name in documented, f"{name} is missing from the classification table" + + +def test_documentation_classifies_async_symbols_as_public_and_optional() -> None: + """Public API status and optional-dependency availability stay separate.""" + documented = _documented_package_root_classifications() + for name in OPTIONAL_ASYNC_PACKAGE_ROOT_SYMBOLS: + status = documented[name] + assert "Public and stable in 1.x" in status, status + assert "`async` extra" in status, status + + # --------------------------------------------------------------------------- # Constructor signatures # ---------------------------------------------------------------------------