From 71699d011e093082ac2cd54cd4b9df9e519e521a Mon Sep 17 00:00:00 2001 From: allen0099 Date: Fri, 2 Oct 2026 17:17:48 +0000 Subject: [PATCH] fix(cache): send the headers and cookies dependencies set on every response (#233) FastAPI merges the shared sub-Response into the result only when the endpoint returns plain data, and the @cache wrapper always returns a Response. A dependency's headers and cookies were lost on a handler without a `response: Response` parameter; with one, they were stored with the entry and replayed on every hit with the values of the request that filled the cache. The wrapper now always receives the sub-response (it injects the parameter when the handler does not declare one) and records the lines the dependencies set before the handler runs. Only the lines the handler adds are part of its response and stored; the dependencies' lines are added to every response the wrapper sends, miss, hit and 304. A cookie a dependency sets keeps the response out of the backend and makes it private, as a cookie the handler sets does. --- changelog.d/233.changed.md | 9 + changelog.d/233.fixed.md | 12 + docs/HTTP_CACHING.md | 26 ++ fastapi_cachex/cache.py | 333 ++++++++++++++++--- i18n/zh-TW/docs/HTTP_CACHING.md | 5 + tests/test_cache_dependency_headers.py | 421 +++++++++++++++++++++++++ tests/test_cache_rendering.py | 20 ++ tests/test_cache_request_param.py | 18 +- 8 files changed, 798 insertions(+), 46 deletions(-) create mode 100644 changelog.d/233.changed.md create mode 100644 changelog.d/233.fixed.md create mode 100644 tests/test_cache_dependency_headers.py diff --git a/changelog.d/233.changed.md b/changelog.d/233.changed.md new file mode 100644 index 0000000..0c3a6f4 --- /dev/null +++ b/changelog.d/233.changed.md @@ -0,0 +1,9 @@ +**A dependency that sets a cookie keeps `@cache` routes out of the backend.** +Before 0.4.1, a handler that did not declare `response: Response` dropped the +cookie and the response was stored; now the cookie is sent and the response is +`private` and not stored. An app-wide dependency that sets a cookie on every +request therefore turns storage off for every `@cache` route it applies to, and +so does one that sets the status code. `@cache` also adds the dependencies' +headers to a `Response` the handler returns itself, which FastAPI does not do. +Entries stored before the upgrade may still replay dependency headers until +their TTL ends. diff --git a/changelog.d/233.fixed.md b/changelog.d/233.fixed.md new file mode 100644 index 0000000..86feec6 --- /dev/null +++ b/changelog.d/233.fixed.md @@ -0,0 +1,12 @@ +**Headers and cookies a dependency sets on the shared `Response` reach the +client on every `@cache` response, with this request's values.** A handler that +did not declare `response: Response` lost them, and one that did stored the +dependency's headers with the entry and replayed the values of the request that +filled the cache (a rate-limit countdown answered 10, 10, 10). They are now +added to every miss, hit and 304 and never stored. A response with a cookie a +dependency set, or with a dependency's `private` or `no-store` `Cache-Control`, +is treated like one where the handler set it: it is not stored, and it is sent +with that header or with `private`. A dependency's other `Cache-Control` and a +header the handler sets over a dependency's are handled as the handler's own, +on a hit too. A status code a dependency sets is sent, and keeps the response +out of the backend. diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index 701fca3..f693a74 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -188,6 +188,32 @@ route's response model (declared or inferred from the return annotation, with the `response_model_*` options), the route's `status_code` applies, and the status and headers set on an injected `response: Response` parameter are kept. +Headers and cookies that dependencies set on FastAPI's shared `Response` reach +the client with this request's values, on a miss, a hit and a 304 alike, whether +or not the handler declares `response: Response`. They are never stored with +the entry, so a header such as `X-RateLimit-Remaining` is not replayed from the +request that filled the cache. A dependency's `Cache-Control` with `private` +or `no-store`, and a cookie a dependency sets, count as the handler's own: the +response is not stored, and it is sent with the dependency's header or with +`private`, a hit or 304 included. A dependency's other `Cache-Control` is +treated like the handler's own: the decorator's replaces it, and a bare +`@cache()` keeps it. A status code a dependency sets is sent but keeps the +response out of the backend, since it may hold for that request only; a hit +keeps the stored status. Lines the handler itself adds to `response`, and the +status code it sets, belong to the stored response; where the handler sets a +header a dependency also set, the handler's value is sent, on a hit as well. A +handler that deletes a dependency's header removes it only when it runs. FastAPI +itself merges these lines only when the handler returns plain data; `@cache` +adds the dependencies' lines to a handler's own `Response` too. Before 0.4.1, a +handler without a `response: Response` parameter lost them, and one with it +replayed the headers of the request that filled the cache (#233). + +> [!NOTE] +> A dependency that sets a cookie on every request, such as an app-wide CSRF or +> session-refresh dependency, therefore keeps every `@cache` route it applies to +> out of the backend. Limit it to the routes that need it, or set the cookie +> only when it changes. + ### Requests with credentials A single-page app that sends `Authorization` on every request, or a site where diff --git a/fastapi_cachex/cache.py b/fastapi_cachex/cache.py index 961d0b0..08b7072 100644 --- a/fastapi_cachex/cache.py +++ b/fastapi_cachex/cache.py @@ -6,6 +6,7 @@ import threading import time import warnings +from collections import Counter from collections.abc import Awaitable from collections.abc import Callable from collections.abc import Iterable @@ -28,6 +29,7 @@ from fastapi import Request from fastapi import Response from fastapi.encoders import jsonable_encoder +from fastapi.params import Depends as DependsParam from fastapi.utils import is_body_allowed_for_status_code from pydantic import TypeAdapter from starlette.concurrency import run_in_threadpool @@ -661,15 +663,52 @@ def _is_request_annotation(annotation: Any) -> bool: return isinstance(annotation, type) and issubclass(annotation, Request) +def _is_response_annotation(annotation: Any) -> bool: + """Whether an annotation asks for the sub-``Response`` FastAPI injects. + + ``Annotated[Response, Depends(...)]`` is a dependency that returns a + ``Response``, not the sub-response. + """ + if get_origin(annotation) is Annotated: + annotation, *metadata = get_args(annotation) + if any(isinstance(item, DependsParam) for item in metadata): + return False + return isinstance(annotation, type) and issubclass(annotation, Response) + + def _find_request_param( func: HandlerCallable, params: list[Parameter] ) -> Parameter | None: - """The handler's own ``Request`` parameter, if it declares one. + """The handler's own ``Request`` parameter, if it declares one (see ``_find_param``).""" + return _find_param(func, params, _is_request_annotation) + + +def _find_response_param( + func: HandlerCallable, params: list[Parameter] +) -> Parameter | None: + """The handler's own ``Response`` parameter, if it declares one (see ``_find_param``). + + A parameter defaulting to ``Depends(...)`` is a dependency's result, not + the sub-response, whatever its annotation. + """ + return _find_param( + func, + [param for param in params if not isinstance(param.default, DependsParam)], + _is_response_annotation, + ) + + +def _find_param( + func: HandlerCallable, + params: list[Parameter], + matches: Callable[[Any], bool], +) -> Parameter | None: + """The handler's first parameter whose annotation ``matches``. Annotations are resolved first, so a handler under ``from __future__ import annotations`` (where the annotation is the string ``"Request"``), ``Annotated[Request, ...]``, or a ``Request`` subclass is recognised - instead of being given a second, unused request parameter. Resolution can + instead of being given a second, unused parameter. Resolution can fail on a forward reference that does not resolve in the handler's module, which must not break decoration: the raw annotations are used instead. """ @@ -679,15 +718,107 @@ def _find_request_param( hints = {} return next( - ( - param - for param in params - if _is_request_annotation(hints.get(param.name, param.annotation)) - ), + (param for param in params if matches(hints.get(param.name, param.annotation))), None, ) +def _split_header_lines( + lines: Iterable[tuple[bytes, bytes]], before: Iterable[tuple[bytes, bytes]] +) -> tuple[list[tuple[bytes, bytes]], list[tuple[bytes, bytes]]]: + """Split a sub-response's header lines into those of ``before`` and the rest. + + FastAPI resolves the dependencies before it calls the handler, so the + sub-response's lines when the wrapper starts (``before``) are the + dependencies' and the lines added since are the handler's. Lines are + matched as a multiset, so a repeated line is counted once per copy. + """ + remaining = Counter(before) + kept: list[tuple[bytes, bytes]] = [] + added: list[tuple[bytes, bytes]] = [] + for line in lines: + if remaining[line] > 0: + remaining[line] -= 1 + kept.append(line) + else: + added.append(line) + return kept, added + + +def _sets_cookie(lines: Iterable[tuple[bytes, bytes]]) -> bool: + return any(name.lower() == b"set-cookie" for name, _ in lines) + + +def _cache_control_values(lines: Iterable[tuple[bytes, bytes]]) -> list[str]: + return [ + value.decode("latin-1") + for name, value in lines + if name.lower() == b"cache-control" + ] + + +def _dependency_unshareable_reason( + dependency_lines: Iterable[tuple[bytes, bytes]], +) -> str | None: + """Why the dependencies' lines keep a response out of the backend, if they do.""" + lines = list(dependency_lines) + if _has_unshareable_directive(_cache_control_values(lines)): + return "a dependency's Cache-Control is private or no-store" + if _sets_cookie(lines): + return "a dependency sets a cookie" + return None + + +def _with_dependency_headers( + response: Response, + sub_response: Response | None, + dependency_lines: Sequence[tuple[bytes, bytes]], + private_cache_control: str, + *, + cacheable_get: bool, +) -> Response: + """Add the header lines the dependencies set on this request (#233). + + FastAPI merges the sub-response into the response only when the handler + returns plain data, and the wrapper always returns a ``Response``, so it + merges them itself: on a miss, a hit and a 304 alike, with the values of + this request rather than those stored with the entry. + + On a cacheable GET response, ``Cache-Control`` is decided as for the + handler's own lines: a handler's ``private`` or ``no-store`` header (or + ``no_store=True``) is kept; otherwise a dependency's ``private`` or + ``no-store`` header replaces the decorator's, and a ``Set-Cookie`` makes it + ``private``. Any header the response already carries (other than + ``Set-Cookie``) is not added, the decorator's ``Cache-Control`` included: + the handler set it over the dependency's, which on a hit is the stored + value. + Any other response gets every line, as FastAPI would send them. + """ + if sub_response is None or not dependency_lines: + return response + current, _ = _split_header_lines(sub_response.headers.raw, dependency_lines) + if not cacheable_get or not ( + response.status_code == HTTP_304_NOT_MODIFIED + or _is_cacheable_status(response.status_code) + ): + response.headers.raw.extend(current) + return response + own = {name.lower() for name, _ in response.headers.raw} + response.headers.raw.extend( + (name, value) + for name, value in current + if name.lower() == b"set-cookie" or name.lower() not in own + ) + if _marked_unshareable(response): + return response + dependency_cache_control = _cache_control_values(current) + if _has_unshareable_directive(dependency_cache_control): + response.headers["Cache-Control"] = ", ".join(dependency_cache_control) + elif _sets_cookie(current): + response.headers["Cache-Control"] = private_cache_control + return response + + def _get_response_body(response: Response) -> bytes | None: """Return response body bytes, or None for streaming/file responses.""" return getattr(response, "body", None) @@ -788,9 +919,14 @@ def _marked_unshareable(response: Response) -> bool: Directive names are matched as whole tokens, case-insensitively, across every ``Cache-Control`` field the response carries. """ + return _has_unshareable_directive(response.headers.getlist("cache-control")) + + +def _has_unshareable_directive(cache_control: Iterable[str]) -> bool: + """Whether any of these ``Cache-Control`` values has ``private`` or ``no-store``.""" return any( directive.split("=", 1)[0].strip().lower() in _UNSHAREABLE_DIRECTIVES - for value in response.headers.getlist("cache-control") + for value in cache_control for directive in value.split(",") ) @@ -836,10 +972,23 @@ def _with_cache_control( async def _render( - func: HandlerCallable, request: Request, /, *args: Any, **kwargs: Any + func: HandlerCallable, + request: Request, + args: tuple[Any, ...], + kwargs: dict[str, Any], + *, + sub_response: Response | None, + dependency_lines: Sequence[tuple[bytes, bytes]], ) -> tuple[Response, bytes | None, str | None]: """Run the handler; the body and ETag are None for streaming/file responses.""" - response = await get_response(func, request, *args, **kwargs) + response = await _respond( + func, + request, + args, + kwargs, + sub_response=sub_response, + dependency_lines=dependency_lines, + ) body = _get_response_body(response) return response, body, None if body is None else _etag_for(body) @@ -900,12 +1049,37 @@ async def get_response( Coroutine handlers are awaited. Sync handlers run in the threadpool, as FastAPI would run them without the (async) cache wrapper, so blocking I/O - in a ``def`` handler does not stall the event loop. + in a ``def`` handler does not stall the event loop. Every header line of a + ``Response`` among ``kwargs`` is carried over onto a plain-data result. + """ + sub_response = next( + (value for value in kwargs.values() if isinstance(value, Response)), None + ) + return await _respond( + __func, __request, args, kwargs, sub_response=sub_response, dependency_lines=() + ) + + +async def _respond( + func: HandlerCallable, + request: Request, + args: tuple[Any, ...], + kwargs: dict[str, Any], + *, + sub_response: Response | None, + dependency_lines: Sequence[tuple[bytes, bytes]], +) -> Response: + """Run the handler and build its response, as ``get_response`` describes. + + Only the lines added to ``sub_response`` beyond ``dependency_lines`` are + carried over here: the dependencies' own lines are added to every + response the wrapper sends (``_with_dependency_headers``), and must not be + stored with the entry. """ - if _is_coroutine_callable(__func): - result = await cast("Callable[..., Awaitable[object]]", __func)(*args, **kwargs) + if _is_coroutine_callable(func): + result = await cast("Callable[..., Awaitable[object]]", func)(*args, **kwargs) else: - result = await run_in_threadpool(__func, *args, **kwargs) + result = await run_in_threadpool(func, *args, **kwargs) # A sync callable can still hand back an awaitable (a lambda wrapping a # coroutine function, say); await it rather than try to encode it. if inspect.isawaitable(result): @@ -916,7 +1090,7 @@ async def get_response( return result # Get response_class from route if available - route: APIRoute | None = __request.scope.get("route") + route: APIRoute | None = request.scope.get("route") if route is None: msg = "Route not found in request scope" raise CacheXError(msg) @@ -924,7 +1098,7 @@ async def get_response( # A placeholder means this route uses the application default. Unwrap the # application value instead of calling the route's DefaultPlaceholder. route_response_class = route.response_class - application_default_response_class = __request.app.router.default_response_class + application_default_response_class = request.app.router.default_response_class response_class: type[Response] = cast( "type[Response]", ( @@ -940,10 +1114,7 @@ async def get_response( # Build the response the way FastAPI would have without the cache wrapper: # serialize through the response model, apply the route's status code and - # carry over what the handler set on an injected `response: Response`. - sub_response = next( - (value for value in kwargs.values() if isinstance(value, Response)), None - ) + # carry over what the handler set on the injected `response: Response`. status_code = route.status_code if sub_response is not None and sub_response.status_code: status_code = sub_response.status_code @@ -955,7 +1126,8 @@ async def get_response( if not is_body_allowed_for_status_code(response.status_code): response.body = b"" if sub_response is not None: - response.headers.raw.extend(sub_response.headers.raw) + _, added = _split_header_lines(sub_response.headers.raw, dependency_lines) + response.headers.raw.extend(added) return response @@ -1167,16 +1339,32 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: # Check if Request is already in the parameters found_request: Parameter | None = _find_request_param(func, params) - # Add Request parameter if it's not present + # FastAPI's sub-response: the wrapper needs it even when the handler + # does not ask for it, to send what the dependencies set on it (#233). + found_response: Parameter | None = _find_response_param(func, params) + + # Add the Request and Response parameters the handler does not declare + injected: list[Parameter] = [] if not found_request: request_name: str = "__cachex_request" - - request_param = inspect.Parameter( - request_name, - inspect.Parameter.KEYWORD_ONLY, - annotation=Request, + injected.append( + inspect.Parameter( + request_name, inspect.Parameter.KEYWORD_ONLY, annotation=Request + ) + ) + else: + request_name = found_request.name + if not found_response: + response_name: str = "__cachex_response" + injected.append( + inspect.Parameter( + response_name, inspect.Parameter.KEYWORD_ONLY, annotation=Response + ) ) + else: + response_name = found_response.name + if injected: # A keyword-only parameter must precede **kwargs; appending it # after one makes `Signature.replace` raise at decoration time, # so a handler taking **kwargs could not be cached at all. @@ -1189,16 +1377,9 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: len(params), ) sig = sig.replace( - parameters=[ - *params[:insert_at], - request_param, - *params[insert_at:], - ] + parameters=[*params[:insert_at], *injected, *params[insert_at:]] ) - else: - request_name = found_request.name - # The header only depends on the decorator arguments, so build it once. cache_control = _build_cache_control( ttl=ttl, @@ -1238,8 +1419,14 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: f"{getattr(func, '__module__', '?')}.{getattr(func, '__qualname__', '?')}" ) - @wraps(func) - async def serve(*args: Any, **kwargs: Any) -> Response: + async def respond( + sub_response: Response | None, + dependency_lines: Sequence[tuple[bytes, bytes]], + dependency_status: int | None, + /, + *args: Any, + **kwargs: Any, + ) -> Response: # Resolve backend on every request to support lifespan-configured backends cache_backend = get_backend_or_fallback() @@ -1258,11 +1445,25 @@ async def serve(*args: Any, **kwargs: Any) -> Response: logger.debug( "Non-GET request; bypassing cache for method=%s", req.method ) - return await get_response(func, req, *args, **kwargs) + return await _respond( + func, + req, + args, + kwargs, + sub_response=sub_response, + dependency_lines=dependency_lines, + ) # Handle special case: no-store (highest priority) if no_store: - response = await get_response(func, req, *args, **kwargs) + response = await _respond( + func, + req, + args, + kwargs, + sub_response=sub_response, + dependency_lines=dependency_lines, + ) logger.debug( "no-store active; bypassed cache for path=%s", req.url.path ) @@ -1312,7 +1513,14 @@ async def serve(*args: Any, **kwargs: Any) -> Response: # cache reuse the answer to an `Authorization` request under # `must-revalidate`, and nothing at all stops one reusing the # answer to a cookie. - response, _, etag = await _render(func, req, *args, **kwargs) + response, _, etag = await _render( + func, + req, + args, + kwargs, + sub_response=sub_response, + dependency_lines=dependency_lines, + ) if not _is_cacheable_status(response.status_code): return response if etag is None: @@ -1359,7 +1567,12 @@ async def serve(*args: Any, **kwargs: Any) -> Response: if no_cache: # Get fresh response first if using no-cache current_response, current_body, current_etag = await _render( - func, req, *args, **kwargs + func, + req, + args, + kwargs, + sub_response=sub_response, + dependency_lines=dependency_lines, ) if not _is_cacheable_status(current_response.status_code): # Error responses carry no validator: never answer 304. @@ -1437,7 +1650,12 @@ async def serve(*args: Any, **kwargs: Any) -> Response: if current_response is None or current_etag is None: # Retrieve the current response if not already done current_response, current_body, current_etag = await _render( - func, req, *args, **kwargs + func, + req, + args, + kwargs, + sub_response=sub_response, + dependency_lines=dependency_lines, ) if not _is_cacheable_status(current_response.status_code): # Leave any existing entry alone: a transient error must not @@ -1462,6 +1680,11 @@ async def serve(*args: Any, **kwargs: Any) -> Response: # stored. An entry already under this key is left alone, as for an # error status: it came from a response that was shareable. skip_reason = _unshareable_reason(current_response) + if skip_reason is None: + skip_reason = _dependency_unshareable_reason(dependency_lines) + if skip_reason is None and dependency_status is not None: + # It may hold for this request only, and a hit would replay it. + skip_reason = "a dependency set the status code" if skip_reason is not None: logger.debug("Not storing key=%s: %s", cache_key, skip_reason) @@ -1512,6 +1735,32 @@ async def serve(*args: Any, **kwargs: Any) -> Response: current_response, response_cache_control, private_cache_control ) + @wraps(func) + async def serve(*args: Any, **kwargs: Any) -> Response: + if found_response: + sub_response: Response | None = kwargs.get(response_name) + else: + sub_response = kwargs.pop(response_name, None) + # The dependencies have run; the handler has not (see + # `_split_header_lines`). + dependency_lines = ( + [] if sub_response is None else list(sub_response.headers.raw) + ) + dependency_status = ( + None if sub_response is None else sub_response.status_code + ) + req: Request | None = kwargs.get(request_name) + response = await respond( + sub_response, dependency_lines, dependency_status, *args, **kwargs + ) + return _with_dependency_headers( + response, + sub_response, + dependency_lines, + private_cache_control, + cacheable_get=req is not None and req.method == "GET", + ) + wrapper: AsyncResponseCallable = serve if vary_names: diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index fad05d9..ebe4b3c 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -99,6 +99,11 @@ GET /items → 200, Cache-Control: max-age=60, Age: 42（儲存後 42 秒送出 handler 回傳一般資料而非 `Response` 時，得到的處理與沒有 `@cache` 時相同：回傳值會經過路由的 response model 驗證與過濾（明確宣告的，或由回傳型別註記推斷，並套用 `response_model_*` 選項），套用路由的 `status_code`，而在注入的 `response: Response` 參數上設定的狀態碼與標頭也會保留。 +依賴項在 FastAPI 共用的 `Response` 上設定的標頭與 cookie，會以本次請求的值送到用戶端，未命中、命中與 304 皆然，不論 handler 是否宣告 `response: Response`。它們不會隨項目儲存，因此 `X-RateLimit-Remaining` 這類標頭不會重播寫入快取那次請求的值。依賴項帶有 `private` 或 `no-store` 的 `Cache-Control`，以及依賴項設定的 cookie，都與 handler 自己設定的視為相同：回應不會儲存，並以依賴項的標頭或 `private` 送出，命中與 304 也一樣。依賴項其他的 `Cache-Control` 與 handler 自己的視為相同：裝飾器的會取代它，不帶參數的 `@cache()` 則保留它。依賴項設定的狀態碼會送出，但回應不會寫入後端，因為它可能只適用於該次請求；命中時沿用儲存的狀態碼。handler 自己加到 `response` 上的標頭，以及它設定的狀態碼，則屬於儲存的回應；handler 設定了依賴項也設定過的標頭時，送出的是 handler 的值，命中時也一樣。handler 刪除依賴項的標頭，只在它執行時才有效。FastAPI 本身只在 handler 回傳一般資料時合併這些標頭；`@cache` 也會把依賴項的標頭加到 handler 自己回傳的 `Response` 上。0.4.1 之前，沒有宣告 `response: Response` 參數的 handler 會遺失它們，有宣告的則會重播寫入快取那次請求的標頭（#233）。 + +> [!NOTE] +> 因此，每次請求都設定 cookie 的依賴項（例如套用到整個應用程式的 CSRF 或 session 更新依賴項），會讓它套用到的每個 `@cache` 路由都不寫入後端。請只把它套用到需要的路由，或只在 cookie 改變時才設定。 + ### 帶有憑證的請求 {#requests-with-credentials} 每個請求都送出 `Authorization` 的單頁應用程式，或每位訪客都有 Session 的網站，在只加上 `@cache` 的路由上完全不會命中快取：每個請求都會繞過後端（見上文）。請依 handler 回傳的內容選擇： diff --git a/tests/test_cache_dependency_headers.py b/tests/test_cache_dependency_headers.py new file mode 100644 index 0000000..4c796c7 --- /dev/null +++ b/tests/test_cache_dependency_headers.py @@ -0,0 +1,421 @@ +"""Headers and cookies the dependencies set on the shared `Response` (#233). + +FastAPI gives every dependency and the handler one sub-`Response` and merges it +into the result only when the endpoint returns plain data. The `@cache` wrapper +always returns a `Response`, so it merges the dependencies' lines itself, on +every response: with the values of this request, never the ones stored with +an entry. Lines the handler adds are part of its response and are stored. +""" + +import inspect +from collections.abc import Iterator +from typing import Annotated +from typing import Any + +import pytest +from fastapi import Depends +from fastapi import FastAPI +from fastapi import Request +from fastapi import Response +from fastapi.responses import JSONResponse +from fastapi.testclient import TestClient + +from fastapi_cachex import cache + + +def _countdown() -> Iterator[int]: + return iter(range(10, 0, -1)) + + +def _rate_limit_app(*, declare_response: bool, **cache_kwargs: Any) -> TestClient: + app = FastAPI() + left = _countdown() + + def rate_limit(response: Response) -> None: + response.headers["X-RateLimit-Remaining"] = str(next(left)) + + if declare_response: + + @app.get("/item", dependencies=[Depends(rate_limit)]) + @cache(**cache_kwargs) + async def item_with_response(response: Response) -> dict[str, int]: + return {"item": 1} + + else: + + @app.get("/item", dependencies=[Depends(rate_limit)]) + @cache(**cache_kwargs) + async def item() -> dict[str, int]: + return {"item": 1} + + return TestClient(app) + + +@pytest.mark.parametrize("declare_response", [False, True], ids=["plain", "declared"]) +def test_a_dependency_header_has_this_requests_value_on_a_miss_and_a_hit( + declare_response: bool, +) -> None: + client = _rate_limit_app(declare_response=declare_response, ttl=60) + + responses = [client.get("/item") for _ in range(4)] + + # Without the response parameter the header was lost; with it, the miss's + # value was stored and replayed: 10, 10, 10, 10. + assert [r.headers.get("x-ratelimit-remaining") for r in responses] == [ + "10", + "9", + "8", + "7", + ] + assert all(len(r.headers.get_list("x-ratelimit-remaining")) == 1 for r in responses) + assert all(r.json() == {"item": 1} for r in responses) + assert "age" in responses[1].headers # the later ones are hits + + +@pytest.mark.parametrize( + "cache_kwargs", + [{"ttl": 60}, {"private": True}, {"ttl": 60, "no_cache": True}, {}], + ids=["hit", "bypass", "no-cache", "bare"], +) +def test_a_304_carries_the_dependency_header(cache_kwargs: dict[str, Any]) -> None: + client = _rate_limit_app(declare_response=False, **cache_kwargs) + + etag = client.get("/item").headers["ETag"] + response = client.get("/item", headers={"If-None-Match": etag}) + + assert response.status_code == 304 + assert response.headers["x-ratelimit-remaining"] == "9" + + +def test_a_header_the_handler_sets_is_still_stored_and_replayed() -> None: + """Handler lines on the sub-response are content: the hit has no handler run (#99).""" + app = FastAPI() + left = _countdown() + calls: list[int] = [] + + def rate_limit(response: Response) -> None: + response.headers["X-RateLimit-Remaining"] = str(next(left)) + + @app.get("/tagged", dependencies=[Depends(rate_limit)]) + @cache(ttl=60) + async def tagged(response: Response) -> dict[str, str]: + calls.append(1) + response.headers["X-Tag"] = "blue" + return {"tag": "blue"} + + client = TestClient(app) + miss = client.get("/tagged") + hit = client.get("/tagged") + + assert len(calls) == 1 + for response, remaining in ((miss, "10"), (hit, "9")): + assert response.headers.get_list("x-tag") == ["blue"] + assert response.headers.get_list("x-ratelimit-remaining") == [remaining] + + +def test_a_handler_returning_a_response_gets_the_dependency_header() -> None: + app = FastAPI() + left = _countdown() + + def rate_limit(response: Response) -> None: + response.headers["X-RateLimit-Remaining"] = str(next(left)) + + @app.get("/raw", dependencies=[Depends(rate_limit)]) + @cache(ttl=60) + async def raw() -> JSONResponse: + return JSONResponse({"raw": True}, headers={"X-Own": "1"}) + + client = TestClient(app) + responses = [client.get("/raw") for _ in range(2)] + + assert [r.headers["x-ratelimit-remaining"] for r in responses] == ["10", "9"] + assert [r.headers["x-own"] for r in responses] == ["1", "1"] + + +def _cookie_app( + sets_cookie: list[bool], **cache_kwargs: Any +) -> tuple[TestClient, list[int]]: + """A dependency that sets a cookie on the requests ``sets_cookie`` marks.""" + app = FastAPI() + requests = iter(sets_cookie) + calls: list[int] = [] + + def visit(response: Response) -> None: + if next(requests): + response.set_cookie("visit", "1") + + @app.get("/page", dependencies=[Depends(visit)]) + @cache(**cache_kwargs) + async def page() -> dict[str, bool]: + calls.append(1) + return {"page": True} + + return TestClient(app), calls + + +def test_a_dependency_cookie_is_sent_private_and_not_stored() -> None: + client, calls = _cookie_app([True, True], ttl=60, public=True) + + responses = [client.get("/page") for _ in range(2)] + + # As for a cookie the handler sets: it may carry per-user state, so + # neither this backend nor a shared cache downstream keeps the response. + assert len(calls) == 2 + for response in responses: + assert response.headers["set-cookie"].startswith("visit=1") + assert response.headers["cache-control"] == "private, max-age=60" + + +def test_a_hit_with_a_dependency_cookie_is_private() -> None: + client, calls = _cookie_app([False, True], ttl=60, public=True) + + stored = client.get("/page") + hit = client.get("/page") + + assert len(calls) == 1 + assert stored.headers["cache-control"] == "public, max-age=60" + assert "set-cookie" not in stored.headers + assert hit.headers["set-cookie"].startswith("visit=1") + assert hit.headers["cache-control"] == "private, max-age=60" + + +def test_a_stored_304_with_a_dependency_cookie_is_private() -> None: + client, _ = _cookie_app([False, True], ttl=60) + + etag = client.get("/page").headers["ETag"] + response = client.get("/page", headers={"If-None-Match": etag}) + + assert response.status_code == 304 + assert response.headers["set-cookie"].startswith("visit=1") + assert response.headers["cache-control"] == "private, max-age=60" + + +def test_no_store_keeps_no_store_with_a_dependency_cookie() -> None: + client, _ = _cookie_app([True], no_store=True) + + response = client.get("/page") + + assert response.headers["set-cookie"].startswith("visit=1") + assert response.headers["cache-control"] == "no-store" + + +def test_a_non_get_request_gets_the_dependency_header() -> None: + app = FastAPI() + + def tag(response: Response) -> None: + response.headers["X-Dep"] = "1" + + @app.post("/submit", dependencies=[Depends(tag)]) + @cache(ttl=60) + async def submit() -> JSONResponse: + return JSONResponse({"ok": True}, headers={"X-Dep": "own"}) + + response = TestClient(app).post("/submit") + + # Every line, as FastAPI sends them: only a cacheable GET drops a repeat. + assert response.headers.get_list("x-dep") == ["own", "1"] + assert "cache-control" not in response.headers + + +class _MyResponse(Response): + """A `Response` subclass, which FastAPI also injects as the sub-response.""" + + +@pytest.mark.parametrize( + "annotation", + [Response, "Response", Annotated[Response, "meta"], _MyResponse], + ids=["plain", "string", "annotated", "subclass"], +) +def test_a_declared_response_is_used_and_not_injected_twice(annotation: Any) -> None: + async def handler(response: Any) -> dict[str, bool]: + response.headers["X-Handler"] = "1" + return {"ok": True} + + handler.__annotations__["response"] = annotation + app = FastAPI() + + def tag(response: Response) -> None: + response.headers["X-Dep"] = "1" + + wrapped = cache(ttl=60)(handler) + app.get("/x", dependencies=[Depends(tag)])(wrapped) + + assert "__cachex_response" not in inspect.signature(wrapped).parameters + for response in (TestClient(app).get("/x"), TestClient(app).get("/x")): + assert response.headers.get_list("x-handler") == ["1"] + assert response.headers.get_list("x-dep") == ["1"] + + +@pytest.mark.parametrize("overwrite", [True, False], ids=["overwrite", "append"]) +def test_the_handlers_value_of_a_dependency_header_wins(overwrite: bool) -> None: + """On a hit the handler did not run: its stored value replaces the dependency's.""" + app = FastAPI() + left = _countdown() + + def rate_limit(response: Response) -> None: + response.headers["X-Rate"] = str(next(left)) + + @app.get("/rate", dependencies=[Depends(rate_limit)]) + @cache(ttl=60) + async def rate(response: Response) -> dict[str, bool]: + if overwrite: + response.headers["X-Rate"] = "handler" + else: + response.headers.append("X-Rate", "handler") + return {"ok": True} + + client = TestClient(app) + miss, hit = client.get("/rate"), client.get("/rate") + + assert miss.headers.get_list("x-rate") == ["handler"] + assert hit.headers.get_list("x-rate") == ["handler"] + + +def _private_dependency_app( + directive: str, **cache_kwargs: Any +) -> tuple[TestClient, list[str]]: + """A dependency that marks a signed-in user's response with ``directive``.""" + app = FastAPI() + calls: list[str] = [] + + def auth(request: Request, response: Response) -> None: + if "x-user" in request.headers: + response.headers["Cache-Control"] = directive + + @app.get("/me", dependencies=[Depends(auth)]) + @cache(**cache_kwargs) + async def me(request: Request) -> dict[str, str]: + user = request.headers.get("x-user", "anonymous") + calls.append(user) + return {"user": user} + + return TestClient(app), calls + + +@pytest.mark.parametrize("directive", ["private, no-store", "private", "no-store"]) +def test_a_private_header_from_a_dependency_keeps_the_response_out_of_the_backend( + directive: str, +) -> None: + client, calls = _private_dependency_app(directive, ttl=60, public=True) + + alice = client.get("/me", headers={"X-User": "alice"}) + anonymous = client.get("/me") + + assert alice.json() == {"user": "alice"} + # One Cache-Control, the dependency's: the decorator's would widen it. + assert alice.headers.get_list("cache-control") == [directive] + # Not stored, so the next caller does not get alice's response. + assert anonymous.json() == {"user": "anonymous"} + assert calls == ["alice", "anonymous"] + + +def test_a_hit_marked_private_by_a_dependency_is_sent_private() -> None: + client, calls = _private_dependency_app("private", ttl=60, public=True) + + client.get("/me") + hit = client.get("/me", headers={"X-User": "alice"}) + + assert calls == ["anonymous"] + assert hit.headers.get_list("cache-control") == ["private"] + + +def test_a_shareable_cache_control_from_a_dependency_is_replaced() -> None: + client, calls = _private_dependency_app("max-age=5", ttl=60) + + first = client.get("/me", headers={"X-User": "alice"}) + second = client.get("/me", headers={"X-User": "alice"}) + + # As a handler's own shareable header: the decorator's replaces it. + assert first.headers.get_list("cache-control") == ["max-age=60"] + assert second.headers.get_list("cache-control") == ["max-age=60"] + assert calls == ["alice"] + + +def test_bare_cache_keeps_a_shareable_cache_control_from_a_dependency() -> None: + client, _ = _private_dependency_app("max-age=5") + + response = client.get("/me", headers={"X-User": "alice"}) + + # As a handler's own header under a bare `@cache()` (#363). + assert response.headers.get_list("cache-control") == ["max-age=5"] + + +def test_a_status_a_dependency_sets_keeps_the_response_out_of_the_backend() -> None: + app = FastAPI() + calls: list[int] = [] + + def beta(request: Request, response: Response) -> None: + if "x-beta" in request.headers: + response.status_code = 203 + + @app.get("/page", dependencies=[Depends(beta)]) + @cache(ttl=60) + async def page() -> dict[str, bool]: + calls.append(1) + return {"page": True} + + client = TestClient(app) + beta_response = client.get("/page", headers={"X-Beta": "1"}) + plain = client.get("/page") + + assert beta_response.status_code == 203 + # Not stored, so the next caller does not get the beta status. + assert plain.status_code == 200 + assert len(calls) == 2 + + +def test_an_error_response_gets_every_dependency_line() -> None: + app = FastAPI() + + def tag(response: Response) -> None: + response.headers["Cache-Control"] = "max-age=5" + response.headers["X-Dep"] = "1" + + @app.get("/missing", dependencies=[Depends(tag)]) + @cache(ttl=60) + async def missing() -> JSONResponse: + return JSONResponse({"error": True}, status_code=404, headers={"X-Dep": "own"}) + + response = TestClient(app).get("/missing") + + # As FastAPI sends them: the decorator leaves uncacheable responses alone. + assert response.status_code == 404 + assert response.headers.get_list("x-dep") == ["own", "1"] + assert response.headers.get_list("cache-control") == ["max-age=5"] + + +def _make_response() -> Response: + return Response(headers={"X-Made": "1"}) + + +async def _made_annotated( + made: Annotated[Response, Depends(_make_response)], +) -> dict[str, bool]: + return {"ok": isinstance(made, Response)} + + +async def _made_default( + made: Response = Depends(_make_response), +) -> dict[str, bool]: + return {"ok": isinstance(made, Response)} + + +@pytest.mark.parametrize( + "handler", [_made_annotated, _made_default], ids=["annotated", "default"] +) +def test_a_response_returned_by_a_dependency_is_not_the_sub_response( + handler: Any, +) -> None: + """A ``Response`` a dependency returns must not stop the injection.""" + app = FastAPI() + + def tag(response: Response) -> None: + response.headers["X-Dep"] = "1" + + wrapped = cache(ttl=60)(handler) + app.get("/made", dependencies=[Depends(tag)])(wrapped) + + assert "__cachex_response" in inspect.signature(wrapped).parameters + response = TestClient(app).get("/made") + assert response.headers.get_list("x-dep") == ["1"] + assert "x-made" not in response.headers diff --git a/tests/test_cache_rendering.py b/tests/test_cache_rendering.py index 4699266..b2e67b4 100644 --- a/tests/test_cache_rendering.py +++ b/tests/test_cache_rendering.py @@ -4,11 +4,13 @@ from uuid import UUID from fastapi import FastAPI +from fastapi import Request from fastapi import Response from fastapi.testclient import TestClient from pydantic import BaseModel from fastapi_cachex.cache import cache +from fastapi_cachex.cache import get_response EVENT_ID = UUID("12345678-1234-5678-1234-567812345678") EVENT_AT = datetime(2026, 9, 25, 12, 0) # noqa: DTZ001 - naive encodes the same both ways @@ -117,3 +119,21 @@ async def tagged(response: Response) -> dict[str, str]: assert response.status_code == 203 assert response.headers["x-tag"] == "blue" assert response.json() == {"tag": "blue"} + + +def test_get_response_still_carries_a_response_argument() -> None: + """`get_response` is no longer used by `@cache`, but keeps its behaviour.""" + app = FastAPI() + + async def handler(response: Response) -> dict[str, bool]: + response.headers["X-Tag"] = "blue" + return {"ok": True} + + @app.get("/direct") + async def direct(request: Request, response: Response) -> Response: + return await get_response(handler, request, response=response) + + result = TestClient(app).get("/direct") + + assert result.headers.get_list("x-tag") == ["blue"] + assert result.json() == {"ok": True} diff --git a/tests/test_cache_request_param.py b/tests/test_cache_request_param.py index 57c33ab..21a4622 100644 --- a/tests/test_cache_request_param.py +++ b/tests/test_cache_request_param.py @@ -12,6 +12,8 @@ from fastapi_cachex import cache INJECTED = "__cachex_request" +# The sub-response is injected the same way, for the dependencies' headers (#233). +INJECTED_RESPONSE = "__cachex_response" class MyRequest(Request): @@ -71,7 +73,7 @@ async def handler(**kwargs: Any) -> dict[str, str]: params = list(inspect.signature(handler).parameters.values()) - assert [param.name for param in params] == [INJECTED, "kwargs"] + assert [param.name for param in params] == [INJECTED, INJECTED_RESPONSE, "kwargs"] assert params[-1].kind is inspect.Parameter.VAR_KEYWORD @@ -135,7 +137,7 @@ async def handler(request: Any) -> dict[str, str]: wrapped = cache(ttl=60)(handler) names = list(inspect.signature(wrapped).parameters) - assert names == ["request"] + assert names == ["request", INJECTED_RESPONSE] assert INJECTED not in names @@ -168,7 +170,11 @@ async def handler(thing: "Local") -> dict[str, bool]: wrapped = cache(ttl=60)(handler) - assert list(inspect.signature(wrapped).parameters) == ["thing", INJECTED] + assert list(inspect.signature(wrapped).parameters) == [ + "thing", + INJECTED, + INJECTED_RESPONSE, + ] def test_unresolvable_annotation_still_finds_a_plain_request(): @@ -182,4 +188,8 @@ async def handler(request: Request, thing: "Local") -> dict[str, str]: wrapped = cache(ttl=60)(handler) - assert list(inspect.signature(wrapped).parameters) == ["request", "thing"] + assert list(inspect.signature(wrapped).parameters) == [ + "request", + "thing", + INJECTED_RESPONSE, + ]