feat: remove waffle forced ccached miss switch (#363)

CHANGELOG.rst

@@ -11,6 +11,13 @@ Change Log
 
 .. There should always be an "Unreleased" section for changes pending release.
 
+[5.9.0] - 2023-11-27
+--------------------
+
+Removed
+~~~~~~~
+* Removed ``edx_django_utils.cache.disable_forced_cache_miss_for_none`` which was added in ``5.7.0``.
+
 [5.8.0] - 2023-11-03
 --------------------
 

edx_django_utils/cache/tests/test_utils.py

@@ -7,7 +7,6 @@
 
 import ddt
 from django.test import TestCase as DjangoTestCase
-from waffle.testutils import override_switch
 
 from edx_django_utils.cache.utils import (
     DEFAULT_REQUEST_CACHE_NAMESPACE,
@@ -170,57 +169,6 @@ 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.monitoring.internal.utils.set_custom_attribute')
-    @override_switch('edx_django_utils.cache.disable_forced_cache_miss_for_none', True)
-    def test_get_cached_response_hit_with_cached_none(self, mock_set_custom_attribute):
-        """
-        Tests cache hit when caching a None.
-
-        For rollout, this test relies on ``disable_forced_cache_miss_for_none`` switch to be on, because
-        by default we are temporarily forcing cache misses for backward compatibility.
-        """
-        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)
-
-        expected_calls = [
-            mock.call('retrieved_cached_none', True)
-        ]
-        mock_set_custom_attribute.assert_has_calls(expected_calls)
-
-    @mock.patch('edx_django_utils.monitoring.internal.utils.set_custom_attribute')
-    def test_get_cached_response_miss_with_cached_none(self, mock_set_custom_attribute):
-        """
-        Temporary tests for cache miss when caching a None.
-
-        Temporarily we are forcing cache misses by default when caching None for
-        backward compatibility purposes. See ``disable_forced_cache_miss_for_none``
-        switch for more details.
-        """
-        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)
-
-        expected_calls = [
-            mock.call('retrieved_cached_none', True)
-        ]
-        mock_set_custom_attribute.assert_has_calls(expected_calls)
-
     @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)

edx_django_utils/cache/utils.py

@@ -4,7 +4,6 @@
 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
@@ -15,7 +14,6 @@
 SHOULD_FORCE_CACHE_MISS_KEY = 'edx_django_utils.cache.should_force_cache_miss'
 
 _CACHE_MISS = object()
-_CACHED_NONE = 'CACHED_NONE'
 
 
 def get_cache_key(**kwargs):
@@ -212,10 +210,7 @@ def set_all_tiers(key, value, django_cache_timeout=DEFAULT_TIMEOUT):
 
         """
         DEFAULT_REQUEST_CACHE.set(key, value)
-        cached_value = value
-        if value is None:
-            cached_value = _CACHED_NONE
-        django_cache.set(key, cached_value, django_cache_timeout)
+        django_cache.set(key, value, django_cache_timeout)
 
     @staticmethod
     def delete_all_tiers(key):
@@ -264,21 +259,6 @@ 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 == _CACHED_NONE:
-            # Avoiding circular import
-            from edx_django_utils.monitoring.internal.utils import \
-                set_custom_attribute  # isort:skip, pylint: disable=cyclic-import, import-outside-toplevel
-
-            # .. custom_attribute_name: retrieved_cached_none
-            # .. custom_attribute_description: Temporary attribute to see when a None would
-            #      have been retrieved, so we know what will be affected by the toggle.
-            set_custom_attribute('retrieved_cached_none', True)
-            if TieredCache._is_forced_cache_miss_for_none_disabled():
-                cached_value = None
-            else:
-                cached_value = _CACHE_MISS
-
         is_found = cached_value is not _CACHE_MISS
         return CachedResponse(is_found, key, cached_value)
 
@@ -323,25 +303,6 @@ 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():
-        """
-        Returns True if disable_forced_cache_miss_for_none is on, and False otherwise.
-        """
-        # .. toggle_name: edx_django_utils.cache.disable_forced_cache_miss_for_none
-        # .. toggle_implementation: WaffleSwitch
-        # .. toggle_default: False
-        # .. toggle_description: By default, this toggle will replicate an existing bug by forcing
-        #      cache misses when setting None. Set the toggle to True to disable this behavior,
-        #      which fixes the bug and returns None when None was cached. This toggle is
-        #      being used for backward compatibility during rollout, until the toggle and old
-        #      broken behavior can be removed.
-        # .. toggle_use_cases: temporary
-        # .. toggle_creation_date: 2023-08-02
-        # .. toggle_target_removal_date: 2023-09-01
-        # .. toggle_tickets: https://github.com/openedx/edx-django-utils/issues/333
-        return waffle.switch_is_active('edx_django_utils.cache.disable_forced_cache_miss_for_none')
-
 
 class CachedResponseError(Exception):
     """