Skip to content

fix(cache): send no empty Cache-Control, and warn when no_store overrides other arguments - #431

Merged
allen0099 merged 2 commits into
masterfrom
fix/363-328-cache-arguments
Oct 2, 2026
Merged

allen0099 merged 2 commits into
masterfrom
fix/363-328-cache-arguments

Conversation

@allen0099

Copy link
Copy Markdown
Owner

Fixes #363. Closes #328.

#363: bare @cache() sent an empty Cache-Control

With neither ttl nor a directive, _build_cache_control returns "" and the decorator set it anyway, replacing the handler's own header. Now:

  • _cache_control_for falls back to the handler's own Cache-Control when the decorator has none, and _with_cache_control / _not_modified skip an empty value, on both the 200 and the 304.
  • A response that sets a cookie, or answers a request with Authorization or a session, still gets private, as on every route (unchanged from before; now documented).

A bare @cache() is therefore an ETag-only mode: ETag, 304 on a matching If-None-Match, the handler's own Cache-Control. It does not warn: it now does something useful, a new warning would fail downstream test suites that run with warnings as errors in a patch release, and the only silencer would be a filter.

#328: arguments that contradict each other

no_store=True combined with ttl, stale/stale_ttl, no_cache, public, private, immutable or must_revalidate emits a UserWarning at decoration time naming the overridden arguments (stacklevel points at the user's @cache(...) line). Warning rather than raising keeps this non-breaking for 0.4.x. Behaviour is unchanged: no-store only.

Out of scope (could be follow-ups): no_cache=True silently dropping public/ttl/stale/immutable from the header, and no_store with key_builder/cache_authorized/sort_query.

Tests

  • tests/test_cache_arguments.py: bare @cache() with no handler header, with the handler's header (200 + 304), with vary, with a streaming response, with a cookie and with Authorization; every no_store combination; combinations that must not warn.
  • Existing tests that deliberately combine no_store with other arguments now assert the warning with pytest.warns, or ignore it per test where a shared parametrized helper builds the combination.
  • Two assertions that encoded the empty header now assert its absence.
  • Each new guard was reverted in turn and a test failed each time.

Docs (EN + zh-TW): HTTP_CACHING.md, CACHE_FLOW.md, and the cache() docstring.

An independent review agent read the change before this PR. It led to dropping the warning on a bare @cache(), documenting the cookie/credential private exception, and adding the missing test cases.

@allen0099 allen0099 added bug Something isn't working http-cache The @cache decorator, cache keys and Cache-Control handling labels Oct 2, 2026
@allen0099
allen0099 merged commit facc8e8 into master Oct 2, 2026
13 of 15 checks passed
@allen0099
allen0099 deleted the fix/363-328-cache-arguments branch October 2, 2026 15:16
@allen0099 allen0099 added this to the 0.4.1 milestone Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working http-cache The @cache decorator, cache keys and Cache-Control handling

Projects

None yet

1 participant