From 3261204451322e2393154fa89a0301ef3c08e669 Mon Sep 17 00:00:00 2001 From: Otto Fernandes Date: Sat, 27 Jul 2024 20:06:12 -0300 Subject: [PATCH 1/2] feat: ignore cache on cache fail When cache fails, exceptions were not captured, so the whole view call would fail. Since a cache hit attempt is an optional feature (it just saves performance), ignoring cache on cache errors is a more resilient approach. Closes #59 --- drf_kit/cache.py | 39 +++++++++++++++- .../tests/tests_views/tests_cached_views.py | 44 ++++++++++++++++++- test_app/views.py | 18 ++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/drf_kit/cache.py b/drf_kit/cache.py index 51ac477..41278d7 100644 --- a/drf_kit/cache.py +++ b/drf_kit/cache.py @@ -38,6 +38,25 @@ class BodyCacheKeyConstructor(CacheKeyConstructor): class CacheResponse(decorators.CacheResponse): + def __init__(self, timeout=None, key_func=None, cache=None, cache_errors=None, cache_error_func=None): + """ + :param cache_error_func: Function to be called when an error occurs while trying to get the cache. + parameters to this function include the exception that was raised, the key that was used to get the cache, + the request that was made and whether the error occurred when fetching the cache or when setting it. + + This function should look like this: + def cache_error_func(exception: Exception, key: str, request: Request, get_cache: bool): + pass + """ + super().__init__( + timeout, + key_func, + cache, + cache_errors, + ) + + self.cache_error_func = cache_error_func + def process_cache_response( self, view_instance, @@ -62,7 +81,7 @@ def process_cache_response( response_dict = None else: valid_cache_control = False - response_dict = self.cache.get(key) + response_dict = self._get_cached_result(key, request) if not response_dict: response = view_method(view_instance, request, *args, **kwargs) @@ -81,7 +100,7 @@ def process_cache_response( response.headers.copy(), ) - self.cache.set(key, response_dict, self.timeout) + self._set_cached_result(key, request, response_dict) cache_hit = False else: try: @@ -100,5 +119,21 @@ def process_cache_response( return response + def _get_cached_result(self, key, request) -> dict | None: + try: + return self.cache.get(key) + except Exception as exc: + if self.cache_error_func: + self.cache_error_func(exception=exc, key=key, request=request, get_cache=True) + + return None + + def _set_cached_result(self, key, request, response_dict): + try: + self.cache.set(key, response_dict, self.timeout) + except Exception as exc: + if self.cache_error_func: + self.cache_error_func(exception=exc, key=key, request=request, get_cache=False) + cache_response = CacheResponse diff --git a/test_app/tests/tests_views/tests_cached_views.py b/test_app/tests/tests_views/tests_cached_views.py index 4e6e543..a6e60fe 100644 --- a/test_app/tests/tests_views/tests_cached_views.py +++ b/test_app/tests/tests_views/tests_cached_views.py @@ -1,7 +1,8 @@ -from unittest.mock import patch +from unittest.mock import ANY, patch from django.core.cache import cache from rest_framework import status +from rest_framework.response import Response from drf_kit.cache import CacheResponse from drf_kit.tests import BaseApiTest @@ -105,3 +106,44 @@ def test_cache_control_invalid_directive(self): self.assertEqual(status.HTTP_200_OK, response_json_miss.status_code) self.assertEqual("HIT", response_json_miss["X-Cache"]) self.assertEqual(None, response_json_miss.get("Cache-Control")) + + def test_cache_skip_on_cache_get_and_call_view_action(self): + url = self.url + + with ( + patch("django.core.cache.backends.locmem.LocMemCache.get", side_effect=Exception), + patch("drf_kit.views.viewsets.MultiSerializerMixin.list", return_value=Response()) as list_method, + ): + self.client.get(url) + + list_method.assert_called_once() + + def test_cache_skip_on_cache_set(self): + url = self.url + + with ( + patch("django.core.cache.backends.locmem.LocMemCache.set", side_effect=Exception), + ): + self.client.get(url) # No exception should be raised from this call + + def test_cache_custom_handler_on_cache_get_error(self): + url = f"{self.url}/patronus" + + with ( + patch("django.core.cache.backends.locmem.LocMemCache.get", side_effect=Exception), + patch("test_app.views.handle_cache_error_helper") as error_handler, + ): + self.client.get(url) + + error_handler.assert_called_once_with(ANY, ANY, ANY, True) + + def test_cache_custom_handler_on_cache_set_error(self): + url = f"{self.url}/patronus" + + with ( + patch("django.core.cache.backends.locmem.LocMemCache.set", side_effect=Exception), + patch("test_app.views.handle_cache_error_helper") as error_handler, + ): + self.client.get(url) + + error_handler.assert_called_once_with(ANY, ANY, ANY, False) diff --git a/test_app/views.py b/test_app/views.py index 13f9429..b51fb15 100644 --- a/test_app/views.py +++ b/test_app/views.py @@ -1,7 +1,9 @@ from django.db.models import Count from rest_framework import status +from rest_framework.decorators import action from rest_framework.response import Response +from drf_kit.cache import cache_response from drf_kit.views import ( BulkMixin, ModelViewSet, @@ -31,12 +33,28 @@ def add_stats_to_queryset(self, queryset): ) +def handle_cache_error_helper(exception, key, request, get_cache): + pass + + +# TODO: make tests mock this function instead of the helper. +# This function calls a helper so the test can mock it. For some reason, +# trying to mock `handle_cache_error` directly does not work. +def handle_cache_error(exception, key, request, get_cache): + handle_cache_error_helper(exception, key, request, get_cache) + + class TeacherViewSet(CachedSearchableModelViewSet): queryset = models.Teacher.objects.all() serializer_class = serializers.TeacherSerializer filterset_class = filters.TeacherFilterSet ordering_fields = ("name", "id") + @action(detail=False, methods=["get"]) + @cache_response(cache_error_func=handle_cache_error) + def patronus(self, request, *args, **kwargs): + return Response(status=status.HTTP_200_OK) + class WizardViewSet(ModelViewSet): queryset = models.Wizard.objects.all() From 6de9312beb9eba23ba90e091a5ff943923b07bc7 Mon Sep 17 00:00:00 2001 From: Otto Fernandes Date: Sat, 27 Jul 2024 20:07:27 -0300 Subject: [PATCH 2/2] build: bump version --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 1832e14..47da7b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "drf-kit" -version = "1.42.4" +version = "1.43.0" description = "DRF Toolkit" authors = ["Nilo Saude "] packages = [