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.

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'],