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 52b4dd4db6..47a8ba115f 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. @@ -77,18 +83,145 @@ def _get_pubsub_keys(self, *args): def parse_subcommand(self, command, **options): return _parse_subcommand(command) + def _get_keys_from_specs(self, cmd_name, args): + """ + 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. + """ + 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. + 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": + lastkey, limit = a, b + if lastkey >= 0: + last = first + lastkey + elif not limit: + last = argc + lastkey + else: + last = first + (argc - first) // limit + lastkey + else: + keynumidx, firstkey = a, b + if first + keynumidx >= argc: + return None + 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, encoder): + """ + The key count as the server reads it, or None for a value it rejects. + 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. + """ + try: + raw = encoder.encode(value) + except DataError: + return None + if not isinstance(raw, (bytes, bytearray)) or not raw.isdigit(): + 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) + + +def _compile_movablekeys_specs(commands): + """ + 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. + """ + compiled = {} + for name, command in commands.items(): + if "movablekeys" in command["flags"]: + specs = _compile_key_specs(command.get("key_specifications")) + if specs is not None: + specs = (int(command["arity"]), specs) + compiled[name] = specs + 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): + 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): """ 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._encoder = None self.redis_connection = redis_connection self.initialize(self.redis_connection) @@ -101,6 +234,8 @@ 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) + 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 @@ -140,7 +275,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(cmd_name, 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: @@ -255,8 +392,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. @@ -266,10 +404,12 @@ 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]] = {} + self._encoder: Optional["Encoder"] = None async def initialize(self, node: Optional["ClusterNode"] = None) -> None: if node: @@ -277,6 +417,8 @@ 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 @@ -316,7 +458,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(cmd_name, 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/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/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_asyncio/test_cluster.py b/tests/test_asyncio/test_cluster.py index fc70346f07..7f9cacfd33 100644 --- a/tests/test_asyncio/test_cluster.py +++ b/tests/test_asyncio/test_cluster.py @@ -5085,7 +5085,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_asyncio/test_command_parser.py b/tests/test_asyncio/test_command_parser.py index 9e52b2832c..0c3a6aa95c 100644 --- a/tests/test_asyncio/test_command_parser.py +++ b/tests/test_asyncio/test_command_parser.py @@ -1,6 +1,8 @@ +from unittest import mock + 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 @@ -27,6 +29,72 @@ 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_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_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 + @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._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) == [] + @pytest.mark.asyncio @skip_if_server_version_gte("8.5.240") async def test_get_command_policies(self, r): diff --git a/tests/test_cluster.py b/tests/test_cluster.py index aa6a0375ac..60fd96e15d 100644 --- a/tests/test_cluster.py +++ b/tests/test_cluster.py @@ -3238,7 +3238,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 diff --git a/tests/test_command_parser.py b/tests/test_command_parser.py index 99519ae58d..fbccbf4002 100644 --- a/tests/test_command_parser.py +++ b/tests/test_command_parser.py @@ -1,5 +1,8 @@ +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 @@ -57,24 +60,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 +85,78 @@ 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() + + @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() + + @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) + + @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 + + @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._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) == [] + + 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): 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}")