Skip to content

Add no-store, no-cache in Cache-Control headers #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ When using the Redis backend, please make sure you pass in a redis client that d

[redis-decode]: https://redis-py.readthedocs.io/en/latest/examples/connection_examples.html#by-default-Redis-return-binary-responses,-to-decode-them-use-decode_responses=True

## Notes on `Cache-Control` header
The cache behavior can be controlled by the client by passing the `Cache-Control` request header. The behavior is described below:
- `no-cache`: doesn't use cache even if the value is present but stores the response in the cache.
- `no-store`: can use cache if present but will not add/update to cache.
- `no-cache,no-store`: i.e. both are passed, it will neither store nor use the cache. Will remove the `max-age` and `ETag` as well from the response header.

## Tests and coverage

```shell
Expand Down
45 changes: 35 additions & 10 deletions fastapi_cache/decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
TypeVar,
Union,
cast,
Set,
)

if sys.version_info >= (3, 10):
Expand Down Expand Up @@ -81,7 +82,23 @@ def _uncacheable(request: Optional[Request]) -> bool:
return False
if request.method != "GET":
return True
return request.headers.get("Cache-Control") == "no-store"
return False

def _extract_cache_control_headers(request: Optional[Request]) -> Set[str]:
"""Extracts Cache-Control header
1. Convert all header to lowercase to make it case insensitive
2. Split on comma (,)
3. Strip whitespaces
4. convert to all lower case

returns an empty set if header not set
"""
if request is not None:
headers = {header_key.lower(): header_val for header_key, header_val in request.headers.items()}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can omit lowecasing since FastAPI uses starlette where headers are case-insensitive https://fastapi.tiangolo.com/tutorial/header-params/?h=insensitive#automatic-conversion

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it looks redundant if Fastapi takes care of it. I pushed the fix.

cache_control_header = headers.get("cache-control", None)
if cache_control_header:
return set([cache_control_val.strip().lower() for cache_control_val in cache_control_header.split(",")])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be set comprehension imo

{cache_control_val.strip().lower() for cache_control_val in cache_control_header.split(",")}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sultaniman thanks for pointing it out. Pushed the suggested change.

return set()


def cache(
Expand Down Expand Up @@ -161,6 +178,8 @@ async def ensure_async_func(*args: P.args, **kwargs: P.kwargs) -> R:
key_builder = key_builder or FastAPICache.get_key_builder()
backend = FastAPICache.get_backend()
cache_status_header = FastAPICache.get_cache_status_header()
cache_control_headers = _extract_cache_control_headers(request=request)
response_headers = {"Cache-Control": cache_control_headers.copy()}

cache_key = key_builder(
func,
Expand All @@ -174,21 +193,30 @@ async def ensure_async_func(*args: P.args, **kwargs: P.kwargs) -> R:
cache_key = await cache_key
assert isinstance(cache_key, str) # noqa: S101 # assertion is a type guard

ttl, cached = 0, None
try:
ttl, cached = await backend.get_with_ttl(cache_key)
# no-cache: Assume cache is not present. i.e. treat it as a miss
if "no-cache" not in cache_control_headers:
ttl, cached = await backend.get_with_ttl(cache_key)
etag = f"W/{hash(cached)}"
response_headers["Cache-Control"].add(f"max-age={ttl}")
response_headers["Etag"] = {f"ETag={etag}"}
except Exception:
logger.warning(
f"Error retrieving cache key '{cache_key}' from backend:",
exc_info=True,
)
ttl, cached = 0, None

if cached is None or (request is not None and request.headers.get("Cache-Control") == "no-cache") : # cache miss
result = await ensure_async_func(*args, **kwargs)
to_cache = coder.encode(result)

try:
await backend.set(cache_key, to_cache, expire)
# no-store: do not store the value in cache
if "no-store" not in cache_control_headers:
await backend.set(cache_key, to_cache, expire)
response_headers["Cache-Control"].add(f"max-age={expire}")
response_headers["Etag"] = {f"W/{hash(to_cache)}"}
except Exception:
logger.warning(
f"Error setting cache key '{cache_key}' in backend:",
Expand All @@ -198,25 +226,22 @@ async def ensure_async_func(*args: P.args, **kwargs: P.kwargs) -> R:
if response:
response.headers.update(
{
"Cache-Control": f"max-age={expire}",
"ETag": f"W/{hash(to_cache)}",
**{header_key: ",".join(sorted(header_val)) for header_key, header_val in response_headers.items()},
cache_status_header: "MISS",
}
)

else: # cache hit
if response:
etag = f"W/{hash(cached)}"
response.headers.update(
{
"Cache-Control": f"max-age={ttl}",
"ETag": etag,
**{header_key: ",".join(sorted(header_val)) for header_key, header_val in response_headers.items()},
cache_status_header: "HIT",
}
)

if_none_match = request and request.headers.get("if-none-match")
if if_none_match == etag:
if "Etag" in response_headers and if_none_match == response_headers["Etag"]:
response.status_code = HTTP_304_NOT_MODIFIED
return response

Expand Down
38 changes: 38 additions & 0 deletions tests/test_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,41 @@ def test_cache_control() -> None:

response = client.get("/cached_put")
assert response.json() == {"value": 2}

def test_cache_control_header() -> None:
"""Test no-cache, no-store cache control header"""
with TestClient(app) as client:
# forcing clear to start a clean cache
client.get("/clear")

# no-store, no-cache will always no use or store cache
response = client.get("/date", headers={"Cache-Control": "no-store,no-cache"})
assert response.headers.get("X-FastAPI-Cache") == "MISS"
assert response.headers.get("Cache-Control") == "no-cache,no-store"
assert response.headers.get("ETag") is None
assert pendulum.parse(response.json()) == pendulum.today() # type: ignore[attr-defined]

# do it again to test cache without header
response = client.get("/date")
assert response.headers.get("X-FastAPI-Cache") == "MISS"
assert pendulum.parse(response.json()) == pendulum.today() # type: ignore[attr-defined]

# do it again to test cache with no-store. Will not store this response but use the cache
response = client.get("/date", headers={"Cache-Control": "no-store"})
assert response.headers.get("X-FastAPI-Cache") == "HIT"
assert response.headers.get("Cache-Control") == "max-age=10,no-store"
assert pendulum.parse(response.json()) == pendulum.today() # type: ignore[attr-defined]

# do it again to test cache with no-cache. Will not store use cache but store it
response = client.get("/date", headers={"Cache-Control": "no-cache"})
assert response.headers.get("X-FastAPI-Cache") == "MISS"
assert response.headers.get("Cache-Control") == "max-age=10,no-cache"
assert pendulum.parse(response.json()) == pendulum.today() # type: ignore[attr-defined]

time.sleep(3)

# call with no headers now to use the value store in previous step
response = client.get("/date")
assert response.headers.get("X-FastAPI-Cache") == "HIT"
assert response.headers.get("Cache-Control") == "max-age=7"
assert pendulum.parse(response.json()) == pendulum.today() # type: ignore[attr-defined]