From a630791c754730cf0389e7bafd8c00dbdc8f8fd7 Mon Sep 17 00:00:00 2001 From: Shubham Kaudewar Date: Sun, 7 Sep 2025 02:37:21 +0530 Subject: [PATCH 1/4] Fix for _current_command_cache_key generating same against key for different fields --- redis/cache.py | 1 + redis/connection.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/redis/cache.py b/redis/cache.py index cb29ffe785..52065fbe3c 100644 --- a/redis/cache.py +++ b/redis/cache.py @@ -19,6 +19,7 @@ class EvictionPolicyType(Enum): class CacheKey: command: str redis_keys: tuple + redis_args: tuple = () class CacheEntry: diff --git a/redis/connection.py b/redis/connection.py index 0e7de5a584..eabbd34e1f 100644 --- a/redis/connection.py +++ b/redis/connection.py @@ -1214,7 +1214,7 @@ def send_command(self, *args, **kwargs): with self._cache_lock: # Command is write command or not allowed # to be cached. - if not self._cache.is_cachable(CacheKey(command=args[0], redis_keys=())): + if not self._cache.is_cachable(CacheKey(command=args[0], redis_keys=(), redis_args=())): self._current_command_cache_key = None self._conn.send_command(*args, **kwargs) return @@ -1224,7 +1224,7 @@ def send_command(self, *args, **kwargs): # Creates cache key. self._current_command_cache_key = CacheKey( - command=args[0], redis_keys=tuple(kwargs.get("keys")) + command=args[0], redis_keys=tuple(kwargs.get("keys")), redis_args=args ) with self._cache_lock: From 3590399d30d7658df61f157b3500947010c59c78 Mon Sep 17 00:00:00 2001 From: Shubham Kaudewar Date: Mon, 8 Sep 2025 20:33:41 +0530 Subject: [PATCH 2/4] correction in test_read_response_returns_cached_reply test case for new redis_args field --- tests/test_connection.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/test_connection.py b/tests/test_connection.py index 89ea04df75..8cd5588067 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -348,12 +348,17 @@ def test_format_error_message(conn, error, expected_message): def test_network_connection_failure(): - exp_err = f"Error {ECONNREFUSED} connecting to localhost:9999. Connection refused." + # Match only the stable part of the error message across OS + exp_err = rf"Error {ECONNREFUSED} connecting to localhost:9999\." with pytest.raises(ConnectionError, match=exp_err): redis = Redis(port=9999) redis.set("a", "b") +@pytest.mark.skipif( + not hasattr(socket, "AF_UNIX"), + reason="Unix domain sockets not supported on this platform" +) def test_unix_socket_connection_failure(): exp_err = "Error 2 connecting to unix:///tmp/a.sock. No such file or directory." with pytest.raises(ConnectionError, match=exp_err): @@ -463,25 +468,25 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): None, None, CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",)), + cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), cache_value=CacheProxyConnection.DUMMY_CACHE_VALUE, status=CacheEntryStatus.IN_PROGRESS, connection_ref=mock_connection, ), CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",)), + cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, ), CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",)), + cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, ), CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",)), + cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, @@ -503,7 +508,7 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): [ call( CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",)), + cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), cache_value=CacheProxyConnection.DUMMY_CACHE_VALUE, status=CacheEntryStatus.IN_PROGRESS, connection_ref=mock_connection, @@ -511,7 +516,7 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): ), call( CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",)), + cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, @@ -522,9 +527,9 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): mock_cache.get.assert_has_calls( [ - call(CacheKey(command="GET", redis_keys=("foo",))), - call(CacheKey(command="GET", redis_keys=("foo",))), - call(CacheKey(command="GET", redis_keys=("foo",))), + call(CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo"))), + call(CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo"))), + call(CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo"))), ] ) From 728c2056323850a0840fd30f51a38833f36f48f0 Mon Sep 17 00:00:00 2001 From: Shubham Kaudewar <38852978+ShubhamKaudewar@users.noreply.github.com> Date: Tue, 16 Sep 2025 23:20:41 +0530 Subject: [PATCH 3/4] Update redis/cache.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- redis/cache.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/redis/cache.py b/redis/cache.py index 52065fbe3c..ee99da64a0 100644 --- a/redis/cache.py +++ b/redis/cache.py @@ -17,9 +17,20 @@ class EvictionPolicyType(Enum): @dataclass(frozen=True) class CacheKey: + """ + Represents a unique key for a cache entry. + + Attributes: + command (str): The Redis command being cached. + redis_keys (tuple): The Redis keys involved in the command. + redis_args (tuple): Additional arguments for the Redis command. + This field is included in the cache key to ensure uniqueness + when commands have the same keys but different arguments. + Changing this field will affect cache key uniqueness. + """ command: str redis_keys: tuple - redis_args: tuple = () + redis_args: tuple = () # Additional arguments for the Redis command; affects cache key uniqueness. class CacheEntry: From 49b5c459239e6916c07f41c0305915c5155ac4b5 Mon Sep 17 00:00:00 2001 From: Shubham Kaudewar Date: Tue, 16 Sep 2025 23:57:15 +0530 Subject: [PATCH 4/4] linter checks --- redis/cache.py | 1 + redis/connection.py | 4 +++- tests/test_connection.py | 48 +++++++++++++++++++++++++++++++--------- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/redis/cache.py b/redis/cache.py index ee99da64a0..dc88f204ec 100644 --- a/redis/cache.py +++ b/redis/cache.py @@ -28,6 +28,7 @@ class CacheKey: when commands have the same keys but different arguments. Changing this field will affect cache key uniqueness. """ + command: str redis_keys: tuple redis_args: tuple = () # Additional arguments for the Redis command; affects cache key uniqueness. diff --git a/redis/connection.py b/redis/connection.py index eabbd34e1f..235d7fa7d3 100644 --- a/redis/connection.py +++ b/redis/connection.py @@ -1214,7 +1214,9 @@ def send_command(self, *args, **kwargs): with self._cache_lock: # Command is write command or not allowed # to be cached. - if not self._cache.is_cachable(CacheKey(command=args[0], redis_keys=(), redis_args=())): + if not self._cache.is_cachable( + CacheKey(command=args[0], redis_keys=(), redis_args=()) + ): self._current_command_cache_key = None self._conn.send_command(*args, **kwargs) return diff --git a/tests/test_connection.py b/tests/test_connection.py index 8cd5588067..f777426edf 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -357,7 +357,7 @@ def test_network_connection_failure(): @pytest.mark.skipif( not hasattr(socket, "AF_UNIX"), - reason="Unix domain sockets not supported on this platform" + reason="Unix domain sockets not supported on this platform", ) def test_unix_socket_connection_failure(): exp_err = "Error 2 connecting to unix:///tmp/a.sock. No such file or directory." @@ -468,25 +468,33 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): None, None, CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), + cache_key=CacheKey( + command="GET", redis_keys=("foo",), redis_args=("GET", "foo") + ), cache_value=CacheProxyConnection.DUMMY_CACHE_VALUE, status=CacheEntryStatus.IN_PROGRESS, connection_ref=mock_connection, ), CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), + cache_key=CacheKey( + command="GET", redis_keys=("foo",), redis_args=("GET", "foo") + ), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, ), CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), + cache_key=CacheKey( + command="GET", redis_keys=("foo",), redis_args=("GET", "foo") + ), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, ), CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), + cache_key=CacheKey( + command="GET", redis_keys=("foo",), redis_args=("GET", "foo") + ), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, @@ -508,7 +516,11 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): [ call( CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), + cache_key=CacheKey( + command="GET", + redis_keys=("foo",), + redis_args=("GET", "foo"), + ), cache_value=CacheProxyConnection.DUMMY_CACHE_VALUE, status=CacheEntryStatus.IN_PROGRESS, connection_ref=mock_connection, @@ -516,7 +528,11 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): ), call( CacheEntry( - cache_key=CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo")), + cache_key=CacheKey( + command="GET", + redis_keys=("foo",), + redis_args=("GET", "foo"), + ), cache_value=b"bar", status=CacheEntryStatus.VALID, connection_ref=mock_connection, @@ -527,9 +543,21 @@ def test_read_response_returns_cached_reply(self, mock_cache, mock_connection): mock_cache.get.assert_has_calls( [ - call(CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo"))), - call(CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo"))), - call(CacheKey(command="GET", redis_keys=("foo",), redis_args=("GET", "foo"))), + call( + CacheKey( + command="GET", redis_keys=("foo",), redis_args=("GET", "foo") + ) + ), + call( + CacheKey( + command="GET", redis_keys=("foo",), redis_args=("GET", "foo") + ) + ), + call( + CacheKey( + command="GET", redis_keys=("foo",), redis_args=("GET", "foo") + ) + ), ] )