Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Cache Misses None Values #20

Merged
merged 10 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python-version: [ "3.7", "3.8", "3.9", "3.10" ]
django-version: [ "3.2" ]
python-version: [ "3.8", "3.9", "3.10" ]
django-version: [ "4.2" ]
Copy link
Author

@anthonyp97 anthonyp97 Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating these versions to align with what we use in main repo while I'm here. Python 3.7 no longer supported by Django 4.2+ so removed that from matrix.

steps:
# Checks-out the repository.
- uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion cache_helper/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
VERSION = (1, 0, 4)
VERSION = (1, 0, 5)
34 changes: 21 additions & 13 deletions cache_helper/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from cache_helper import utils

logger = logging.getLogger(__name__)
CACHE_KEY_NOT_FOUND = 'cache_key_not_found'


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

try:
value = cache.get(cache_key)
# Attempts to get cache key and defaults to a string constant which will be returned if the cache
# key does not exist due to expiry or never being set.
value = cache.get(cache_key, CACHE_KEY_NOT_FOUND)
except Exception:
logger.warning(
f'Error retrieving value from Cache for Key: {function_cache_key}',
exc_info=True,
)
value = None
value = CACHE_KEY_NOT_FOUND

if value is None:
if value == CACHE_KEY_NOT_FOUND:
value = func(*args, **kwargs)
# Try and set the key, value pair in the cache.
# But if it fails on an error from the underlying
# cache system, handle it.
try:
cache.set(cache_key, value, timeout)

except CacheSetError:
logger.warning(
f'Error saving value to Cache for Key: {function_cache_key}',
Expand Down Expand Up @@ -73,21 +76,21 @@ def _cached(func):
@wraps(func)
def wrapper(*args, **kwargs):
# skip the first arg because it will be the class itself
function_cache_key = utils.get_function_cache_key(
func_name, args[1:], kwargs
)
function_cache_key = utils.get_function_cache_key(func_name, args[1:], kwargs)
cache_key = utils.get_hashed_cache_key(function_cache_key)

try:
value = cache.get(cache_key)
# Attempts to get cache key and defaults to a string constant which will be returned if the cache
# key does not exist due to expiry or never being set.
value = cache.get(cache_key, CACHE_KEY_NOT_FOUND)
except Exception:
logger.warning(
f'Error retrieving value from Cache for Key: {function_cache_key}',
exc_info=True,
)
value = None
value = CACHE_KEY_NOT_FOUND

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

def __call__(self, *args, **kwargs):
cache_key, function_cache_key = self.create_cache_key(*args, **kwargs)

try:
value = cache.get(cache_key)
# Attempts to get cache key and defaults to a string constant which will be returned if the cache
# key does not exist due to expiry or never being set.
value = cache.get(cache_key, CACHE_KEY_NOT_FOUND)
except Exception:
logger.warning(
f'Error retrieving value from Cache for Key: {function_cache_key}',
exc_info=True,
)
value = None
if value is None:
value = CACHE_KEY_NOT_FOUND

if value == CACHE_KEY_NOT_FOUND:
value = self.func(*args, **kwargs)
# Try and set the key, value pair in the cache.
# But if it fails on an error from the underlying
Expand All @@ -170,6 +177,7 @@ def __call__(self, *args, **kwargs):
f'Error saving value to Cache for Key: {function_cache_key}',
exc_info=True,
)

return value

def _invalidate(self, *args, **kwargs):
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ authors = [
]
description = "a simple tool for making caching functions, methods, and class methods a little bit easier."
readme = "README.md"
requires-python = ">=3.7"
requires-python = ">=3.8"
keywords = [
"cache",
"django"
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Used for Test Project
Django>= 3.2, <4.0
Django>= 4.2, <5.0

# Used for tests coverage
coverage
Expand Down
80 changes: 80 additions & 0 deletions test_project/test_project/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

from datetime import datetime

GLOBAL_COUNTER = 200


class Incrementer:
class_counter = 500
Expand Down Expand Up @@ -39,6 +41,25 @@ def func_with_multiple_args_and_kwargs(
):
return datetime.utcnow()

@cached_instance_method(60 * 60)
def instance_increment_by_none_return(self, num):
self.instance_counter += num
return None

@classmethod
@cached_class_method(60 * 60)
def class_increment_by_none_return(cls, num):
cls.class_counter += num
return None

@staticmethod
@cached(60 * 60)
def static_increment_by_none_return(num):
global GLOBAL_COUNTER
GLOBAL_COUNTER += num

return None


class SubclassIncrementer(Incrementer):
class_counter = 500
Expand Down Expand Up @@ -268,6 +289,26 @@ def test_invalidate_instance_method(self):
# but 2 is still stale
self.assertEqual(incrementer.instance_increment_by(2), 103)

def test_cached_instance_method_none_return(self):
"""
Tests that calling a cached instance method returning None still uses the cache as expected.
"""
incrementer = Incrementer(100)

# Hasn't been computed before, so the function actually gets called
incrementer.instance_increment_by_none_return(1)
self.assertEqual(incrementer.instance_counter, 101)

incrementer.instance_increment_by_none_return(2)
self.assertEqual(incrementer.instance_counter, 103)

# We expect the instance counter to stay the same as the last time it was updated and not increment again
incrementer.instance_increment_by_none_return(1)
self.assertEqual(incrementer.instance_counter, 103)

incrementer.instance_increment_by_none_return(2)
self.assertEqual(incrementer.instance_counter, 103)


class CachedClassMethodTests(TestCase):
def tearDown(self):
Expand Down Expand Up @@ -410,10 +451,31 @@ def test_invalidate_class_method(self):
# but 2 is still stale
self.assertEqual(Incrementer.class_increment_by(2), 503)

def test_cached_class_method_none_return(self):
"""
Tests that calling a cached class method returning None still uses the cache as expected.
"""
# Hasn't been computed before, so the function actually gets called
Incrementer.class_increment_by_none_return(1)
self.assertEqual(Incrementer.class_counter, 501)

Incrementer.class_increment_by_none_return(2)
self.assertEqual(Incrementer.class_counter, 503)

# We expect the class counter to stay the same as the last time it was updated and not increment again
Incrementer.class_increment_by_none_return(1)
self.assertEqual(Incrementer.class_counter, 503)

Incrementer.class_increment_by_none_return(2)
self.assertEqual(Incrementer.class_counter, 503)


class CachedStaticMethodTests(TestCase):
def tearDown(self):
super().tearDown()

global GLOBAL_COUNTER
GLOBAL_COUNTER = 200
cache.clear()

def test_exception_during_cache_retrieval(self):
Expand Down Expand Up @@ -549,6 +611,24 @@ def test_invalidate_static_method(self):
# but 2 is still stale
self.assertEqual(Incrementer.get_datetime(2), initial_datetime_2)

def test_cached_class_method_none_return(self):
"""
Tests that calling a cached static method returning None still uses the cache as expected.
"""
# Hasn't been computed before, so the function actually gets called
Incrementer.static_increment_by_none_return(1)
self.assertEqual(GLOBAL_COUNTER, 201)

Incrementer.static_increment_by_none_return(2)
self.assertEqual(GLOBAL_COUNTER, 203)

# We expect the global counter to stay the same as the last time it was updated and not increment again
Incrementer.static_increment_by_none_return(1)
self.assertEqual(GLOBAL_COUNTER, 203)

Incrementer.static_increment_by_none_return(2)
self.assertEqual(GLOBAL_COUNTER, 203)


class CacheHelperCacheableTests(TestCase):
def tearDown(self):
Expand Down
Loading