diff --git a/scrython/__init__.py b/scrython/__init__.py index 16b9199..b4b92ce 100644 --- a/scrython/__init__.py +++ b/scrython/__init__.py @@ -1,3 +1,20 @@ from . import bulk_data, cards, catalogs, migrations, rulings, sets, symbology +from .connector import get_connector, set_default_connector, use_connector +from .connectors.scryfall_api import ScryfallConnector +from .rate_limiter import NullRateLimiter, RateLimitWarning -__all__ = ["bulk_data", "cards", "catalogs", "migrations", "rulings", "sets", "symbology"] +__all__ = [ + "bulk_data", + "cards", + "catalogs", + "migrations", + "rulings", + "sets", + "symbology", + "ScryfallConnector", + "set_default_connector", + "use_connector", + "get_connector", + "RateLimitWarning", + "NullRateLimiter", +] diff --git a/scrython/base.py b/scrython/base.py index 130fae2..7905332 100644 --- a/scrython/base.py +++ b/scrython/base.py @@ -1,12 +1,15 @@ import json import types -import urllib.error import urllib.parse +import warnings from typing import Any -from urllib.request import Request, urlopen from .cache import generate_cache_key, get_global_cache -from .rate_limiter import RateLimiter +from .connector import Connector + +_HANDLER_KWARGS: frozenset[str] = frozenset( + {"rate_limit", "cache", "cache_ttl", "data", "connector"} +) class ScryfallError(Exception): @@ -44,51 +47,19 @@ class ScrythonRequestHandler: """ Base class for all Scryfall API requests. - This class handles HTTP communication with the Scryfall API including - path building, query parameter encoding, and error handling. - - API Requirements: - - User-Agent header is required (default: 'Scrython/2.0') - - Accept header is required (default: 'application/json') - - HTTPS with TLS 1.2+ is required + Builds endpoint paths and query parameters, then delegates HTTP execution + to a Connector. The resolved connector is chosen by: per-request connector= + kwarg, then use_connector() scope, then set_default_connector(), then the + built-in ScryfallConnector default. """ _scryfall_data: dict[str, Any] = {} - _user_agent: str = "Scrython/2.0 (https://github.com/NandaScott/Scrython)" - _accept: str = "application/json" - _content_type: str = "application/json" _endpoint: str = "" - _rate_limiter_class: type[RateLimiter] = RateLimiter - _override_limiter: RateLimiter | None = None - - @classmethod - def set_user_agent(cls, user_agent: str) -> None: - """ - Set a custom User-Agent header for all Scrython requests. - - Scryfall recommends identifying your application in the User-Agent. - - Args: - user_agent: Custom User-Agent string - - Example: - scrython.set_user_agent('MyMTGApp/1.0 (contact@example.com)') - """ - cls._user_agent = user_agent @property def scryfall_data(self) -> types.SimpleNamespace: """ - Read-only access to Scryfall API response data. - - Returns a SimpleNamespace object allowing dot-notation access to all - fields returned by the Scryfall API. This is a read-only view - - modifications will not affect the internal data. - - Example: - card = scrython.cards.Named(exact='Black Lotus') - print(card.scryfall_data.name) # 'Black Lotus' - print(card.scryfall_data.mana_cost) # '{0}' + Read-only access to Scryfall API response data as dot-notation namespace. Returns: SimpleNamespace object with API response data @@ -98,15 +69,6 @@ def scryfall_data(self) -> types.SimpleNamespace: return self._scryfall_namespace def _dict_to_namespace(self, data: Any) -> Any: - """ - Recursively convert dict to SimpleNamespace for nested objects. - - Args: - data: The data to convert (dict, list, or other) - - Returns: - SimpleNamespace for dicts, list of converted items for lists, or original data - """ if isinstance(data, dict): return types.SimpleNamespace(**{k: self._dict_to_namespace(v) for k, v in data.items()}) elif isinstance(data, list): @@ -124,20 +86,11 @@ def __init__(self, **kwargs: Any) -> None: Args: **kwargs: Endpoint-specific parameters, plus optional: - - rate_limit (bool): Enable rate limiting (default: True) - - rate_limit_per_second (float): Override the default rate limit - for this handler instance. Creates a per-instance limiter, - so separate instantiations do not coordinate with each other - or with the class default limiter. Pagination within a single - handler (e.g., iter_all()) is properly throttled. + - connector (Connector): Override the connector for this request - cache (bool): Enable caching (default: False) - cache_ttl (int): Cache TTL in seconds (default: 3600) """ - rate_limit_per_second = kwargs.get("rate_limit_per_second") - self._override_limiter: RateLimiter | None = None - if rate_limit_per_second is not None: - self._override_limiter = RateLimiter(rate_limit_per_second) - + self._warn_removed_kwargs(kwargs) self._build_path(**kwargs) self._build_params(**kwargs) self._fetch(**kwargs) @@ -145,127 +98,90 @@ def __init__(self, **kwargs: Any) -> None: if self._scryfall_data["object"] == "error": raise ScryfallError(self._scryfall_data, self._scryfall_data["details"]) + def _warn_removed_kwargs(self, kwargs: dict[str, Any]) -> None: + if "rate_limit" in kwargs: + warnings.warn( + "rate_limit= is removed and has no effect; configure throttling on " + "the connector instead, e.g. " + "ScryfallConnector(rate_limiter=NullRateLimiter()).", + DeprecationWarning, + stacklevel=3, + ) + + def _get_connector(self, **kwargs: Any) -> Connector: + connector: Connector | None = kwargs.get("connector") + if connector is not None: + return connector + return Connector.current() + def _fetch_raw(self, url: str, cache_key: str | None = None, **kwargs: Any) -> dict[str, Any]: """ - Low-level HTTP fetch for absolute URLs. + Low-level fetch for absolute URLs (used by iter_all for pagination). - This method handles rate limiting, caching, and HTTP request execution - for any URL (including pagination URLs). It's used by both _fetch() for - endpoint-based requests and iter_all() for pagination. + Handles caching and delegates HTTP execution to the resolved connector. Args: url: Full absolute URL to fetch - cache_key: Optional cache key to use (if not provided, caching is skipped) + cache_key: Optional cache key (caching skipped if not provided) **kwargs: Optional parameters: - cache (bool): Enable caching (default: False) - cache_ttl (int): Cache TTL in seconds (default: 3600) - - rate_limit (bool): Enable rate limiting (default: True) - - data (dict): POST data (optional) + - data (dict): POST body data (optional) + - connector (Connector): Override the connector for this request Returns: dict: Parsed JSON response from Scryfall API Raises: - Exception: On HTTP errors or request failures + Exception: On transport-level failures """ - # Caching (disabled by default, requires cache_key) use_cache = kwargs.get("cache", False) - cache_ttl = kwargs.get("cache_ttl", 3600) # Default 1 hour + cache_ttl = kwargs.get("cache_ttl", 3600) - # Check cache first if enabled and cache_key provided if use_cache and cache_key is not None: - cache = get_global_cache() - cached_data = cache.get(cache_key) - + cached_data = get_global_cache().get(cache_key) if cached_data is not None: - # Cache hit - return cached data return cached_data - # Rate limiting (enabled by default) - rate_limit = kwargs.get("rate_limit", True) - - if rate_limit: - # Use the instance override limiter if set, otherwise fall back - # to the class-level global limiter for the endpoint's tier. - if self._override_limiter is not None: - limiter = self._override_limiter - else: - limiter = self._rate_limiter_class.get_global_limiter() - - limiter.wait() - - # Prepare POST data if provided - data: bytes | None = None - if data_param := kwargs.get("data"): - data = json.dumps(data_param).encode("utf-8") - - # Create and configure HTTP request - request = Request(url, data=data) - request.add_header("User-Agent", self._user_agent) - request.add_header("Accept", self._accept) - request.add_header("Content-Type", self._content_type) - - # Execute HTTP request - try: - with urlopen(request) as response: - charset = response.info().get_param("charset") or "utf-8" - decoded = response.read().decode(charset) - - response_data = json.loads(decoded) - - # Store in cache if enabled and cache_key provided - if use_cache and cache_key is not None and response_data.get("object") != "error": - cache = get_global_cache() - cache.set(cache_key, response_data, cache_ttl) - - return response_data - except urllib.error.HTTPError as exc: - # Scryfall returns JSON error bodies on 4xx/5xx responses. - # urllib raises HTTPError before we can read the body normally, - # but the HTTPError itself is a file-like object containing it. - try: - charset = exc.headers.get_param("charset") - if not isinstance(charset, str): - charset = "utf-8" - error_data = json.loads(exc.read().decode(charset)) - except (json.JSONDecodeError, UnicodeDecodeError): - raise Exception(f"{exc}: {request.get_full_url()}") from exc - - if error_data.get("object") == "error": - raise ScryfallError(error_data, error_data["details"]) from exc - - raise Exception(f"{exc}: {request.get_full_url()}") from exc + connector = self._get_connector(**kwargs) + data_param: dict[str, Any] | None = kwargs.get("data") + + parsed = urllib.parse.urlparse(url) + endpoint = parsed.path.lstrip("/") + params: dict[str, Any] = dict(urllib.parse.parse_qsl(parsed.query)) + + response_data = connector.fetch(endpoint, params, data=data_param) + + if use_cache and cache_key is not None and response_data.get("object") != "error": + get_global_cache().set(cache_key, response_data, cache_ttl) + + return response_data def _fetch(self, **kwargs: Any) -> None: """ Fetch data from Scryfall API using the endpoint template. Builds the full URL from self.endpoint and query parameters, - then delegates to _fetch_raw() for actual HTTP execution. + then delegates to _fetch_raw() for caching and connector execution. Args: **kwargs: Optional parameters passed to _fetch_raw() """ - # Build full URL from endpoint template and query parameters url = f"https://api.scryfall.com/{self.endpoint}?{self._encoded_query_params}" - - # Generate cache key from endpoint and params (order-independent) cache_key = generate_cache_key(self.endpoint, self._query_params) - - # Delegate to _fetch_raw for HTTP execution self._scryfall_data = self._fetch_raw(url, cache_key=cache_key, **kwargs) - # Invalidate namespace cache when new data is fetched if hasattr(self, "_scryfall_namespace"): delattr(self, "_scryfall_namespace") def _build_params(self, **kwargs: Any) -> None: + api_kwargs = {k: v for k, v in kwargs.items() if k not in _HANDLER_KWARGS} self._query_params: dict[str, Any] = { - "format": kwargs.get("format", "json"), - "face": kwargs.get("face", ""), - "version": kwargs.get("version", ""), - "pretty": kwargs.get("pretty", ""), - **kwargs, + "format": api_kwargs.get("format", "json"), + "face": api_kwargs.get("face", ""), + "version": api_kwargs.get("version", ""), + "pretty": api_kwargs.get("pretty", ""), + **api_kwargs, } self._encoded_query_params: str = urllib.parse.urlencode(self._query_params) @@ -295,20 +211,8 @@ def _build_path(self, **kwargs: Any) -> None: self._endpoint = "/".join(resolved) def __repr__(self) -> str: - """ - Developer-friendly representation showing class name and key identifiers. - - Returns a string in the format: ClassName(id='...', key_field='...') - - Example: - Named(id='bd8fa327-dd41-4737-8f19-2cf5eb1f7cdd', name='Lightning Bolt') - """ class_name = self.__class__.__name__ - - # Try to get the ID field (common for most objects) obj_id = self._scryfall_data.get("id") - - # Try to get a meaningful identifier (name, code, etc.) name = self._scryfall_data.get("name") code = self._scryfall_data.get("code") @@ -322,17 +226,6 @@ def __repr__(self) -> str: return f"{class_name}({', '.join(parts)})" def __str__(self) -> str: - """ - User-friendly string representation. - - For cards: Returns "Card Name (SET)" format - For sets: Returns "Set Name (CODE)" format - For other objects: Returns the name or a basic representation - - Example: - "Lightning Bolt (LEA)" - "Limited Edition Alpha (LEA)" - """ obj_type = self._scryfall_data.get("object", "") name = self._scryfall_data.get("name", "") @@ -343,135 +236,41 @@ def __str__(self) -> str: code = self._scryfall_data.get("code", "").upper() return f"{name} ({code})" if code else name elif obj_type == "list": - # For list objects, show summary total = self._scryfall_data.get("total_cards", 0) return f"List with {total} items" elif obj_type == "catalog": - # For catalog objects, show summary data = self._scryfall_data.get("data", []) return f"Catalog with {len(data)} items" else: - # Fallback to name or class name return name if name else f"{self.__class__.__name__} object" def __eq__(self, other: object) -> bool: - """ - Compare objects by their Scryfall ID. - - Two objects are considered equal if: - 1. They are both ScrythonRequestHandler instances - 2. They have the same Scryfall ID - - Args: - other: Another object to compare with - - Returns: - True if objects have the same Scryfall ID, False otherwise - - Example: - card1 = scrython.cards.Named(fuzzy='Lightning Bolt') - card2 = scrython.cards.Named(exact='Lightning Bolt') - card1 == card2 # True (same card, same ID) - """ if not isinstance(other, ScrythonRequestHandler): return False - # Compare by ID if both objects have one self_id = self._scryfall_data.get("id") other_id = other._scryfall_data.get("id") if self_id and other_id: return self_id == other_id - # Fallback to object comparison if no IDs return self is other def __hash__(self) -> int: - """ - Generate hash based on Scryfall ID to enable use in sets and dicts. - - Returns: - Hash of the Scryfall ID, or hash of class name if no ID available - - Example: - unique_cards = {card1, card2, card3} - card_lookup = {card1: 'owned', card2: 'wanted'} - """ obj_id = self._scryfall_data.get("id") if obj_id: return hash(obj_id) - # Fallback to instance hash if no ID - # Note: This makes objects without IDs unhashable across instances return hash(id(self)) def to_dict(self) -> dict[str, Any]: - """ - Export object data as a dictionary. - - Returns a copy of the internal Scryfall data dictionary. Modifications - to the returned dict will not affect the object's internal state. - - Returns: - Dictionary containing all Scryfall API response data - - Example: - card = scrython.cards.Named(fuzzy='Lightning Bolt') - card_dict = card.to_dict() - print(card_dict['name']) # 'Lightning Bolt' - """ return self._scryfall_data.copy() def to_json(self, **kwargs: Any) -> str: - """ - Export object data as a JSON string. - - Args: - **kwargs: Additional arguments passed to json.dumps() - Common options: indent, sort_keys, ensure_ascii - - Returns: - JSON string representation of the object data - - Example: - card = scrython.cards.Named(fuzzy='Lightning Bolt') - - # Compact JSON - json_str = card.to_json() - - # Pretty-printed JSON - json_str = card.to_json(indent=2, sort_keys=True) - - # Save to file - with open('card.json', 'w') as f: - f.write(card.to_json(indent=2)) - """ return json.dumps(self._scryfall_data, **kwargs) @classmethod def from_dict(cls, data: dict[str, Any]) -> "ScrythonRequestHandler": - """ - Construct an object from a dictionary without making an API request. - - This is useful for rehydrating cached objects or constructing objects - from saved data. The object is created without making any HTTP requests. - - Args: - data: Dictionary containing Scryfall API response data - - Returns: - Instance of the class populated with the provided data - - Example: - # Save card data - card = scrython.cards.Named(fuzzy='Lightning Bolt') - card_dict = card.to_dict() - - # Later, restore from dict (no API call) - restored_card = scrython.cards.Named.from_dict(card_dict) - print(restored_card.name) # 'Lightning Bolt' - """ - # Create instance without calling __init__ instance = cls.__new__(cls) instance._scryfall_data = data.copy() return instance diff --git a/scrython/base_mixins.py b/scrython/base_mixins.py index 6e97a38..1ec9e19 100644 --- a/scrython/base_mixins.py +++ b/scrython/base_mixins.py @@ -1,4 +1,3 @@ -import warnings from functools import cache from typing import Any @@ -125,15 +124,6 @@ def iter_all(self, **kwargs): """ import hashlib - if "rate_limit_per_second" in kwargs: - warnings.warn( - "rate_limit_per_second must be set at construction time, " - "not passed to iter_all(). This kwarg is ignored.", - UserWarning, - stacklevel=2, - ) - kwargs.pop("rate_limit_per_second") - # Yield items from current page yield from self.data diff --git a/scrython/bulk_data/bulk_data_mixins.py b/scrython/bulk_data/bulk_data_mixins.py index bf49317..8ed2162 100644 --- a/scrython/bulk_data/bulk_data_mixins.py +++ b/scrython/bulk_data/bulk_data_mixins.py @@ -3,7 +3,7 @@ from typing import Any from urllib.request import Request, urlopen -from ..base import ScrythonRequestHandler +from ..connectors.scryfall_api import ScryfallConnector class BulkDataObjectMixin: @@ -164,7 +164,7 @@ def download( download_url = self.download_uri request = Request(download_url) - request.add_header("User-Agent", ScrythonRequestHandler._user_agent) + request.add_header("User-Agent", ScryfallConnector._user_agent) request.add_header("Accept-Encoding", "gzip, identity") # Optional progress bar diff --git a/scrython/cards/cards.py b/scrython/cards/cards.py index 242bbb0..743a4b4 100644 --- a/scrython/cards/cards.py +++ b/scrython/cards/cards.py @@ -1,6 +1,5 @@ from ..base import ScrythonRequestHandler from ..base_mixins import ScryfallCatalogMixin, ScryfallListMixin -from ..rate_limiter import SlowRateLimiter from ..types import ScryfallCardData from .cards_mixins import CardsObjectMixin @@ -159,7 +158,6 @@ class Search(ScryfallListMixin, ScrythonRequestHandler): """ _endpoint = "/cards/search" - _rate_limiter_class = SlowRateLimiter list_data_type = Object @@ -191,7 +189,6 @@ class Named(CardsObjectMixin, ScrythonRequestHandler): """ _endpoint = "/cards/named" - _rate_limiter_class = SlowRateLimiter class Autocomplete(ScryfallCatalogMixin, ScrythonRequestHandler): @@ -247,7 +244,6 @@ class Random(CardsObjectMixin, ScrythonRequestHandler): """ _endpoint = "/cards/random" - _rate_limiter_class = SlowRateLimiter class Collection(ScryfallListMixin, ScrythonRequestHandler): @@ -280,7 +276,6 @@ class Collection(ScryfallListMixin, ScrythonRequestHandler): """ _endpoint = "/cards/collection" - _rate_limiter_class = SlowRateLimiter list_data_type = Object diff --git a/scrython/connector.py b/scrython/connector.py new file mode 100644 index 0000000..89579cf --- /dev/null +++ b/scrython/connector.py @@ -0,0 +1,58 @@ +from abc import ABC, abstractmethod +from collections.abc import Generator +from contextlib import contextmanager +from contextvars import ContextVar +from typing import Any, ClassVar + + +class Connector(ABC): + # Connector resolution state lives on the type: a per-context override set + # by use(), and a process-wide default set by set_default(). + _connector_var: ClassVar[ContextVar["Connector | None"]] = ContextVar( + "scrython_connector", default=None + ) + _default: ClassVar["Connector | None"] = None + + @abstractmethod + def fetch( + self, + endpoint: str, + params: dict[str, Any], + *, + data: dict[str, Any] | None = None, + ) -> dict[str, Any]: + """Return a Scryfall-shaped response dict, including error dicts.""" + + @classmethod + def set_default(cls, connector: "Connector | None") -> None: + """Set the process-wide default connector (lowest precedence after builtin).""" + cls._default = connector + + @classmethod + @contextmanager + def use(cls, connector: "Connector") -> Generator[None, None, None]: + """Scope a connector to the current context (overrides the default).""" + token = cls._connector_var.set(connector) + try: + yield + finally: + cls._connector_var.reset(token) + + @classmethod + def current(cls) -> "Connector": + """Resolve the active connector: use() scope, then default, then builtin.""" + ctx = cls._connector_var.get() + if ctx is not None: + return ctx + if cls._default is not None: + return cls._default + from .connectors.scryfall_api import ScryfallConnector + + return ScryfallConnector() + + +# Module-level aliases keep the documented `scrython.set_default_connector(...)` +# call site working while the implementation lives on Connector. +set_default_connector = Connector.set_default +use_connector = Connector.use +get_connector = Connector.current diff --git a/scrython/connectors/__init__.py b/scrython/connectors/__init__.py new file mode 100644 index 0000000..b4dd309 --- /dev/null +++ b/scrython/connectors/__init__.py @@ -0,0 +1,3 @@ +from .scryfall_api import ScryfallConnector + +__all__ = ["ScryfallConnector"] diff --git a/scrython/connectors/scryfall_api.py b/scrython/connectors/scryfall_api.py new file mode 100644 index 0000000..cb580b1 --- /dev/null +++ b/scrython/connectors/scryfall_api.py @@ -0,0 +1,123 @@ +import json +import urllib.error +import urllib.parse +import warnings +from typing import Any +from urllib.request import Request, urlopen + +from ..connector import Connector +from ..rate_limiter import NullRateLimiter, RateLimiter, RateLimitWarning, SlowRateLimiter + + +class ScryfallConnector(Connector): + """ + Default HTTP connector for the Scryfall API. + + Handles urllib, User-Agent/Accept/Content-Type headers, and rate limiting. + Inject a custom instance via the connector= kwarg or set_default_connector() + to control rate limiting or headers on a per-use basis. + """ + + _user_agent: str = "Scrython/2.0 (https://github.com/NandaScott/Scrython)" + _accept: str = "application/json" + _content_type: str = "application/json" + _BASE_URL: str = "https://api.scryfall.com" + + # Scryfall throttles these endpoints harder than the 10/s default. + _SLOW_ENDPOINTS: frozenset[str] = frozenset( + {"cards/search", "cards/named", "cards/random", "cards/collection"} + ) + + def __init__(self, rate_limiter: RateLimiter | None = None) -> None: + # An injected limiter overrides per-endpoint tiering for every request. + # Left as None, each request uses the limiter for its endpoint's tier. + self._rate_limiter = rate_limiter + + @classmethod + def set_user_agent(cls, user_agent: str) -> None: + """ + Set a custom User-Agent header for all Scrython requests. + + Scryfall recommends identifying your application in the User-Agent. + + Args: + user_agent: Custom User-Agent string + + Example: + ScryfallConnector.set_user_agent('MyMTGApp/1.0 (contact@example.com)') + """ + cls._user_agent = user_agent + + def fetch( + self, + endpoint: str, + params: dict[str, Any], + *, + data: dict[str, Any] | None = None, + ) -> dict[str, Any]: + """Return a Scryfall-shaped response dict, including error dicts.""" + url = f"{self._BASE_URL}/{endpoint}?{urllib.parse.urlencode(params)}" + return self._fetch_url(url, limiter=self._limiter_for(endpoint), data=data) + + def _tier_class(self, endpoint: str) -> type[RateLimiter]: + if endpoint.strip("/") in self._SLOW_ENDPOINTS: + return SlowRateLimiter + return RateLimiter + + def _limiter_for(self, endpoint: str) -> RateLimiter: + tier_default = self._tier_class(endpoint).get_global_limiter() + limiter = self._rate_limiter if self._rate_limiter is not None else tier_default + self._warn_if_over_limit(limiter, tier_default, endpoint) + return limiter + + def _warn_if_over_limit( + self, limiter: RateLimiter, tier_default: RateLimiter, endpoint: str + ) -> None: + if isinstance(limiter, NullRateLimiter): + return + if limiter.calls_per_second > tier_default.calls_per_second: + warnings.warn( + f"Rate limiter is set to {limiter.calls_per_second}/s for " + f"'{endpoint}', exceeding Scryfall's {tier_default.calls_per_second}/s " + f"limit for this endpoint; requests risk being throttled or banned.", + RateLimitWarning, + stacklevel=2, + ) + + def _fetch_url( + self, + url: str, + *, + limiter: RateLimiter | None = None, + data: dict[str, Any] | None = None, + rate_limit: bool = True, + ) -> dict[str, Any]: + if rate_limit and limiter is not None: + limiter.wait() + + post_data: bytes | None = None + if data is not None: + post_data = json.dumps(data).encode("utf-8") + + request = Request(url, data=post_data) + request.add_header("User-Agent", self._user_agent) + request.add_header("Accept", self._accept) + request.add_header("Content-Type", self._content_type) + + try: + with urlopen(request) as response: + charset = response.info().get_param("charset") or "utf-8" + return json.loads(response.read().decode(charset)) + except urllib.error.HTTPError as exc: + try: + charset = exc.headers.get_param("charset") + if not isinstance(charset, str): + charset = "utf-8" + error_data: dict[str, Any] = json.loads(exc.read().decode(charset)) + except (json.JSONDecodeError, UnicodeDecodeError): + raise Exception(f"{exc}: {request.get_full_url()}") from exc + + if error_data.get("object") == "error": + return error_data + + raise Exception(f"{exc}: {request.get_full_url()}") from exc diff --git a/scrython/rate_limiter.py b/scrython/rate_limiter.py index 18f9368..a6ded31 100644 --- a/scrython/rate_limiter.py +++ b/scrython/rate_limiter.py @@ -15,6 +15,17 @@ from typing import ClassVar +class RateLimitWarning(UserWarning): + """ + Warns that the active rate limiter exceeds Scryfall's limit for an endpoint. + + Emitted per-request when an injected limiter is faster than the tier the + endpoint belongs to. Filter it precisely with + ``warnings.filterwarnings("ignore", category=scrython.RateLimitWarning)``, + or escalate it to an error in tests with ``"error"``. + """ + + class RateLimiter: """ Thread-safe rate limiter using token bucket algorithm. @@ -127,3 +138,20 @@ class SlowRateLimiter(RateLimiter): def __init__(self, calls_per_second: float = 2.0) -> None: super().__init__(calls_per_second) + + +class NullRateLimiter(RateLimiter): + """ + No-op limiter for callers who explicitly opt out of throttling. + + ``wait()`` does nothing. The connector treats this as a sanctioned bypass + and suppresses RateLimitWarning for it. Use at your own risk: exceeding + Scryfall's published limits can get your client throttled or banned. Inject + via ``ScryfallConnector(rate_limiter=NullRateLimiter())``. + """ + + def __init__(self) -> None: + super().__init__(float("inf")) + + def wait(self) -> None: + return diff --git a/tests/conftest.py b/tests/conftest.py index 181a5a4..bc7dc4b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,11 +57,19 @@ def disable_rate_limiting(): Fixture that disables rate limiting for tests. This makes tests run much faster by removing rate limit delays. + + Both rate-limiter tiers are patched so slow endpoints (search/named/random/ + collection) do not sleep either. """ - with patch("scrython.base.RateLimiter") as mock_limiter_class: - mock_instance = Mock() - mock_instance.wait = Mock() # No-op wait method + mock_instance = Mock() + mock_instance.wait = Mock() # No-op wait method + mock_instance.calls_per_second = float("inf") + with ( + patch("scrython.connectors.scryfall_api.RateLimiter") as mock_limiter_class, + patch("scrython.connectors.scryfall_api.SlowRateLimiter") as mock_slow_limiter_class, + ): mock_limiter_class.get_global_limiter.return_value = mock_instance + mock_slow_limiter_class.get_global_limiter.return_value = mock_instance yield @@ -174,7 +182,7 @@ def __call__(self, request): mock = MockURLOpen() - with patch("scrython.base.urlopen", side_effect=mock): + with patch("scrython.connectors.scryfall_api.urlopen", side_effect=mock): yield mock diff --git a/tests/test_bulk_data.py b/tests/test_bulk_data.py index d05cd14..6b56fe2 100644 --- a/tests/test_bulk_data.py +++ b/tests/test_bulk_data.py @@ -10,6 +10,7 @@ import pytest from scrython.bulk_data import All, ById, ByType +from scrython.connectors.scryfall_api import ScryfallConnector class TestAll: @@ -317,8 +318,6 @@ def test_download_sets_headers(self, mock_urlopen): """Test download sets proper User-Agent and Accept-Encoding headers.""" from urllib.request import Request - from scrython.base import ScrythonRequestHandler - mock_urlopen.set_response("bulk_data/by_id.json") bulk = ByType(type="oracle_cards") @@ -339,5 +338,5 @@ def test_download_sets_headers(self, mock_urlopen): assert isinstance(request, Request) # Verify headers are set correctly - assert request.get_header("User-agent") == ScrythonRequestHandler._user_agent + assert request.get_header("User-agent") == ScryfallConnector._user_agent assert request.get_header("Accept-encoding") == "gzip, identity" diff --git a/tests/test_caching.py b/tests/test_caching.py index 768eb0d..d283c88 100644 --- a/tests/test_caching.py +++ b/tests/test_caching.py @@ -288,12 +288,12 @@ class TestHandler(ScrythonRequestHandler): # First call with both cache and rate limit start = time.time() - _handler1 = TestHandler(fuzzy="bolt", cache=True, rate_limit=True) + _handler1 = TestHandler(fuzzy="bolt", cache=True) first_call = time.time() - start # Second call - should use cache (no rate limit delay) start = time.time() - _handler2 = TestHandler(fuzzy="bolt", cache=True, rate_limit=True) + _handler2 = TestHandler(fuzzy="bolt", cache=True) second_call = time.time() - start # Second call should be much faster (cache hit) diff --git a/tests/test_connector.py b/tests/test_connector.py new file mode 100644 index 0000000..4562652 --- /dev/null +++ b/tests/test_connector.py @@ -0,0 +1,80 @@ +"""Tests for connector resolution (issue #170 review). + +Resolution lives on the Connector type as classmethods; module-level aliases +keep the documented scrython.set_default_connector(...) call site working. +""" + +import pytest + +import scrython +from scrython.connector import ( + Connector, + get_connector, + set_default_connector, + use_connector, +) +from scrython.connectors.scryfall_api import ScryfallConnector + + +class DummyConnector(Connector): + """A no-network connector that echoes its name into the response.""" + + def __init__(self, name: str = "dummy") -> None: + self.name = name + + def fetch(self, endpoint, params, *, data=None): # noqa: ARG002 + return {"object": "card", "id": self.name, "name": self.name} + + +@pytest.fixture(autouse=True) +def reset_connector_default(): + Connector.set_default(None) + yield + Connector.set_default(None) + + +class TestConnectorResolution: + def test_builtin_default_is_scryfall_connector(self): + assert isinstance(Connector.current(), ScryfallConnector) + + def test_set_default_is_resolved(self): + dummy = DummyConnector() + Connector.set_default(dummy) + assert Connector.current() is dummy + + def test_use_scope_overrides_default(self): + default = DummyConnector("default") + scoped = DummyConnector("scoped") + Connector.set_default(default) + with Connector.use(scoped): + assert Connector.current() is scoped + assert Connector.current() is default + + def test_per_request_kwarg_beats_scope_and_default(self): + Connector.set_default(DummyConnector("default")) + with Connector.use(DummyConnector("scoped")): + card = scrython.cards.ById(id="x", connector=DummyConnector("kwarg")) + assert card.to_dict()["id"] == "kwarg" + + +class TestModuleAliases: + def test_aliases_point_at_classmethods(self): + assert set_default_connector == Connector.set_default + assert get_connector == Connector.current + assert use_connector == Connector.use + + def test_alias_set_default_and_get(self): + dummy = DummyConnector() + set_default_connector(dummy) + assert get_connector() is dummy + + def test_alias_use_scope(self): + dummy = DummyConnector() + with use_connector(dummy): + assert get_connector() is dummy + assert get_connector() is not dummy + + def test_scrython_namespace_call_site(self): + dummy = DummyConnector() + scrython.set_default_connector(dummy) + assert scrython.get_connector() is dummy diff --git a/tests/test_rate_limiting.py b/tests/test_rate_limiting.py index 5a3c711..e0ff888 100644 --- a/tests/test_rate_limiting.py +++ b/tests/test_rate_limiting.py @@ -5,6 +5,7 @@ import pytest +import scrython from scrython.base import ScryfallError, ScrythonRequestHandler from scrython.rate_limiter import RateLimiter @@ -229,7 +230,7 @@ def __call__(self, request): mock = MockURLOpen() - with patch("scrython.base.urlopen", side_effect=mock): + with patch("scrython.connectors.scryfall_api.urlopen", side_effect=mock): yield mock def test_rate_limit_enabled_by_default(self, mock_urlopen_with_rate_limit, sample_card): @@ -251,112 +252,6 @@ class TestHandler(ScrythonRequestHandler): # Second call should have been rate limited (~0.1s delay) assert elapsed > 0.08 - def test_rate_limit_can_be_disabled(self, mock_urlopen_with_rate_limit, sample_card): - """Test that rate limiting can be disabled.""" - # Reset rate limiter - RateLimiter.reset_all_limiters() - - mock_urlopen_with_rate_limit.set_response(data=sample_card) - - class TestHandler(ScrythonRequestHandler): - _endpoint = "cards/named" - - # Make two calls quickly with rate limiting disabled - start = time.time() - _handler1 = TestHandler(fuzzy="Card 1", rate_limit=False) - _handler2 = TestHandler(fuzzy="Card 2", rate_limit=False) - elapsed = time.time() - start - - # Should be very fast (no rate limiting) - assert elapsed < 0.3 - - def test_default_rate_limiter_class_is_base(self): - """Test that ScrythonRequestHandler defaults to RateLimiter.""" - assert ScrythonRequestHandler._rate_limiter_class is RateLimiter - - def test_slow_endpoint_uses_slow_limiter(self, mock_urlopen_with_rate_limit, sample_card): - """Test that an endpoint with SlowRateLimiter uses the slow rate.""" - from scrython.rate_limiter import SlowRateLimiter - - mock_urlopen_with_rate_limit.set_response(data=sample_card) - - class SlowHandler(ScrythonRequestHandler): - _endpoint = "cards/search" - _rate_limiter_class = SlowRateLimiter - - start = time.time() - _handler1 = SlowHandler(q="Card 1") - _handler2 = SlowHandler(q="Card 2") - elapsed = time.time() - start - - # SlowRateLimiter at 2/s means ~0.5s between calls - assert elapsed > 0.45 - - def test_rate_limit_per_second_kwarg_overrides_class( - self, mock_urlopen_with_rate_limit, sample_card - ): - """Test that rate_limit_per_second kwarg overrides the class default.""" - from scrython.rate_limiter import SlowRateLimiter - - mock_urlopen_with_rate_limit.set_response(data=sample_card) - - class SlowHandler(ScrythonRequestHandler): - _endpoint = "cards/search" - _rate_limiter_class = SlowRateLimiter - - # Override to a fast rate — should NOT wait 500ms - start = time.time() - _handler1 = SlowHandler(q="Card 1", rate_limit_per_second=20.0) - _handler2 = SlowHandler(q="Card 2", rate_limit_per_second=20.0) - elapsed = time.time() - start - - # Should be fast (~0.05s), not slow (~0.5s) - assert elapsed < 0.5 - - def test_custom_rate_limit(self, mock_urlopen_with_rate_limit, sample_card): - """Test that rate_limit_per_second creates a per-instance limiter.""" - # Reset rate limiter - RateLimiter.reset_all_limiters() - - mock_urlopen_with_rate_limit.set_response(data=sample_card) - - class TestHandler(ScrythonRequestHandler): - _endpoint = "cards/named" - - # Each handler gets its own limiter, so separate instantiations - # do not throttle against each other (only pagination within - # a single handler instance is throttled). - start = time.time() - _handler1 = TestHandler(fuzzy="Card 1", rate_limit_per_second=5.0) - _handler2 = TestHandler(fuzzy="Card 2", rate_limit_per_second=5.0) - elapsed = time.time() - start - - assert elapsed < 0.2 - - def test_custom_rate_limit_throttles_within_instance( - self, mock_urlopen_with_rate_limit, sample_card - ): - """Test that rate_limit_per_second throttles repeated calls on the same handler.""" - RateLimiter.reset_all_limiters() - - mock_urlopen_with_rate_limit.set_response(data=sample_card) - - class TestHandler(ScrythonRequestHandler): - _endpoint = "cards/named" - - handler = TestHandler(fuzzy="Card 1", rate_limit_per_second=5.0) - - # Call _fetch_raw again on the same instance (simulates pagination) - start = time.time() - handler._fetch_raw( - "https://api.scryfall.com/cards/named?fuzzy=Card+2", - rate_limit=True, - ) - elapsed = time.time() - start - - # Should wait ~0.2s (5 calls/sec = 200ms interval) - assert elapsed > 0.15 - def test_slow_rate_limiter_attributes_via_global( self, mock_urlopen_with_rate_limit, sample_card ): @@ -380,8 +275,11 @@ def test_rate_limit_respects_previous_calls(self, mock_urlopen_with_rate_limit, mock_urlopen_with_rate_limit.set_response(data=sample_card) + # Fast-tier endpoint (10/s, 0.1s interval) — the slow endpoints + # (search/named/random/collection) run at 2/s, which this timing math + # would not match. class TestHandler(ScrythonRequestHandler): - _endpoint = "cards/named" + _endpoint = "cards/multiverse/123" # First call _handler1 = TestHandler(fuzzy="Card 1") @@ -474,41 +372,102 @@ def test_slow_rate_limiter_enforces_delay(self): assert 0.45 < elapsed < 1.0 -class TestEndpointRateLimiterAssignment: - """Test that endpoint classes declare the correct rate limiter class.""" +class TestPerEndpointTiering: + """ScryfallConnector owns per-endpoint rate-limit tiering (issue #170 review).""" - def test_search_uses_slow_limiter(self): - from scrython.cards.cards import Search - from scrython.rate_limiter import SlowRateLimiter + def _connector(self, **kwargs): + from scrython.connectors.scryfall_api import ScryfallConnector - assert Search._rate_limiter_class is SlowRateLimiter + return ScryfallConnector(**kwargs) - def test_named_uses_slow_limiter(self): - from scrython.cards.cards import Named + def test_slow_endpoints_use_slow_tier(self): from scrython.rate_limiter import SlowRateLimiter - assert Named._rate_limiter_class is SlowRateLimiter + conn = self._connector() + for endpoint in ("cards/search", "cards/named", "cards/random", "cards/collection"): + assert isinstance(conn._limiter_for(endpoint), SlowRateLimiter) - def test_random_uses_slow_limiter(self): - from scrython.cards.cards import Random - from scrython.rate_limiter import SlowRateLimiter + def test_fast_endpoints_use_default_tier(self): + from scrython.rate_limiter import RateLimiter, SlowRateLimiter - assert Random._rate_limiter_class is SlowRateLimiter + limiter = self._connector()._limiter_for("cards/some-id") + assert isinstance(limiter, RateLimiter) + assert not isinstance(limiter, SlowRateLimiter) - def test_collection_uses_slow_limiter(self): - from scrython.cards.cards import Collection - from scrython.rate_limiter import SlowRateLimiter + def test_injected_limiter_overrides_tiering(self): + import warnings + + from scrython.rate_limiter import RateLimitWarning + + fixed = RateLimiter(20.0) + conn = self._connector(rate_limiter=fixed) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RateLimitWarning) + assert conn._limiter_for("cards/search") is fixed + assert conn._limiter_for("cards/some-id") is fixed + + def test_over_limit_injection_warns(self): + from scrython.rate_limiter import RateLimiter, RateLimitWarning + + conn = self._connector(rate_limiter=RateLimiter(10.0)) # 10/s > slow 2/s limit + with pytest.warns(RateLimitWarning): + conn._limiter_for("cards/search") + + def test_within_limit_injection_is_silent(self, recwarn): + from scrython.rate_limiter import RateLimiter, RateLimitWarning + + self._connector(rate_limiter=RateLimiter(2.0))._limiter_for("cards/search") + assert not [w for w in recwarn.list if issubclass(w.category, RateLimitWarning)] + + def test_default_tiered_path_is_silent(self, recwarn): + from scrython.rate_limiter import RateLimitWarning + + conn = self._connector() + conn._limiter_for("cards/search") + conn._limiter_for("cards/some-id") + assert not [w for w in recwarn.list if issubclass(w.category, RateLimitWarning)] + + def test_null_rate_limiter_suppresses_warning(self, recwarn): + from scrython.rate_limiter import NullRateLimiter, RateLimitWarning + + self._connector(rate_limiter=NullRateLimiter())._limiter_for("cards/search") + assert not [w for w in recwarn.list if issubclass(w.category, RateLimitWarning)] + + def test_null_rate_limiter_wait_is_noop(self): + from scrython.rate_limiter import NullRateLimiter + + limiter = NullRateLimiter() + start = time.time() + for _ in range(5): + limiter.wait() + assert time.time() - start < 0.05 + + def test_slow_endpoint_set_matches_card_endpoints(self): + """Drift guard: every slow endpoint string maps to a real cards.py endpoint.""" + from scrython.cards import cards as cards_module + from scrython.connectors.scryfall_api import ScryfallConnector + + defined: set[str] = set() + for name in dir(cards_module): + obj = getattr(cards_module, name) + if isinstance(obj, type) and issubclass(obj, ScrythonRequestHandler): + endpoint = getattr(obj, "_endpoint", "") + if endpoint: + defined.add(endpoint.strip("/")) - assert Collection._rate_limiter_class is SlowRateLimiter + assert defined >= ScryfallConnector._SLOW_ENDPOINTS - def test_autocomplete_uses_default_limiter(self): - from scrython.cards.cards import Autocomplete - from scrython.rate_limiter import RateLimiter - assert Autocomplete._rate_limiter_class is RateLimiter +class TestRemovedRateLimitKwarg: + """The per-request rate_limit= toggle is removed (issue #170 review).""" - def test_by_code_number_uses_default_limiter(self): - from scrython.cards.cards import ByCodeNumber - from scrython.rate_limiter import RateLimiter + def test_rate_limit_kwarg_warns_deprecation(self, mock_urlopen, sample_card): + mock_urlopen.set_response(data=sample_card) + with pytest.warns(DeprecationWarning): + scrython.cards.ById(id="abc", rate_limit=False) - assert ByCodeNumber._rate_limiter_class is RateLimiter + def test_rate_limit_kwarg_not_sent_as_query_param(self, mock_urlopen, sample_card): + mock_urlopen.set_response(data=sample_card) + with pytest.warns(DeprecationWarning): + scrython.cards.ById(id="abc", rate_limit=False) + assert "rate_limit" not in mock_urlopen.calls[0]["url"]