Skip to content

Commit d4d1b32

Browse files
kdhamoychartskdhamoycharts
authored
Merge pull request #20 from anthonyp97/fix_none_cahce_misses
Fix Cache Misses None Values
2 parents b6bd5e0 + a840d21 commit d4d1b32

File tree

6 files changed

+106
-18
lines changed

6 files changed

+106
-18
lines changed

‎.github/workflows/main.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ jobs:
1919
runs-on: ubuntu-latest
2020
strategy:
2121
matrix:
22-
python-version: [ "3.7", "3.8", "3.9", "3.10" ]
23-
django-version: [ "3.2" ]
22+
python-version: [ "3.8", "3.9", "3.10" ]
23+
django-version: [ "4.2" ]
2424
steps:
2525
# Checks-out the repository.
2626
- uses: actions/checkout@v2

‎cache_helper/__init__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
VERSION = (1, 0, 4)
1+
VERSION = (1, 0, 5)

‎cache_helper/decorators.py‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
from cache_helper import utils
1414

1515
logger = logging.getLogger(__name__)
16+
CACHE_KEY_NOT_FOUND = 'cache_key_not_found'
17+
1618

1719
def cached(timeout):
1820
def _cached(func):
@@ -24,22 +26,23 @@ def wrapper(*args, **kwargs):
2426
cache_key = utils.get_hashed_cache_key(function_cache_key)
2527

2628
try:
27-
value = cache.get(cache_key)
29+
# Attempts to get cache key and defaults to a string constant which will be returned if the cache
30+
# key does not exist due to expiry or never being set.
31+
value = cache.get(cache_key, CACHE_KEY_NOT_FOUND)
2832
except Exception:
2933
logger.warning(
3034
f'Error retrieving value from Cache for Key: {function_cache_key}',
3135
exc_info=True,
3236
)
33-
value = None
37+
value = CACHE_KEY_NOT_FOUND
3438

35-
if value is None:
39+
if value == CACHE_KEY_NOT_FOUND:
3640
value = func(*args, **kwargs)
3741
# Try and set the key, value pair in the cache.
3842
# But if it fails on an error from the underlying
3943
# cache system, handle it.
4044
try:
4145
cache.set(cache_key, value, timeout)
42-
4346
except CacheSetError:
4447
logger.warning(
4548
f'Error saving value to Cache for Key: {function_cache_key}',
@@ -73,21 +76,21 @@ def _cached(func):
7376
@wraps(func)
7477
def wrapper(*args, **kwargs):
7578
# skip the first arg because it will be the class itself
76-
function_cache_key = utils.get_function_cache_key(
77-
func_name, args[1:], kwargs
78-
)
79+
function_cache_key = utils.get_function_cache_key(func_name, args[1:], kwargs)
7980
cache_key = utils.get_hashed_cache_key(function_cache_key)
8081

8182
try:
82-
value = cache.get(cache_key)
83+
# Attempts to get cache key and defaults to a string constant which will be returned if the cache
84+
# key does not exist due to expiry or never being set.
85+
value = cache.get(cache_key, CACHE_KEY_NOT_FOUND)
8386
except Exception:
8487
logger.warning(
8588
f'Error retrieving value from Cache for Key: {function_cache_key}',
8689
exc_info=True,
8790
)
88-
value = None
91+
value = CACHE_KEY_NOT_FOUND
8992

90-
if value is None:
93+
if value == CACHE_KEY_NOT_FOUND:
9194
value = func(*args, **kwargs)
9295
# Try and set the key, value pair in the cache.
9396
# But if it fails on an error from the underlying
@@ -150,15 +153,19 @@ def __get__(self, obj, objtype):
150153

151154
def __call__(self, *args, **kwargs):
152155
cache_key, function_cache_key = self.create_cache_key(*args, **kwargs)
156+
153157
try:
154-
value = cache.get(cache_key)
158+
# Attempts to get cache key and defaults to a string constant which will be returned if the cache
159+
# key does not exist due to expiry or never being set.
160+
value = cache.get(cache_key, CACHE_KEY_NOT_FOUND)
155161
except Exception:
156162
logger.warning(
157163
f'Error retrieving value from Cache for Key: {function_cache_key}',
158164
exc_info=True,
159165
)
160-
value = None
161-
if value is None:
166+
value = CACHE_KEY_NOT_FOUND
167+
168+
if value == CACHE_KEY_NOT_FOUND:
162169
value = self.func(*args, **kwargs)
163170
# Try and set the key, value pair in the cache.
164171
# But if it fails on an error from the underlying
@@ -170,6 +177,7 @@ def __call__(self, *args, **kwargs):
170177
f'Error saving value to Cache for Key: {function_cache_key}',
171178
exc_info=True,
172179
)
180+
173181
return value
174182

175183
def _invalidate(self, *args, **kwargs):

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ authors = [
99
]
1010
description = "a simple tool for making caching functions, methods, and class methods a little bit easier."
1111
readme = "README.md"
12-
requires-python = ">=3.7"
12+
requires-python = ">=3.8"
1313
keywords = [
1414
"cache",
1515
"django"

‎requirements.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Used for Test Project
2-
Django>= 3.2, <4.0
2+
Django>= 4.2, <5.0
33

44
# Used for tests coverage
55
coverage

‎test_project/test_project/tests.py‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
from datetime import datetime
1111

12+
GLOBAL_COUNTER = 200
13+
1214

1315
class Incrementer:
1416
class_counter = 500
@@ -39,6 +41,25 @@ def func_with_multiple_args_and_kwargs(
3941
):
4042
return datetime.utcnow()
4143

44+
@cached_instance_method(60 * 60)
45+
def instance_increment_by_none_return(self, num):
46+
self.instance_counter += num
47+
return None
48+
49+
@classmethod
50+
@cached_class_method(60 * 60)
51+
def class_increment_by_none_return(cls, num):
52+
cls.class_counter += num
53+
return None
54+
55+
@staticmethod
56+
@cached(60 * 60)
57+
def static_increment_by_none_return(num):
58+
global GLOBAL_COUNTER
59+
GLOBAL_COUNTER += num
60+
61+
return None
62+
4263

4364
class SubclassIncrementer(Incrementer):
4465
class_counter = 500
@@ -268,6 +289,26 @@ def test_invalidate_instance_method(self):
268289
# but 2 is still stale
269290
self.assertEqual(incrementer.instance_increment_by(2), 103)
270291

292+
def test_cached_instance_method_none_return(self):
293+
"""
294+
Tests that calling a cached instance method returning None still uses the cache as expected.
295+
"""
296+
incrementer = Incrementer(100)
297+
298+
# Hasn't been computed before, so the function actually gets called
299+
incrementer.instance_increment_by_none_return(1)
300+
self.assertEqual(incrementer.instance_counter, 101)
301+
302+
incrementer.instance_increment_by_none_return(2)
303+
self.assertEqual(incrementer.instance_counter, 103)
304+
305+
# We expect the instance counter to stay the same as the last time it was updated and not increment again
306+
incrementer.instance_increment_by_none_return(1)
307+
self.assertEqual(incrementer.instance_counter, 103)
308+
309+
incrementer.instance_increment_by_none_return(2)
310+
self.assertEqual(incrementer.instance_counter, 103)
311+
271312

272313
class CachedClassMethodTests(TestCase):
273314
def tearDown(self):
@@ -410,10 +451,31 @@ def test_invalidate_class_method(self):
410451
# but 2 is still stale
411452
self.assertEqual(Incrementer.class_increment_by(2), 503)
412453

454+
def test_cached_class_method_none_return(self):
455+
"""
456+
Tests that calling a cached class method returning None still uses the cache as expected.
457+
"""
458+
# Hasn't been computed before, so the function actually gets called
459+
Incrementer.class_increment_by_none_return(1)
460+
self.assertEqual(Incrementer.class_counter, 501)
461+
462+
Incrementer.class_increment_by_none_return(2)
463+
self.assertEqual(Incrementer.class_counter, 503)
464+
465+
# We expect the class counter to stay the same as the last time it was updated and not increment again
466+
Incrementer.class_increment_by_none_return(1)
467+
self.assertEqual(Incrementer.class_counter, 503)
468+
469+
Incrementer.class_increment_by_none_return(2)
470+
self.assertEqual(Incrementer.class_counter, 503)
471+
413472

414473
class CachedStaticMethodTests(TestCase):
415474
def tearDown(self):
416475
super().tearDown()
476+
477+
global GLOBAL_COUNTER
478+
GLOBAL_COUNTER = 200
417479
cache.clear()
418480

419481
def test_exception_during_cache_retrieval(self):
@@ -549,6 +611,24 @@ def test_invalidate_static_method(self):
549611
# but 2 is still stale
550612
self.assertEqual(Incrementer.get_datetime(2), initial_datetime_2)
551613

614+
def test_cached_class_method_none_return(self):
615+
"""
616+
Tests that calling a cached static method returning None still uses the cache as expected.
617+
"""
618+
# Hasn't been computed before, so the function actually gets called
619+
Incrementer.static_increment_by_none_return(1)
620+
self.assertEqual(GLOBAL_COUNTER, 201)
621+
622+
Incrementer.static_increment_by_none_return(2)
623+
self.assertEqual(GLOBAL_COUNTER, 203)
624+
625+
# We expect the global counter to stay the same as the last time it was updated and not increment again
626+
Incrementer.static_increment_by_none_return(1)
627+
self.assertEqual(GLOBAL_COUNTER, 203)
628+
629+
Incrementer.static_increment_by_none_return(2)
630+
self.assertEqual(GLOBAL_COUNTER, 203)
631+
552632

553633
class CacheHelperCacheableTests(TestCase):
554634
def tearDown(self):

0 commit comments

Comments
 (0)