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 6cac54f
Show file tree
Hide file tree
Showing 3 changed files with 44 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
27 changes: 27 additions & 0 deletions edx_django_utils/cache/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from unittest import TestCase, mock

import ddt
from django.test.utils import override_settings

from edx_django_utils.cache.utils import (
DEFAULT_REQUEST_CACHE_NAMESPACE,
Expand Down Expand Up @@ -168,6 +169,32 @@ 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.')

@override_settings(ENABLE_FORCE_CACHE_MISS_FOR_NONE=False)
def test_get_cached_response_hit_with_cached_none(self):
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)

def test_get_cached_response_miss_with_cached_none(self):
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
15 changes: 15 additions & 0 deletions edx_django_utils/cache/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
import hashlib
import threading

from django.conf import settings
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


FORCE_CACHE_MISS_PARAM = 'force_cache_miss'
DEFAULT_NAMESPACE = 'edx_django_utils.cache'
DEFAULT_REQUEST_CACHE_NAMESPACE = f'{DEFAULT_NAMESPACE}.default'
Expand Down Expand Up @@ -259,6 +261,19 @@ 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 getattr(settings, 'ENABLE_FORCE_CACHE_MISS_FOR_NONE', True):
# `ENABLE_FORCE_CACHE_MISS_FOR_NONE` is to provide backward compatibility.
# As we have been treating None as CACHE_MISS.
cached_value = _CACHE_MISS

# Avoiding circular import
from edx_django_utils.monitoring import set_custom_attribute
# .. custom_attribute_name: forced_cache_miss_occurred_for_none
# .. custom_attribute_description: True if we explicitly do CACHE MISS when None is retrieved
# from cache.
set_custom_attribute('forced_cache_miss_occurred_for_none', True)

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

Expand Down

0 comments on commit 6cac54f

Please sign in to comment.