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

default cached to not cache None #141

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion flask_caching/__init__.py
Expand Up @@ -292,6 +292,7 @@ def cached(
response_filter=None,
query_string=False,
hash_method=hashlib.md5,
cache_none=False,
):
"""Decorator. Use this to cache a function. By default the cache key
is `view/request.path`. You are able to use this decorator with any
Expand Down Expand Up @@ -374,6 +375,10 @@ def get_list():

:param hash_method: Default hashlib.md5. The hash method used to
generate the keys for cached results.
:param cache_none: Default False. If set to True, add a key exists
check when cache.get returns None. This will likely
lead to wrongly returned None values in concurrent
situations and is not recommended to use.

"""

Expand Down Expand Up @@ -411,7 +416,16 @@ def decorated_function(*args, **kwargs):
# might be because the key is not found in the cache
# or because the cached value is actually None
if rv is None:
found = self.cache.has(cache_key)
# If we're sure we don't need to cache None values
# (cache_none=False), don't bother checking for
# key existence, as it can lead to false positives
# if a concurrent call already cached the
# key between steps. This would cause us to
# return None when we shouldn't
if not cache_none:
found = False
else:
found = self.cache.has(cache_key)
except Exception:
if self.app.debug:
raise
Expand Down
13 changes: 0 additions & 13 deletions flask_caching/backends/rediscache.py
Expand Up @@ -203,19 +203,6 @@ def unlink(self, *keys):
return self._write_client.unlink(*keys)
return self._write_client.delete(*keys)

def unlink(self, *keys):
"""when redis-py >= 3.0.0 and redis > 4, support this operation
"""
if not keys:
return
if self.key_prefix:
keys = [self.key_prefix + key for key in keys]

unlink = getattr(self._write_client, "unlink", None)
if unlink is not None and callable(unlink):
return self._write_client.unlink(*keys)
return self._write_client.delete(*keys)


class RedisSentinelCache(RedisCache):
"""Uses the Redis key-value store as a cache backend.
Expand Down
35 changes: 34 additions & 1 deletion tests/test_cache.py
Expand Up @@ -113,7 +113,7 @@ def test_cached_none(app, cache):

call_counter = Counter()

@cache.cached()
@cache.cached(cache_none=True)
def cache_none(param):
call_counter[param] += 1

Expand All @@ -131,6 +131,39 @@ def cache_none(param):
assert call_counter[1] == 2


def test_cached_doesnt_cache_none(app, cache):
"""Asserting that when cache_none is False, we always
assume a None value returned from .get() means the key is not found
"""
with app.test_request_context():
from collections import Counter

call_counter = Counter()

@cache.cached()
def cache_none(param):
call_counter[param] += 1

return None

cache_none(1)

# The cached function should have been called
assert call_counter[1] == 1

# Next time we call the function, the value should be coming from the cache…
# But the value is None and so we treat it as uncached.
assert cache_none(1) is None

# …thus, the call counter should increment to 2
assert call_counter[1] == 2

cache.clear()

cache_none(1)
assert call_counter[1] == 3


def test_cache_forced_update(app, cache):
from collections import Counter

Expand Down