From 94d75927037584ef615c34b13016dbf5d07e4ed9 Mon Sep 17 00:00:00 2001 From: Muhammad Umar Khan <42294172+mumarkhan999@users.noreply.github.com> Date: Mon, 7 Aug 2023 14:59:53 +0500 Subject: [PATCH] fix: check None before returning config (#296) * fix: check None before returning config * squash! add testcase --- CHANGELOG.rst | 1 + config_models/models.py | 2 +- tests/test_config_models.py | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 59039b6..abe7f38 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -16,6 +16,7 @@ Unreleased * Switch from ``edx-sphinx-theme`` to ``sphinx-book-theme`` since the former is deprecated +* Fixed ConfigurationModel.current: it will make sure that it does not return None for current configuration. [2.3.0] - 2022-01-19 ~~~~~~~~~~~~~~~~~~~~ diff --git a/config_models/models.py b/config_models/models.py index ef63284..d537220 100644 --- a/config_models/models.py +++ b/config_models/models.py @@ -131,7 +131,7 @@ def current(cls, *args): """ cache_key = cls.cache_key_name(*args) cached_response = TieredCache.get_cached_response(cache_key) - if cached_response.is_found: + if cached_response.is_found and cached_response.value is not None: return cached_response.value key_dict = dict(zip(cls.KEY_FIELDS, args)) diff --git a/tests/test_config_models.py b/tests/test_config_models.py index efe8a2c..7142d91 100644 --- a/tests/test_config_models.py +++ b/tests/test_config_models.py @@ -1,10 +1,11 @@ """ Tests of ConfigurationModel """ - +from unittest import mock import ddt from django.contrib.auth import get_user_model +from edx_django_utils.cache.utils import CachedResponse from example.models import (ExampleConfig, ExampleKeyedConfig, ManyToManyExampleConfig) from freezegun import freeze_time @@ -45,6 +46,21 @@ def test_no_config_empty_cache(self): self.assertEqual(cached_current.int_field, 10) self.assertEqual(cached_current.string_field, '') + @mock.patch('config_models.models.TieredCache.get_cached_response') + def test_config_with_cached_response_value_none(self, mock_cached_response): + mock_cached_response.return_value = CachedResponse(is_found=True, key='test-key', value=None) + # First time reads from the database + with self.assertNumQueries(1): + current = ExampleConfig.current() + self.assertEqual(current.int_field, 10) + self.assertEqual(current.string_field, '') + + # Follow-on request reads from database instead of cache as cache value will be None. + with self.assertNumQueries(1): + cached_current = ExampleConfig.current() + self.assertEqual(cached_current.int_field, 10) + self.assertEqual(cached_current.string_field, '') + def test_config_ordering(self): with freeze_time('2012-01-01'): first = ExampleConfig(changed_by=self.user)