Fixed #29867 -- Added support for storing None value in caches.

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 @@ def get_many(self, keys, version=None):
         """
         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 @@ def get_or_set(self, key, default, timeout=DEFAULT_TIMEOUT, version=None):
 
         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 @@ def incr_version(self, key, delta=1, version=None):
         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)

django/core/cache/backends/memcached.py

@@ -165,6 +165,11 @@ def validate_key(self, key):
 
 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 '

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

docs/topics/cache.txt

@@ -839,9 +839,21 @@ If the object doesn't exist in the cache, ``cache.get()`` returns ``None``::
     >>> cache.get('my_key')
     None
 
-We advise against storing the literal value ``None`` in the cache, because you
-won't be able to distinguish between your stored ``None`` value and a cache
-miss signified by a return value of ``None``.
+If you need to determine whether the object exists in the cache and you have
+stored a literal value ``None``, use a sentinel object as the default::
+
+    >>> sentinel = object()
+    >>> cache.get('my_key', sentinel) is sentinel
+    False
+    >>> # Wait 30 seconds for 'my_key' to expire...
+    >>> cache.get('my_key', sentinel) is sentinel
+    True
+
+.. admonition:: ``MemcachedCache``
+
+    Due to a ``python-memcached`` limitation, it's not possible to distinguish
+    between stored ``None`` value and a cache miss signified by a return value
+    of ``None`` on the deprecated ``MemcachedCache`` backend.
 
 ``cache.get()`` can take a ``default`` argument. This specifies which value to
 return if the object doesn't exist in the cache::

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 @@ def test_get_many(self):
         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 @@ def test_has_key(self):
         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 @@ def test_incr(self):
         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 @@ def test_decr(self):
         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 @@ def test_incr_version(self):
         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 @@ def test_decr_version(self):
         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 @@ def test_get_or_set(self):
         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 @@ def my_callable():
         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 @@ def fail_set_multi(mapping, *args, **kwargs):
 ))
 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 @@ def test_pylibmc_client_servers(self):
 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'],