Fixed #30759 -- Made cache.delete() return whether it succeeded.

Thanks Simon Charette for the review.

9 files changed, 43 insertions, 15 deletions

diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py
index b3a65ab40f..cf9629bc03 100644
--- a/django/core/cache/backends/base.py
+++ b/django/core/cache/backends/base.py
@@ -132,7 +132,8 @@ class BaseCache:
 
     def delete(self, key, version=None):
         """
-        Delete a key from the cache, failing silently.
+        Delete a key from the cache and return whether it succeeded, failing
+        silently.
         """
         raise NotImplementedError('subclasses of BaseCache must provide a delete() method')
 
diff --git a/django/core/cache/backends/db.py b/django/core/cache/backends/db.py
index 17b5209659..33c79c5a14 100644
--- a/django/core/cache/backends/db.py
+++ b/django/core/cache/backends/db.py
@@ -197,7 +197,8 @@ class DatabaseCache(BaseDatabaseCache):
                 return True
 
     def delete(self, key, version=None):
-        self.delete_many([key], version)
+        self.validate_key(key)
+        return self._base_delete_many([self.make_key(key, version)])
 
     def delete_many(self, keys, version=None):
         key_list = []
@@ -208,7 +209,7 @@ class DatabaseCache(BaseDatabaseCache):
 
     def _base_delete_many(self, keys):
         if not keys:
-            return
+            return False
 
         db = router.db_for_write(self.cache_model_class)
         connection = connections[db]
@@ -224,6 +225,7 @@ class DatabaseCache(BaseDatabaseCache):
                 ),
                 keys,
             )
+        return bool(cursor.rowcount)
 
     def has_key(self, key, version=None):
         key = self.make_key(key, version=version)
diff --git a/django/core/cache/backends/dummy.py b/django/core/cache/backends/dummy.py
index 942b72ef89..3913746c49 100644
--- a/django/core/cache/backends/dummy.py
+++ b/django/core/cache/backends/dummy.py
@@ -28,6 +28,7 @@ class DummyCache(BaseCache):
     def delete(self, key, version=None):
         key = self.make_key(key, version=version)
         self.validate_key(key)
+        return False
 
     def has_key(self, key, version=None):
         key = self.make_key(key, version=version)
diff --git a/django/core/cache/backends/filebased.py b/django/core/cache/backends/filebased.py
index 48b8df90ab..3d76f0371b 100644
--- a/django/core/cache/backends/filebased.py
+++ b/django/core/cache/backends/filebased.py
@@ -76,16 +76,17 @@ class FileBasedCache(BaseCache):
             return False
 
     def delete(self, key, version=None):
-        self._delete(self._key_to_file(key, version))
+        return self._delete(self._key_to_file(key, version))
 
     def _delete(self, fname):
         if not fname.startswith(self._dir) or not os.path.exists(fname):
-            return
+            return False
         try:
             os.remove(fname)
         except FileNotFoundError:
             # The file may have been removed by another process.
-            pass
+            return False
+        return True
 
     def has_key(self, key, version=None):
         fname = self._key_to_file(key, version)
diff --git a/django/core/cache/backends/locmem.py b/django/core/cache/backends/locmem.py
index 6058bf5f7e..2573d14497 100644
--- a/django/core/cache/backends/locmem.py
+++ b/django/core/cache/backends/locmem.py
@@ -108,13 +108,14 @@ class LocMemCache(BaseCache):
             del self._cache[key]
             del self._expire_info[key]
         except KeyError:
-            pass
+            return False
+        return True
 
     def delete(self, key, version=None):
         key = self.make_key(key, version=version)
         self.validate_key(key)
         with self._lock:
-            self._delete(key)
+            return self._delete(key)
 
     def clear(self):
         with self._lock:
diff --git a/django/core/cache/backends/memcached.py b/django/core/cache/backends/memcached.py
index 48cfb8310b..8202005045 100644
--- a/django/core/cache/backends/memcached.py
+++ b/django/core/cache/backends/memcached.py
@@ -78,7 +78,7 @@ class BaseMemcachedCache(BaseCache):
 
     def delete(self, key, version=None):
         key = self.make_key(key, version=version)
-        self._cache.delete(key)
+        return bool(self._cache.delete(key))
 
     def get_many(self, keys, version=None):
         key_map = {self.make_key(key, version=version): key for key in keys}
@@ -170,6 +170,13 @@ class MemcachedCache(BaseMemcachedCache):
             return default
         return val
 
+    def delete(self, key, version=None):
+        # python-memcached's delete() returns True when key doesn't exist.
+        # https://github.com/linsomniac/python-memcached/issues/170
+        # Call _deletetouch() without the NOT_FOUND in expected results.
+        key = self.make_key(key, version=version)
+        return bool(self._cache._deletetouch([b'DELETED'], 'delete', key))
+
 
 class PyLibMCCache(BaseMemcachedCache):
     "An implementation of a cache binding using pylibmc"
diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt
index de114e7098..f725e4e056 100644
--- a/docs/releases/3.1.txt
+++ b/docs/releases/3.1.txt
@@ -118,6 +118,9 @@ Cache
   field names in the ``no-cache`` directive for the ``Cache-Control`` header,
   according to :rfc:`7234#section-5.2.2.2`.
 
+* :meth:`~django.core.caches.cache.delete` now returns ``True`` if the key was
+  successfully deleted, ``False`` otherwise.
+
 CSRF
 ~~~~
 
diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt
index ce030d4221..a038183d0d 100644
--- a/docs/topics/cache.txt
+++ b/docs/topics/cache.txt
@@ -724,6 +724,7 @@ a cached item, for example:
     # cache key for {% cache 500 sidebar username %}
     >>> key = make_template_fragment_key('sidebar', [username])
     >>> cache.delete(key) # invalidates cached template fragment
+    True
 
 .. _low-level-cache-api:
 
@@ -884,6 +885,14 @@ You can delete keys explicitly with ``delete()`` to clear the cache for a
 particular object::
 
     >>> cache.delete('a')
+    True
+
+``delete()`` returns ``True`` if the key was successfully deleted, ``False``
+otherwise.
+
+.. versionchanged:: 3.1
+
+    The boolean return value was added.
 
 .. method:: cache.delete_many(keys, version=None)
 
diff --git a/tests/cache/tests.py b/tests/cache/tests.py
index 1b2a9490f6..eebcaf6cb5 100644
--- a/tests/cache/tests.py
+++ b/tests/cache/tests.py
@@ -105,7 +105,7 @@ class DummyCacheTests(SimpleTestCase):
         "Cache deletion is transparently ignored on the dummy cache backend"
         cache.set_many({'key1': 'spam', 'key2': 'eggs'})
         self.assertIsNone(cache.get("key1"))
-        cache.delete("key1")
+        self.assertFalse(cache.delete("key1"))
         self.assertIsNone(cache.get("key1"))
         self.assertIsNone(cache.get("key2"))
 
@@ -315,10 +315,13 @@ class BaseCacheTests:
         # Cache keys can be deleted
         cache.set_many({'key1': 'spam', 'key2': 'eggs'})
         self.assertEqual(cache.get("key1"), "spam")
-        cache.delete("key1")
+        self.assertTrue(cache.delete("key1"))
         self.assertIsNone(cache.get("key1"))
         self.assertEqual(cache.get("key2"), "eggs")
 
+    def test_delete_nonexistent(self):
+        self.assertFalse(cache.delete('nonexistent_key'))
+
     def test_has_key(self):
         # The cache can be inspected for cache keys
         cache.set("hello1", "goodbye1")
@@ -741,25 +744,25 @@ class BaseCacheTests:
     def test_cache_versioning_delete(self):
         cache.set('answer1', 37, version=1)
         cache.set('answer1', 42, version=2)
-        cache.delete('answer1')
+        self.assertTrue(cache.delete('answer1'))
         self.assertIsNone(cache.get('answer1', version=1))
         self.assertEqual(cache.get('answer1', version=2), 42)
 
         cache.set('answer2', 37, version=1)
         cache.set('answer2', 42, version=2)
-        cache.delete('answer2', version=2)
+        self.assertTrue(cache.delete('answer2', version=2))
         self.assertEqual(cache.get('answer2', version=1), 37)
         self.assertIsNone(cache.get('answer2', version=2))
 
         cache.set('answer3', 37, version=1)
         cache.set('answer3', 42, version=2)
-        caches['v2'].delete('answer3')
+        self.assertTrue(caches['v2'].delete('answer3'))
         self.assertEqual(cache.get('answer3', version=1), 37)
         self.assertIsNone(cache.get('answer3', version=2))
 
         cache.set('answer4', 37, version=1)
         cache.set('answer4', 42, version=2)
-        caches['v2'].delete('answer4', version=1)
+        self.assertTrue(caches['v2'].delete('answer4', version=1))
         self.assertIsNone(cache.get('answer4', version=1))
         self.assertEqual(cache.get('answer4', version=2), 42)