From 4813005920c1e6cde108be20a8567e4be06f709c Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Mon, 9 Nov 2026 13:49:57 +0100 Subject: [PATCH 1/9] Resolve movablekeys from key specs instead of COMMAND GETKEYS The cluster client sent COMMAND GETKEYS before every movablekeys command (ZUNIONSTORE, LMPOP, SINTERCARD, ...) just to find the slot. The key specs from COMMAND already describe where the keys are; read them locally and only ask the server when a spec is incomplete or uses a keyword search (XREAD, SORT, MIGRATE, GEORADIUS). --- redis/_parsers/commands.py | 65 ++++++++++++++++++++++++++++++++++-- tests/test_command_parser.py | 31 ++++++++++------- 2 files changed, 82 insertions(+), 14 deletions(-) diff --git a/redis/_parsers/commands.py b/redis/_parsers/commands.py index 52b4dd4db6..0c8b37432e 100644 --- a/redis/_parsers/commands.py +++ b/redis/_parsers/commands.py @@ -77,6 +77,63 @@ def _get_pubsub_keys(self, *args): def parse_subcommand(self, command, **options): return _parse_subcommand(command) + def _get_keys_from_specs(self, command, args): + """ + Find the keys of a movablekeys command from its key specs, the way the + server does, which saves the COMMAND GETKEYS round trip. Returns None + when the specs are not enough and the server has to be asked. + """ + specs = command.get("key_specifications") + if not specs: + return None + args = args[0].split() + list(args[1:]) + argc = len(args) + keys = [] + for spec in specs: + spec = _spec_dict(spec) + begin = _spec_dict(spec.get("begin_search", ())) + find = _spec_dict(spec.get("find_keys", ())) + if ( + "incomplete" in {str_if_bytes(f) for f in spec.get("flags", ())} + or str_if_bytes(begin.get("type")) != "index" + ): + return None + first = _spec_dict(begin["spec"])["index"] + find_type = str_if_bytes(find.get("type")) + find_spec = _spec_dict(find.get("spec", ())) + if find_type == "range": + lastkey = find_spec["lastkey"] + limit = find_spec["limit"] + if lastkey >= 0: + last = first + lastkey + elif not limit: + last = argc + lastkey + else: + last = first + (argc - first) // limit + lastkey + elif find_type == "keynum": + try: + numkeys = int(args[first + find_spec["keynumidx"]]) + except (IndexError, TypeError, ValueError): + return None + if numkeys < 0: + return None + first += find_spec["firstkey"] + last = first + numkeys - 1 + else: + return None + step = find_spec["keystep"] + if step < 1 or first < 1 or last >= argc: + return None + keys.extend(args[first : last + 1 : step]) + return keys + + +def _spec_dict(spec): + # RESP2 sends a key spec as a flat [name, value, ...] list, RESP3 as a map. + if isinstance(spec, dict): + return {str_if_bytes(k): v for k, v in spec.items()} + return {str_if_bytes(k): v for k, v in zip(spec[::2], spec[1::2])} + class CommandsParser(AbstractCommandsParser): """ @@ -140,7 +197,9 @@ def get_keys(self, redis_conn, *args): command = self.commands.get(cmd_name) if "movablekeys" in command["flags"]: - keys = self._get_moveable_keys(redis_conn, *args) + keys = self._get_keys_from_specs(command, args) + if keys is None: + keys = self._get_moveable_keys(redis_conn, *args) elif "pubsub" in command["flags"] or command["name"] == "pubsub": keys = self._get_pubsub_keys(*args) else: @@ -316,7 +375,9 @@ async def get_keys(self, *args: Any) -> Optional[Tuple[str, ...]]: command = self.commands.get(cmd_name) if "movablekeys" in command["flags"]: - keys = await self._get_moveable_keys(*args) + keys = self._get_keys_from_specs(command, args) + if keys is None: + keys = await self._get_moveable_keys(*args) elif "pubsub" in command["flags"] or command["name"] == "pubsub": keys = self._get_pubsub_keys(*args) else: diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index 99519ae58d..6e03cbfe89 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -1,3 +1,5 @@ +from unittest import mock + import pytest from redis._parsers import CommandsParser from redis.commands.metadata import RequestPolicy, ResponsePolicy @@ -57,24 +59,14 @@ def test_get_moveable_keys(self, r): ] args7 = ["MIGRATE", "192.168.1.34", 6379, "key1", 0, 5000] - assert_resp_response( - r, - sorted(commands_parser.get_keys(r, *args1)), - ["key1", "key2"], - [b"key1", b"key2"], - ) + assert commands_parser.get_keys(r, *args1) == ["key1", "key2"] assert_resp_response( r, sorted(commands_parser.get_keys(r, *args2)), ["mystream", "writers"], [b"mystream", b"writers"], ) - assert_resp_response( - r, - sorted(commands_parser.get_keys(r, *args3)), - ["out", "zset1", "zset2"], - [b"out", b"zset1", b"zset2"], - ) + assert commands_parser.get_keys(r, *args3) == ["out", "zset1", "zset2"] assert_resp_response( r, sorted(commands_parser.get_keys(r, *args4)), @@ -92,6 +84,21 @@ def test_get_moveable_keys(self, r): r, sorted(commands_parser.get_keys(r, *args7)), ["key1"], [b"key1"] ) + @skip_if_server_version_lt("7.0.0") + def test_get_moveable_keys_from_key_specs(self, r): + commands_parser = CommandsParser(r) + cases = [ + (["ZUNIONSTORE", "out", 2, "a", "b", "WEIGHTS", 2, 3], ["out", "a", "b"]), + (["LMPOP", 2, "a", "b", "LEFT"], ["a", "b"]), + (["BZMPOP", 0, 1, "a", "MIN"], ["a"]), + (["SINTERCARD", 2, "a", "b", "LIMIT", 1], ["a", "b"]), + (["FCALL", "f", 0, "arg"], []), + ] + with mock.patch.object(commands_parser, "_get_moveable_keys") as server: + for args, keys in cases: + assert commands_parser.get_keys(r, *args) == keys + server.assert_not_called() + # A bug in redis<7.0 causes this to fail: https://github.com/redis/redis/issues/9493 @skip_if_server_version_lt("7.0.0") def test_get_eval_keys_with_0_keys(self, r): From 989e6b6c2b8baf76f43f006aed8052fe95e2201f Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Mon, 9 Nov 2026 14:13:36 +0100 Subject: [PATCH 2/9] Apply keystep to the last key of keynum specs MSETEX counts keys, not arguments, so only its first key was returned. --- redis/_parsers/commands.py | 2 +- tests/test_command_parser.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/redis/_parsers/commands.py b/redis/_parsers/commands.py index 0c8b37432e..bb6e60506e 100644 --- a/redis/_parsers/commands.py +++ b/redis/_parsers/commands.py @@ -118,7 +118,7 @@ def _get_keys_from_specs(self, command, args): if numkeys < 0: return None first += find_spec["firstkey"] - last = first + numkeys - 1 + last = first + (numkeys - 1) * find_spec["keystep"] else: return None step = find_spec["keystep"] diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index 6e03cbfe89..44274c33a4 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -99,6 +99,14 @@ def test_get_moveable_keys_from_key_specs(self, r): assert commands_parser.get_keys(r, *args) == keys server.assert_not_called() + @skip_if_server_version_lt("8.4.0") + def test_get_moveable_keys_from_key_specs_with_keystep(self, r): + commands_parser = CommandsParser(r) + args = ["MSETEX", 2, "k1", "v1", "k2", "v2", "EX", 10] + with mock.patch.object(commands_parser, "_get_moveable_keys") as server: + assert commands_parser.get_keys(r, *args) == ["k1", "k2"] + server.assert_not_called() + # A bug in redis<7.0 causes this to fail: https://github.com/redis/redis/issues/9493 @skip_if_server_version_lt("7.0.0") def test_get_eval_keys_with_0_keys(self, r): From 60b4874cf2ca67997c6f63b00be1b1fb385bf685 Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Mon, 9 Nov 2026 14:33:52 +0100 Subject: [PATCH 3/9] Accept memoryview keys in key_slot --- redis/crc.py | 3 +++ tests/test_crc.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/redis/crc.py b/redis/crc.py index e261241178..3d0bb4b339 100644 --- a/redis/crc.py +++ b/redis/crc.py @@ -15,6 +15,9 @@ def key_slot(key: EncodedT, bucket: int = REDIS_CLUSTER_HASH_SLOTS) -> int: :param key - bytes :param bucket - int """ + if isinstance(key, memoryview): + # The encoder passes memoryviews through, and they have no find(). + key = key.tobytes() start = key.find(b"{") if start > -1: end = key.find(b"}", start + 1) diff --git a/tests/test_crc.py b/tests/test_crc.py index 8a37d5eeb5..c7df398410 100644 --- a/tests/test_crc.py +++ b/tests/test_crc.py @@ -57,3 +57,7 @@ def test_unclosed_brace_falls_back_to_whole_key(self): def test_custom_bucket_is_honored(self): assert key_slot(b"foo", 100) == crc_hqx(b"foo", 0) % 100 assert 0 <= key_slot(b"123456789", 100) < 100 + + def test_memoryview_key_matches_bytes(self): + assert key_slot(memoryview(b"foo")) == key_slot(b"foo") + assert key_slot(memoryview(b"{user1000}.x")) == key_slot(b"{user1000}") From 770c5ce03b8e7374b1cf1eb8ce7e115d27124ccf Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Mon, 9 Nov 2026 17:30:34 +0100 Subject: [PATCH 4/9] Compile movablekeys key specs once in initialize() Parse the key specs of every movablekeys command when the parser initializes, for both the sync and the async parser, instead of on every get_keys() call. Match COMMAND GETKEYS for the two flags it treats specially: specs flagged not_key are skipped, and a command with a spec flagged variable_flags is left to the server. The args[0].split() copy is gone, since get_keys() already splits multi-word command names. Add the async counterparts of the key-spec tests, and update the parser docstrings, docs/resp3_features.rst and AGENTS.md. --- AGENTS.md | 2 +- docs/resp3_features.rst | 6 +- redis/_parsers/commands.py | 116 +++++++++++++++------- tests/test_asyncio/test_command_parser.py | 28 ++++++ tests/test_command_parser.py | 18 ++++ 5 files changed, 131 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b9f9713b06..efc8bee367 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,7 +57,7 @@ Command surface is composed via mixin classes in `redis/commands/`: - `sentinel.py` — sentinel-specific commands. - `redismodules.py` — aggregates module mixins (`bf/` Bloom, `json/`, `search/`, `timeseries/`, `vectorset/`). Each module subpackage owns its own command surface and response parsing. - `policies.py` — `PolicyResolver` / `StaticPolicyResolver`, the routing view the cluster client resolves per command. Also holds `STATIC_POLICIES`, which is **deprecated and unread**: a frozen verbatim copy of the 7.1.0 table, kept only for external importers and deliberately not derived from or synced with `_STATIC_COMMAND_METADATA`. Change routing in `metadata.py`, never there. -- `metadata.py` — the command metadata record types: `RequestPolicy` / `ResponsePolicy`, `CommandPolicies` / `PolicyRecords`, and `CommandMetadata` / `CommandMetadataRecordsCache`, plus the metadata resolvers. `CommandMetadata` is the superset record; `_to_command_policies` projects one down to the routing view a `PolicyResolver` serves - projecting to `None` when the record withholds its routing policies, which is how a resolver tells the cluster client to resolve the target itself (`SCAN`, `RANDOMKEY` and the other `COMMAND_FLAGS` commands need this). The `movablekeys` reads are recorded `DEFAULT_KEYED`, which `_build_command_records` also derives from their key specs, and the cluster client finds their keys through `COMMAND GETKEYS`. The two types are kept separate deliberately: `CommandPolicies` shipped in 7.1.0 as a mutable class, and `CommandMetadata` is frozen so one record can be shared across many commands. `redis/_parsers/commands.py` re-exports the policy types from here for backwards compatibility; import them from `redis.commands.metadata`. A resolver is a client-level constructor argument (`metadata_resolver=` on `Redis`, `ConnectionPool`, `RedisCluster`, `NodesManager` and `ClusterPipeline`; on the async side only on `redis.asyncio.RedisCluster`, because the async stack has no client-side cache to be the second consumer), built once and shared with every node client; it serves cluster routing through `resolve_policies`, replica safety through `is_replica_safe`, and client-side-cache eligibility through `is_cacheable`. When only `metadata_resolver` is given the routing resolver is derived from it; an explicit `policy_resolver` still wins for the routing view - which nodes a command targets - but not for anything that view cannot express: replica safety and cache eligibility keep resolving through `metadata_resolver` either way, because a `CommandPolicies` record carries no `is_readonly` flag to answer them with. +- `metadata.py` — the command metadata record types: `RequestPolicy` / `ResponsePolicy`, `CommandPolicies` / `PolicyRecords`, and `CommandMetadata` / `CommandMetadataRecordsCache`, plus the metadata resolvers. `CommandMetadata` is the superset record; `_to_command_policies` projects one down to the routing view a `PolicyResolver` serves - projecting to `None` when the record withholds its routing policies, which is how a resolver tells the cluster client to resolve the target itself (`SCAN`, `RANDOMKEY` and the other `COMMAND_FLAGS` commands need this). The `movablekeys` reads are recorded `DEFAULT_KEYED`, which `_build_command_records` also derives from their key specs, and the cluster client finds their keys from the key specs `CommandsParser.initialize()` compiles, asking `COMMAND GETKEYS` only where those specs are not enough. The two types are kept separate deliberately: `CommandPolicies` shipped in 7.1.0 as a mutable class, and `CommandMetadata` is frozen so one record can be shared across many commands. `redis/_parsers/commands.py` re-exports the policy types from here for backwards compatibility; import them from `redis.commands.metadata`. A resolver is a client-level constructor argument (`metadata_resolver=` on `Redis`, `ConnectionPool`, `RedisCluster`, `NodesManager` and `ClusterPipeline`; on the async side only on `redis.asyncio.RedisCluster`, because the async stack has no client-side cache to be the second consumer), built once and shared with every node client; it serves cluster routing through `resolve_policies`, replica safety through `is_replica_safe`, and client-side-cache eligibility through `is_cacheable`. When only `metadata_resolver` is given the routing resolver is derived from it; an explicit `policy_resolver` still wins for the routing view - which nodes a command targets - but not for anything that view cannot express: replica safety and cache eligibility keep resolving through `metadata_resolver` either way, because a `CommandPolicies` record carries no `is_readonly` flag to answer them with. - `helpers.py` — shared utilities (`list_or_args`, pubsub subscription partitioning, etc.). `Redis` (sync) and `redis.asyncio.Redis` are assembled by inheriting these mixins plus the connection/pool plumbing. Adding a new command means editing both the sync and async mixin classes and, for cluster routing, possibly the command metadata in `metadata.py`, which drives both the routing policies and replica eligibility. Not `READ_COMMANDS` - it is deprecated and unread. There is a `/add-new-command` skill (`.claude/commands/add-new-command.md`) that follows the project's expected workflow; use the spec template at `.agents/commands/add-new-command/command-specification-template.md`. Before adding commands, also read this file (AGENTS.md) and `.agents/sync_async_type_hints_overload_guide.md` — they document the sync/async type-overload convention that command additions must follow. The `/sync-claude-md` skill audits and refreshes this file when the project structure changes. diff --git a/docs/resp3_features.rst b/docs/resp3_features.rst index 6fecaf9007..c85e40f54d 100644 --- a/docs/resp3_features.rst +++ b/docs/resp3_features.rst @@ -166,8 +166,10 @@ commands with the ``movablekeys`` flag. Examples are ``SINTERCARD``, ``ZDIFF``, - The static table lists them as keyed commands. - With the server metadata, the parser reads their key specs and marks them as keyed. -In both cases, the Cluster client asks the server for their keys with ``COMMAND GETKEYS`` -and routes the command by those keys. +In both cases, the Cluster client finds their keys from the key specs that ``COMMAND`` +reports and routes the command by those keys. It asks the server with ``COMMAND GETKEYS`` +only when the specs are not enough: a server older than 7.0 without key specs, a spec +flagged ``incomplete`` or ``variable_flags``, or a keyword search. The same argument is accepted by ``ConnectionPool`` (configure it there when you supply your own ``connection_pool=``) and by ``RedisCluster``, which shares the one resolver with every diff --git a/redis/_parsers/commands.py b/redis/_parsers/commands.py index bb6e60506e..edb7107fe2 100644 --- a/redis/_parsers/commands.py +++ b/redis/_parsers/commands.py @@ -77,57 +77,96 @@ def _get_pubsub_keys(self, *args): def parse_subcommand(self, command, **options): return _parse_subcommand(command) - def _get_keys_from_specs(self, command, args): + def _get_keys_from_specs(self, cmd_name, args): """ - Find the keys of a movablekeys command from its key specs, the way the - server does, which saves the COMMAND GETKEYS round trip. Returns None - when the specs are not enough and the server has to be asked. + Find the keys of a movablekeys command from the key specs compiled in + initialize(), the way the server does, which saves the COMMAND GETKEYS + round trip. Returns None when the server has to be asked. """ - specs = command.get("key_specifications") - if not specs: + specs = self._movablekeys_specs.get(cmd_name) + if specs is None: return None - args = args[0].split() + list(args[1:]) argc = len(args) keys = [] - for spec in specs: - spec = _spec_dict(spec) - begin = _spec_dict(spec.get("begin_search", ())) - find = _spec_dict(spec.get("find_keys", ())) - if ( - "incomplete" in {str_if_bytes(f) for f in spec.get("flags", ())} - or str_if_bytes(begin.get("type")) != "index" - ): - return None - first = _spec_dict(begin["spec"])["index"] - find_type = str_if_bytes(find.get("type")) - find_spec = _spec_dict(find.get("spec", ())) + for find_type, first, a, b, step in specs: if find_type == "range": - lastkey = find_spec["lastkey"] - limit = find_spec["limit"] + lastkey, limit = a, b if lastkey >= 0: last = first + lastkey elif not limit: last = argc + lastkey else: last = first + (argc - first) // limit + lastkey - elif find_type == "keynum": + else: + keynumidx, firstkey = a, b try: - numkeys = int(args[first + find_spec["keynumidx"]]) + numkeys = int(args[first + keynumidx]) except (IndexError, TypeError, ValueError): return None if numkeys < 0: return None - first += find_spec["firstkey"] - last = first + (numkeys - 1) * find_spec["keystep"] - else: - return None - step = find_spec["keystep"] - if step < 1 or first < 1 or last >= argc: + first += firstkey + last = first + (numkeys - 1) * step + if last >= argc: return None keys.extend(args[first : last + 1 : step]) return keys +def _compile_movablekeys_specs(commands): + """ + Compile the key specs of every movablekeys command into tuples that + _get_keys_from_specs evaluates, or None where COMMAND GETKEYS has to be + asked: no specs, a spec flagged incomplete or variable_flags (the server + then ignores the specs too), or a search the client does not implement. + """ + compiled = {} + for name, command in commands.items(): + if "movablekeys" in command["flags"]: + compiled[name] = _compile_key_specs(command.get("key_specifications")) + return compiled + + +def _compile_key_specs(specs): + if not specs: + return None + compiled = [] + for spec in specs: + flags = _key_spec_flags(spec) + if "incomplete" in flags or "variable_flags" in flags: + return None + if "not_key" in flags: + continue + spec = _spec_dict(spec) + begin = _spec_dict(spec.get("begin_search", ())) + find = _spec_dict(spec.get("find_keys", ())) + if str_if_bytes(begin.get("type")) != "index": + return None + first = _spec_dict(begin["spec"])["index"] + find_type = str_if_bytes(find.get("type")) + find_spec = _spec_dict(find.get("spec", ())) + step = find_spec.get("keystep", 0) + if first < 1 or step < 1: + return None + if find_type == "range": + compiled.append( + (find_type, first, find_spec["lastkey"], find_spec["limit"], step) + ) + elif find_type == "keynum": + compiled.append( + ( + find_type, + first, + find_spec["keynumidx"], + find_spec["firstkey"], + step, + ) + ) + else: + return None + return tuple(compiled) + + def _spec_dict(spec): # RESP2 sends a key spec as a flat [name, value, ...] list, RESP3 as a map. if isinstance(spec, dict): @@ -140,12 +179,13 @@ class CommandsParser(AbstractCommandsParser): Parses Redis commands to get command keys. COMMAND output is used to determine key locations. Commands that do not have a predefined key location are flagged with - 'movablekeys', and these commands' keys are determined by the command - 'COMMAND GETKEYS'. + 'movablekeys'. Their keys are read from the command's key specs, and + 'COMMAND GETKEYS' is asked only when the specs are not enough. """ def __init__(self, redis_connection): self.commands = {} + self._movablekeys_specs = {} self.redis_connection = redis_connection self.initialize(self.redis_connection) @@ -158,6 +198,7 @@ def initialize(self, r): for cmd in uppercase_commands: commands[cmd.lower()] = commands.pop(cmd) self.commands = commands + self._movablekeys_specs = _compile_movablekeys_specs(commands) # As soon as this PR is merged into Redis, we should reimplement # our logic to use COMMAND INFO changes to determine the key positions @@ -197,7 +238,7 @@ def get_keys(self, redis_conn, *args): command = self.commands.get(cmd_name) if "movablekeys" in command["flags"]: - keys = self._get_keys_from_specs(command, args) + keys = self._get_keys_from_specs(cmd_name, args) if keys is None: keys = self._get_moveable_keys(redis_conn, *args) elif "pubsub" in command["flags"] or command["name"] == "pubsub": @@ -314,8 +355,9 @@ class AsyncCommandsParser(AbstractCommandsParser): Parses Redis commands to get command keys. COMMAND output is used to determine key locations. - Commands that do not have a predefined key location are flagged with 'movablekeys', - and these commands' keys are determined by the command 'COMMAND GETKEYS'. + Commands that do not have a predefined key location are flagged with 'movablekeys'. + Their keys are read from the command's key specs, and 'COMMAND GETKEYS' is asked + only when the specs are not enough. NOTE: Due to a bug in redis<7.0, this does not work properly for EVAL or EVALSHA when the `numkeys` arg is 0. @@ -325,10 +367,11 @@ class AsyncCommandsParser(AbstractCommandsParser): So, don't use this with EVAL or EVALSHA. """ - __slots__ = ("commands", "node") + __slots__ = ("commands", "node", "_movablekeys_specs") def __init__(self) -> None: self.commands: Dict[str, Union[int, Dict[str, Any]]] = {} + self._movablekeys_specs: Dict[str, Optional[Tuple]] = {} async def initialize(self, node: Optional["ClusterNode"] = None) -> None: if node: @@ -336,6 +379,7 @@ async def initialize(self, node: Optional["ClusterNode"] = None) -> None: commands = await self.node.execute_command("COMMAND") self.commands = {cmd.lower(): command for cmd, command in commands.items()} + self._movablekeys_specs = _compile_movablekeys_specs(self.commands) # As soon as this PR is merged into Redis, we should reimplement # our logic to use COMMAND INFO changes to determine the key positions @@ -375,7 +419,7 @@ async def get_keys(self, *args: Any) -> Optional[Tuple[str, ...]]: command = self.commands.get(cmd_name) if "movablekeys" in command["flags"]: - keys = self._get_keys_from_specs(command, args) + keys = self._get_keys_from_specs(cmd_name, args) if keys is None: keys = await self._get_moveable_keys(*args) elif "pubsub" in command["flags"] or command["name"] == "pubsub": diff --git a/tests/test_asyncio/test_command_parser.py b/tests/test_asyncio/test_command_parser.py index 9e52b2832c..852fc217be 100644 --- a/tests/test_asyncio/test_command_parser.py +++ b/tests/test_asyncio/test_command_parser.py @@ -1,3 +1,5 @@ +from unittest import mock + import pytest from redis._parsers import AsyncCommandsParser @@ -27,6 +29,32 @@ async def test_movablekeys_commands_derive_the_keyed_defaults(self, r): assert metadata["spublish"].request_policy is RequestPolicy.DEFAULT_KEYED assert metadata["lastsave"].request_policy is RequestPolicy.DEFAULT_KEYLESS + @pytest.mark.asyncio + async def test_get_moveable_keys_from_key_specs(self, r): + commands_parser = AsyncCommandsParser() + await commands_parser.initialize(node=r.get_default_node()) + cases = [ + (["ZUNIONSTORE", "out", 2, "a", "b", "WEIGHTS", 2, 3], ["out", "a", "b"]), + (["LMPOP", 2, "a", "b", "LEFT"], ["a", "b"]), + (["BZMPOP", 0, 1, "a", "MIN"], ["a"]), + (["SINTERCARD", 2, "a", "b", "LIMIT", 1], ["a", "b"]), + (["FCALL", "f", 0, "arg"], []), + ] + with mock.patch.object(commands_parser, "_get_moveable_keys") as server: + for args, keys in cases: + assert await commands_parser.get_keys(*args) == keys + server.assert_not_called() + + @pytest.mark.asyncio + @skip_if_server_version_lt("8.4.0") + async def test_get_moveable_keys_from_key_specs_with_keystep(self, r): + commands_parser = AsyncCommandsParser() + await commands_parser.initialize(node=r.get_default_node()) + args = ["MSETEX", 2, "k1", "v1", "k2", "v2", "EX", 10] + with mock.patch.object(commands_parser, "_get_moveable_keys") as server: + assert await commands_parser.get_keys(*args) == ["k1", "k2"] + server.assert_not_called() + @pytest.mark.asyncio @skip_if_server_version_gte("8.5.240") async def test_get_command_policies(self, r): diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index 44274c33a4..f722f6cb74 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -2,6 +2,7 @@ import pytest from redis._parsers import CommandsParser +from redis._parsers.commands import _compile_key_specs from redis.commands.metadata import RequestPolicy, ResponsePolicy from tests.helpers import get_expected_command_policies @@ -107,6 +108,23 @@ def test_get_moveable_keys_from_key_specs_with_keystep(self, r): assert commands_parser.get_keys(r, *args) == ["k1", "k2"] server.assert_not_called() + def test_compile_key_specs_follows_the_server_flags(self): + def spec(*flags): + return { + "flags": list(flags), + "begin_search": {"type": "index", "spec": {"index": 1}}, + "find_keys": { + "type": "range", + "spec": {"lastkey": 0, "keystep": 1, "limit": 0}, + }, + } + + assert _compile_key_specs([spec("RW"), spec("not_key")]) == ( + ("range", 1, 0, 0, 1), + ) + assert _compile_key_specs([spec("RW"), spec("variable_flags")]) is None + assert _compile_key_specs([spec("incomplete")]) is None + # A bug in redis<7.0 causes this to fail: https://github.com/redis/redis/issues/9493 @skip_if_server_version_lt("7.0.0") def test_get_eval_keys_with_0_keys(self, r): From 76666b6e0b11ab9486449f245618fafdb71ec0ff Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Tue, 10 Nov 2026 11:16:57 +0100 Subject: [PATCH 5/9] Update comments that still credit COMMAND GETKEYS for movablekeys keys --- redis/commands/metadata.py | 6 +++--- tests/test_asyncio/test_cluster.py | 3 ++- tests/test_cluster.py | 3 ++- tests/test_command_metadata.py | 6 +++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/redis/commands/metadata.py b/redis/commands/metadata.py index 94eee7d67e..8b4702f932 100644 --- a/redis/commands/metadata.py +++ b/redis/commands/metadata.py @@ -1140,9 +1140,9 @@ def _load_commands_metadata_cache( # # This includes the ``movablekeys`` commands. Their ``first_key_pos`` is 0, but they do take # keys, so ``has_key_argument`` is True. The cluster client routes them like any other keyed -# command: ``determine_slot`` asks the server for their keys with ``COMMAND GETKEYS``. The -# parsers read their key specs and also mark them keyed, so a live resolver agrees with this -# record. +# command: ``determine_slot`` reads their keys from the ``COMMAND`` key specs, and asks the +# server with ``COMMAND GETKEYS`` only where a spec is incomplete. The parsers also mark them +# keyed, so a live resolver agrees with this record. _CACHEABLE_KEYED = CommandMetadata( request_policy=RequestPolicy.DEFAULT_KEYED, response_policy=ResponsePolicy.DEFAULT_KEYED, diff --git a/tests/test_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index 46653ae1a0..af76ae0778 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -5004,7 +5004,8 @@ class TestStaticMetadataRouting: The ``movablekeys`` reads are the sharp case, and get their own test. Their keys live only in the ``COMMAND`` key specifications, so ``first_key_pos`` is 0; their records carry the keyed - defaults, and the client finds the keys through ``COMMAND GETKEYS``. + defaults, and the client reads the keys from those specs, asking the server with + ``COMMAND GETKEYS`` only where a spec is incomplete. Key extraction itself is covered by ``tests/test_asyncio/test_command_parser.py``, so it is stubbed here and these tests assert only the node the command is dispatched to. diff --git a/tests/test_cluster.py b/tests/test_cluster.py index 1dc86b3de9..d0c0944dd9 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -3237,7 +3237,8 @@ class TestStaticMetadataRouting: The ``movablekeys`` reads are the sharp case, and get their own test. Their keys live only in the ``COMMAND`` key specifications, so ``first_key_pos`` is 0; their records carry the keyed - defaults, and the client finds the keys through ``COMMAND GETKEYS``. + defaults, and the client reads the keys from those specs, asking the server with + ``COMMAND GETKEYS`` only where a spec is incomplete. Key extraction itself is covered by ``tests/test_command_parser.py``, so it is stubbed here and these tests assert only the node the command is dispatched to. diff --git a/tests/test_command_metadata.py b/tests/test_command_metadata.py index b5d931e25e..5362cbb754 100644 --- a/tests/test_command_metadata.py +++ b/tests/test_command_metadata.py @@ -293,8 +293,8 @@ def slot_routed_args(name: str) -> tuple[Any, ...]: Arguments that put the key ``{foo}a`` where ``name`` reads its keys from. The read-only EVAL variants count their keys after the script, and the cluster client - reads them from there itself rather than asking the server; every other command is - resolved through ``COMMAND GETKEYS``, which the routing tests stub. + reads them from there itself; every other command is resolved through its key specs + (or ``COMMAND GETKEYS``), which the routing tests stub. """ if name in ("eval_ro", "evalsha_ro"): return "return 1", 1, "{foo}a" @@ -549,7 +549,7 @@ class TestWithheldRoutingPolicies: records that exist only for their cacheability inputs. Withholding makes a policy resolver report the command the way it reports one the records do not carry, which is what sends the cluster client back to its own target resolution. The ``movablekeys`` reads do not withhold: - they are recorded keyed, and ``determine_slot`` -> ``COMMAND GETKEYS`` finds their keys. + they are recorded keyed, and ``determine_slot`` finds their keys from the key specs. Only the sync suite covers the projection function and the shipped table: both are module-level and shared by the two stacks. The resolver behaviour is mirrored in the async From 5bdcf535690c0ce234fadd82c0645dfdbe84d3b3 Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Tue, 10 Nov 2026 11:29:56 +0100 Subject: [PATCH 6/9] Leave key counts the server would reject to COMMAND GETKEYS int() accepts "+2", " 2", "02", "2_0" and the float 2.5, which the server rejects, so the spec path routed commands the server would refuse as malformed. Only a count in its canonical form is read locally now. --- redis/_parsers/commands.py | 27 +++++++++++++++++++---- tests/test_asyncio/test_command_parser.py | 13 +++++++++++ tests/test_command_parser.py | 11 +++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/redis/_parsers/commands.py b/redis/_parsers/commands.py index edb7107fe2..9fb9ee9e0b 100644 --- a/redis/_parsers/commands.py +++ b/redis/_parsers/commands.py @@ -99,11 +99,10 @@ def _get_keys_from_specs(self, cmd_name, args): last = first + (argc - first) // limit + lastkey else: keynumidx, firstkey = a, b - try: - numkeys = int(args[first + keynumidx]) - except (IndexError, TypeError, ValueError): + if first + keynumidx >= argc: return None - if numkeys < 0: + numkeys = _parse_numkeys(args[first + keynumidx]) + if numkeys is None or numkeys < 0: return None first += firstkey last = first + (numkeys - 1) * step @@ -113,6 +112,26 @@ def _get_keys_from_specs(self, cmd_name, args): return keys +def _parse_numkeys(value): + """ + The key count as the server reads it, or None for a value it rejects. + int() also accepts "+2", " 2", "02", "2_0" and the float 2.5. + """ + if isinstance(value, bool): + return None + if isinstance(value, int): + value = repr(value) + elif isinstance(value, (bytes, bytearray)): + value = value.decode("ascii", "replace") + elif not isinstance(value, str): + return None + try: + numkeys = int(value) + except ValueError: + return None + return numkeys if str(numkeys) == value else None + + def _compile_movablekeys_specs(commands): """ Compile the key specs of every movablekeys command into tuples that diff --git a/tests/test_asyncio/test_command_parser.py b/tests/test_asyncio/test_command_parser.py index 852fc217be..a3c612497c 100644 --- a/tests/test_asyncio/test_command_parser.py +++ b/tests/test_asyncio/test_command_parser.py @@ -55,6 +55,19 @@ async def test_get_moveable_keys_from_key_specs_with_keystep(self, r): assert await commands_parser.get_keys(*args) == ["k1", "k2"] server.assert_not_called() + @pytest.mark.asyncio + @skip_if_server_version_lt("7.0.0") + async def test_key_count_the_server_rejects_is_left_to_the_server(self, r): + commands_parser = AsyncCommandsParser() + await commands_parser.initialize(node=r.get_default_node()) + cases = ["+2", " 2", "02", "2_0", "2.5", 2.5, b"+2"] + with mock.patch.object( + commands_parser, "_get_moveable_keys", return_value=None + ) as server: + for numkeys in cases: + await commands_parser.get_keys("LMPOP", numkeys, "{a}x", "{b}y", "LEFT") + assert server.call_count == len(cases) + @pytest.mark.asyncio @skip_if_server_version_gte("8.5.240") async def test_get_command_policies(self, r): diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index f722f6cb74..c8425fad56 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -108,6 +108,17 @@ def test_get_moveable_keys_from_key_specs_with_keystep(self, r): assert commands_parser.get_keys(r, *args) == ["k1", "k2"] server.assert_not_called() + @skip_if_server_version_lt("7.0.0") + def test_key_count_the_server_rejects_is_left_to_the_server(self, r): + commands_parser = CommandsParser(r) + cases = ["+2", " 2", "02", "2_0", "2.5", 2.5, b"+2"] + with mock.patch.object( + commands_parser, "_get_moveable_keys", return_value=None + ) as server: + for numkeys in cases: + commands_parser.get_keys(r, "LMPOP", numkeys, "{a}x", "{b}y", "LEFT") + assert server.call_count == len(cases) + def test_compile_key_specs_follows_the_server_flags(self): def spec(*flags): return { From 95bb27a2e1b803af4f6509d86f85877ecaf66745 Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Tue, 10 Nov 2026 11:44:46 +0100 Subject: [PATCH 7/9] Read the key count from the bytes sent and skip empty key ranges A str key count is checked after encoding, since a non-ASCII client encoding changes the bytes the server parses. A keynum or range spec whose last key falls before the first now yields no keys, as on the server, instead of a slice that wraps around. --- redis/_parsers/commands.py | 38 ++++++++++++++--------- tests/test_asyncio/test_command_parser.py | 21 ++++++++++++- tests/test_command_parser.py | 17 +++++++++- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/redis/_parsers/commands.py b/redis/_parsers/commands.py index 9fb9ee9e0b..079d5a6098 100644 --- a/redis/_parsers/commands.py +++ b/redis/_parsers/commands.py @@ -20,10 +20,16 @@ RequestPolicy, ResponsePolicy, ) -from redis.exceptions import IncorrectPolicyType, RedisError, ResponseError +from redis.exceptions import ( + DataError, + IncorrectPolicyType, + RedisError, + ResponseError, +) from redis.utils import deprecated_function, str_if_bytes if TYPE_CHECKING: + from redis._parsers.encoders import Encoder from redis.asyncio.cluster import ClusterNode # The record type ``_build_command_records`` builds per command: policies or full metadata. @@ -101,35 +107,35 @@ def _get_keys_from_specs(self, cmd_name, args): keynumidx, firstkey = a, b if first + keynumidx >= argc: return None - numkeys = _parse_numkeys(args[first + keynumidx]) + numkeys = _parse_numkeys(args[first + keynumidx], self._encoder) if numkeys is None or numkeys < 0: return None first += firstkey last = first + (numkeys - 1) * step + if last < first: + # No keys, as for the server; a negative slice end would wrap. + continue if last >= argc: return None keys.extend(args[first : last + 1 : step]) return keys -def _parse_numkeys(value): +def _parse_numkeys(value, encoder): """ The key count as the server reads it, or None for a value it rejects. - int() also accepts "+2", " 2", "02", "2_0" and the float 2.5. + Checks the bytes the client sends: int() also accepts "+2", " 2", "02", + "2_0" and 2.5, and a str is sent in the client's encoding. """ - if isinstance(value, bool): + try: + raw = encoder.encode(value) + except DataError: return None - if isinstance(value, int): - value = repr(value) - elif isinstance(value, (bytes, bytearray)): - value = value.decode("ascii", "replace") - elif not isinstance(value, str): + if not isinstance(raw, (bytes, bytearray)) or not raw.isdigit(): return None - try: - numkeys = int(value) - except ValueError: + if len(raw) > 1 and raw.startswith(b"0"): return None - return numkeys if str(numkeys) == value else None + return int(raw) def _compile_movablekeys_specs(commands): @@ -205,6 +211,7 @@ class CommandsParser(AbstractCommandsParser): def __init__(self, redis_connection): self.commands = {} self._movablekeys_specs = {} + self._encoder = None self.redis_connection = redis_connection self.initialize(self.redis_connection) @@ -218,6 +225,7 @@ def initialize(self, r): commands[cmd.lower()] = commands.pop(cmd) self.commands = commands self._movablekeys_specs = _compile_movablekeys_specs(commands) + self._encoder = r.get_encoder() # As soon as this PR is merged into Redis, we should reimplement # our logic to use COMMAND INFO changes to determine the key positions @@ -391,6 +399,7 @@ class AsyncCommandsParser(AbstractCommandsParser): def __init__(self) -> None: self.commands: Dict[str, Union[int, Dict[str, Any]]] = {} self._movablekeys_specs: Dict[str, Optional[Tuple]] = {} + self._encoder: Optional["Encoder"] = None async def initialize(self, node: Optional["ClusterNode"] = None) -> None: if node: @@ -399,6 +408,7 @@ async def initialize(self, node: Optional["ClusterNode"] = None) -> None: commands = await self.node.execute_command("COMMAND") self.commands = {cmd.lower(): command for cmd, command in commands.items()} self._movablekeys_specs = _compile_movablekeys_specs(self.commands) + self._encoder = self.node.get_encoder() # As soon as this PR is merged into Redis, we should reimplement # our logic to use COMMAND INFO changes to determine the key positions diff --git a/tests/test_asyncio/test_command_parser.py b/tests/test_asyncio/test_command_parser.py index a3c612497c..0f8abf5c99 100644 --- a/tests/test_asyncio/test_command_parser.py +++ b/tests/test_asyncio/test_command_parser.py @@ -2,7 +2,7 @@ import pytest -from redis._parsers import AsyncCommandsParser +from redis._parsers import AsyncCommandsParser, Encoder from redis.commands.metadata import RequestPolicy, ResponsePolicy from tests.conftest import skip_if_server_version_gte, skip_if_server_version_lt from tests.helpers import get_expected_command_policies @@ -68,6 +68,25 @@ async def test_key_count_the_server_rejects_is_left_to_the_server(self, r): await commands_parser.get_keys("LMPOP", numkeys, "{a}x", "{b}y", "LEFT") assert server.call_count == len(cases) + @pytest.mark.asyncio + @skip_if_server_version_lt("7.0.0") + async def test_key_count_is_read_from_the_encoded_bytes(self, r): + commands_parser = AsyncCommandsParser() + await commands_parser.initialize(node=r.get_default_node()) + args = ["lmpop", "2", "{a}x", "{b}y", "LEFT"] + assert commands_parser._get_keys_from_specs("lmpop", args) == ["{a}x", "{b}y"] + # The server would get b"2\x00" and reject it. + commands_parser._encoder = Encoder("utf-16-le", "strict", False) + assert commands_parser._get_keys_from_specs("lmpop", args) is None + + @pytest.mark.asyncio + async def test_empty_key_range_yields_no_keys(self, r): + commands_parser = AsyncCommandsParser() + await commands_parser.initialize(node=r.get_default_node()) + commands_parser._movablekeys_specs = {"modcmd": (("keynum", 1, 0, 1, 4),)} + args = ["modcmd", 0, "a", "b", "c", "d", "e", "f", "g"] + assert commands_parser._get_keys_from_specs("modcmd", args) == [] + @pytest.mark.asyncio @skip_if_server_version_gte("8.5.240") async def test_get_command_policies(self, r): diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index c8425fad56..5e6356fc44 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -1,7 +1,7 @@ from unittest import mock import pytest -from redis._parsers import CommandsParser +from redis._parsers import CommandsParser, Encoder from redis._parsers.commands import _compile_key_specs from redis.commands.metadata import RequestPolicy, ResponsePolicy from tests.helpers import get_expected_command_policies @@ -119,6 +119,21 @@ def test_key_count_the_server_rejects_is_left_to_the_server(self, r): commands_parser.get_keys(r, "LMPOP", numkeys, "{a}x", "{b}y", "LEFT") assert server.call_count == len(cases) + @skip_if_server_version_lt("7.0.0") + def test_key_count_is_read_from_the_encoded_bytes(self, r): + commands_parser = CommandsParser(r) + args = ["lmpop", "2", "{a}x", "{b}y", "LEFT"] + assert commands_parser._get_keys_from_specs("lmpop", args) == ["{a}x", "{b}y"] + # The server would get b"2\x00" and reject it. + commands_parser._encoder = Encoder("utf-16-le", "strict", False) + assert commands_parser._get_keys_from_specs("lmpop", args) is None + + def test_empty_key_range_yields_no_keys(self, r): + commands_parser = CommandsParser(r) + commands_parser._movablekeys_specs = {"modcmd": (("keynum", 1, 0, 1, 4),)} + args = ["modcmd", 0, "a", "b", "c", "d", "e", "f", "g"] + assert commands_parser._get_keys_from_specs("modcmd", args) == [] + def test_compile_key_specs_follows_the_server_flags(self): def spec(*flags): return { From 57350b6857e17ffdd30cd8273c872d029bcc4319 Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Tue, 10 Nov 2026 11:54:39 +0100 Subject: [PATCH 8/9] Leave wrong arity and overlong key counts to COMMAND GETKEYS The server checks the argument count before it reads key specs, so a call below or off the command's arity no longer gets keys from the specs. A key count with more than 19 digits is out of range for the server and made int() raise past 4300 digits; it now falls back too. --- redis/_parsers/commands.py | 7 +++++++ tests/test_asyncio/test_command_parser.py | 9 +++++++++ tests/test_command_parser.py | 7 +++++++ 3 files changed, 23 insertions(+) diff --git a/redis/_parsers/commands.py b/redis/_parsers/commands.py index 079d5a6098..48c284aee5 100644 --- a/redis/_parsers/commands.py +++ b/redis/_parsers/commands.py @@ -93,6 +93,10 @@ def _get_keys_from_specs(self, cmd_name, args): if specs is None: return None argc = len(args) + # COMMAND GETKEYS rejects a wrong argument count before reading specs. + arity = int(self.commands[cmd_name]["arity"]) + if argc < abs(arity) or (arity > 0 and argc != arity): + return None keys = [] for find_type, first, a, b, step in specs: if find_type == "range": @@ -135,6 +139,9 @@ def _parse_numkeys(value, encoder): return None if len(raw) > 1 and raw.startswith(b"0"): return None + # More digits than a signed 64-bit count; int() would also refuse 4300+. + if len(raw) > 19: + return None return int(raw) diff --git a/tests/test_asyncio/test_command_parser.py b/tests/test_asyncio/test_command_parser.py index 0f8abf5c99..fdf5134bae 100644 --- a/tests/test_asyncio/test_command_parser.py +++ b/tests/test_asyncio/test_command_parser.py @@ -79,10 +79,19 @@ async def test_key_count_is_read_from_the_encoded_bytes(self, r): commands_parser._encoder = Encoder("utf-16-le", "strict", False) assert commands_parser._get_keys_from_specs("lmpop", args) is None + @pytest.mark.asyncio + @skip_if_server_version_lt("7.0.0") + async def test_malformed_calls_are_left_to_the_server(self, r): + commands_parser = AsyncCommandsParser() + await commands_parser.initialize(node=r.get_default_node()) + for args in (["lmpop", 1, "k"], ["lmpop", b"9" * 5000, "k", "LEFT"]): + assert commands_parser._get_keys_from_specs("lmpop", args) is None + @pytest.mark.asyncio async def test_empty_key_range_yields_no_keys(self, r): commands_parser = AsyncCommandsParser() await commands_parser.initialize(node=r.get_default_node()) + commands_parser.commands["modcmd"] = {"arity": -2} commands_parser._movablekeys_specs = {"modcmd": (("keynum", 1, 0, 1, 4),)} args = ["modcmd", 0, "a", "b", "c", "d", "e", "f", "g"] assert commands_parser._get_keys_from_specs("modcmd", args) == [] diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index 5e6356fc44..0d177865e7 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -128,8 +128,15 @@ def test_key_count_is_read_from_the_encoded_bytes(self, r): commands_parser._encoder = Encoder("utf-16-le", "strict", False) assert commands_parser._get_keys_from_specs("lmpop", args) is None + @skip_if_server_version_lt("7.0.0") + def test_malformed_calls_are_left_to_the_server(self, r): + commands_parser = CommandsParser(r) + for args in (["lmpop", 1, "k"], ["lmpop", b"9" * 5000, "k", "LEFT"]): + assert commands_parser._get_keys_from_specs("lmpop", args) is None + def test_empty_key_range_yields_no_keys(self, r): commands_parser = CommandsParser(r) + commands_parser.commands["modcmd"] = {"arity": -2} commands_parser._movablekeys_specs = {"modcmd": (("keynum", 1, 0, 1, 4),)} args = ["modcmd", 0, "a", "b", "c", "d", "e", "f", "g"] assert commands_parser._get_keys_from_specs("modcmd", args) == [] From 1971410eaa0cef9d9e0afce1e3518d67f788298c Mon Sep 17 00:00:00 2001 From: patrick-immometrica Date: Tue, 10 Nov 2026 13:22:14 +0100 Subject: [PATCH 9/9] Store the arity with the compiled key specs --- redis/_parsers/commands.py | 13 ++++++++----- tests/test_asyncio/test_command_parser.py | 3 +-- tests/test_command_parser.py | 3 +-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/redis/_parsers/commands.py b/redis/_parsers/commands.py index 48c284aee5..47a8ba115f 100644 --- a/redis/_parsers/commands.py +++ b/redis/_parsers/commands.py @@ -89,12 +89,12 @@ def _get_keys_from_specs(self, cmd_name, args): initialize(), the way the server does, which saves the COMMAND GETKEYS round trip. Returns None when the server has to be asked. """ - specs = self._movablekeys_specs.get(cmd_name) - if specs is None: + compiled = self._movablekeys_specs.get(cmd_name) + if compiled is None: return None + arity, specs = compiled argc = len(args) # COMMAND GETKEYS rejects a wrong argument count before reading specs. - arity = int(self.commands[cmd_name]["arity"]) if argc < abs(arity) or (arity > 0 and argc != arity): return None keys = [] @@ -147,7 +147,7 @@ def _parse_numkeys(value, encoder): def _compile_movablekeys_specs(commands): """ - Compile the key specs of every movablekeys command into tuples that + Compile every movablekeys command into an ``(arity, specs)`` pair that _get_keys_from_specs evaluates, or None where COMMAND GETKEYS has to be asked: no specs, a spec flagged incomplete or variable_flags (the server then ignores the specs too), or a search the client does not implement. @@ -155,7 +155,10 @@ def _compile_movablekeys_specs(commands): compiled = {} for name, command in commands.items(): if "movablekeys" in command["flags"]: - compiled[name] = _compile_key_specs(command.get("key_specifications")) + specs = _compile_key_specs(command.get("key_specifications")) + if specs is not None: + specs = (int(command["arity"]), specs) + compiled[name] = specs return compiled diff --git a/tests/test_asyncio/test_command_parser.py b/tests/test_asyncio/test_command_parser.py index fdf5134bae..0c3a6aa95c 100644 --- a/tests/test_asyncio/test_command_parser.py +++ b/tests/test_asyncio/test_command_parser.py @@ -91,8 +91,7 @@ async def test_malformed_calls_are_left_to_the_server(self, r): async def test_empty_key_range_yields_no_keys(self, r): commands_parser = AsyncCommandsParser() await commands_parser.initialize(node=r.get_default_node()) - commands_parser.commands["modcmd"] = {"arity": -2} - commands_parser._movablekeys_specs = {"modcmd": (("keynum", 1, 0, 1, 4),)} + commands_parser._movablekeys_specs = {"modcmd": (-2, (("keynum", 1, 0, 1, 4),))} args = ["modcmd", 0, "a", "b", "c", "d", "e", "f", "g"] assert commands_parser._get_keys_from_specs("modcmd", args) == [] diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index 0d177865e7..fbccbf4002 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -136,8 +136,7 @@ def test_malformed_calls_are_left_to_the_server(self, r): def test_empty_key_range_yields_no_keys(self, r): commands_parser = CommandsParser(r) - commands_parser.commands["modcmd"] = {"arity": -2} - commands_parser._movablekeys_specs = {"modcmd": (("keynum", 1, 0, 1, 4),)} + commands_parser._movablekeys_specs = {"modcmd": (-2, (("keynum", 1, 0, 1, 4),))} args = ["modcmd", 0, "a", "b", "c", "d", "e", "f", "g"] assert commands_parser._get_keys_from_specs("modcmd", args) == []