Fixed #29867 -- Added support for storing None value in caches.
Many of the cache operations make use of the default argument to the .get() operation to determine whether the key was found in the cache. The default value of the default argument is None, so this results in these operations assuming that None is not stored in the cache when it actually is. Adding a sentinel object solves this issue. Unfortunately the unmaintained python-memcached library does not support a default argument to .get(), so the previous behavior is preserved for the deprecated MemcachedCache backend.
This commit is contained in:
parent
d23dad5778
commit
bb64b99b78
|
@ -52,6 +52,8 @@ def get_key_func(key_func):
|
||||||
|
|
||||||
|
|
||||||
class BaseCache:
|
class BaseCache:
|
||||||
|
_missing_key = object()
|
||||||
|
|
||||||
def __init__(self, params):
|
def __init__(self, params):
|
||||||
timeout = params.get('timeout', params.get('TIMEOUT', 300))
|
timeout = params.get('timeout', params.get('TIMEOUT', 300))
|
||||||
if timeout is not None:
|
if timeout is not None:
|
||||||
|
@ -151,8 +153,8 @@ class BaseCache:
|
||||||
"""
|
"""
|
||||||
d = {}
|
d = {}
|
||||||
for k in keys:
|
for k in keys:
|
||||||
val = self.get(k, version=version)
|
val = self.get(k, self._missing_key, version=version)
|
||||||
if val is not None:
|
if val is not self._missing_key:
|
||||||
d[k] = val
|
d[k] = val
|
||||||
return d
|
return d
|
||||||
|
|
||||||
|
@ -165,31 +167,29 @@ class BaseCache:
|
||||||
|
|
||||||
Return the value of the key stored or retrieved.
|
Return the value of the key stored or retrieved.
|
||||||
"""
|
"""
|
||||||
val = self.get(key, version=version)
|
val = self.get(key, self._missing_key, version=version)
|
||||||
if val is None:
|
if val is self._missing_key:
|
||||||
if callable(default):
|
if callable(default):
|
||||||
default = default()
|
default = default()
|
||||||
if default is not None:
|
self.add(key, default, timeout=timeout, version=version)
|
||||||
self.add(key, default, timeout=timeout, version=version)
|
# Fetch the value again to avoid a race condition if another caller
|
||||||
# Fetch the value again to avoid a race condition if another
|
# added a value between the first get() and the add() above.
|
||||||
# caller added a value between the first get() and the add()
|
return self.get(key, default, version=version)
|
||||||
# above.
|
|
||||||
return self.get(key, default, version=version)
|
|
||||||
return val
|
return val
|
||||||
|
|
||||||
def has_key(self, key, version=None):
|
def has_key(self, key, version=None):
|
||||||
"""
|
"""
|
||||||
Return True if the key is in the cache and has not expired.
|
Return True if the key is in the cache and has not expired.
|
||||||
"""
|
"""
|
||||||
return self.get(key, version=version) is not None
|
return self.get(key, self._missing_key, version=version) is not self._missing_key
|
||||||
|
|
||||||
def incr(self, key, delta=1, version=None):
|
def incr(self, key, delta=1, version=None):
|
||||||
"""
|
"""
|
||||||
Add delta to value in the cache. If the key does not exist, raise a
|
Add delta to value in the cache. If the key does not exist, raise a
|
||||||
ValueError exception.
|
ValueError exception.
|
||||||
"""
|
"""
|
||||||
value = self.get(key, version=version)
|
value = self.get(key, self._missing_key, version=version)
|
||||||
if value is None:
|
if value is self._missing_key:
|
||||||
raise ValueError("Key '%s' not found" % key)
|
raise ValueError("Key '%s' not found" % key)
|
||||||
new_value = value + delta
|
new_value = value + delta
|
||||||
self.set(key, new_value, version=version)
|
self.set(key, new_value, version=version)
|
||||||
|
@ -257,8 +257,8 @@ class BaseCache:
|
||||||
if version is None:
|
if version is None:
|
||||||
version = self.version
|
version = self.version
|
||||||
|
|
||||||
value = self.get(key, version=version)
|
value = self.get(key, self._missing_key, version=version)
|
||||||
if value is None:
|
if value is self._missing_key:
|
||||||
raise ValueError("Key '%s' not found" % key)
|
raise ValueError("Key '%s' not found" % key)
|
||||||
|
|
||||||
self.set(key, value, version=version + delta)
|
self.set(key, value, version=version + delta)
|
||||||
|
|
|
@ -165,6 +165,11 @@ class BaseMemcachedCache(BaseCache):
|
||||||
|
|
||||||
class MemcachedCache(BaseMemcachedCache):
|
class MemcachedCache(BaseMemcachedCache):
|
||||||
"An implementation of a cache binding using python-memcached"
|
"An implementation of a cache binding using python-memcached"
|
||||||
|
|
||||||
|
# python-memcached doesn't support default values in get().
|
||||||
|
# https://github.com/linsomniac/python-memcached/issues/159
|
||||||
|
_missing_key = None
|
||||||
|
|
||||||
def __init__(self, server, params):
|
def __init__(self, server, params):
|
||||||
warnings.warn(
|
warnings.warn(
|
||||||
'MemcachedCache is deprecated in favor of PyMemcacheCache and '
|
'MemcachedCache is deprecated in favor of PyMemcacheCache and '
|
||||||
|
|
|
@ -681,6 +681,14 @@ Miscellaneous
|
||||||
``UserChangeForm.clean_password()`` is no longer required to return the
|
``UserChangeForm.clean_password()`` is no longer required to return the
|
||||||
initial value.
|
initial value.
|
||||||
|
|
||||||
|
* The ``cache.get_many()``, ``get_or_set()``, ``has_key()``, ``incr()``,
|
||||||
|
``decr()``, ``incr_version()``, and ``decr_version()`` cache operations now
|
||||||
|
correctly handle ``None`` stored in the cache, in the same way as any other
|
||||||
|
value, instead of behaving as though the key didn't exist.
|
||||||
|
|
||||||
|
Due to a ``python-memcached`` limitation, the previous behavior is kept for
|
||||||
|
the deprecated ``MemcachedCache`` backend.
|
||||||
|
|
||||||
.. _deprecated-features-3.2:
|
.. _deprecated-features-3.2:
|
||||||
|
|
||||||
Features deprecated in 3.2
|
Features deprecated in 3.2
|
||||||
|
|
|
@ -278,6 +278,14 @@ class BaseCacheTests:
|
||||||
# A common set of tests to apply to all cache backends
|
# A common set of tests to apply to all cache backends
|
||||||
factory = RequestFactory()
|
factory = RequestFactory()
|
||||||
|
|
||||||
|
# RemovedInDjango41Warning: python-memcached doesn't support .get() with
|
||||||
|
# default.
|
||||||
|
supports_get_with_default = True
|
||||||
|
|
||||||
|
# Some clients raise custom exceptions when .incr() or .decr() are called
|
||||||
|
# with a non-integer value.
|
||||||
|
incr_decr_type_error = TypeError
|
||||||
|
|
||||||
def tearDown(self):
|
def tearDown(self):
|
||||||
cache.clear()
|
cache.clear()
|
||||||
|
|
||||||
|
@ -320,6 +328,8 @@ class BaseCacheTests:
|
||||||
self.assertEqual(cache.get_many(['a', 'c', 'd']), {'a': 'a', 'c': 'c', 'd': 'd'})
|
self.assertEqual(cache.get_many(['a', 'c', 'd']), {'a': 'a', 'c': 'c', 'd': 'd'})
|
||||||
self.assertEqual(cache.get_many(['a', 'b', 'e']), {'a': 'a', 'b': 'b'})
|
self.assertEqual(cache.get_many(['a', 'b', 'e']), {'a': 'a', 'b': 'b'})
|
||||||
self.assertEqual(cache.get_many(iter(['a', 'b', 'e'])), {'a': 'a', 'b': 'b'})
|
self.assertEqual(cache.get_many(iter(['a', 'b', 'e'])), {'a': 'a', 'b': 'b'})
|
||||||
|
cache.set_many({'x': None, 'y': 1})
|
||||||
|
self.assertEqual(cache.get_many(['x', 'y']), {'x': None, 'y': 1})
|
||||||
|
|
||||||
def test_delete(self):
|
def test_delete(self):
|
||||||
# Cache keys can be deleted
|
# Cache keys can be deleted
|
||||||
|
@ -339,12 +349,22 @@ class BaseCacheTests:
|
||||||
self.assertIs(cache.has_key("goodbye1"), False)
|
self.assertIs(cache.has_key("goodbye1"), False)
|
||||||
cache.set("no_expiry", "here", None)
|
cache.set("no_expiry", "here", None)
|
||||||
self.assertIs(cache.has_key("no_expiry"), True)
|
self.assertIs(cache.has_key("no_expiry"), True)
|
||||||
|
cache.set('null', None)
|
||||||
|
self.assertIs(
|
||||||
|
cache.has_key('null'),
|
||||||
|
True if self.supports_get_with_default else False,
|
||||||
|
)
|
||||||
|
|
||||||
def test_in(self):
|
def test_in(self):
|
||||||
# The in operator can be used to inspect cache contents
|
# The in operator can be used to inspect cache contents
|
||||||
cache.set("hello2", "goodbye2")
|
cache.set("hello2", "goodbye2")
|
||||||
self.assertIn("hello2", cache)
|
self.assertIn("hello2", cache)
|
||||||
self.assertNotIn("goodbye2", cache)
|
self.assertNotIn("goodbye2", cache)
|
||||||
|
cache.set('null', None)
|
||||||
|
if self.supports_get_with_default:
|
||||||
|
self.assertIn('null', cache)
|
||||||
|
else:
|
||||||
|
self.assertNotIn('null', cache)
|
||||||
|
|
||||||
def test_incr(self):
|
def test_incr(self):
|
||||||
# Cache values can be incremented
|
# Cache values can be incremented
|
||||||
|
@ -356,6 +376,9 @@ class BaseCacheTests:
|
||||||
self.assertEqual(cache.incr('answer', -10), 42)
|
self.assertEqual(cache.incr('answer', -10), 42)
|
||||||
with self.assertRaises(ValueError):
|
with self.assertRaises(ValueError):
|
||||||
cache.incr('does_not_exist')
|
cache.incr('does_not_exist')
|
||||||
|
cache.set('null', None)
|
||||||
|
with self.assertRaises(self.incr_decr_type_error):
|
||||||
|
cache.incr('null')
|
||||||
|
|
||||||
def test_decr(self):
|
def test_decr(self):
|
||||||
# Cache values can be decremented
|
# Cache values can be decremented
|
||||||
|
@ -367,6 +390,9 @@ class BaseCacheTests:
|
||||||
self.assertEqual(cache.decr('answer', -10), 42)
|
self.assertEqual(cache.decr('answer', -10), 42)
|
||||||
with self.assertRaises(ValueError):
|
with self.assertRaises(ValueError):
|
||||||
cache.decr('does_not_exist')
|
cache.decr('does_not_exist')
|
||||||
|
cache.set('null', None)
|
||||||
|
with self.assertRaises(self.incr_decr_type_error):
|
||||||
|
cache.decr('null')
|
||||||
|
|
||||||
def test_close(self):
|
def test_close(self):
|
||||||
self.assertTrue(hasattr(cache, 'close'))
|
self.assertTrue(hasattr(cache, 'close'))
|
||||||
|
@ -914,6 +940,13 @@ class BaseCacheTests:
|
||||||
with self.assertRaises(ValueError):
|
with self.assertRaises(ValueError):
|
||||||
cache.incr_version('does_not_exist')
|
cache.incr_version('does_not_exist')
|
||||||
|
|
||||||
|
cache.set('null', None)
|
||||||
|
if self.supports_get_with_default:
|
||||||
|
self.assertEqual(cache.incr_version('null'), 2)
|
||||||
|
else:
|
||||||
|
with self.assertRaises(self.incr_decr_type_error):
|
||||||
|
cache.incr_version('null')
|
||||||
|
|
||||||
def test_decr_version(self):
|
def test_decr_version(self):
|
||||||
cache.set('answer', 42, version=2)
|
cache.set('answer', 42, version=2)
|
||||||
self.assertIsNone(cache.get('answer'))
|
self.assertIsNone(cache.get('answer'))
|
||||||
|
@ -938,6 +971,13 @@ class BaseCacheTests:
|
||||||
with self.assertRaises(ValueError):
|
with self.assertRaises(ValueError):
|
||||||
cache.decr_version('does_not_exist', version=2)
|
cache.decr_version('does_not_exist', version=2)
|
||||||
|
|
||||||
|
cache.set('null', None, version=2)
|
||||||
|
if self.supports_get_with_default:
|
||||||
|
self.assertEqual(cache.decr_version('null', version=2), 1)
|
||||||
|
else:
|
||||||
|
with self.assertRaises(self.incr_decr_type_error):
|
||||||
|
cache.decr_version('null', version=2)
|
||||||
|
|
||||||
def test_custom_key_func(self):
|
def test_custom_key_func(self):
|
||||||
# Two caches with different key functions aren't visible to each other
|
# Two caches with different key functions aren't visible to each other
|
||||||
cache.set('answer1', 42)
|
cache.set('answer1', 42)
|
||||||
|
@ -995,6 +1035,11 @@ class BaseCacheTests:
|
||||||
self.assertEqual(cache.get_or_set('projector', 42), 42)
|
self.assertEqual(cache.get_or_set('projector', 42), 42)
|
||||||
self.assertEqual(cache.get('projector'), 42)
|
self.assertEqual(cache.get('projector'), 42)
|
||||||
self.assertIsNone(cache.get_or_set('null', None))
|
self.assertIsNone(cache.get_or_set('null', None))
|
||||||
|
if self.supports_get_with_default:
|
||||||
|
# Previous get_or_set() stores None in the cache.
|
||||||
|
self.assertIsNone(cache.get('null', 'default'))
|
||||||
|
else:
|
||||||
|
self.assertEqual(cache.get('null', 'default'), 'default')
|
||||||
|
|
||||||
def test_get_or_set_callable(self):
|
def test_get_or_set_callable(self):
|
||||||
def my_callable():
|
def my_callable():
|
||||||
|
@ -1003,10 +1048,12 @@ class BaseCacheTests:
|
||||||
self.assertEqual(cache.get_or_set('mykey', my_callable), 'value')
|
self.assertEqual(cache.get_or_set('mykey', my_callable), 'value')
|
||||||
self.assertEqual(cache.get_or_set('mykey', my_callable()), 'value')
|
self.assertEqual(cache.get_or_set('mykey', my_callable()), 'value')
|
||||||
|
|
||||||
def test_get_or_set_callable_returning_none(self):
|
self.assertIsNone(cache.get_or_set('null', lambda: None))
|
||||||
self.assertIsNone(cache.get_or_set('mykey', lambda: None))
|
if self.supports_get_with_default:
|
||||||
# Previous get_or_set() doesn't store None in the cache.
|
# Previous get_or_set() stores None in the cache.
|
||||||
self.assertEqual(cache.get('mykey', 'default'), 'default')
|
self.assertIsNone(cache.get('null', 'default'))
|
||||||
|
else:
|
||||||
|
self.assertEqual(cache.get('null', 'default'), 'default')
|
||||||
|
|
||||||
def test_get_or_set_version(self):
|
def test_get_or_set_version(self):
|
||||||
msg = "get_or_set() missing 1 required positional argument: 'default'"
|
msg = "get_or_set() missing 1 required positional argument: 'default'"
|
||||||
|
@ -1399,6 +1446,8 @@ MemcachedCache_params = configured_caches.get('django.core.cache.backends.memcac
|
||||||
))
|
))
|
||||||
class MemcachedCacheTests(BaseMemcachedTests, TestCase):
|
class MemcachedCacheTests(BaseMemcachedTests, TestCase):
|
||||||
base_params = MemcachedCache_params
|
base_params = MemcachedCache_params
|
||||||
|
supports_get_with_default = False
|
||||||
|
incr_decr_type_error = ValueError
|
||||||
|
|
||||||
def test_memcached_uses_highest_pickle_version(self):
|
def test_memcached_uses_highest_pickle_version(self):
|
||||||
# Regression test for #19810
|
# Regression test for #19810
|
||||||
|
@ -1459,6 +1508,10 @@ class PyLibMCCacheTests(BaseMemcachedTests, TestCase):
|
||||||
# libmemcached manages its own connections.
|
# libmemcached manages its own connections.
|
||||||
should_disconnect_on_close = False
|
should_disconnect_on_close = False
|
||||||
|
|
||||||
|
@property
|
||||||
|
def incr_decr_type_error(self):
|
||||||
|
return cache._lib.ClientError
|
||||||
|
|
||||||
@override_settings(CACHES=caches_setting_for_tests(
|
@override_settings(CACHES=caches_setting_for_tests(
|
||||||
base=PyLibMCCache_params,
|
base=PyLibMCCache_params,
|
||||||
exclude=memcached_excluded_caches,
|
exclude=memcached_excluded_caches,
|
||||||
|
@ -1497,6 +1550,10 @@ class PyLibMCCacheTests(BaseMemcachedTests, TestCase):
|
||||||
class PyMemcacheCacheTests(BaseMemcachedTests, TestCase):
|
class PyMemcacheCacheTests(BaseMemcachedTests, TestCase):
|
||||||
base_params = PyMemcacheCache_params
|
base_params = PyMemcacheCache_params
|
||||||
|
|
||||||
|
@property
|
||||||
|
def incr_decr_type_error(self):
|
||||||
|
return cache._lib.exceptions.MemcacheClientError
|
||||||
|
|
||||||
def test_pymemcache_highest_pickle_version(self):
|
def test_pymemcache_highest_pickle_version(self):
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
cache._cache.default_kwargs['serde']._serialize_func.keywords['pickle_version'],
|
cache._cache.default_kwargs['serde']._serialize_func.keywords['pickle_version'],
|
||||||
|
|
Loading…
Reference in New Issue