Skip to content

Commit

Permalink
fix: Make Tiered cache compatible with both cache backends (python-me…
Browse files Browse the repository at this point in the history
…mcache and pymemcache)
  • Loading branch information
mumarkhan999 committed Aug 2, 2023
1 parent f9836a1 commit b82cc25
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ Unreleased
----------

Added

~~~~~
* Support added for Django 4.2
* A new django setting ``ENABLE_FORCE_CACHE_MISS_FOR_NONE`` has been added to make ``TieredCache`` backward compatible

[5.6.0] - 2023-07-24
--------------------
Expand Down
29 changes: 29 additions & 0 deletions edx_django_utils/cache/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,35 @@ def test_get_cached_response_django_cache_hit(self, mock_cache_get):
cached_response = self.request_cache.get_cached_response(TEST_KEY)
self.assertTrue(cached_response.is_found, 'Django cache hit should cache value in request cache.')

@mock.patch('edx_django_utils.cache.utils.waffle.switch_is_active')
def test_get_cached_response_hit_with_cached_none(self, mock_switch_is_active):
mock_switch_is_active.return_value = True
TieredCache.set_all_tiers(TEST_KEY, None)
# Test retrieval from tier 1: RequestCache
cached_response = TieredCache.get_cached_response(TEST_KEY)
self.assertTrue(cached_response.is_found)
self.assertEqual(cached_response.value, None)

self.request_cache.clear()
# Test retrieval from tier 2: Django Cache
cached_response = TieredCache.get_cached_response(TEST_KEY)
self.assertTrue(cached_response.is_found)
self.assertEqual(cached_response.value, None)

@mock.patch('edx_django_utils.cache.utils.waffle.switch_is_active')
def test_get_cached_response_miss_with_cached_none(self, mock_switch_is_active):
mock_switch_is_active.return_value = False
TieredCache.set_all_tiers(TEST_KEY, None)
# Test retrieval from tier 1: RequestCache
cached_response = TieredCache.get_cached_response(TEST_KEY)
self.assertTrue(cached_response.is_found)
self.assertEqual(cached_response.value, None)

self.request_cache.clear()
# Test retrieval from tier 2: Django Cache
cached_response = TieredCache.get_cached_response(TEST_KEY)
self.assertFalse(cached_response.is_found)

@mock.patch('django.core.cache.cache.get')
def test_get_cached_response_force_cache_miss(self, mock_cache_get):
self.request_cache.set(SHOULD_FORCE_CACHE_MISS_KEY, True)
Expand Down
11 changes: 11 additions & 0 deletions edx_django_utils/cache/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import hashlib
import threading

import waffle # pylint: disable=invalid-django-waffle-import
from django.core.cache import cache as django_cache
from django.core.cache.backends.base import DEFAULT_TIMEOUT
from django.utils.encoding import force_str
Expand Down Expand Up @@ -259,6 +260,10 @@ def _get_cached_response_from_django_cache(key):
return CachedResponse(is_found=False, key=key, value=None)

cached_value = django_cache.get(key, _CACHE_MISS)

if cached_value is None and not TieredCache._is_forced_cache_miss_for_none_disabled():
cached_value = _CACHE_MISS

is_found = cached_value is not _CACHE_MISS
return CachedResponse(is_found, key, cached_value)

Expand Down Expand Up @@ -303,6 +308,12 @@ def _should_force_django_cache_miss(cls):
cached_response = DEFAULT_REQUEST_CACHE.get_cached_response(SHOULD_FORCE_CACHE_MISS_KEY)
return False if not cached_response.is_found else cached_response.value

@staticmethod
def _is_forced_cache_miss_for_none_disabled():
# This switch is to provide backward compatibility.
# As we have been treating None as CACHE_MISS.
return waffle.switch_is_active('edx_django_utils.cache.utils.disable_forced_cache_miss_for_none')


class CachedResponseError(Exception):
"""
Expand Down

0 comments on commit b82cc25

Please sign in to comment.