Skip to content

Commit f84b16f

Browse files
committed
Extend cache-key handling to xpending_range and sort_ro
Following review, cover the other allow-listed commands that reached execute_command without `keys`: - xpending_range() sent XPENDING with no keys, so it raised ValueError("Cannot create cache key.") under CSC. Pass keys=[name]. - sort_ro() delegated to sort(), which sends SORT, so SORT_RO (on the allow list) was never cached. Send SORT_RO with keys=[key] via a private _command argument to sort(); the write path (sort(store=...)) still sends SORT. The SORT response callback is a no-op without `groups` (which sort_ro never uses), so response parsing is unaffected. Add a regression test per command in tests/test_cache.py. (Deferred: georadius/georadiusbymember _RO caching. The GEO response callbacks are keyed on the base command names, so sending the _RO variants would change response parsing for withdist/withcoord/withhash calls, and the _RO commands only exist since Redis 3.2.10. That needs _RO parser aliases and version gating and is better handled separately.)
1 parent c5b22b2 commit f84b16f

2 files changed

Lines changed: 86 additions & 3 deletions

File tree

redis/commands/core.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5399,6 +5399,7 @@ def sort(
53995399
alpha: bool = False,
54005400
store: str | None = None,
54015401
groups: bool | None = False,
5402+
_command: str = "SORT",
54025403
) -> SortResponse | Awaitable[SortResponse]:
54035404
"""
54045405
Sort and return the list, set or sorted set at ``name``.
@@ -5459,7 +5460,7 @@ def sort(
54595460

54605461
options = {"groups": len(get) if groups else None}
54615462
options["keys"] = [name]
5462-
return self.execute_command("SORT", *pieces, **options)
5463+
return self.execute_command(_command, *pieces, **options)
54635464

54645465
@overload
54655466
def sort_ro(
@@ -5515,7 +5516,14 @@ def sort_ro(
55155516
For more information, see https://redis.io/commands/sort_ro
55165517
"""
55175518
return self.sort(
5518-
key, start=start, num=num, by=by, get=get, desc=desc, alpha=alpha
5519+
key,
5520+
start=start,
5521+
num=num,
5522+
by=by,
5523+
get=get,
5524+
desc=desc,
5525+
alpha=alpha,
5526+
_command="SORT_RO",
55195527
)
55205528

55215529

@@ -7796,7 +7804,7 @@ def xpending_range(
77967804
if consumername:
77977805
pieces.append(consumername)
77987806

7799-
return self.execute_command("XPENDING", *pieces, parse_detail=True)
7807+
return self.execute_command("XPENDING", *pieces, parse_detail=True, keys=[name])
78007808

78017809
@overload
78027810
def xrange(

tests/test_cache.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,81 @@ def test_zrank_zrevrank_are_cacheable(self, r, r2):
194194
# rank of "b" shifts from 1 to 2 after inserting a lower-scored member
195195
assert r.zrank("myzset", "b") == 2
196196

197+
@pytest.mark.parametrize(
198+
"r",
199+
[
200+
{
201+
"cache": DefaultCache(CacheConfig(max_size=5)),
202+
"single_connection_client": True,
203+
},
204+
{
205+
"cache": DefaultCache(CacheConfig(max_size=5)),
206+
"single_connection_client": False,
207+
},
208+
],
209+
ids=["single", "pool"],
210+
indirect=True,
211+
)
212+
@pytest.mark.onlynoncluster
213+
def test_xpending_range_is_cacheable(self, r):
214+
# XPENDING is on the allow list, but xpending_range passed no `keys`,
215+
# so client-side caching raised ValueError("Cannot create cache key.").
216+
cache = r.get_cache()
217+
stream, group = "stream", "group"
218+
r.delete(stream)
219+
r.xadd(stream, {"foo": "bar"})
220+
r.xgroup_create(stream, group, 0)
221+
# must not raise, and the result must be cached under the stream key
222+
assert r.xpending_range(stream, group, min="-", max="+", count=5) == []
223+
assert (
224+
cache.get(
225+
CacheKey(
226+
command="XPENDING",
227+
redis_keys=(stream,),
228+
redis_args=("XPENDING", stream, group, "-", "+", 5),
229+
)
230+
)
231+
is not None
232+
)
233+
234+
@pytest.mark.parametrize(
235+
"r",
236+
[
237+
{
238+
"cache": DefaultCache(CacheConfig(max_size=5)),
239+
"single_connection_client": True,
240+
},
241+
{
242+
"cache": DefaultCache(CacheConfig(max_size=5)),
243+
"single_connection_client": False,
244+
},
245+
],
246+
ids=["single", "pool"],
247+
indirect=True,
248+
)
249+
@pytest.mark.onlynoncluster
250+
def test_sort_ro_is_cacheable(self, r, r2):
251+
# sort_ro delegated to sort() and sent SORT, so SORT_RO (on the allow
252+
# list) was never actually cached. It now sends SORT_RO with the key.
253+
cache = r.get_cache()
254+
r.delete("mylist")
255+
r.rpush("mylist", "3", "1", "2")
256+
assert r.sort_ro("mylist") == [b"1", b"2", b"3"]
257+
assert (
258+
cache.get(
259+
CacheKey(
260+
command="SORT_RO",
261+
redis_keys=("mylist",),
262+
redis_args=("SORT_RO", "mylist"),
263+
)
264+
)
265+
is not None
266+
)
267+
# mutate from a second client -> invalidation
268+
r2.rpush("mylist", "0")
269+
time.sleep(0.1)
270+
assert r.sort_ro("mylist") == [b"0", b"1", b"2", b"3"]
271+
197272
@pytest.mark.parametrize(
198273
"r",
199274
[

0 commit comments

Comments
 (0)