From c1c53a766ec9625b82f74a08777e5007ac3bb0c8 Mon Sep 17 00:00:00 2001 From: allen0099 Date: Fri, 2 Oct 2026 15:10:22 +0000 Subject: [PATCH 1/2] fix(cache): send no empty Cache-Control, and warn about @cache arguments that do nothing (#363, #328) --- changelog.d/328.changed.md | 4 + changelog.d/363.fixed.md | 3 + docs/CACHE_FLOW.md | 1 + docs/HTTP_CACHING.md | 10 +++ fastapi_cachex/cache.py | 61 ++++++++++++-- i18n/zh-TW/docs/CACHE_FLOW.md | 1 + i18n/zh-TW/docs/HTTP_CACHING.md | 5 ++ tests/test_cache.py | 22 +++-- tests/test_cache_age.py | 1 + tests/test_cache_arguments.py | 129 +++++++++++++++++++++++++++++ tests/test_cache_bypass_private.py | 1 + tests/test_cache_internals.py | 2 + tests/test_cache_private.py | 11 ++- tests/test_cache_unshareable.py | 12 +-- tests/test_cache_vary.py | 1 + 15 files changed, 242 insertions(+), 22 deletions(-) create mode 100644 changelog.d/328.changed.md create mode 100644 changelog.d/363.fixed.md create mode 100644 tests/test_cache_arguments.py diff --git a/changelog.d/328.changed.md b/changelog.d/328.changed.md new file mode 100644 index 0000000..0540336 --- /dev/null +++ b/changelog.d/328.changed.md @@ -0,0 +1,4 @@ +**`@cache` warns about arguments that do nothing.** A `UserWarning` is emitted +when the decorator is applied with `no_store=True` and another caching +argument it overrides (`ttl`, `public`, and so on), or with neither `ttl` nor +a directive, which stores nothing. Behaviour is unchanged. diff --git a/changelog.d/363.fixed.md b/changelog.d/363.fixed.md new file mode 100644 index 0000000..0cbacb8 --- /dev/null +++ b/changelog.d/363.fixed.md @@ -0,0 +1,3 @@ +**A bare `@cache()` no longer sends an empty `Cache-Control` header.** With +neither `ttl` nor a directive, the decorator has nothing to send, so the +handler's own `Cache-Control` is kept, or none is sent. diff --git a/docs/CACHE_FLOW.md b/docs/CACHE_FLOW.md index b43e0dd..a167bb1 100644 --- a/docs/CACHE_FLOW.md +++ b/docs/CACHE_FLOW.md @@ -163,6 +163,7 @@ The header value is built once per decorated route: |-----------|----------------------| | `no_store=True` | `no-store` (overrides everything else) | | `no_cache=True` | `no-cache`, plus `must-revalidate` if requested; `public`/`private`/`max-age`/`stale-*`/`immutable` are omitted | +| none (a bare `@cache()`) | none of its own; the handler's header, if any, is kept | | anything else | in order: `public` or `private`, `max-age=`, `must-revalidate`, `stale-while-revalidate=` or `stale-if-error=`, `immutable` | > [!NOTE] diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index d808bf7..978b671 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -78,6 +78,16 @@ header, and the server-side cache behaves the same with or without them. `no_store` only `no-store`. How the other arguments combine is described in [Cache flow](CACHE_FLOW.md#2-cache-control-directives). +Two kinds of arguments do nothing, and `@cache` emits a `UserWarning` pointing +at your `@cache(...)` line when the decorator is applied: + +- `no_store=True` together with any other caching argument (`ttl`, `stale`, + `no_cache`, `public`, `private`, `immutable`, `must_revalidate`): `no_store` + overrides them, and the warning names them. +- A bare `@cache()`, with neither `ttl` nor a directive: nothing is stored and + the decorator has no `Cache-Control` to send, so it keeps the handler's own + header (or sends none) and only adds an ETag. + ### The request's `Cache-Control` is ignored A client's own `Cache-Control` request header (`no-cache`, `max-age=0`, as sent diff --git a/fastapi_cachex/cache.py b/fastapi_cachex/cache.py index 36356c3..14f9929 100644 --- a/fastapi_cachex/cache.py +++ b/fastapi_cachex/cache.py @@ -258,6 +258,21 @@ def _request_credential(request: Request) -> str | None: "warnings.filterwarnings('ignore', message='cache vary on Cookie')." ) +_NOTHING_TO_CACHE_WARNING = ( + "cache has nothing to do: @cache() without ttl or a Cache-Control option " + "stores nothing and sends no Cache-Control header; it only adds an ETag. " + "Set ttl= to cache the response, or pass a directive such as " + "no_cache=True." +) + + +def _no_store_ignored_warning(ignored: list[str]) -> str: + return ( + f"cache no_store ignores {', '.join(ignored)}: @cache(no_store=True) " + "stores nothing and sends only Cache-Control: no-store, so the other " + "arguments have no effect. Remove them, or drop no_store." + ) + # How the one-time bypass warning names each `_request_credential` result. _CREDENTIAL_DESCRIPTIONS = { @@ -739,7 +754,8 @@ def _not_modified( response = Response(status_code=HTTP_304_NOT_MODIFIED) _append_headers(response, _revalidation_headers(headers)) response.headers["ETag"] = etag - response.headers["Cache-Control"] = cache_control + if cache_control: + response.headers["Cache-Control"] = cache_control response.headers.update(age or {}) return response @@ -781,12 +797,16 @@ def _cache_control_for( A handler that marked its response ``private`` or ``no-store`` keeps its own header; the decorator's would widen what the handler allowed. A response that sets a cookie gets ``private_cache_control``, so a shared cache in - front of the app does not store it either. + front of the app does not store it either. When the decorator has no + directive to send (a bare ``@cache()``), the handler's own header is kept, + or none is sent (#363). """ if _marked_unshareable(response): return ", ".join(response.headers.getlist("cache-control")) if "set-cookie" in response.headers: return private_cache_control + if not cache_control: + return ", ".join(response.headers.getlist("cache-control")) return cache_control @@ -794,9 +814,9 @@ def _with_cache_control( response: Response, cache_control: str, private_cache_control: str ) -> Response: if not _marked_unshareable(response): - response.headers["Cache-Control"] = _cache_control_for( - response, cache_control, private_cache_control - ) + value = _cache_control_for(response, cache_control, private_cache_control) + if value: + response.headers["Cache-Control"] = value return response @@ -1062,6 +1082,13 @@ def cache( ``key_builder``, or if ``key_builder`` is an ``async`` callable. At request time, if ``key_builder`` returns anything but a ``str``. + + Warns: + UserWarning: When the decorator is applied, if ``vary`` lists + ``Cookie``, if ``no_store`` is combined with another caching + argument it overrides, or if neither ``ttl`` nor any directive is + given (a bare ``@cache()`` stores nothing and keeps the handler's + own ``Cache-Control``). """ def decorator(func: HandlerCallable) -> AsyncResponseCallable: @@ -1092,6 +1119,28 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: # stacklevel=2: the caller applying the decorator, i.e. the line # of the user's @cache(...). warnings.warn(_COOKIE_VARY_WARNING, UserWarning, stacklevel=2) + # Combinations that cache nothing, or that no_store overrides, are + # accepted but warned about (#328); rejecting them would break routes + # that work today. + if no_store: + ignored = [ + name + for name, value in ( + ("ttl", ttl), + ("stale", stale), + ("stale_ttl", stale_ttl), + ("no_cache", no_cache), + ("public", public), + ("private", private), + ("immutable", immutable), + ("must_revalidate", must_revalidate), + ) + if value is not None and value is not False + ] + if ignored: + warnings.warn( + _no_store_ignored_warning(ignored), UserWarning, stacklevel=2 + ) # Analyze the original function's signature sig: Signature = inspect.signature(func) @@ -1143,6 +1192,8 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: immutable=immutable, must_revalidate=must_revalidate, ) + if ttl is None and not no_store and not cache_control: + warnings.warn(_NOTHING_TO_CACHE_WARNING, UserWarning, stacklevel=2) # Sent instead for a response that must not be stored downstream # either: one that sets a cookie, or one answering an `Authorization` # or session request the backend was bypassed for. `public` becomes `private`; diff --git a/i18n/zh-TW/docs/CACHE_FLOW.md b/i18n/zh-TW/docs/CACHE_FLOW.md index efc0a1a..a3b1850 100644 --- a/i18n/zh-TW/docs/CACHE_FLOW.md +++ b/i18n/zh-TW/docs/CACHE_FLOW.md @@ -126,6 +126,7 @@ cache_key = "|".join( |------|------------------------| | `no_store=True` | `no-store`（覆蓋其他所有設定） | | `no_cache=True` | `no-cache`，若有要求則加上 `must-revalidate`；省略 `public`／`private`／`max-age`／`stale-*`／`immutable` | +| 無（不帶參數的 `@cache()`） | 不送出自己的標頭；保留 handler 的標頭（若有） | | 其他情況 | 依序為：`public` 或 `private`、`max-age=`、`must-revalidate`、`stale-while-revalidate=` 或 `stale-if-error=`、`immutable` | > [!NOTE] diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index 0c12382..a437079 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -57,6 +57,11 @@ async def items(): ... `no_cache=True` 與 `no_store=True` 會取代標頭的其餘內容：使用 `no_cache` 時只送出 `no-cache`（若有設定，再加上 `must-revalidate`），使用 `no_store` 時只送出 `no-store`。其他參數如何組合，請見[快取流程](CACHE_FLOW.md#2-cache-control-directives)。 +有兩種參數組合不會有任何效果，套用裝飾器時 `@cache` 會發出指向你 `@cache(...)` 那一行的 `UserWarning`： + +- `no_store=True` 搭配其他任何快取參數（`ttl`、`stale`、`no_cache`、`public`、`private`、`immutable`、`must_revalidate`）：`no_store` 會覆蓋它們，警告會列出這些參數。 +- 不帶參數的 `@cache()`，既沒有 `ttl` 也沒有任何指令：不會儲存任何內容，裝飾器也沒有 `Cache-Control` 可送，因此會保留 handler 自己的標頭（沒有就不送），只加上 ETag。 + ### 請求的 `Cache-Control` 會被忽略 {#the-requests-cache-control-is-ignored} 用戶端自己送出的 `Cache-Control` 請求標頭（例如瀏覽器強制重新整理時送出的 `no-cache`、`max-age=0` 等）不會改變 `@cache` 的行為。這是刻意的設計：若請求標頭能繞過快取，任何用戶端都能讓每個請求直接打到你的 handler。條件式請求仍會處理：`If-None-Match` 相符時回 304。 diff --git a/tests/test_cache.py b/tests/test_cache.py index dbbf77b..718ccf4 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -19,6 +19,10 @@ from fastapi_cachex.proxy import BackendProxy from fastapi_cachex.types import CacheEntry +# Most tests here use a bare @cache() for the ETag handling alone; the warning +# it emits is tested in test_cache_arguments.py. +pytestmark = pytest.mark.filterwarnings("ignore:cache has nothing to do:UserWarning") + app = FastAPI() client = TestClient(app) @@ -34,7 +38,7 @@ async def default_cache_endpoint(): response = client.get("/default") assert response.status_code == 200 - assert response.headers["Cache-Control"] == "" + assert "Cache-Control" not in response.headers assert "ETag" in response.headers @@ -627,10 +631,12 @@ def test_no_store_and_no_cache_combined(): """no_store=True takes priority; only 'no-store' appears in Cache-Control.""" combo_app = FastAPI() - @combo_app.get("/no-store-no-cache") - @cache(no_store=True, no_cache=True) - async def no_store_no_cache_endpoint(): - return Response(content=b"data", media_type="text/plain") + with pytest.warns(UserWarning, match="no_store ignores no_cache"): + + @combo_app.get("/no-store-no-cache") + @cache(no_store=True, no_cache=True) + async def no_store_no_cache_endpoint(): + return Response(content=b"data", media_type="text/plain") combo_client = TestClient(combo_app) r = combo_client.get("/no-store-no-cache") @@ -640,7 +646,7 @@ async def no_store_no_cache_endpoint(): assert "no-cache" not in cc -@pytest.mark.parametrize(("ttl", "cache_control"), [(None, ""), (0, "max-age=0")]) +@pytest.mark.parametrize(("ttl", "cache_control"), [(None, None), (0, "max-age=0")]) def test_without_a_positive_ttl_nothing_is_stored_or_served(ttl, cache_control): """No positive ttl means no server-side cache, and no 304 from a stale ETag (#110). @@ -664,14 +670,14 @@ async def no_ttl_endpoint(): r1 = no_ttl_client.get("/no-ttl") assert r1.status_code == 200 - assert r1.headers["Cache-Control"] == cache_control + assert r1.headers.get("Cache-Control") == cache_control old_etag = r1.headers["ETag"] assert backend.cache == {} # An unchanged response still revalidates, against the fresh render. r2 = no_ttl_client.get("/no-ttl", headers={"If-None-Match": old_etag}) assert r2.status_code == 304 - assert r2.headers["Cache-Control"] == cache_control + assert r2.headers.get("Cache-Control") == cache_control assert len(calls) == 2 # The data changes: the old validator must not get a 304 any more. diff --git a/tests/test_cache_age.py b/tests/test_cache_age.py index d89d371..feb49e6 100644 --- a/tests/test_cache_age.py +++ b/tests/test_cache_age.py @@ -175,6 +175,7 @@ async def test_no_cache_revalidation_sends_no_age(clock, client_for): assert "age" not in full.headers +@pytest.mark.filterwarnings("ignore:cache no_store ignores:UserWarning") @pytest.mark.parametrize( ("cache_kwargs", "method", "headers"), [ diff --git a/tests/test_cache_arguments.py b/tests/test_cache_arguments.py new file mode 100644 index 0000000..9a80885 --- /dev/null +++ b/tests/test_cache_arguments.py @@ -0,0 +1,129 @@ +"""`@cache` arguments that cache nothing or contradict each other (#328, #363). + +A bare `@cache()` has no directive to send: it used to set an empty +`Cache-Control` header, replacing the handler's own one. It now sends none of +its own and warns when applied. `no_store` overrides every other caching +argument, so passing one with it warns as well. +""" + +import pytest +from fastapi import FastAPI +from fastapi import Response +from fastapi.testclient import TestClient + +from fastapi_cachex.cache import cache + + +def _bare_app(handler_cache_control: str | None = None) -> TestClient: + app = FastAPI() + + with pytest.warns(UserWarning, match="cache has nothing to do"): + + @app.get("/bare") + @cache() + async def bare(response: Response): + if handler_cache_control is not None: + response.headers["Cache-Control"] = handler_cache_control + return {"ok": True} + + return TestClient(app) + + +def test_bare_cache_warns_at_the_decorator_line() -> None: + with pytest.warns(UserWarning, match="cache has nothing to do") as record: + + @cache() + async def handler(): + return {} + + assert record[0].filename == __file__ + + +def test_bare_cache_sends_no_cache_control() -> None: + client = _bare_app() + + first = client.get("/bare") + assert "Cache-Control" not in first.headers + assert "ETag" in first.headers + + revalidated = client.get("/bare", headers={"If-None-Match": first.headers["ETag"]}) + assert revalidated.status_code == 304 + assert "Cache-Control" not in revalidated.headers + + +def test_bare_cache_keeps_the_handlers_cache_control() -> None: + client = _bare_app("max-age=30") + + first = client.get("/bare") + assert first.headers["Cache-Control"] == "max-age=30" + + revalidated = client.get("/bare", headers={"If-None-Match": first.headers["ETag"]}) + assert revalidated.status_code == 304 + assert revalidated.headers["Cache-Control"] == "max-age=30" + + +def test_bare_cache_still_sends_private_for_a_cookie() -> None: + app = FastAPI() + + with pytest.warns(UserWarning, match="cache has nothing to do"): + + @app.get("/cookie") + @cache() + async def cookie(response: Response): + response.set_cookie("c", "1") + return {} + + assert TestClient(app).get("/cookie").headers["Cache-Control"] == "private" + + +@pytest.mark.parametrize( + ("kwargs", "ignored"), + [ + ({"ttl": 60}, "ttl"), + ({"ttl": 0}, "ttl"), + ({"stale": "revalidate", "stale_ttl": 30}, "stale, stale_ttl"), + ({"no_cache": True}, "no_cache"), + ({"public": True}, "public"), + ({"private": True}, "private"), + ({"immutable": True}, "immutable"), + ({"must_revalidate": True}, "must_revalidate"), + ({"ttl": 60, "public": True}, "ttl, public"), + ], +) +def test_no_store_warns_about_the_arguments_it_overrides( + kwargs: dict[str, object], ignored: str +) -> None: + app = FastAPI() + + with pytest.warns( + UserWarning, match=f"cache no_store ignores {ignored}:" + ) as record: + + @app.get("/none") + @cache(no_store=True, **kwargs) # type: ignore[arg-type] + async def none(): + return {} + + assert len(record) == 1 + assert record[0].filename == __file__ + # The warning changes nothing at request time. + assert TestClient(app).get("/none").headers["Cache-Control"] == "no-store" + + +@pytest.mark.parametrize( + "kwargs", + [ + {"no_store": True}, + {"ttl": 60}, + {"ttl": 0}, + {"public": True}, + {"no_cache": True}, + {"must_revalidate": True}, + {"ttl": 60, "public": False, "no_cache": False}, + ], +) +def test_arguments_that_do_something_do_not_warn(kwargs: dict[str, object]) -> None: + # filterwarnings = error turns any warning into a failure here. + @cache(**kwargs) # type: ignore[arg-type] + async def handler(): + return {} diff --git a/tests/test_cache_bypass_private.py b/tests/test_cache_bypass_private.py index dc47a50..2932e52 100644 --- a/tests/test_cache_bypass_private.py +++ b/tests/test_cache_bypass_private.py @@ -34,6 +34,7 @@ async def me() -> dict[str, str]: return TestClient(app) +@pytest.mark.filterwarnings("ignore:cache has nothing to do:UserWarning") @pytest.mark.parametrize( ("cache_kwargs", "expected"), [ diff --git a/tests/test_cache_internals.py b/tests/test_cache_internals.py index 8d2ba2f..1fed293 100644 --- a/tests/test_cache_internals.py +++ b/tests/test_cache_internals.py @@ -27,6 +27,8 @@ async def item() -> dict[str, str]: return TestClient(app), calls +@pytest.mark.filterwarnings("ignore:cache no_store ignores:UserWarning") +@pytest.mark.filterwarnings("ignore:cache has nothing to do:UserWarning") @pytest.mark.parametrize( "cache_kwargs", [ diff --git a/tests/test_cache_private.py b/tests/test_cache_private.py index 85ca05e..7aa6174 100644 --- a/tests/test_cache_private.py +++ b/tests/test_cache_private.py @@ -8,6 +8,7 @@ from collections.abc import AsyncIterator +import pytest from fastapi import FastAPI from fastapi import Request from fastapi import Response @@ -134,10 +135,12 @@ def test_no_store_still_wins_over_private(): app = FastAPI() client = TestClient(app) - @app.get("/private-no-store") - @cache(private=True, no_store=True) - async def private_no_store(): - return Response(content="x", media_type="text/plain") + with pytest.warns(UserWarning, match="no_store ignores private"): + + @app.get("/private-no-store") + @cache(private=True, no_store=True) + async def private_no_store(): + return Response(content="x", media_type="text/plain") assert client.get("/private-no-store").headers["Cache-Control"] == "no-store" diff --git a/tests/test_cache_unshareable.py b/tests/test_cache_unshareable.py index fa5d0f6..df5aa8a 100644 --- a/tests/test_cache_unshareable.py +++ b/tests/test_cache_unshareable.py @@ -482,10 +482,12 @@ async def opted_in(): def test_no_store_decorator_wins_over_set_cookie(): app = FastAPI() - @app.get("/nothing") - @cache(no_store=True, public=True) - async def nothing(response: Response): - response.set_cookie("c", "1") - return {} + with pytest.warns(UserWarning, match="no_store ignores public"): + + @app.get("/nothing") + @cache(no_store=True, public=True) + async def nothing(response: Response): + response.set_cookie("c", "1") + return {} assert TestClient(app).get("/nothing").headers["Cache-Control"] == "no-store" diff --git a/tests/test_cache_vary.py b/tests/test_cache_vary.py index e9abe7d..11e96e2 100644 --- a/tests/test_cache_vary.py +++ b/tests/test_cache_vary.py @@ -154,6 +154,7 @@ def test_vary_header_on_miss_hit_and_304() -> None: assert response.headers["vary"] == "Accept-Language" +@pytest.mark.filterwarnings("ignore:cache no_store ignores:UserWarning") @pytest.mark.parametrize( ("cache_kwargs", "request_headers"), [ From 01ab56cef2b8c52d9de41b7d76adc4eea52726f7 Mon Sep 17 00:00:00 2001 From: allen0099 Date: Fri, 2 Oct 2026 15:14:44 +0000 Subject: [PATCH 2/2] fix(cache): keep a bare @cache() as an ETag-only mode without a warning; document the private exception --- changelog.d/328.changed.md | 8 +-- changelog.d/363.fixed.md | 6 +- docs/CACHE_FLOW.md | 2 +- docs/HTTP_CACHING.md | 19 +++--- fastapi_cachex/cache.py | 28 ++++----- i18n/zh-TW/docs/CACHE_FLOW.md | 2 +- i18n/zh-TW/docs/HTTP_CACHING.md | 5 +- tests/test_cache.py | 4 -- tests/test_cache_arguments.py | 96 +++++++++++++++++++----------- tests/test_cache_bypass_private.py | 1 - tests/test_cache_internals.py | 1 - 11 files changed, 95 insertions(+), 77 deletions(-) diff --git a/changelog.d/328.changed.md b/changelog.d/328.changed.md index 0540336..cc6efdb 100644 --- a/changelog.d/328.changed.md +++ b/changelog.d/328.changed.md @@ -1,4 +1,4 @@ -**`@cache` warns about arguments that do nothing.** A `UserWarning` is emitted -when the decorator is applied with `no_store=True` and another caching -argument it overrides (`ttl`, `public`, and so on), or with neither `ttl` nor -a directive, which stores nothing. Behaviour is unchanged. +**`@cache(no_store=True)` warns about the arguments it overrides.** A +`UserWarning` naming them is emitted when the decorator is applied with +`no_store=True` and `ttl`, `stale`, `no_cache`, `public`, `private`, +`immutable` or `must_revalidate`. The response is unchanged: `no-store` only. diff --git a/changelog.d/363.fixed.md b/changelog.d/363.fixed.md index 0cbacb8..4483e86 100644 --- a/changelog.d/363.fixed.md +++ b/changelog.d/363.fixed.md @@ -1,3 +1,5 @@ **A bare `@cache()` no longer sends an empty `Cache-Control` header.** With -neither `ttl` nor a directive, the decorator has nothing to send, so the -handler's own `Cache-Control` is kept, or none is sent. +neither `ttl` nor a directive, the decorator has no directive of its own: it +keeps the handler's `Cache-Control`, or sends none, and adds only the ETag. A +response that sets a cookie or answers a request with credentials is still +sent with `private`. diff --git a/docs/CACHE_FLOW.md b/docs/CACHE_FLOW.md index a167bb1..148380d 100644 --- a/docs/CACHE_FLOW.md +++ b/docs/CACHE_FLOW.md @@ -163,7 +163,7 @@ The header value is built once per decorated route: |-----------|----------------------| | `no_store=True` | `no-store` (overrides everything else) | | `no_cache=True` | `no-cache`, plus `must-revalidate` if requested; `public`/`private`/`max-age`/`stale-*`/`immutable` are omitted | -| none (a bare `@cache()`) | none of its own; the handler's header, if any, is kept | +| none (a bare `@cache()`) | none of its own; the handler's header, if any, is kept (`private` for a cookie or credentials, as on every route) | | anything else | in order: `public` or `private`, `max-age=`, `must-revalidate`, `stale-while-revalidate=` or `stale-if-error=`, `immutable` | > [!NOTE] diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index 978b671..8b14868 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -78,15 +78,16 @@ header, and the server-side cache behaves the same with or without them. `no_store` only `no-store`. How the other arguments combine is described in [Cache flow](CACHE_FLOW.md#2-cache-control-directives). -Two kinds of arguments do nothing, and `@cache` emits a `UserWarning` pointing -at your `@cache(...)` line when the decorator is applied: - -- `no_store=True` together with any other caching argument (`ttl`, `stale`, - `no_cache`, `public`, `private`, `immutable`, `must_revalidate`): `no_store` - overrides them, and the warning names them. -- A bare `@cache()`, with neither `ttl` nor a directive: nothing is stored and - the decorator has no `Cache-Control` to send, so it keeps the handler's own - header (or sends none) and only adds an ETag. +`no_store=True` together with any other caching argument (`ttl`, `stale`, +`no_cache`, `public`, `private`, `immutable`, `must_revalidate`) emits a +`UserWarning` pointing at your `@cache(...)` line when the decorator is +applied: `no_store` overrides them, and the warning names them. + +A bare `@cache()`, with neither `ttl` nor a directive, stores nothing and has no +`Cache-Control` of its own. It adds an ETag and answers a matching +`If-None-Match` with `304`, and keeps the handler's own `Cache-Control` (or +sends none). As on every route, a response that sets a cookie or answers a +request with `Authorization` or a session is still sent with `private`. ### The request's `Cache-Control` is ignored diff --git a/fastapi_cachex/cache.py b/fastapi_cachex/cache.py index 14f9929..21699bb 100644 --- a/fastapi_cachex/cache.py +++ b/fastapi_cachex/cache.py @@ -258,13 +258,6 @@ def _request_credential(request: Request) -> str | None: "warnings.filterwarnings('ignore', message='cache vary on Cookie')." ) -_NOTHING_TO_CACHE_WARNING = ( - "cache has nothing to do: @cache() without ttl or a Cache-Control option " - "stores nothing and sends no Cache-Control header; it only adds an ETag. " - "Set ttl= to cache the response, or pass a directive such as " - "no_cache=True." -) - def _no_store_ignored_warning(ignored: list[str]) -> str: return ( @@ -799,7 +792,7 @@ def _cache_control_for( that sets a cookie gets ``private_cache_control``, so a shared cache in front of the app does not store it either. When the decorator has no directive to send (a bare ``@cache()``), the handler's own header is kept, - or none is sent (#363). + or none is sent (#363); a cookie still gets ``private_cache_control``. """ if _marked_unshareable(response): return ", ".join(response.headers.getlist("cache-control")) @@ -1083,12 +1076,16 @@ def cache( At request time, if ``key_builder`` returns anything but a ``str``. + Without ``ttl`` or any directive (a bare ``@cache()``), nothing is stored + and the decorator has no ``Cache-Control`` of its own: it adds an ETag, + answers a matching ``If-None-Match`` with 304 and keeps the handler's own + ``Cache-Control``, except that a response setting a cookie or answering a + request with credentials is still sent with ``private``. + Warns: UserWarning: When the decorator is applied, if ``vary`` lists - ``Cookie``, if ``no_store`` is combined with another caching - argument it overrides, or if neither ``ttl`` nor any directive is - given (a bare ``@cache()`` stores nothing and keeps the handler's - own ``Cache-Control``). + ``Cookie``, or if ``no_store`` is combined with another caching + argument it overrides. """ def decorator(func: HandlerCallable) -> AsyncResponseCallable: @@ -1119,9 +1116,8 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: # stacklevel=2: the caller applying the decorator, i.e. the line # of the user's @cache(...). warnings.warn(_COOKIE_VARY_WARNING, UserWarning, stacklevel=2) - # Combinations that cache nothing, or that no_store overrides, are - # accepted but warned about (#328); rejecting them would break routes - # that work today. + # Arguments no_store overrides are accepted but warned about (#328); + # rejecting them would break routes that work today. if no_store: ignored = [ name @@ -1192,8 +1188,6 @@ def decorator(func: HandlerCallable) -> AsyncResponseCallable: immutable=immutable, must_revalidate=must_revalidate, ) - if ttl is None and not no_store and not cache_control: - warnings.warn(_NOTHING_TO_CACHE_WARNING, UserWarning, stacklevel=2) # Sent instead for a response that must not be stored downstream # either: one that sets a cookie, or one answering an `Authorization` # or session request the backend was bypassed for. `public` becomes `private`; diff --git a/i18n/zh-TW/docs/CACHE_FLOW.md b/i18n/zh-TW/docs/CACHE_FLOW.md index a3b1850..0b97fbc 100644 --- a/i18n/zh-TW/docs/CACHE_FLOW.md +++ b/i18n/zh-TW/docs/CACHE_FLOW.md @@ -126,7 +126,7 @@ cache_key = "|".join( |------|------------------------| | `no_store=True` | `no-store`（覆蓋其他所有設定） | | `no_cache=True` | `no-cache`，若有要求則加上 `must-revalidate`；省略 `public`／`private`／`max-age`／`stale-*`／`immutable` | -| 無（不帶參數的 `@cache()`） | 不送出自己的標頭；保留 handler 的標頭（若有） | +| 無（不帶參數的 `@cache()`） | 不送出自己的標頭；保留 handler 的標頭（若有）（遇到 Cookie 或憑證時送出 `private`，與其他路由相同） | | 其他情況 | 依序為：`public` 或 `private`、`max-age=`、`must-revalidate`、`stale-while-revalidate=` 或 `stale-if-error=`、`immutable` | > [!NOTE] diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index a437079..6aa2217 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -57,10 +57,9 @@ async def items(): ... `no_cache=True` 與 `no_store=True` 會取代標頭的其餘內容：使用 `no_cache` 時只送出 `no-cache`（若有設定，再加上 `must-revalidate`），使用 `no_store` 時只送出 `no-store`。其他參數如何組合，請見[快取流程](CACHE_FLOW.md#2-cache-control-directives)。 -有兩種參數組合不會有任何效果，套用裝飾器時 `@cache` 會發出指向你 `@cache(...)` 那一行的 `UserWarning`： +`no_store=True` 搭配其他任何快取參數（`ttl`、`stale`、`no_cache`、`public`、`private`、`immutable`、`must_revalidate`）時，套用裝飾器時 `@cache` 會發出指向你 `@cache(...)` 那一行的 `UserWarning`：`no_store` 會覆蓋這些參數，警告會列出它們。 -- `no_store=True` 搭配其他任何快取參數（`ttl`、`stale`、`no_cache`、`public`、`private`、`immutable`、`must_revalidate`）：`no_store` 會覆蓋它們，警告會列出這些參數。 -- 不帶參數的 `@cache()`，既沒有 `ttl` 也沒有任何指令：不會儲存任何內容，裝飾器也沒有 `Cache-Control` 可送，因此會保留 handler 自己的標頭（沒有就不送），只加上 ETag。 +不帶參數的 `@cache()`，既沒有 `ttl` 也沒有任何指令，不會儲存任何內容，也沒有自己的 `Cache-Control`。它會加上 ETag，以 `304` 回應相符的 `If-None-Match`，並保留 handler 自己的 `Cache-Control`（沒有就不送）。與其他路由一樣，設定 Cookie 的回應，或回應帶有 `Authorization` 或 Session 的請求時，仍會送出 `private`。 ### 請求的 `Cache-Control` 會被忽略 {#the-requests-cache-control-is-ignored} diff --git a/tests/test_cache.py b/tests/test_cache.py index 718ccf4..26d4028 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -19,10 +19,6 @@ from fastapi_cachex.proxy import BackendProxy from fastapi_cachex.types import CacheEntry -# Most tests here use a bare @cache() for the ETag handling alone; the warning -# it emits is tested in test_cache_arguments.py. -pytestmark = pytest.mark.filterwarnings("ignore:cache has nothing to do:UserWarning") - app = FastAPI() client = TestClient(app) diff --git a/tests/test_cache_arguments.py b/tests/test_cache_arguments.py index 9a80885..7f1b819 100644 --- a/tests/test_cache_arguments.py +++ b/tests/test_cache_arguments.py @@ -1,44 +1,36 @@ -"""`@cache` arguments that cache nothing or contradict each other (#328, #363). +"""`@cache` arguments that send no directive or contradict each other (#328, #363). A bare `@cache()` has no directive to send: it used to set an empty -`Cache-Control` header, replacing the handler's own one. It now sends none of -its own and warns when applied. `no_store` overrides every other caching -argument, so passing one with it warns as well. +`Cache-Control` header, replacing the handler's own one. It now keeps the +handler's header and only adds the ETag, which makes it an ETag-only mode, so +it does not warn. `no_store` overrides every other caching argument, so passing +one with it warns. """ +from collections.abc import AsyncIterator + import pytest from fastapi import FastAPI from fastapi import Response +from fastapi.responses import StreamingResponse from fastapi.testclient import TestClient from fastapi_cachex.cache import cache -def _bare_app(handler_cache_control: str | None = None) -> TestClient: +def _bare_app(handler_cache_control: str | None = None, **kwargs: object) -> TestClient: app = FastAPI() - with pytest.warns(UserWarning, match="cache has nothing to do"): - - @app.get("/bare") - @cache() - async def bare(response: Response): - if handler_cache_control is not None: - response.headers["Cache-Control"] = handler_cache_control - return {"ok": True} + @app.get("/bare") + @cache(**kwargs) # type: ignore[arg-type] + async def bare(response: Response): + if handler_cache_control is not None: + response.headers["Cache-Control"] = handler_cache_control + return {"ok": True} return TestClient(app) -def test_bare_cache_warns_at_the_decorator_line() -> None: - with pytest.warns(UserWarning, match="cache has nothing to do") as record: - - @cache() - async def handler(): - return {} - - assert record[0].filename == __file__ - - def test_bare_cache_sends_no_cache_control() -> None: client = _bare_app() @@ -51,31 +43,62 @@ def test_bare_cache_sends_no_cache_control() -> None: assert "Cache-Control" not in revalidated.headers -def test_bare_cache_keeps_the_handlers_cache_control() -> None: - client = _bare_app("max-age=30") +@pytest.mark.parametrize("handler_header", ["max-age=30", "private, max-age=30"]) +def test_bare_cache_keeps_the_handlers_cache_control(handler_header: str) -> None: + client = _bare_app(handler_header) first = client.get("/bare") - assert first.headers["Cache-Control"] == "max-age=30" + assert first.headers["Cache-Control"] == handler_header revalidated = client.get("/bare", headers={"If-None-Match": first.headers["ETag"]}) assert revalidated.status_code == 304 - assert revalidated.headers["Cache-Control"] == "max-age=30" + assert revalidated.headers["Cache-Control"] == handler_header -def test_bare_cache_still_sends_private_for_a_cookie() -> None: +def test_bare_cache_with_vary_sends_vary_and_no_cache_control() -> None: + response = _bare_app(vary=["Accept-Language"]).get("/bare") + + assert "Cache-Control" not in response.headers + assert response.headers["Vary"] == "Accept-Language" + + +def test_bare_cache_streaming_response_gets_no_cache_control() -> None: app = FastAPI() - with pytest.warns(UserWarning, match="cache has nothing to do"): + async def chunks() -> AsyncIterator[bytes]: + yield b"x" - @app.get("/cookie") - @cache() - async def cookie(response: Response): - response.set_cookie("c", "1") - return {} + @app.get("/stream") + @cache() + async def stream(): + return StreamingResponse(chunks(), media_type="text/plain") + + assert "Cache-Control" not in TestClient(app).get("/stream").headers + + +def test_bare_cache_still_sends_private_for_a_cookie() -> None: + app = FastAPI() + + @app.get("/cookie") + @cache() + async def cookie(response: Response): + response.set_cookie("c", "1") + response.headers["Cache-Control"] = "max-age=30" + return {} + # As on every route: a response that sets a cookie is sent with the + # decorator's private header, in place of the handler's own one. assert TestClient(app).get("/cookie").headers["Cache-Control"] == "private" +def test_bare_cache_still_sends_private_for_a_credential_request() -> None: + response = _bare_app("max-age=30").get( + "/bare", headers={"Authorization": "Bearer t"} + ) + + assert response.headers["Cache-Control"] == "private" + + @pytest.mark.parametrize( ("kwargs", "ignored"), [ @@ -119,6 +142,11 @@ async def none(): {"public": True}, {"no_cache": True}, {"must_revalidate": True}, + {"private": True}, + {"immutable": True}, + {"stale": "revalidate", "stale_ttl": 30}, + {}, + {"vary": ["Accept-Language"]}, {"ttl": 60, "public": False, "no_cache": False}, ], ) diff --git a/tests/test_cache_bypass_private.py b/tests/test_cache_bypass_private.py index 2932e52..dc47a50 100644 --- a/tests/test_cache_bypass_private.py +++ b/tests/test_cache_bypass_private.py @@ -34,7 +34,6 @@ async def me() -> dict[str, str]: return TestClient(app) -@pytest.mark.filterwarnings("ignore:cache has nothing to do:UserWarning") @pytest.mark.parametrize( ("cache_kwargs", "expected"), [ diff --git a/tests/test_cache_internals.py b/tests/test_cache_internals.py index 1fed293..83b3def 100644 --- a/tests/test_cache_internals.py +++ b/tests/test_cache_internals.py @@ -28,7 +28,6 @@ async def item() -> dict[str, str]: @pytest.mark.filterwarnings("ignore:cache no_store ignores:UserWarning") -@pytest.mark.filterwarnings("ignore:cache has nothing to do:UserWarning") @pytest.mark.parametrize( "cache_kwargs", [