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

cache_none option for memoize to deal with concurrency problem #140

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 @@ -695,6 +695,7 @@ def memoize(
unless=None,
forced_update=None,
hash_method=hashlib.md5,
cache_none=False,
):
"""Use this to cache the result of a function, taking its arguments
into account in the cache key.
Expand Down Expand Up @@ -752,6 +753,10 @@ def big_foo(a, b):
renewal of cached functions.
: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.

.. versionadded:: 0.5
params ``make_name``, ``unless``
Expand Down Expand Up @@ -788,7 +793,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
35 changes: 34 additions & 1 deletion tests/test_memoize.py
Expand Up @@ -656,7 +656,7 @@ def test_memoize_none(app, cache):

call_counter = Counter()

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

Expand All @@ -677,3 +677,36 @@ def memoize_none(param):

memoize_none(1)
assert call_counter[1] == 2


def test_memoize_never_accept_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.memoize()
def memoize_none(param):
call_counter[param] += 1

return None

memoize_none(1)

# The memoized 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 memoize_none(1) is None

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

cache.clear()

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