From b6ff77f660f5b7ddb65c367de944d525000ad2b8 Mon Sep 17 00:00:00 2001 From: Aaron Luna Date: Tue, 29 Jun 2021 04:21:55 -0700 Subject: [PATCH 1/6] bug: 304 response to request with if-none-metch header field must contain cache-control, expires, etag header fields (#44) Fixes #39 --- src/fastapi_redis_cache/cache.py | 22 ++++++++++++++++------ tests/test_cache.py | 10 ++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/fastapi_redis_cache/cache.py b/src/fastapi_redis_cache/cache.py index 7573739..6d4c360 100644 --- a/src/fastapi_redis_cache/cache.py +++ b/src/fastapi_redis_cache/cache.py @@ -46,14 +46,24 @@ async def inner_wrapper(*args, **kwargs): key = redis_cache.get_cache_key(func, *args, **kwargs) ttl, in_cache = redis_cache.check_cache(key) if in_cache: + redis_cache.set_response_headers(response, True, deserialize_json(in_cache), ttl) if redis_cache.requested_resource_not_modified(request, in_cache): response.status_code = int(HTTPStatus.NOT_MODIFIED) - return response - cached_data = deserialize_json(in_cache) - redis_cache.set_response_headers(response, cache_hit=True, response_data=cached_data, ttl=ttl) - if create_response_directly: - return Response(content=in_cache, media_type="application/json", headers=response.headers) - return cached_data + return ( + Response( + content=None, + status_code=response.status_code, + media_type="application/json", + headers=response.headers, + ) + if create_response_directly + else response + ) + return ( + Response(content=in_cache, media_type="application/json", headers=response.headers) + if create_response_directly + else deserialize_json(in_cache) + ) response_data = await get_api_response_async(func, *args, **kwargs) ttl = calculate_ttl(expire) cached = redis_cache.add_to_cache(key, response_data, ttl) diff --git a/tests/test_cache.py b/tests/test_cache.py index c668ec1..4e261fa 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -113,8 +113,18 @@ def test_if_none_match(): invalid_etag = "W/-5480454928453453778" response = client.get("/cache_never_expire", headers={"if-none-match": f"{etag}, {invalid_etag}"}) assert response.status_code == 304 + assert not response.content + assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Hit" + assert "cache-control" in response.headers + assert "expires" in response.headers + assert "etag" in response.headers response = client.get("/cache_never_expire", headers={"if-none-match": "*"}) assert response.status_code == 304 + assert not response.content + assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Hit" + assert "cache-control" in response.headers + assert "expires" in response.headers + assert "etag" in response.headers response = client.get("/cache_never_expire", headers={"if-none-match": invalid_etag}) assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data can be cached indefinitely"} From d0ffba80954374e623bf5b6e2b34d7d13ac1b1fb Mon Sep 17 00:00:00 2001 From: Aaron Luna Date: Tue, 29 Jun 2021 04:29:45 -0700 Subject: [PATCH 2/6] missing test coverage: response data is not json-serializable (#45) Fixes #40 --- tests/main.py | 12 +++++++++++- tests/test_cache.py | 12 ++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/main.py b/tests/main.py index e430181..ca78ba2 100644 --- a/tests/main.py +++ b/tests/main.py @@ -1,9 +1,10 @@ +import logging from datetime import date, datetime, timedelta from decimal import Decimal from fastapi import FastAPI, Request, Response -from fastapi_redis_cache import cache, cache_one_hour +from fastapi_redis_cache import cache, cache_one_hour, cache_one_minute app = FastAPI(title="FastAPI Redis Cache Test App") @@ -35,3 +36,12 @@ def cache_json_encoder(): @cache_one_hour() def partial_cache_one_hour(response: Response): return {"success": True, "message": "this data should be cached for one hour"} + + +@app.get("/cache_invalid_type") +@cache_one_minute() +def cache_invalid_type(request: Request, response: Response): + logging.basicConfig() + logger = logging.getLogger(__name__) + logger.setLevel(logging.INFO) + return logger diff --git a/tests/test_cache.py b/tests/test_cache.py index 4e261fa..d1a6a55 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -144,3 +144,15 @@ def test_partial_cache_one_hour(): assert match and int(match.groupdict()["ttl"]) == 3600 assert "expires" in response.headers assert "etag" in response.headers + + +def test_cache_invalid_type(): + # Simple test that verifies the correct behavior when a value that is not JSON-serializable is returned + # as response data + with pytest.raises(ValueError): + response = client.get("/cache_invalid_type") + assert response.status_code == 200 + assert "x-fastapi-cache" not in response.headers + assert "cache-control" not in response.headers + assert "expires" not in response.headers + assert "etag" not in response.headers From e42e4e96ebb5a40b32b37aa82bc7fde29a3200ca Mon Sep 17 00:00:00 2001 From: Aaron Luna Date: Tue, 29 Jun 2021 04:39:20 -0700 Subject: [PATCH 3/6] bug: serialize_json should not be called on data if attempt to add data to cache fails (#47) Fixes #43 --- src/fastapi_redis_cache/cache.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/fastapi_redis_cache/cache.py b/src/fastapi_redis_cache/cache.py index 6d4c360..0df8dd3 100644 --- a/src/fastapi_redis_cache/cache.py +++ b/src/fastapi_redis_cache/cache.py @@ -69,11 +69,12 @@ async def inner_wrapper(*args, **kwargs): cached = redis_cache.add_to_cache(key, response_data, ttl) if cached: redis_cache.set_response_headers(response, cache_hit=False, response_data=response_data, ttl=ttl) - if create_response_directly: - return Response( - content=serialize_json(response_data), - media_type="application/json", - headers=response.headers, + return ( + Response( + content=serialize_json(response_data), media_type="application/json", headers=response.headers + ) + if create_response_directly + else response_data ) return response_data From 56aee43ea81d97b4081622de4f70899bfdd7a2d1 Mon Sep 17 00:00:00 2001 From: Aaron Luna Date: Tue, 29 Jun 2021 04:43:50 -0700 Subject: [PATCH 4/6] add comments to test functions in order to better document the intended design (#48) Fixes #41 --- src/fastapi_redis_cache/cache.py | 1 + src/fastapi_redis_cache/client.py | 6 --- tests/main.py | 4 +- tests/test_cache.py | 78 +++++++++++++++++++++++++++---- 4 files changed, 71 insertions(+), 18 deletions(-) diff --git a/src/fastapi_redis_cache/cache.py b/src/fastapi_redis_cache/cache.py index 0df8dd3..5257a14 100644 --- a/src/fastapi_redis_cache/cache.py +++ b/src/fastapi_redis_cache/cache.py @@ -89,6 +89,7 @@ async def get_api_response_async(func, *args, **kwargs): def calculate_ttl(expire: Union[int, timedelta]) -> int: + """"Converts expire time to total seconds and ensures that ttl is capped at one year.""" if isinstance(expire, timedelta): expire = int(expire.total_seconds()) return min(expire, ONE_YEAR_IN_SECONDS) diff --git a/src/fastapi_redis_cache/client.py b/src/fastapi_redis_cache/client.py index fa0f3d4..d763488 100644 --- a/src/fastapi_redis_cache/client.py +++ b/src/fastapi_redis_cache/client.py @@ -158,9 +158,3 @@ def get_etag(cached_data: Union[str, bytes, Dict]) -> str: def get_log_time(): """Get a timestamp to include with a log message.""" return datetime.now().strftime(LOG_TIMESTAMP) - - @staticmethod - def hasmethod(obj, method_name): - """Return True if obj.method_name exists and is callable. Otherwise, return False.""" - obj_method = getattr(obj, method_name, None) - return callable(obj_method) if obj_method else False diff --git a/tests/main.py b/tests/main.py index ca78ba2..be2b693 100644 --- a/tests/main.py +++ b/tests/main.py @@ -16,9 +16,9 @@ def cache_never_expire(request: Request, response: Response): @app.get("/cache_expires") -@cache(expire=timedelta(seconds=8)) +@cache(expire=timedelta(seconds=5)) async def cache_expires(): - return {"success": True, "message": "this data should be cached for eight seconds"} + return {"success": True, "message": "this data should be cached for five seconds"} @app.get("/cache_json_encoder") diff --git a/tests/test_cache.py b/tests/test_cache.py index d1a6a55..bdc0ccb 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -4,7 +4,9 @@ from datetime import datetime from decimal import Decimal +import pytest from fastapi.testclient import TestClient +from fastapi_redis_cache.client import HTTP_TIME from fastapi_redis_cache.util import deserialize_json from tests.main import app @@ -14,6 +16,7 @@ def test_cache_never_expire(): + # Initial request, X-FastAPI-Cache header field should equal "Miss" response = client.get("/cache_never_expire") assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data can be cached indefinitely"} @@ -21,6 +24,8 @@ def test_cache_never_expire(): assert "cache-control" in response.headers assert "expires" in response.headers assert "etag" in response.headers + + # Send request to same endpoint, X-FastAPI-Cache header field should now equal "Hit" response = client.get("/cache_never_expire") assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data can be cached indefinitely"} @@ -31,38 +36,72 @@ def test_cache_never_expire(): def test_cache_expires(): - start = datetime.now() + # Store time when response data was added to cache + added_at_utc = datetime.utcnow() + + # Initial request, X-FastAPI-Cache header field should equal "Miss" response = client.get("/cache_expires") assert response.status_code == 200 - assert response.json() == {"success": True, "message": "this data should be cached for eight seconds"} + assert response.json() == {"success": True, "message": "this data should be cached for five seconds"} assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Miss" assert "cache-control" in response.headers assert "expires" in response.headers assert "etag" in response.headers + + # Store eTag value from response header check_etag = response.headers["etag"] + + # Send request, X-FastAPI-Cache header field should now equal "Hit" response = client.get("/cache_expires") assert response.status_code == 200 - assert response.json() == {"success": True, "message": "this data should be cached for eight seconds"} + assert response.json() == {"success": True, "message": "this data should be cached for five seconds"} assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Hit" - assert "cache-control" in response.headers - assert "expires" in response.headers + + # Verify eTag value matches the value stored from the initial response assert "etag" in response.headers assert response.headers["etag"] == check_etag - elapsed = (datetime.now() - start).total_seconds() - remaining = 8 - elapsed - if remaining > 0: - time.sleep(remaining) + + # Store 'max-age' value of 'cache-control' header field + assert "cache-control" in response.headers + match = MAX_AGE_REGEX.search(response.headers.get("cache-control")) + assert match + ttl = int(match.groupdict()["ttl"]) + assert ttl <= 5 + + # Store value of 'expires' header field + assert "expires" in response.headers + expire_at_utc = datetime.strptime(response.headers["expires"], HTTP_TIME) + + # Wait until expire time has passed + now = datetime.utcnow() + while expire_at_utc > now: + time.sleep(1) + now = datetime.utcnow() + + # Wait one additional second to ensure redis has deleted the expired response data + time.sleep(1) + second_request_utc = datetime.utcnow() + + # Verify that the time elapsed since the data was added to the cache is greater than the ttl value + elapsed = (second_request_utc - added_at_utc).total_seconds() + assert elapsed > ttl + + # Send request, X-FastAPI-Cache header field should equal "Miss" since the cached value has been evicted response = client.get("/cache_expires") assert response.status_code == 200 - assert response.json() == {"success": True, "message": "this data should be cached for eight seconds"} + assert response.json() == {"success": True, "message": "this data should be cached for five seconds"} assert "x-fastapi-cache" in response.headers and response.headers["x-fastapi-cache"] == "Miss" assert "cache-control" in response.headers assert "expires" in response.headers assert "etag" in response.headers + + # Check eTag value again. Since data is the same, the value should still match assert response.headers["etag"] == check_etag def test_cache_json_encoder(): + # In order to verify that our custom BetterJsonEncoder is working correctly, the /cache_json_encoder + # endpoint returns a dict containing datetime.datetime, datetime.date and decimal.Decimal objects. response = client.get("/cache_json_encoder") assert response.status_code == 200 response_json = response.json() @@ -75,6 +114,9 @@ def test_cache_json_encoder(): "val": "3.140000000000000124344978758017532527446746826171875", }, } + + # To verify that our custom object_hook function which deserializes types that are not typically + # JSON-serializable is working correctly, we test it with the serialized values sent in the response. json_dict = deserialize_json(json.dumps(response_json)) assert json_dict["start_time"] == datetime(2021, 4, 20, 7, 17, 17) assert json_dict["finish_by"] == datetime(2021, 4, 21) @@ -82,6 +124,8 @@ def test_cache_json_encoder(): def test_cache_control_no_cache(): + # Simple test that verifies if a request is recieved with the cache-control header field containing "no-cache", + # no caching behavior is performed response = client.get("/cache_never_expire", headers={"cache-control": "no-cache"}) assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data can be cached indefinitely"} @@ -92,6 +136,8 @@ def test_cache_control_no_cache(): def test_cache_control_no_store(): + # Simple test that verifies if a request is recieved with the cache-control header field containing "no-store", + # no caching behavior is performed response = client.get("/cache_never_expire", headers={"cache-control": "no-store"}) assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data can be cached indefinitely"} @@ -102,6 +148,7 @@ def test_cache_control_no_store(): def test_if_none_match(): + # Initial request, response data is added to cache response = client.get("/cache_never_expire") assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data can be cached indefinitely"} @@ -109,8 +156,13 @@ def test_if_none_match(): assert "cache-control" in response.headers assert "expires" in response.headers assert "etag" in response.headers + + # Store correct eTag value from response header etag = response.headers["etag"] + # Create another eTag value that is different from the correct value invalid_etag = "W/-5480454928453453778" + + # Send request to same endpoint where If-None-Match header contains both valid and invalid eTag values response = client.get("/cache_never_expire", headers={"if-none-match": f"{etag}, {invalid_etag}"}) assert response.status_code == 304 assert not response.content @@ -118,6 +170,8 @@ def test_if_none_match(): assert "cache-control" in response.headers assert "expires" in response.headers assert "etag" in response.headers + + # Send request to same endpoint where If-None-Match header contains just the wildcard (*) character response = client.get("/cache_never_expire", headers={"if-none-match": "*"}) assert response.status_code == 304 assert not response.content @@ -125,6 +179,8 @@ def test_if_none_match(): assert "cache-control" in response.headers assert "expires" in response.headers assert "etag" in response.headers + + # Send request to same endpoint where If-None-Match header contains only the invalid eTag value response = client.get("/cache_never_expire", headers={"if-none-match": invalid_etag}) assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data can be cached indefinitely"} @@ -135,6 +191,8 @@ def test_if_none_match(): def test_partial_cache_one_hour(): + # Simple test that verifies that the @cache_for_one_hour partial function version of the @cache decorator + # is working correctly. response = client.get("/cache_one_hour") assert response.status_code == 200 assert response.json() == {"success": True, "message": "this data should be cached for one hour"} From 2ef5d9a477c5bf4ee688c1b1a21fe8f726793902 Mon Sep 17 00:00:00 2001 From: Aaron Luna Date: Tue, 29 Jun 2021 04:46:37 -0700 Subject: [PATCH 5/6] add toc to readme (#49) Fixes #42 --- README.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 967cbd4..6378ccd 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -## fastapi-redis-cache +## fastapi-redis-cache [![PyPI version](https://badge.fury.io/py/fastapi-redis-cache.svg)](https://badge.fury.io/py/fastapi-redis-cache) ![PyPI - Downloads](https://img.shields.io/pypi/dm/fastapi-redis-cache?color=%234DC71F) @@ -7,6 +7,17 @@ [![Maintainability](https://api.codeclimate.com/v1/badges/ec0b1d7afb21bd8c23dc/maintainability)](https://codeclimate.com/github/a-luna/fastapi-redis-cache/maintainability) [![codecov](https://codecov.io/gh/a-luna/fastapi-redis-cache/branch/main/graph/badge.svg?token=dUaILJcgWY)](https://codecov.io/gh/a-luna/fastapi-redis-cache) +- [Features](#features) +- [Installation](#installation) +- [Usage](#usage) + - [Initialize Redis](#initialize-redis) + - [`@cache` Decorator](#cache-decorator) + - [Response Headers](#response-headers) + - [Pre-defined Lifetimes](#pre-defined-lifetimes) + - [Cache Keys](#cache-keys) + - [Cache Keys Pt 2.](#cache-keys-pt-2) +- [Questions/Contributions](#questionscontributions) + ## Features - Cache response data for async and non-async path operation functions. From 946f8ae4cca59f395703370dcf56ac3ab32b01dd Mon Sep 17 00:00:00 2001 From: Aaron Luna Date: Tue, 29 Jun 2021 04:48:48 -0700 Subject: [PATCH 6/6] update version number and requirements --- requirements-dev.txt | 2 +- src/fastapi_redis_cache/version.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 4299b22..ee12438 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,7 +2,7 @@ black==20.8b1 coverage==5.5 fakeredis==1.5.2 flake8==3.9.2 -isort==5.8.0 +isort==5.9.1 pytest==6.2.4 pytest-cov==2.12.1 pytest-flake8==1.0.7 diff --git a/src/fastapi_redis_cache/version.py b/src/fastapi_redis_cache/version.py index 7da5656..4bf3b7b 100644 --- a/src/fastapi_redis_cache/version.py +++ b/src/fastapi_redis_cache/version.py @@ -1,3 +1,3 @@ # flake8: noqa -__version_info__ = ("0", "2", "4") # pragma: no cover +__version_info__ = ("0", "2", "5") # pragma: no cover __version__ = ".".join(__version_info__) # pragma: no cover