feat(backends)!: make delete() return whether a key was removed

changelog.d/71.changed.md

@@ -0,0 +1,9 @@
+**`BaseCacheBackend.delete()` returns whether a key was removed.** The memory,
+Redis and Memcached backends return `True` when the key held an entry and
+`False` otherwise; on memory an entry that had already expired counts as
+absent, as it does on Redis. The base `delete_many()` fallback now counts only
+the keys that existed, and the non-atomic `get_and_delete()` and
+`delete_if_equals()` fallbacks use the result, so a caller whose delete found
+the key already gone no longer wins as well. A third-party backend that declares `-> None` fails type
+checking and should return the result instead. See the
+[migration guide](https://fastapi-cachex.readthedocs.io/en/stable/MIGRATING_0_4/#backend-delete).

fastapi_cachex/backends/base.py

@@ -140,21 +140,24 @@ async def set(self, key: str, value: CacheEntry, ttl: int | None = None) -> None
         """
 
     @abstractmethod
-    async def delete(self, key: str) -> None:
-        """Remove a response from the cache."""
+    async def delete(self, key: str) -> bool:
+        """Remove a response from the cache.
+
+        Returns:
+            Whether ``key`` held an entry that had not expired yet
+        """
 
     async def delete_many(self, keys: Iterable[str]) -> int:
         """Remove every key in ``keys``; returns how many were removed.
 
-        The base implementation deletes one key at a time and reports how
-        many were attempted, since ``delete`` does not say whether the key
-        existed. The built-in backends override it and count what was
-        actually removed.
+        The base implementation deletes one key at a time and counts the
+        deletes that found an entry. The built-in backends override it with
+        batched deletes.
         """
         count = 0
         for key in keys:
-            await self.delete(key)
-            count += 1
+            if await self.delete(key):
+                count += 1
         return count
 
     async def get_and_delete(self, key: str) -> CacheEntry | None:
@@ -172,8 +175,10 @@ async def get_and_delete(self, key: str) -> CacheEntry | None:
             The entry that was stored under ``key``, or ``None`` if there was none
         """
         value = await self.get(key)
-        if value is not None:
-            await self.delete(key)
+        if value is None or not await self.delete(key):
+            # Absent, or another caller removed it between the get and the
+            # delete: that caller got the entry.
+            return None
         return value
 
     async def set_if_absent(
@@ -226,8 +231,7 @@ async def delete_if_equals(self, key: str, expected: CacheEntry) -> bool:
         """
         if await self.get(key) != expected:
             return False
-        await self.delete(key)
-        return True
+        return await self.delete(key)
 
     async def expire_if_equals(self, key: str, expected: CacheEntry, ttl: int) -> bool:
         """Update expiry on ``key`` to ``ttl`` seconds only while it still holds ``expected``.

fastapi_cachex/backends/memcached.py

@@ -443,15 +443,21 @@ async def increment(self, key: str, delta: int = 1, ttl: int | None = None) -> i
         logger.debug("Memcached INCREMENT; key=%s value=%s ttl=%s", key, value, ttl)
         return value
 
-    async def delete(self, key: str) -> None:
+    async def delete(self, key: str) -> bool:
         """Delete value from cache.
 
         Args:
             key: Cache key to delete
+
+        Returns:
+            Whether the server held ``key``
         """
         prefixed = self._make_key(key)
-        await asyncio.to_thread(self.client.delete, prefixed)
-        logger.debug("Memcached DELETE; key=%s", key)
+        removed = bool(
+            await asyncio.to_thread(self.client.delete, prefixed, noreply=False)
+        )
+        logger.debug("Memcached DELETE; key=%s removed=%s", key, removed)
+        return removed
 
     async def delete_many(self, keys: Iterable[str]) -> int:
         """Remove every key in ``keys`` in one worker call; returns how many existed.
@@ -525,17 +531,14 @@ async def clear_path(self, path: str, include_params: bool = False) -> int:
         )
 
         # Try to delete the prefixed key (exact match only)
-        prefixed_key = self._make_key(path)
-        result = await asyncio.to_thread(
-            self.client.delete, prefixed_key, noreply=False
-        )
+        removed = int(await self.delete(path))
         logger.debug(
             "Memcached CLEAR_PATH; path=%s include_params=%s removed=%s",
             path,
             include_params,
-            1 if result else 0,
+            removed,
         )
-        return 1 if result else 0
+        return removed
 
     async def clear_pattern(self, pattern: str) -> int:
         """Clear cached responses matching a pattern.

fastapi_cachex/backends/memory.py

@@ -166,11 +166,13 @@ async def set(self, key: str, value: CacheEntry, ttl: int | None = None) -> None
             self.cache[key] = CacheItem(value=value, expiry=expiry)
             logger.debug("Memory cache SET; key=%s ttl=%s", key, ttl)
 
-    async def delete(self, key: str) -> None:
-        """Remove a response from the cache."""
+    async def delete(self, key: str) -> bool:
+        """Remove a response from the cache; returns whether it had not expired."""
         async with self.lock:
-            self.cache.pop(key, None)
-            logger.debug("Memory cache DELETE; key=%s", key)
+            item = self.cache.pop(key, None)
+            removed = item is not None and _is_live(item, time.time())
+            logger.debug("Memory cache DELETE; key=%s removed=%s", key, removed)
+            return removed
 
     async def delete_many(self, keys: Iterable[str]) -> int:
         """Remove every key in ``keys`` under one lock; returns how many existed."""

fastapi_cachex/backends/redis.py

@@ -309,10 +309,11 @@ async def set(self, key: str, value: CacheEntry, ttl: int | None = None) -> None
         await self.client.set(self._make_key(key), encode_entry(value), ex=ttl)
         logger.debug("Redis SET; key=%s ttl=%s", key, ttl)
 
-    async def delete(self, key: str) -> None:
-        """Remove a response from the cache."""
-        await self.client.delete(self._make_key(key))
-        logger.debug("Redis DELETE; key=%s", key)
+    async def delete(self, key: str) -> bool:
+        """Remove a response from the cache; returns whether the key existed."""
+        removed = bool(await self.client.delete(self._make_key(key)))
+        logger.debug("Redis DELETE; key=%s removed=%s", key, removed)
+        return removed
 
     async def delete_many(self, keys: Iterable[str]) -> int:
         """Remove every key in ``keys`` with batched DELs; returns how many existed."""

tests/backends/test_base.py

@@ -24,8 +24,8 @@ async def get(self, key: str) -> CacheEntry | None:
     async def set(self, key: str, value: CacheEntry, ttl: int | None = None) -> None:
         self.store[key] = (value, ttl)
 
-    async def delete(self, key: str) -> None:
-        self.store.pop(key, None)
+    async def delete(self, key: str) -> bool:
+        return self.store.pop(key, None) is not None
 
     async def clear(self) -> None:
         self.store.clear()
@@ -76,11 +76,29 @@ async def test_get_and_delete_fallback_returns_then_removes(
     assert await backend.get_and_delete("once") is None
 
 
+async def test_fallbacks_lose_to_a_delete_between_get_and_delete(
+    backend: DictBackend, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Another caller removed the key after the get: only it got the entry."""
+    value = CacheEntry(fingerprint="e", content=b"once")
+    await backend.set("once", value)
+
+    async def lost_the_race(key: str) -> bool:
+        backend.store.pop(key, None)
+        return False
+
+    monkeypatch.setattr(backend, "delete", lost_the_race)
+
+    assert await backend.get_and_delete("once") is None
+    await backend.set("once", value)
+    assert await backend.delete_if_equals("once", value) is False
+
+
 async def test_delete_many_fallback_deletes_one_by_one(backend: DictBackend) -> None:
     await backend.set("a", CacheEntry(fingerprint="e", content=b"1"))
     await backend.set("b", CacheEntry(fingerprint="e", content=b"2"))
 
-    assert await backend.delete_many(["a", "b", "missing"]) == 3
+    assert await backend.delete_many(["a", "b", "missing"]) == 2
     assert backend.store == {}
 
 

tests/backends/test_memcached.py

@@ -113,10 +113,11 @@ async def test_memcached_delete(memcached_backend: MemcachedBackend):
     value = CacheEntry(fingerprint="test_etag", content=b"test_content")
 
     await memcached_backend.set(key, value)
-    await memcached_backend.delete(key)
+    assert await memcached_backend.delete(key) is True
     retrieved_value = await memcached_backend.get(key)
 
     assert retrieved_value is None
+    assert await memcached_backend.delete(key) is False
 
 
 @requires_memcached
@@ -1079,6 +1080,18 @@ async def test_memcached_delete_many_sends_each_namespaced_key_once() -> None:
     )
 
 
+@pytest.mark.parametrize("present", [True, False])
+async def test_memcached_delete_reports_what_the_server_answered(
+    present: bool,
+) -> None:
+    backend = stubbed_backend()
+    backend.client.delete.return_value = present
+
+    assert await backend.delete("k") is present
+
+    backend.client.delete.assert_called_once_with(backend._make_key("k"), noreply=False)
+
+
 async def test_memcached_delete_many_with_no_keys_does_no_io() -> None:
     backend = stubbed_backend()
 

tests/backends/test_memory.py

@@ -69,10 +69,11 @@ async def test_memory_backend_delete(memory_backend: MemoryBackend):
     ttl = 60
 
     await memory_backend.set(key, value, ttl)
-    await memory_backend.delete(key)
+    assert await memory_backend.delete(key) is True
     retrieved_value = await memory_backend.get(key)
 
     assert retrieved_value is None
+    assert await memory_backend.delete(key) is False
 
 
 async def test_memory_backend_clear(memory_backend: MemoryBackend):
@@ -599,8 +600,9 @@ async def test_memory_backend_enumeration_skips_expired_entries(
         lambda backend: backend.clear_pattern("stale"),
         lambda backend: backend.clear_path("stale"),
         lambda backend: backend.delete_many(["stale"]),
+        lambda backend: backend.delete("stale"),
     ],
-    ids=["clear_pattern", "clear_path", "delete_many"],
+    ids=["clear_pattern", "clear_path", "delete_many", "delete"],
 )
 async def test_memory_backend_does_not_count_expired_entries_as_cleared(
     memory_backend: MemoryBackend, clear: Callable[[MemoryBackend], Awaitable[int]]

tests/backends/test_redis.py

@@ -353,8 +353,9 @@ async def test_set_with_ttl(self, async_redis_backend: AsyncRedisCacheBackend):
     async def test_delete(self, async_redis_backend: AsyncRedisCacheBackend):
         value = CacheEntry(fingerprint="test-etag", content=b"test-content")
         await async_redis_backend.set("test-key", value)
-        await async_redis_backend.delete("test-key")
+        assert await async_redis_backend.delete("test-key") is True
         assert await async_redis_backend.get("test-key") is None
+        assert await async_redis_backend.delete("test-key") is False
 
     @requires_redis
     async def test_clear(self, async_redis_backend: AsyncRedisCacheBackend):

tests/session/test_manager.py

@@ -755,9 +755,9 @@ def __init__(self) -> None:
             self.deletes = 0
             self.batches: list[int] = []
 
-        async def delete(self, key: str) -> None:
+        async def delete(self, key: str) -> bool:
             self.deletes += 1
-            await super().delete(key)
+            return await super().delete(key)
 
         async def delete_many(self, keys: Iterable[str]) -> int:
             keys = list(keys)