diff options
| -rw-r--r-- | django/core/cache/backends/base.py | 30 | ||||
| -rw-r--r-- | django/core/cache/backends/memcached.py | 5 | ||||
| -rw-r--r-- | docs/releases/3.2.txt | 8 | ||||
| -rw-r--r-- | tests/cache/tests.py | 65 |
4 files changed, 89 insertions, 19 deletions
diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py index 22b8397cac..1e2c7c9509 100644 --- a/django/core/cache/backends/base.py +++ b/django/core/cache/backends/base.py @@ -52,6 +52,8 @@ def get_key_func(key_func): class BaseCache: + _missing_key = object() + def __init__(self, params): timeout = params.get('timeout', params.get('TIMEOUT', 300)) if timeout is not None: @@ -151,8 +153,8 @@ class BaseCache: """ d = {} for k in keys: - val = self.get(k, version=version) - if val is not None: + val = self.get(k, self._missing_key, version=version) + if val is not self._missing_key: d[k] = val return d @@ -165,31 +167,29 @@ class BaseCache: Return the value of the key stored or retrieved. """ - val = self.get(key, version=version) - if val is None: + val = self.get(key, self._missing_key, version=version) + if val is self._missing_key: if callable(default): default = default() - if default is not None: - self.add(key, default, timeout=timeout, version=version) - # Fetch the value again to avoid a race condition if another - # caller added a value between the first get() and the add() - # above. - return self.get(key, default, version=version) + self.add(key, default, timeout=timeout, version=version) + # Fetch the value again to avoid a race condition if another caller + # added a value between the first get() and the add() above. + return self.get(key, default, version=version) return val def has_key(self, key, version=None): """ 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): """ Add delta to value in the cache. If the key does not exist, raise a ValueError exception. """ - value = self.get(key, version=version) - if value is None: + value = self.get(key, self._missing_key, version=version) + if value is self._missing_key: raise ValueError("Key '%s' not found" % key) new_value = value + delta self.set(key, new_value, version=version) @@ -257,8 +257,8 @@ class BaseCache: if version is None: version = self.version - value = self.get(key, version=version) - if value is None: + value = self.get(key, self._missing_key, version=version) + if value is self._missing_key: raise ValueError("Key '%s' not found" % key) self.set(key, value, version=version + delta) diff --git a/django/core/cache/backends/memcached.py b/django/core/cache/backends/memcached.py index 9a717359b8..112dbdd1ff 100644 --- a/django/core/cache/backends/memcached.py +++ b/django/core/cache/backends/memcached.py @@ -165,6 +165,11 @@ class BaseMemcachedCache(BaseCache): class MemcachedCache(BaseMemcachedCache): "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): warnings.warn( 'MemcachedCache is deprecated in favor of PyMemcacheCache and ' diff --git a/docs/releases/3.2.txt b/docs/releases/3.2.txt index 5b1969698c..8cd77ffad0 100644 --- a/docs/releases/3.2.txt +++ b/docs/releases/3.2.txt @@ -681,6 +681,14 @@ Miscellaneous ``UserChangeForm.clean_password()`` is no longer required to return the 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: Features deprecated in 3.2 diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 2853ada233..9d79e6e758 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -278,6 +278,14 @@ class BaseCacheTests: # A common set of tests to apply to all cache backends 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): 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', '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): # Cache keys can be deleted @@ -339,12 +349,22 @@ class BaseCacheTests: self.assertIs(cache.has_key("goodbye1"), False) cache.set("no_expiry", "here", None) 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): # The in operator can be used to inspect cache contents cache.set("hello2", "goodbye2") self.assertIn("hello2", 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): # Cache values can be incremented @@ -356,6 +376,9 @@ class BaseCacheTests: self.assertEqual(cache.incr('answer', -10), 42) with self.assertRaises(ValueError): cache.incr('does_not_exist') + cache.set('null', None) + with self.assertRaises(self.incr_decr_type_error): + cache.incr('null') def test_decr(self): # Cache values can be decremented @@ -367,6 +390,9 @@ class BaseCacheTests: self.assertEqual(cache.decr('answer', -10), 42) with self.assertRaises(ValueError): cache.decr('does_not_exist') + cache.set('null', None) + with self.assertRaises(self.incr_decr_type_error): + cache.decr('null') def test_close(self): self.assertTrue(hasattr(cache, 'close')) @@ -914,6 +940,13 @@ class BaseCacheTests: with self.assertRaises(ValueError): 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): cache.set('answer', 42, version=2) self.assertIsNone(cache.get('answer')) @@ -938,6 +971,13 @@ class BaseCacheTests: with self.assertRaises(ValueError): 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): # Two caches with different key functions aren't visible to each other cache.set('answer1', 42) @@ -995,6 +1035,11 @@ class BaseCacheTests: self.assertEqual(cache.get_or_set('projector', 42), 42) self.assertEqual(cache.get('projector'), 42) 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 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') - def test_get_or_set_callable_returning_none(self): - self.assertIsNone(cache.get_or_set('mykey', lambda: None)) - # Previous get_or_set() doesn't store None in the cache. - self.assertEqual(cache.get('mykey', 'default'), 'default') + self.assertIsNone(cache.get_or_set('null', lambda: 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_version(self): 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): base_params = MemcachedCache_params + supports_get_with_default = False + incr_decr_type_error = ValueError def test_memcached_uses_highest_pickle_version(self): # Regression test for #19810 @@ -1459,6 +1508,10 @@ class PyLibMCCacheTests(BaseMemcachedTests, TestCase): # libmemcached manages its own connections. should_disconnect_on_close = False + @property + def incr_decr_type_error(self): + return cache._lib.ClientError + @override_settings(CACHES=caches_setting_for_tests( base=PyLibMCCache_params, exclude=memcached_excluded_caches, @@ -1497,6 +1550,10 @@ class PyLibMCCacheTests(BaseMemcachedTests, TestCase): class PyMemcacheCacheTests(BaseMemcachedTests, TestCase): base_params = PyMemcacheCache_params + @property + def incr_decr_type_error(self): + return cache._lib.exceptions.MemcacheClientError + def test_pymemcache_highest_pickle_version(self): self.assertEqual( cache._cache.default_kwargs['serde']._serialize_func.keywords['pickle_version'], |