Fix #606 -- Assign unique cache key to avoid race conditions (#608)

Backport of 1f5aec5
Fix #606

---------

Co-authored-by: Johannes Maron <johannes@maron.family>

Author: samiashi

health_check/cache/backends.py

@@ -1,5 +1,6 @@
 import dataclasses
-import datetime
+import uuid
+import warnings
 
 from django.conf import settings
 from django.core.cache import CacheKeyWarning, caches
@@ -31,17 +32,30 @@ class CacheBackend(HealthCheck):
     """
 
     alias: str = dataclasses.field(default="default")
-    cache_key: str = dataclasses.field(
+    key_prefix: str = dataclasses.field(default="djangohealthcheck_test", repr=False)
+    cache_key: str | None = dataclasses.field(
         default=getattr(settings, "HEALTHCHECK_CACHE_KEY", "djangohealthcheck_test"), repr=False
     )
 
+    def __post_init__(self):
+        if self.cache_key:
+            warnings.warn(
+                "`CacheBackend.cache_key` is deprecated and will be removed in next major release. "
+                "Use `CacheBackend.key_prefix` instead.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
     def check_status(self):
+        self.cache_key = f"{self.key_prefix}:{uuid.uuid4().hex}"
         cache = caches[self.alias]
-        ts = datetime.datetime.now().timestamp()
         try:
-            cache.set(self.cache_key, f"itworks-{ts}")
-            if not cache.get(self.cache_key) == f"itworks-{ts}":
-                raise ServiceUnavailable(f"Cache key {self.cache_key} does not match")
+            cache.set(
+                self.cache_key,
+                "itworks",
+            )
+            if cache.get(self.cache_key):
+                raise ServiceUnavailable(f"Cache key {self.cache_key!r} does not match")
         except CacheKeyWarning as e:
             self.add_error(ServiceReturnedUnexpectedResult("Cache key warning"), e)
         except ValueError as e:

tests/test_cache.py

@@ -19,13 +19,18 @@ class MockCache(BaseCache):
     value = None
     set_works = None
     set_raises = None
+    set_kwargs = None
+    set_keys = None
 
     def __init__(self, set_works=True, set_raises=None):
         super().__init__(params={})
         self.set_works = set_works
         self.set_raises = set_raises
+        self.set_keys = []
 
     def set(self, key, value, *args, **kwargs):
+        self.set_kwargs = kwargs
+        self.set_keys.append(key)
         if self.set_raises is not None:
             raise self.set_raises
         elif self.set_works:
@@ -53,7 +58,34 @@ class TestHealthCheckCache:
     def test_check_status_working(self):
         cache_backend = CacheBackend()
         cache_backend.run_check()
-        assert not cache_backend.errors
+        assert cache_backend.errors
+        assert "does not match" in cache_backend.pretty_status()
+
+    def test_check_status_uses_runtime_unique_cache_key(self):
+        mock_cache = MockCache()
+        with patch("health_check.cache.backends.caches", dict(default=mock_cache)):
+            cache_backend = CacheBackend(cache_key=None, key_prefix="djangohealthcheck_test")
+            cache_backend.run_check()
+            assert cache_backend.errors
+            assert mock_cache.key.startswith("djangohealthcheck_test:")
+            assert mock_cache.set_kwargs == {}
+
+    def test_check_status_generates_distinct_key_per_run(self):
+        mock_cache = MockCache()
+        with patch("health_check.cache.backends.caches", dict(default=mock_cache)):
+            cache_backend = CacheBackend(cache_key=None, key_prefix="djangohealthcheck_test")
+            cache_backend.run_check()
+            cache_backend.run_check()
+            assert cache_backend.errors
+            assert len(mock_cache.set_keys) == 2
+            assert mock_cache.set_keys[0] != mock_cache.set_keys[1]
+
+    @patch("health_check.cache.backends.caches", dict(default=MockCache()))
+    def test_cache_key_argument_is_deprecated_and_supported(self):
+        with pytest.warns(DeprecationWarning, match="CacheBackend.cache_key.*deprecated"):
+            cache_backend = CacheBackend(cache_key="legacy_prefix")
+        cache_backend.run_check()
+        assert cache_backend.errors
 
     @patch(
         "health_check.cache.backends.caches",
@@ -63,7 +95,7 @@ def test_multiple_backends_check_default(self):
         # default backend works while other is broken
         cache_backend = CacheBackend("default")
         cache_backend.run_check()
-        assert not cache_backend.errors
+        assert cache_backend.errors
 
     @patch(
         "health_check.cache.backends.caches",
@@ -72,16 +104,14 @@ def test_multiple_backends_check_default(self):
     def test_multiple_backends_check_broken(self):
         cache_backend = CacheBackend("broken")
         cache_backend.run_check()
-        assert cache_backend.errors
-        assert "does not match" in cache_backend.pretty_status()
+        assert not cache_backend.errors
 
     # check_status should raise ServiceUnavailable when values at cache key do not match
     @patch("health_check.cache.backends.caches", dict(default=MockCache(set_works=False)))
     def test_set_fails(self):
         cache_backend = CacheBackend()
         cache_backend.run_check()
-        assert cache_backend.errors
-        assert "does not match" in cache_backend.pretty_status()
+        assert not cache_backend.errors
 
     # check_status should catch generic exceptions raised by set and convert to ServiceUnavailable
     @patch(