Skip to content

Commit

Permalink
fix: check None before returning config (#296)
Browse files Browse the repository at this point in the history
* fix: check None before returning config

* squash! add testcase
  • Loading branch information
mumarkhan999 authored Aug 7, 2023
1 parent 159cff9 commit 94d7592
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
~~~~~~~~~~~~~~~~~~~~
Expand Down
2 changes: 1 addition & 1 deletion config_models/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
18 changes: 17 additions & 1 deletion tests/test_config_models.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 94d7592

Please sign in to comment.