From 30c6798e414e5f3e12ddc7872892b2287ae111d0 Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sat, 26 Sep 2026 11:25:16 +0000 Subject: [PATCH] fix(cache): skip the backend for routes without a positive ttl With ttl=None or ttl=0 the response was stored without expiry, and a request whose If-None-Match matched the stored ETag got a 304 without the handler running. Once the data changed, a client revalidating with the old ETag kept getting 304 until another request rewrote the entry, and every query-string variant stayed in the backend forever. A ttl that allows no reuse now means no server-side cache: such routes take the private=True path, which neither reads nor writes the backend, runs the handler on every request and answers 304 only when If-None-Match matches the freshly rendered response. Tests that relied on ttl-less no_cache routes storing entries now set ttl=60. Closes #110 --- CHANGELOG.md | 12 ++++ docs/CACHE_FLOW.md | 17 ++--- docs/HTTP_CACHING.md | 4 +- fastapi_cachex/cache.py | 38 ++++++----- tests/test_cache.py | 100 +++++++++++++++++++---------- tests/test_cache_hit.py | 2 +- tests/test_cache_revalidation.py | 2 +- tests/test_cache_status_headers.py | 9 +-- 8 files changed, 116 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b56c2e3..4d939af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,18 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. read as counters. ([#111](https://github.com/allen0099/FastAPI-CacheX/issues/111)) +- **`@cache` without a positive `ttl` no longer stores responses or answers + 304 from a stored ETag.** With `ttl=None` or `ttl=0`, the response was stored + without expiry, and a request whose `If-None-Match` matched the stored ETag + got a 304 without the handler running. After the data changed, a client + revalidating with the old ETag kept getting 304 until another request + rewrote the entry, and entries for every query string accumulated. These + routes now skip the backend like `private=True` ones: the handler runs on + every request, and a 304 is sent only when `If-None-Match` matches the + freshly rendered response. Entries that earlier versions stored for them + are no longer read; `clear()` removes them. + ([#110](https://github.com/allen0099/FastAPI-CacheX/issues/110)) + ## [0.3.7] - 2026-09-25 ### Added diff --git a/docs/CACHE_FLOW.md b/docs/CACHE_FLOW.md index fe8fd76..42d68ee 100644 --- a/docs/CACHE_FLOW.md +++ b/docs/CACHE_FLOW.md @@ -113,10 +113,11 @@ The header value is built once per decorated route: | anything else | in order: `public` or `private`, `max-age=`, `must-revalidate`, `stale-while-revalidate=` or `stale-if-error=`, `immutable` | > [!NOTE] -> Without `ttl` (or with `ttl=0`, which sends `max-age=0`), an entry is still -> written (with no expiry) but is never served directly: it is only used to -> answer a matching `If-None-Match` with `304`. Set a positive `ttl` to have the -> server replay cached responses. +> Without `ttl` (or with `ttl=0`, which sends `max-age=0`), the backend is +> neither read nor written: the handler runs on every request, and a matching +> `If-None-Match` is answered with `304` only after comparing it against the +> freshly rendered response. Set a positive `ttl` to have the server store and +> replay responses. > [!WARNING] > **The default cache key does not include the user's identity**, and the backend @@ -169,8 +170,8 @@ if request.method != "GET": if no_store: return await render() # no read, no write -if private: - response, etag = await render() # shared backend neither read nor written +if private or not ttl: + response, etag = await render() # backend neither read nor written return not_modified(...) if etag_matches(client_etag, etag) else response entry = await backend.get(cache_key) # expired entries are already skipped here @@ -182,7 +183,7 @@ if client_etag and no_cache: elif client_etag and entry and etag_matches(client_etag, entry.fingerprint): return not_modified(...) # 304, handler does not run -if entry and not no_cache and ttl is not None: +if entry and not no_cache: return Response( # 200, handler does not run content=entry.content, status_code=entry.status_code, @@ -373,7 +374,7 @@ lookup. Which backend to pick is covered in [Backends](BACKENDS.md#choosing-a-ba | `no_store=True` | The cache is neither read nor written; the endpoint runs every time | | `no_cache=True` | The endpoint runs every time to recompute the ETag; a match with the client's `If-None-Match` still returns 304, and the cache is updated when the ETag changes | | `private=True` | The **shared backend** is neither read nor written; `Cache-Control: private` is still sent and the ETag is compared against fresh content | -| No `ttl` | Entries are written without expiry but only used for `If-None-Match` revalidation; the handler runs on every request without a matching validator | +| No `ttl` (or `ttl=0`) | The backend is neither read nor written, as with `private=True`; the endpoint runs every time and the ETag is compared against fresh content | | Cache expired (TTL elapsed) | The endpoint runs again; `MemoryBackend` deletes the expired entry in place when it reads it | | Non-2xx or 206 response | Returned as-is, not written, and any existing entry is left untouched | | Streaming/file response | No ETag can be computed; returned as-is and not written | diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index eaddfe4..02946ae 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -45,7 +45,7 @@ header, and the server-side cache behaves the same with or without them. | Directive | Set with | Sent in header | Effect on the server-side cache | |--------------------------|------------------------------------------|--------------------|----------------------------------------------------------------------------------------------------------------| -| `max-age` | `ttl=N` | :white_check_mark: | The stored response is served for `N` seconds without running the handler (`ttl=0` or unset: never served directly). | +| `max-age` | `ttl=N` | :white_check_mark: | The stored response is served for `N` seconds without running the handler (`ttl=0` or unset: nothing is stored). | | `no-cache` | `no_cache=True` | :white_check_mark: | The handler runs on every request; the response is still stored, and a matching `If-None-Match` gets a 304. | | `no-store` | `no_store=True` | :white_check_mark: | Nothing is read or stored, and no ETag is set. | | `private` | `private=True` | :white_check_mark: | The backend is bypassed; the handler runs on every request, and ETag revalidation still works. | @@ -80,7 +80,7 @@ When a cached entry is valid (within TTL): - **With `If-None-Match` header**: Returns HTTP 304 Not Modified if the ETag matches - **With `no-cache` directive**: Forces revalidation with fresh content before deciding on 304 - **With `private=True`**: Nothing is read from or written to the shared backend; the handler runs every time and only `If-None-Match` revalidation applies -- **Without `ttl`** (`ttl=None`): The cached body is never served directly; the handler runs on every request except one whose `If-None-Match` matches the stored ETag, which gets a 304 +- **Without `ttl`** (`ttl=None`): Nothing is read from or written to the backend, as with `private=True`. The handler runs on every request, and `If-None-Match` gets a 304 only when it matches the freshly rendered response, so an old ETag never gets a 304 once the content has changed - **With `ttl=0`**: Sends `max-age=0` and otherwise behaves like `ttl=None`. A negative `ttl` is rejected with `CacheXError` when the decorator is applied Only successful responses are stored. A response the handler *returns* with a diff --git a/fastapi_cachex/cache.py b/fastapi_cachex/cache.py index 66a5f49..c07f476 100644 --- a/fastapi_cachex/cache.py +++ b/fastapi_cachex/cache.py @@ -443,18 +443,19 @@ def cache( Args: ttl: How long, in seconds, a stored response may be served without running the handler. The same value is sent as ``max-age``. - ``ttl=0`` sends ``max-age=0`` and, like ``None``, keeps the entry - only for ETag revalidation: the body is never served from the - cache, but a matching ``If-None-Match`` still gets a 304. Negative - values are rejected. + Without a positive ``ttl`` (``None``, or ``0``, which sends + ``max-age=0``) nothing is read from or written to the backend: the + handler runs on every request, and ``If-None-Match`` gets a 304 + only when it matches the freshly rendered response. Negative values + are rejected. stale_ttl: Seconds sent with the directive chosen by ``stale``. It only shapes the ``Cache-Control`` header; the backend entry still expires after ``ttl``. Must be given together with ``stale``. stale: ``"revalidate"`` sends ``stale-while-revalidate=``, ``"error"`` sends ``stale-if-error=``. no_cache: Run the handler on every request and send ``no-cache``. The - response is still stored and ``If-None-Match`` still gets a 304 - when it matches the fresh ETag. The header then carries only + response is still stored when ``ttl`` is positive, and + ``If-None-Match`` still gets a 304 when it matches the fresh ETag. The header then carries only ``no-cache`` (plus ``must-revalidate`` when set); ``ttl``, ``stale``, ``public``/``private`` and ``immutable`` are left out. no_store: Run the handler, store nothing, and send ``no-store``. Takes @@ -569,10 +570,12 @@ def build_cache_control() -> str: # The header only depends on the decorator arguments, so build it once. cache_control = build_cache_control() builder = key_builder or default_key_builder - # `max-age=0` is a legal header, but backends disagree on what a zero - # TTL means, so such an entry is stored like `ttl=None`: kept only to - # answer ETag revalidation, never served directly. - store_ttl = ttl or None + # Without a positive ttl nothing may be served from storage, and a 304 + # answered from a stored ETag would be exactly that: it would keep + # confirming a copy that the handler no longer produces (#110). Such + # routes skip the backend like private ones. `ttl=0` is included, since + # `max-age=0` allows no reuse either. + bypass_backend = private or not ttl @wraps(func) async def wrapper(*args: Any, **kwargs: Any) -> Response: @@ -609,9 +612,10 @@ async def wrapper(*args: Any, **kwargs: Any) -> Response: # A private response belongs to exactly one user, so it must never # be read from or written to the shared backend — the default cache # key carries no identity, so a stored copy would be served to the - # next caller. ETag revalidation still works: it compares the - # client's validator against freshly rendered content. - if private: + # next caller. The same path serves routes without a positive ttl + # (see `bypass_backend`). ETag revalidation still works: it + # compares the client's validator against freshly rendered content. + if bypass_backend: response, _, etag = await _render(func, req, *args, **kwargs) if not _is_cacheable_status(response.status_code): return response @@ -619,10 +623,10 @@ async def wrapper(*args: Any, **kwargs: Any) -> Response: # StreamingResponse/FileResponse — cannot compute ETag return _with_cache_control(response, cache_control) if _etag_matches(client_etag, etag): - logger.debug("304 Not Modified (private); key=%s", cache_key) + logger.debug("304 Not Modified (uncached); key=%s", cache_key) return _not_modified(etag, cache_control, response.headers) response.headers["ETag"] = etag - logger.debug("Private response; bypassed shared cache") + logger.debug("Bypassed the backend; key=%s", cache_key) return _with_cache_control(response, cache_control) cached_data = await cache_backend.get(cache_key) @@ -670,7 +674,7 @@ async def wrapper(*args: Any, **kwargs: Any) -> Response: # If we don't have If-None-Match header, check if we have a valid cached copy # and can serve it directly (cache hit without ETag comparison) - if cached_data and not no_cache and store_ttl is not None: + if cached_data and not no_cache: logger.debug("Cache HIT (TTL valid); key=%s", cache_key) return Response( content=cached_data.content, @@ -719,7 +723,7 @@ async def wrapper(*args: Any, **kwargs: Any) -> Response: status_code=current_response.status_code, headers=_cacheable_headers(current_response), ), - ttl=store_ttl, + ttl=ttl, ) logger.debug("Updated cache entry; key=%s ttl=%s", cache_key, ttl) diff --git a/tests/test_cache.py b/tests/test_cache.py index 6bf5d18..707d05b 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -1,3 +1,4 @@ +import asyncio import threading from collections.abc import AsyncGenerator from functools import partial @@ -76,7 +77,7 @@ async def ttl_endpoint(): def test_no_cache_endpoint(): @app.get("/no-cache") - @cache(no_cache=True) + @cache(ttl=60, no_cache=True) async def no_cache_endpoint(): return Response( content=b'{"message": "This endpoint should not be cached"}', @@ -274,7 +275,7 @@ def test_handler_returning_a_coroutine_is_awaited( def test_no_cache_with_revalidate(): @app.get("/no-cache-revalidate") - @cache(no_cache=True, must_revalidate=True) + @cache(ttl=60, no_cache=True, must_revalidate=True) async def no_cache_revalidate_endpoint(): return Response( content=b'{"message": "This endpoint should not be cached but must revalidate"}', @@ -452,7 +453,7 @@ def test_no_cache_with_unchanged_data(): counter = 0 @app.get("/no-cache-unchanged") - @cache(no_cache=True) + @cache(ttl=60, no_cache=True) async def no_cache_unchanged_endpoint(): return {"message": "This endpoint uses no-cache", "counter": counter} @@ -481,7 +482,7 @@ def test_no_cache_with_changing_data(): counter = {"value": 0} @app.get("/no-cache-changing") - @cache(no_cache=True) + @cache(ttl=60, no_cache=True) async def no_cache_changing_endpoint(): counter["value"] += 1 return {"message": "This endpoint uses no-cache", "counter": counter["value"]} @@ -587,7 +588,7 @@ def test_streaming_response_with_no_cache_and_if_none_match(): stream_app2 = FastAPI() @stream_app2.get("/stream-nocache") - @cache(no_cache=True) + @cache(ttl=60, no_cache=True) async def streaming_nocache(): async def gen() -> AsyncGenerator[bytes, None]: yield b"data" @@ -639,43 +640,72 @@ async def no_store_no_cache_endpoint(): assert "no-cache" not in cc -def test_ttl_zero_sends_max_age_zero_and_only_revalidates(): - """ttl=0 is `max-age=0`: the body is never replayed, a matching ETag gets 304.""" - call_count = {"n": 0} - ttl0_app = FastAPI() - ttl0_backend = MemoryBackend() - BackendProxy.set(ttl0_backend) +@pytest.mark.parametrize(("ttl", "cache_control"), [(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). - @ttl0_app.get("/ttl-zero") - @cache(ttl=0) - async def ttl_zero_endpoint(): - call_count["n"] += 1 - return Response(content=b"hello", media_type="text/plain") + The handler runs on every request. A 304 is still sent, but only when the + client's validator matches the freshly rendered response: once the data + changes, the old ETag gets the new body instead of a 304. + """ + body = {"current": b"v1"} + calls: list[None] = [] + no_ttl_app = FastAPI() + backend = MemoryBackend() + BackendProxy.set(backend) + + @no_ttl_app.get("/no-ttl") + @cache(ttl=ttl) + async def no_ttl_endpoint(): + calls.append(None) + return Response(content=body["current"], media_type="text/plain") - ttl0_client = TestClient(ttl0_app) + no_ttl_client = TestClient(no_ttl_app) - r1 = ttl0_client.get("/ttl-zero") + r1 = no_ttl_client.get("/no-ttl") assert r1.status_code == 200 - assert r1.headers["Cache-Control"] == "max-age=0" - etag = r1.headers["ETag"] + assert r1.headers["Cache-Control"] == cache_control + old_etag = r1.headers["ETag"] + assert backend.cache == {} - # The entry is kept without an expiry, like ttl=None; the backend is never - # handed a zero TTL, which every backend used to read differently. - (item,) = ttl0_backend.cache.values() - assert item.expiry is None + # 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 len(calls) == 2 - # Without a validator the handler runs again: nothing is served directly. - r2 = ttl0_client.get("/ttl-zero") - assert r2.status_code == 200 - assert r2.content == b"hello" - assert call_count["n"] == 2 + # The data changes: the old validator must not get a 304 any more. + body["current"] = b"v2" + r3 = no_ttl_client.get("/no-ttl", headers={"If-None-Match": old_etag}) + assert r3.status_code == 200 + assert r3.content == b"v2" + assert r3.headers["ETag"] != old_etag + assert len(calls) == 3 + assert backend.cache == {} + backend.stop_cleanup() - # A matching validator is answered from the stored ETag. - r3 = ttl0_client.get("/ttl-zero", headers={"If-None-Match": etag}) - assert r3.status_code == 304 - assert r3.headers["Cache-Control"] == "max-age=0" - assert call_count["n"] == 2 - ttl0_backend.stop_cleanup() + +def test_without_a_ttl_an_entry_left_by_an_older_version_is_ignored(): + """Entries 0.3.7 stored without expiry are neither served nor refreshed (#110).""" + legacy_app = FastAPI() + backend = MemoryBackend() + BackendProxy.set(backend) + + @legacy_app.get("/legacy") + @cache() + async def legacy_endpoint(): + return Response(content=b"new", media_type="text/plain") + + legacy_client = TestClient(legacy_app) + key = "GET|||testserver|||/legacy|||" + stale = CacheEntry(fingerprint='W/"old"', content=b"old", media_type="text/plain") + asyncio.run(backend.set(key, stale)) + + r = legacy_client.get("/legacy", headers={"If-None-Match": 'W/"old"'}) + assert r.status_code == 200 + assert r.content == b"new" + assert backend.cache[key].value == stale + backend.stop_cleanup() def test_negative_ttl_is_rejected_at_decoration(): diff --git a/tests/test_cache_hit.py b/tests/test_cache_hit.py index d32e5df..4d01b65 100644 --- a/tests/test_cache_hit.py +++ b/tests/test_cache_hit.py @@ -143,7 +143,7 @@ def test_no_cache_still_returns_304_on_etag_match(): execution_count = {"value": 0} @app.get("/no-cache-with-etag") - @cache(no_cache=True) + @cache(ttl=60, no_cache=True) async def no_cache_endpoint(): execution_count["value"] += 1 return Response( diff --git a/tests/test_cache_revalidation.py b/tests/test_cache_revalidation.py index 11f1d99..02d16c6 100644 --- a/tests/test_cache_revalidation.py +++ b/tests/test_cache_revalidation.py @@ -117,7 +117,7 @@ def test_not_modified_from_a_fresh_render_repeats_the_headers(): app = FastAPI() @app.get("/fresh") - @cache(no_cache=True) + @cache(ttl=60, no_cache=True) async def fresh(): return Response( content="body", diff --git a/tests/test_cache_status_headers.py b/tests/test_cache_status_headers.py index adc7e90..4b14423 100644 --- a/tests/test_cache_status_headers.py +++ b/tests/test_cache_status_headers.py @@ -69,15 +69,16 @@ async def boom(): async def test_error_does_not_overwrite_a_good_cached_entry(): """A transient failure must not evict the last good response. - Uses ETag-only mode (no ``ttl``) so the handler runs on every request; with - a live TTL the cached copy would be served without calling it at all. + Uses ``no_cache`` so the handler runs on every request while the response + is still stored; otherwise the cached copy would be served without calling + it at all. """ app = FastAPI() client = TestClient(app) state = {"fail": False} @app.get("/flaky") - @cache() + @cache(ttl=60, no_cache=True) async def flaky(): if state["fail"]: return Response(content="down", status_code=503) @@ -169,7 +170,7 @@ def test_no_cache_with_if_none_match_serves_error_instead_of_304(): state = {"fail": False} @app.get("/maybe") - @cache(no_cache=True) + @cache(ttl=60, no_cache=True) async def maybe(): if state["fail"]: return Response(content="gone", status_code=410)