Browse Source

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

Thanks Simon Charette for the review.
daniel a rios 5 years ago
parent
commit
efc3e32d6d

+ 2 - 1
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')
 

+ 4 - 2
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)

+ 1 - 0
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)

+ 4 - 3
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)

+ 3 - 2
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:

+ 8 - 1
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"

+ 3 - 0
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
 ~~~~
 

+ 9 - 0
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)
 

+ 9 - 6
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)