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, + ]