From f57118ff77330686548bd40750bb2e996f2b2d33 Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sat, 26 Sep 2026 22:48:46 +0000 Subject: [PATCH] feat(proxy): one locked get_or_create for lazy defaults; CacheBackend falls back get_backend_or_fallback() and get_app_cache() each had their own double-checked lock, and get_state_manager() had none, so concurrent first requests could each build and register a StateManager. Add ProxyBase.get_or_create(factory) with a per-class threading.Lock and build all three on it. The lock is per class so the default CacheManager can create the fallback backend inside its own creation. get_cache_backend (CacheBackend) now uses get_backend_or_fallback(), so it no longer answers 500 until some @cache route has installed the fallback. get_state_manager still has no memory fallback. Closes #112 --- CLAUDE.md | 3 +- docs/HTTP_CACHING.md | 6 +- fastapi_cachex/dependencies.py | 34 ++++----- fastapi_cachex/proxy.py | 67 ++++++++++++----- fastapi_cachex/state/dependencies.py | 17 +++-- i18n/zh-TW/docs/HTTP_CACHING.md | 2 +- tests/state/test_proxy.py | 53 ++++++++++++++ tests/test_dependencies.py | 19 +++-- tests/test_proxybackend.py | 103 +++++++++++++++++++++++++++ 9 files changed, 247 insertions(+), 57 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a9ed035..89028b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,7 +54,8 @@ The library has four independent subsystems: - Fails open by default (`fail_open=True`): a backend error on `get` is logged and treated as a miss, one on `set` is logged and the response served unstored. `fail_open=False` propagates the error. - Only GET requests are cached; other methods bypass the cache entirely. - Cache keys follow the format `method|||host|||path|||query_params` (separator defined in `types.py`). Host and path go through `escape_key_component` (`|` → `%7C`, `%` → `%25`) so client input cannot inject the separator; `clear_path` encodes its argument and `routes.py` decodes for display. -- `BackendProxy` is a non-instantiable class-level singleton (via `ProxyMeta`). Call `BackendProxy.set(backend)` at app startup; `BackendProxy.get()` raises `BackendNotFoundError` if unset. Falls back to `MemoryBackend` automatically inside `@cache` if no backend is set. +- `BackendProxy` is a non-instantiable class-level singleton (via `ProxyMeta`). Call `BackendProxy.set(backend)` at app startup; `BackendProxy.get()` raises `BackendNotFoundError` if unset. `get_backend_or_fallback()` registers a `MemoryBackend` when none is set; `@cache`, `CacheBackend` and `AppCache` use it. +- `ProxyBase.get_or_create(factory)` is the one lazy get-or-create: a per-class `threading.Lock` (sync dependencies run in worker threads; per class so a factory can call another proxy's `get_or_create`). Used by `get_backend_or_fallback`, `get_app_cache` and `get_state_manager` (no memory fallback for states). - Cache values are stored as `CacheEntry(fingerprint, content, media_type)` dataclass (defined in `types.py`). **2. Application-Level Caching (`fastapi_cachex/manager.py`, `manager_proxy.py`)** diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index c2e9988..8bd5248 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -246,7 +246,11 @@ something a shared cache cannot see, use option 1 instead. ### By path or pattern The clearing methods live on the backend, which you can inject with the -`CacheBackend` dependency or fetch with `BackendProxy.get()`: +`CacheBackend` dependency or fetch with `BackendProxy.get()`. With no backend +configured, `CacheBackend` registers the same `MemoryBackend` fallback that +`@cache` would, so it works before any cached route has run (before 0.3.8 it +answered `500` until then); `BackendProxy.get()` still raises +`BackendNotFoundError`. ```python from fastapi_cachex import CacheBackend diff --git a/fastapi_cachex/dependencies.py b/fastapi_cachex/dependencies.py index bba226b..75f8d0e 100644 --- a/fastapi_cachex/dependencies.py +++ b/fastapi_cachex/dependencies.py @@ -1,23 +1,24 @@ """FastAPI dependency injection utilities for cache control.""" -import threading from typing import Annotated from fastapi import Depends from .backends.base import BaseCacheBackend -from .exceptions import ProxyNotSetError from .manager import CacheManager from .manager_proxy import CacheManagerProxy -from .proxy import BackendProxy from .proxy import get_backend_or_fallback -_manager_lock = threading.Lock() - def get_cache_backend() -> BaseCacheBackend: - """Dependency to get the current cache backend instance.""" - return BackendProxy.get() + """Dependency to get the current cache backend instance. + + With no backend configured this falls back to a `MemoryBackend` and + registers it, the same way `@cache` and `AppCache` do. It used to raise + `BackendNotFoundError` (a 500) until some `@cache` route had run and + installed the fallback first. + """ + return get_backend_or_fallback() CacheBackend = Annotated[BaseCacheBackend, Depends(get_cache_backend)] @@ -36,20 +37,11 @@ def get_app_cache() -> CacheManager: dependency worked depended on whether some `@cache` route had already run and installed the fallback first. """ - try: - return CacheManagerProxy.get() - except ProxyNotSetError: - pass - # Checked again under the lock: FastAPI runs this sync dependency in a - # worker thread, so concurrent first requests would otherwise each build - # and register their own manager (and fallback backend). - with _manager_lock: - try: - return CacheManagerProxy.get() - except ProxyNotSetError: - manager = CacheManager(backend=get_backend_or_fallback()) - CacheManagerProxy.set(manager) - return manager + return CacheManagerProxy.get_or_create(_default_manager) + + +def _default_manager() -> CacheManager: + return CacheManager(backend=get_backend_or_fallback()) AppCache = Annotated[CacheManager, Depends(get_app_cache)] diff --git a/fastapi_cachex/proxy.py b/fastapi_cachex/proxy.py index bf5b11a..2e445a8 100644 --- a/fastapi_cachex/proxy.py +++ b/fastapi_cachex/proxy.py @@ -5,6 +5,7 @@ import threading import warnings from logging import getLogger +from typing import TYPE_CHECKING from typing import ClassVar from typing import Generic from typing import NoReturn @@ -15,14 +16,13 @@ from .exceptions import BackendNotFoundError from .exceptions import ProxyNotSetError +if TYPE_CHECKING: + from collections.abc import Callable + ProxyInstance = TypeVar("ProxyInstance") logger = getLogger(__name__) -# Serialises the lazy fallback below. `get_app_cache` is a sync dependency that -# FastAPI runs in a worker thread, so two first requests can reach it at once. -_fallback_lock = threading.Lock() - class ProxyMeta(type): """Metaclass for BackendProxy to prevent instantiation.""" @@ -39,6 +39,16 @@ class ProxyBase(Generic[ProxyInstance], metaclass=ProxyMeta): _instance: ProxyInstance | None = None # Raised by `get()` while no instance is set. _not_set_error: ClassVar[type[BackendNotFoundError]] = ProxyNotSetError + # Serialises `get_or_create`. A threading lock, because the sync FastAPI + # dependencies built on it run in worker threads. One per class, so a + # factory may call another proxy's `get_or_create` (the default + # `CacheManager` needs a backend) without deadlocking. + _create_lock: ClassVar[threading.Lock] = threading.Lock() + + def __init_subclass__(cls, **kwargs: object) -> None: + """Give every proxy class its own creation lock.""" + super().__init_subclass__(**kwargs) + cls._create_lock = threading.Lock() @classmethod def get(cls) -> ProxyInstance: @@ -56,6 +66,31 @@ def get(cls) -> ProxyInstance: raise cls._not_set_error(msg) return cls._instance + @classmethod + def get_or_create(cls, factory: Callable[[], ProxyInstance]) -> ProxyInstance: + """Return the current instance, creating and registering one if unset. + + The check and the registration happen under the class's lock, so + concurrent first callers, including ones on worker threads, all get + the same instance: ``factory`` runs at most once. If it raises, nothing + is registered and the error propagates. + + Args: + factory: Builds the instance when none is set + + Returns: + The registered instance + """ + instance = cls._instance + if instance is not None: + return instance + with cls._create_lock: + instance = cls._instance + if instance is None: + instance = factory() + cls.set(instance) + return instance + @classmethod def set(cls, instance: ProxyInstance | None) -> None: """Set the instance for the proxy. @@ -115,20 +150,14 @@ def set_backend(backend: BaseCacheBackend | None) -> None: def get_backend_or_fallback() -> BaseCacheBackend: """Return the configured backend, registering a `MemoryBackend` if none is. - Used by `@cache` and the `AppCache` dependency. The check and the - registration happen under one lock, so concurrent first callers — including - ones on worker threads — all end up with the same fallback instead of each + Used by `@cache`, `CacheBackend` and `AppCache`. Built on + `BackendProxy.get_or_create`, so concurrent first callers, including ones + on worker threads, all end up with the same fallback instead of each installing its own and overwriting the others. """ - try: - return BackendProxy.get() - except BackendNotFoundError: - pass - with _fallback_lock: - try: - return BackendProxy.get() - except BackendNotFoundError: - backend = MemoryBackend() - BackendProxy.set(backend) - logger.debug("No backend configured; using MemoryBackend fallback") - return backend + return BackendProxy.get_or_create(_memory_fallback) + + +def _memory_fallback() -> BaseCacheBackend: + logger.debug("No backend configured; using MemoryBackend fallback") + return MemoryBackend() diff --git a/fastapi_cachex/state/dependencies.py b/fastapi_cachex/state/dependencies.py index e3f3781..3301b7f 100644 --- a/fastapi_cachex/state/dependencies.py +++ b/fastapi_cachex/state/dependencies.py @@ -4,8 +4,6 @@ from fastapi import Depends -from fastapi_cachex.exceptions import ProxyNotSetError - from .manager import StateManager from .proxy import StateManagerProxy @@ -15,14 +13,15 @@ def get_state_manager() -> StateManager: Lazily creates and registers a default StateManager (backed by BackendProxy) the first time it's requested, unless one was already - set via StateManagerProxy.set(...). + set via StateManagerProxy.set(...). Concurrent first calls share one + instance. + + Unlike `AppCache`, it does not fall back to a `MemoryBackend`: OAuth + states must be readable by whichever worker handles the callback, so with + no backend configured it raises `BackendNotFoundError` and registers + nothing. """ - try: - return StateManagerProxy.get() - except ProxyNotSetError: - manager = StateManager() - StateManagerProxy.set(manager) - return manager + return StateManagerProxy.get_or_create(StateManager) StateManagerDep = Annotated[StateManager, Depends(get_state_manager)] diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index 9dfd438..3e31cf2 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -183,7 +183,7 @@ async def my_dashboard(user: CurrentUser, response: Response): ### 依路徑或模式 {#by-path-or-pattern} -清除用的方法位於後端上，可以透過 `CacheBackend` 依賴項注入，或以 `BackendProxy.get()` 取得： +清除用的方法位於後端上，可以透過 `CacheBackend` 依賴項注入，或以 `BackendProxy.get()` 取得。尚未設定後端時，`CacheBackend` 會註冊與 `@cache` 相同的 `MemoryBackend` 後備後端，因此在任何快取路由執行之前也能使用（0.3.8 之前在那之前會回應 `500`）；`BackendProxy.get()` 則仍會引發 `BackendNotFoundError`。 ```python from fastapi_cachex import CacheBackend diff --git a/tests/state/test_proxy.py b/tests/state/test_proxy.py index e514574..6c68351 100644 --- a/tests/state/test_proxy.py +++ b/tests/state/test_proxy.py @@ -1,10 +1,15 @@ """Tests for StateManagerProxy and get_state_manager dependency.""" +import threading +import time +from concurrent.futures import ThreadPoolExecutor + import pytest from fastapi_cachex.backends.memory import MemoryBackend from fastapi_cachex.exceptions import BackendNotFoundError from fastapi_cachex.proxy import BackendProxy +from fastapi_cachex.state import dependencies as state_dependencies from fastapi_cachex.state.dependencies import get_state_manager from fastapi_cachex.state.manager import StateManager from fastapi_cachex.state.proxy import StateManagerProxy @@ -57,3 +62,51 @@ def test_get_state_manager_reuses_existing_proxy_instance( assert get_state_manager() is existing finally: StateManagerProxy.set(None) + + +def test_get_state_manager_concurrent_first_calls_share_one_instance( + memory_backend: MemoryBackend, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """FastAPI runs the sync dependency in worker threads; racers must agree. + + Without a lock each first request built and registered its own + `StateManager`, and the later `set()` replaced the earlier one. + """ + + class SlowStateManager(StateManager): + def __init__(self) -> None: + time.sleep(0.05) # widen the window between the check and the set + super().__init__() + + monkeypatch.setattr(state_dependencies, "StateManager", SlowStateManager) + BackendProxy.set(memory_backend) + StateManagerProxy.set(None) + workers = 8 + barrier = threading.Barrier(workers) + + def first_call(_: int) -> StateManager: + barrier.wait() + return get_state_manager() + + try: + with ThreadPoolExecutor(max_workers=workers) as pool: + managers = list(pool.map(first_call, range(workers))) + assert len({id(manager) for manager in managers}) == 1 + assert StateManagerProxy.get() is managers[0] + finally: + StateManagerProxy.set(None) + + +def test_get_state_manager_without_a_backend_raises_and_registers_nothing() -> None: + """OAuth states need a shared backend, so there is no memory fallback.""" + BackendProxy.set(None) + StateManagerProxy.set(None) + + with pytest.raises(BackendNotFoundError): + get_state_manager() + + with pytest.raises(BackendNotFoundError): + StateManagerProxy.get() + with pytest.raises(BackendNotFoundError): + BackendProxy.get() diff --git a/tests/test_dependencies.py b/tests/test_dependencies.py index d017e58..67e4c3a 100644 --- a/tests/test_dependencies.py +++ b/tests/test_dependencies.py @@ -6,7 +6,6 @@ from fastapi_cachex import CacheBackend from fastapi_cachex.backends import MemoryBackend from fastapi_cachex.dependencies import get_app_cache -from fastapi_cachex.exceptions import BackendNotFoundError from fastapi_cachex.manager import CacheManager from fastapi_cachex.manager_proxy import CacheManagerProxy @@ -23,11 +22,21 @@ async def backend_endpoint(backend: CacheBackend): # Actual test functions @pytest.mark.asyncio -async def test_get_cache_backend_no_backend(): - """Test that get_cache_backend raises BackendNotFoundError when no backend is set.""" +async def test_get_cache_backend_falls_back_to_memory_without_a_backend(): + """`CacheBackend` must work before any `@cache` route has run. + + It used to answer 500 (`BackendNotFoundError`) until a `@cache` route had + installed the fallback, the order dependence `AppCache` no longer had. + """ BackendProxy.set(None) - with pytest.raises(BackendNotFoundError): - client.get("/test-backend") + + response = client.get("/test-backend") + + assert response.status_code == 200 + assert response.json() == {"backend_type": "MemoryBackend"} + # The fallback is registered, so `@cache` and `AppCache` share it. + assert isinstance(BackendProxy.get(), MemoryBackend) + assert get_app_cache().backend is BackendProxy.get() @pytest.mark.asyncio diff --git a/tests/test_proxybackend.py b/tests/test_proxybackend.py index d655b70..13e0056 100644 --- a/tests/test_proxybackend.py +++ b/tests/test_proxybackend.py @@ -1,4 +1,8 @@ import asyncio +import threading +import time +from collections.abc import Iterator +from concurrent.futures import ThreadPoolExecutor import pytest import pytest_asyncio @@ -12,6 +16,8 @@ from fastapi_cachex.exceptions import BackendNotFoundError from fastapi_cachex.exceptions import ProxyNotSetError from fastapi_cachex.manager_proxy import CacheManagerProxy +from fastapi_cachex.proxy import ProxyBase +from fastapi_cachex.proxy import get_backend_or_fallback from fastapi_cachex.session.proxy import SessionManagerProxy from fastapi_cachex.state.proxy import StateManagerProxy from fastapi_cachex.types import CacheEntry @@ -162,3 +168,100 @@ def test_backend_proxy_still_raises_backend_not_found_error() -> None: with pytest.raises(BackendNotFoundError) as exc_info: BackendProxy.get() assert not isinstance(exc_info.value, ProxyNotSetError) + + +class _Thing: + """A stand-in instance for the generic `get_or_create` tests.""" + + +class _ThingProxy(ProxyBase[_Thing]): + """A proxy of its own, so these tests leave the library's proxies alone.""" + + +@pytest.fixture +def thing_proxy() -> Iterator[type[_ThingProxy]]: + _ThingProxy.set(None) + yield _ThingProxy + _ThingProxy.set(None) + + +def test_get_or_create_returns_the_registered_instance( + thing_proxy: type[_ThingProxy], +) -> None: + existing = _Thing() + thing_proxy.set(existing) + + def factory() -> _Thing: + pytest.fail("factory must not run while an instance is registered") + + assert thing_proxy.get_or_create(factory) is existing + + +def test_get_or_create_concurrent_first_calls_run_the_factory_once( + thing_proxy: type[_ThingProxy], +) -> None: + """Racing first callers on worker threads all get one registered instance.""" + calls = 0 + + def slow_factory() -> _Thing: + nonlocal calls + calls += 1 + time.sleep(0.05) # widen the window between the check and the set + return _Thing() + + workers = 8 + barrier = threading.Barrier(workers) + + def first_call(_: int) -> _Thing: + barrier.wait() + return thing_proxy.get_or_create(slow_factory) + + with ThreadPoolExecutor(max_workers=workers) as pool: + things = list(pool.map(first_call, range(workers))) + + assert calls == 1 + assert len({id(thing) for thing in things}) == 1 + assert thing_proxy.get() is things[0] + + +def test_get_or_create_registers_nothing_when_the_factory_raises( + thing_proxy: type[_ThingProxy], +) -> None: + def failing_factory() -> _Thing: + msg = "boom" + raise RuntimeError(msg) + + with pytest.raises(RuntimeError, match="boom"): + thing_proxy.get_or_create(failing_factory) + with pytest.raises(ProxyNotSetError): + thing_proxy.get() + # The lock was released: the next caller can still create one. + assert isinstance(thing_proxy.get_or_create(_Thing), _Thing) + + +def test_get_or_create_factory_may_use_another_proxy( + thing_proxy: type[_ThingProxy], +) -> None: + """Each proxy class has its own lock, so nested creation cannot deadlock. + + The default `CacheManager` is built inside `CacheManagerProxy`'s lock and + needs `BackendProxy.get_or_create` for its backend. Run in a thread with a + timeout so a shared lock fails the test instead of hanging the suite. + """ + BackendProxy.set(None) + result: list[_Thing] = [] + + def factory() -> _Thing: + get_backend_or_fallback() + return _Thing() + + worker = threading.Thread( + target=lambda: result.append(thing_proxy.get_or_create(factory)), + daemon=True, + ) + worker.start() + worker.join(timeout=5) + + assert not worker.is_alive(), "nested get_or_create deadlocked" + assert result == [thing_proxy.get()] + assert isinstance(BackendProxy.get(), MemoryBackend)