diff --git a/tests/test_asyncio/test_cache.py b/tests/test_asyncio/test_cache.py index 375fb502..993da2c2 100644 --- a/tests/test_asyncio/test_cache.py +++ b/tests/test_asyncio/test_cache.py @@ -300,6 +300,35 @@ async def test_flush_entire_cache(self, r): assert await r.get("c") == "1" +class TestLocalCacheEviction: + """Unit tests for the _LocalCache eviction bookkeeping (no server needed).""" + + @pytest.mark.parametrize("eviction_policy", list(EvictionPolicy)) + async def test_eviction_with_ttl_does_not_raise(self, eviction_policy): + # a cache configured with a max size and a ttl used to raise KeyError + # from _is_expired() on its second eviction + cache = _LocalCache(max_size=2, ttl=60, eviction_policy=eviction_policy) + for i in range(10): + cache.set(f"GET key{i}", i, [f"key{i}"]) + assert len(cache.cache) == 2 + + @pytest.mark.parametrize("eviction_policy", list(EvictionPolicy)) + async def test_evicted_commands_are_dropped_from_ttl_list(self, eviction_policy): + cache = _LocalCache(max_size=2, ttl=60, eviction_policy=eviction_policy) + for i in range(5): + cache.set(f"GET key{i}", i, [f"key{i}"]) + # commands_ttl_list must only track the commands still cached + assert sorted(cache.commands_ttl_list) == sorted(cache.cache) + assert len(cache.commands_ttl_list) == len(set(cache.commands_ttl_list)) + + async def test_recaching_a_command_does_not_duplicate_ttl_entry(self): + cache = _LocalCache(max_size=2, ttl=60) + cache.set("GET key", 1, ["key"]) + cache.set("GET key", 2, ["key"]) + assert cache.commands_ttl_list == ["GET key"] + assert cache.get("GET key") == 2 + + @pytest.mark.skipif(LIBVALKEY_AVAILABLE, reason="PythonParser only") @pytest.mark.onlycluster class TestClusterLocalCache: diff --git a/tests/test_cache.py b/tests/test_cache.py index c6ba16af..8334dd5f 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -418,6 +418,35 @@ def test_get_from_cache_invalidate_via_get(self, r, r2): assert r.get("foo") == b"barbar" +class TestLocalCacheEviction: + """Unit tests for the _LocalCache eviction bookkeeping (no server needed).""" + + @pytest.mark.parametrize("eviction_policy", list(EvictionPolicy)) + def test_eviction_with_ttl_does_not_raise(self, eviction_policy): + # a cache configured with a max size and a ttl used to raise KeyError + # from _is_expired() on its second eviction + cache = _LocalCache(max_size=2, ttl=60, eviction_policy=eviction_policy) + for i in range(10): + cache.set(f"GET key{i}", i, [f"key{i}"]) + assert len(cache.cache) == 2 + + @pytest.mark.parametrize("eviction_policy", list(EvictionPolicy)) + def test_evicted_commands_are_dropped_from_ttl_list(self, eviction_policy): + cache = _LocalCache(max_size=2, ttl=60, eviction_policy=eviction_policy) + for i in range(5): + cache.set(f"GET key{i}", i, [f"key{i}"]) + # commands_ttl_list must only track the commands still cached + assert sorted(cache.commands_ttl_list) == sorted(cache.cache) + assert len(cache.commands_ttl_list) == len(set(cache.commands_ttl_list)) + + def test_recaching_a_command_does_not_duplicate_ttl_entry(self): + cache = _LocalCache(max_size=2, ttl=60) + cache.set("GET key", 1, ["key"]) + cache.set("GET key", 2, ["key"]) + assert cache.commands_ttl_list == ["GET key"] + assert cache.get("GET key") == 2 + + @pytest.mark.skipif(LIBVALKEY_AVAILABLE, reason="PythonParser only") @pytest.mark.onlycluster class TestClusterLocalCache: diff --git a/valkey/_cache.py b/valkey/_cache.py index fa78403b..baf5bab8 100644 --- a/valkey/_cache.py +++ b/valkey/_cache.py @@ -254,6 +254,11 @@ def set( _ACCESS_COUNT: 0, # Used only for LFU } self._update_key_commands_map(keys_in_command, command) + if command in self.commands_ttl_list: + # the command is cached again, refresh its position so that + # commands_ttl_list has no duplicates and stays in sync with + # the commands currently held by the cache. + self.commands_ttl_list.remove(command) self.commands_ttl_list.append(command) def get(self, command: Union[str, Sequence[str]]) -> ResponseT | None: @@ -345,15 +350,15 @@ def _evict(self): if self._is_expired(self.commands_ttl_list[0]): self.delete_command(self.commands_ttl_list[0]) elif self.eviction_policy == EvictionPolicy.LRU: - self.cache.popitem(last=False) + self.delete_command(next(iter(self.cache))) elif self.eviction_policy == EvictionPolicy.LFU: min_access_command = min( self.cache, key=lambda k: self.cache[k].get("access_count", 0) ) - self.cache.pop(min_access_command) + self.delete_command(min_access_command) elif self.eviction_policy == EvictionPolicy.RANDOM: random_command = random.choice(list(self.cache.keys())) - self.cache.pop(random_command) + self.delete_command(random_command) def _update_key_commands_map( self, keys: List[KeyT], command: Union[str, Sequence[str]]