Skip to content

Commit 854660d

Browse files
Pass zrevrange key as a list in options["keys"] to match sibling range commands (#4244)
* Pass zrevrange key as a list to match sibling range commands ZREVRANGE stored its key in options["keys"] as a bare string, while every sibling range command (zrange, zrangebyscore, zrevrangebyscore) passes it as a list: options["keys"] = [name]. Consumers of options["keys"] iterate it as a sequence of keys. For a bare string that walks the key character by character -- for example client-side cache key construction builds CacheKey(redis_keys=tuple(keys)), turning key "myzset" into ('m', 'y', 'z', 's', 'e', 't') instead of ('myzset',). Wrap the key in a list to match the other range commands, and add a regression test asserting zrevrange passes keys=[name]. * Move zrevrange regression test to the cache level Address review: replace the test that asserted the internal `keys` kwarg with a behavior-level regression test in tests/test_cache.py. It caches a `zrevrange` result, asserts the entry is stored under the whole-key cache key (`redis_keys=("myzset",)`) rather than per-character, and that mutating the sorted set from a second client invalidates it. This fails on master and passes with the fix, and protects the behavior rather than the plumbing. --------- Co-authored-by: uttam12331 <uttam12331@users.noreply.github.com> Co-authored-by: petyaslavova <petya.slavova@redis.com>
1 parent 1627c98 commit 854660d

2 files changed

Lines changed: 42 additions & 1 deletion

File tree

redis/commands/core.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8988,7 +8988,7 @@ def zrevrange(
89888988
if withscores:
89898989
pieces.append(b"WITHSCORES")
89908990
options = {"withscores": withscores, "score_cast_func": score_cast_func}
8991-
options["keys"] = name
8991+
options["keys"] = [name]
89928992
return self.execute_command(*pieces, **options)
89938993

89948994
@overload

tests/test_cache.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,47 @@ def test_get_from_given_cache(self, r, r2):
9999
"barbar",
100100
]
101101

102+
@pytest.mark.parametrize(
103+
"r",
104+
[
105+
{
106+
"cache": DefaultCache(CacheConfig(max_size=5)),
107+
"single_connection_client": True,
108+
},
109+
{
110+
"cache": DefaultCache(CacheConfig(max_size=5)),
111+
"single_connection_client": False,
112+
},
113+
],
114+
ids=["single", "pool"],
115+
indirect=True,
116+
)
117+
@pytest.mark.onlynoncluster
118+
def test_zrevrange_cache_key_uses_whole_key(self, r, r2):
119+
# Regression: zrevrange stored options["keys"] as a bare string, so the
120+
# cache key was built from the key's individual characters
121+
# (("m", "y", "z", ...)) instead of the whole key. Verify the result is
122+
# cached under redis_keys=("myzset",) and invalidated when the set
123+
# changes from another client.
124+
cache = r.get_cache()
125+
r.delete("myzset")
126+
r.zadd("myzset", {"a": 1, "b": 2})
127+
# populate the local cache
128+
assert r.zrevrange("myzset", 0, -1) == [b"b", b"a"]
129+
# the entry must be stored under the whole key, not per-character
130+
cache_key = CacheKey(
131+
command="ZREVRANGE",
132+
redis_keys=("myzset",),
133+
redis_args=("ZREVRANGE", "myzset", 0, -1),
134+
)
135+
assert cache.get(cache_key) is not None
136+
# change the sorted set from a second client (causes invalidation)
137+
r2.zadd("myzset", {"c": 3})
138+
# Add a small delay to allow invalidation to be processed
139+
time.sleep(0.1)
140+
# a fresh value is fetched and re-cached
141+
assert r.zrevrange("myzset", 0, -1) == [b"c", b"b", b"a"]
142+
102143
@pytest.mark.parametrize(
103144
"r",
104145
[

0 commit comments

Comments
 (0)