From ee0474497ddc3a8c996ab662e96fe9b3ac0927e8 Mon Sep 17 00:00:00 2001 From: BoazBD Date: Wed, 23 Oct 2024 07:10:43 +0000 Subject: [PATCH 01/10] implement python json mget command Signed-off-by: BoazBD --- .../async_commands/server_modules/json.py | 44 +++++++ .../tests/tests_server_modules/test_json.py | 116 ++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index 40e4e80c51..4438c72c23 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -147,6 +147,50 @@ async def get( return cast(TJsonResponse[Optional[bytes]], await client.custom_command(args)) +async def mget( + client: TGlideClient, + keys: List[TEncodable], + paths: Optional[Union[TEncodable, List[TEncodable]]] = None, + options: Optional[JsonGetOptions] = None, +) -> Optional[List[bytes]]: + """ + Retrieves the JSON values at the specified `paths` stored at multiple `keys`. + + See https://valkey.io/commands/json.mget/ for more details. + + Args: + client (TGlideClient): The Redis client to execute the command. + keys (List[TEncodable]): A list of keys for the JSON documents. + paths (Optional[Union[TEncodable, List[TEncodable]]]): The path or list of paths within the JSON documents. Default is root `$`. + options (Optional[JsonGetOptions]): Options for formatting the byte representation of the JSON data. See `JsonGetOptions`. + + Returns: + Optional[List[bytes]]: A list of bytes representations of the returned values. + If a key doesn't exist, its corresponding entry will be `None`. + + Examples: + >>> from glide import json as redisJson + >>> import json + >>> json_strs = await redisJson.mget(client, ["doc1", "doc2"], ["$"]) + >>> [json.loads(js) for js in json_strs] # Parse JSON strings to Python data + [[{"a": 1.0, "b": 2}], [{"a": 2.0, "b": {"a": 3.0, "b" : 4.0}}]] # JSON objects retrieved from keys `doc1` and `doc2` + >>> await redisJson.mget(client, ["doc1", "doc2"], ["$.a"]) + [b"[1.0]", b"[2.0]"] # Returns values at path '$.a' for the JSON documents stored at `doc1` and `doc2`. + >>> await redisJson.mget(client, ["doc1"], ["$.non_existing_path"]) + [None] # Returns an empty array since the path '$.non_existing_path' does not exist in the JSON document stored at `doc1`. + """ + args = ["JSON.MGET"] + keys + if options: + args.extend(options.get_options()) + if paths: + if isinstance(paths, (str, bytes)): + paths = [paths] + args.extend(paths) + + results = await client.custom_command(args) + return [result if result is not None else None for result in results] + + async def arrinsert( client: TGlideClient, key: TEncodable, diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index 5989cb9f2e..9c86857032 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -143,6 +143,122 @@ async def test_json_get_formatting(self, glide_client: TGlideClient): expected_result = b'[\n~{\n~~"a":*1.0,\n~~"b":*2,\n~~"c":*{\n~~~"d":*3,\n~~~"e":*4\n~~}\n~}\n]' assert result == expected_result + @pytest.mark.parametrize("cluster_mode", [True, False]) + @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) + async def test_json_mget(self, glide_client: TGlideClient): + key = get_random_string(5) + key1 = f"{{{key}}}1" + key2 = f"{{{key}}}2" + # The prefix ensures that both keys hash to the same slot + + json1_value = {"a": 1.0, "b": {"a": 1, "b": 2.5, "c": True}} + json2_value = {"a": 3.0, "b": {"a": 1, "b": 4}} + + assert ( + await json.set(glide_client, key1, "$", OuterJson.dumps(json1_value)) == OK + ) + assert ( + await json.set(glide_client, key2, "$", OuterJson.dumps(json2_value)) == OK + ) + + result = await json.mget( + glide_client, + [key1, key2], + ["$"], + ) + expected_result = [ + b'[{"a":1.0,"b":{"a":1,"b":2.5,"c":true}}]', + b'[{"a":3.0,"b":{"a":1,"b":4}}]', + ] + assert result == expected_result + + result = await json.mget( + glide_client, + [key1, key2], + ["."], + ) + expected_result = [ + b'{"a":1.0,"b":{"a":1,"b":2.5,"c":true}}', + b'{"a":3.0,"b":{"a":1,"b":4}}', + ] + assert result == expected_result + + result = await json.mget( + glide_client, + [key1, key2], + ["$.a"], + ) + expected_result = [b"[1.0]", b"[3.0]"] + assert result == expected_result + + result = await json.mget( + glide_client, + [key1, key2], + ["$.b"], + ) + expected_result = [b'[{"a":1,"b":2.5,"c":true}]', b'[{"a":1,"b":4}]'] + assert result == expected_result + + result = await json.mget( + glide_client, + [key1, key2], + ["$..b"], + ) + expected_result = [b'[{"a":1,"b":2.5,"c":true},2.5]', b'[{"a":1,"b":4},4]'] + assert result == expected_result + + result = await json.mget( + glide_client, + [key1, key2], + [".b.b"], + ) + expected_result = [b"2.5", b"4"] + assert result == expected_result + + # Path doesn't exist + result = await json.mget( + glide_client, + [key1, key2], + ["$non_existing_path"], + ) + expected_result = [b"[]", b"[]"] + assert result == expected_result + + # Keys don't exist + result = await json.mget( + glide_client, + ["{non_existing_key}1", "{non_existing_key}2"], + ["$a"], + ) + expected_result = [None, None] + assert result == expected_result + + # Test with only one key + result = await json.mget( + glide_client, + [key1], + ["$.a"], + ) + expected_result = [b"[1.0]"] + assert result == expected_result + + # Value at path isnt an object + result = await json.mget( + glide_client, + [key1, key2], + ["$.e"], + ) + expected_result = [b"[]", b"[]"] + assert result == expected_result + + # No path given + result = await json.mget( + glide_client, + [key1, key2], + ) + expected_result = [None] + assert result == expected_result + @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) async def test_json_del(self, glide_client: TGlideClient): From 0853c09a3525a3480b3c67890d4f92b274c15eac Mon Sep 17 00:00:00 2001 From: BoazBD Date: Wed, 23 Oct 2024 08:21:06 +0000 Subject: [PATCH 02/10] update changelog Signed-off-by: BoazBD --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1bec5d60b..f3182c1ec7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Python: Add commands FT.ALIASADD, FT.ALIASDEL, FT.ALIASUPDATE([#2471](https://github.com/valkey-io/valkey-glide/pull/2471)) * Python: Python FT.DROPINDEX command ([#2437](https://github.com/valkey-io/valkey-glide/pull/2437)) * Python: Python: Added FT.CREATE command([#2413](https://github.com/valkey-io/valkey-glide/pull/2413)) +* Python: Add JSON.MGET command ([#2507](https://github.com/valkey-io/valkey-glide/pull/2507)) * Python: Add JSON.ARRLEN command ([#2403](https://github.com/valkey-io/valkey-glide/pull/2403)) * Python: Add JSON.CLEAR command ([#2418](https://github.com/valkey-io/valkey-glide/pull/2418)) * Python: Add JSON.TYPE command ([#2409](https://github.com/valkey-io/valkey-glide/pull/2409)) From 4133d049c0b3377304530b4aae14d43f56c0d9b0 Mon Sep 17 00:00:00 2001 From: BoazBD Date: Wed, 23 Oct 2024 08:24:50 +0000 Subject: [PATCH 03/10] fix type of mget Signed-off-by: BoazBD --- python/python/glide/async_commands/server_modules/json.py | 7 +++---- python/python/tests/tests_server_modules/test_json.py | 6 ++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index 4438c72c23..0f5b0b8849 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -152,7 +152,7 @@ async def mget( keys: List[TEncodable], paths: Optional[Union[TEncodable, List[TEncodable]]] = None, options: Optional[JsonGetOptions] = None, -) -> Optional[List[bytes]]: +) -> List[Optional[bytes]]: """ Retrieves the JSON values at the specified `paths` stored at multiple `keys`. @@ -165,7 +165,7 @@ async def mget( options (Optional[JsonGetOptions]): Options for formatting the byte representation of the JSON data. See `JsonGetOptions`. Returns: - Optional[List[bytes]]: A list of bytes representations of the returned values. + List[Optional[bytes]]: A list of bytes representations of the returned values. If a key doesn't exist, its corresponding entry will be `None`. Examples: @@ -187,8 +187,7 @@ async def mget( paths = [paths] args.extend(paths) - results = await client.custom_command(args) - return [result if result is not None else None for result in results] + return cast(List[Optional[bytes]], await client.custom_command(args)) async def arrinsert( diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index 9c86857032..635bd0b422 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -230,8 +230,7 @@ async def test_json_mget(self, glide_client: TGlideClient): ["{non_existing_key}1", "{non_existing_key}2"], ["$a"], ) - expected_result = [None, None] - assert result == expected_result + assert result == [None, None] # Test with only one key result = await json.mget( @@ -256,8 +255,7 @@ async def test_json_mget(self, glide_client: TGlideClient): glide_client, [key1, key2], ) - expected_result = [None] - assert result == expected_result + assert result == [None] @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) From 099d77fd8a1d4c99a35f9beae378ff05724e5a02 Mon Sep 17 00:00:00 2001 From: BoazBD Date: Sun, 27 Oct 2024 09:02:33 +0000 Subject: [PATCH 04/10] fix signature and docstring Signed-off-by: BoazBD --- .../async_commands/server_modules/json.py | 36 +++++++++--------- .../tests/tests_server_modules/test_json.py | 38 +++++++++---------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index 0f5b0b8849..63fef358ea 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -150,42 +150,42 @@ async def get( async def mget( client: TGlideClient, keys: List[TEncodable], - paths: Optional[Union[TEncodable, List[TEncodable]]] = None, - options: Optional[JsonGetOptions] = None, + path: Optional[TEncodable] = None, ) -> List[Optional[bytes]]: """ - Retrieves the JSON values at the specified `paths` stored at multiple `keys`. - - See https://valkey.io/commands/json.mget/ for more details. + Retrieves the JSON values at the specified `path` stored at multiple `keys`. + Note: + When in cluster mode, the command may route to multiple nodes when `keys` map to different hash slots. Args: client (TGlideClient): The Redis client to execute the command. keys (List[TEncodable]): A list of keys for the JSON documents. - paths (Optional[Union[TEncodable, List[TEncodable]]]): The path or list of paths within the JSON documents. Default is root `$`. - options (Optional[JsonGetOptions]): Options for formatting the byte representation of the JSON data. See `JsonGetOptions`. + path (Optional[TEncodable]): The path within the JSON documents. Default is root `$`. Returns: - List[Optional[bytes]]: A list of bytes representations of the returned values. - If a key doesn't exist, its corresponding entry will be `None`. + List[Optional[bytes]]: + For JSONPath (`path` starts with `$`): + Returns a list of byte representations of the values found at the given path for each key. If the path does not exist, + the entry will be an empty array. + For legacy path (`path` starts with `.`): + Returns a string representation of the value at the specified path. If the path does not exist, the entry will be None. + If a key doesn't exist, the corresponding list element will also be `None`. + Examples: >>> from glide import json as redisJson >>> import json - >>> json_strs = await redisJson.mget(client, ["doc1", "doc2"], ["$"]) + >>> json_strs = await redisJson.mget(client, ["doc1", "doc2"], "$") >>> [json.loads(js) for js in json_strs] # Parse JSON strings to Python data [[{"a": 1.0, "b": 2}], [{"a": 2.0, "b": {"a": 3.0, "b" : 4.0}}]] # JSON objects retrieved from keys `doc1` and `doc2` - >>> await redisJson.mget(client, ["doc1", "doc2"], ["$.a"]) + >>> await redisJson.mget(client, ["doc1", "doc2"], "$.a") [b"[1.0]", b"[2.0]"] # Returns values at path '$.a' for the JSON documents stored at `doc1` and `doc2`. - >>> await redisJson.mget(client, ["doc1"], ["$.non_existing_path"]) + >>> await redisJson.mget(client, ["doc1"], "$.non_existing_path") [None] # Returns an empty array since the path '$.non_existing_path' does not exist in the JSON document stored at `doc1`. """ args = ["JSON.MGET"] + keys - if options: - args.extend(options.get_options()) - if paths: - if isinstance(paths, (str, bytes)): - paths = [paths] - args.extend(paths) + if path: + args.append(path) return cast(List[Optional[bytes]], await client.custom_command(args)) diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index 635bd0b422..bb4495665e 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -164,7 +164,7 @@ async def test_json_mget(self, glide_client: TGlideClient): result = await json.mget( glide_client, [key1, key2], - ["$"], + "$", ) expected_result = [ b'[{"a":1.0,"b":{"a":1,"b":2.5,"c":true}}]', @@ -175,7 +175,7 @@ async def test_json_mget(self, glide_client: TGlideClient): result = await json.mget( glide_client, [key1, key2], - ["."], + ".", ) expected_result = [ b'{"a":1.0,"b":{"a":1,"b":2.5,"c":true}}', @@ -186,7 +186,7 @@ async def test_json_mget(self, glide_client: TGlideClient): result = await json.mget( glide_client, [key1, key2], - ["$.a"], + "$.a", ) expected_result = [b"[1.0]", b"[3.0]"] assert result == expected_result @@ -194,7 +194,7 @@ async def test_json_mget(self, glide_client: TGlideClient): result = await json.mget( glide_client, [key1, key2], - ["$.b"], + "$.b", ) expected_result = [b'[{"a":1,"b":2.5,"c":true}]', b'[{"a":1,"b":4}]'] assert result == expected_result @@ -202,7 +202,7 @@ async def test_json_mget(self, glide_client: TGlideClient): result = await json.mget( glide_client, [key1, key2], - ["$..b"], + "$..b", ) expected_result = [b'[{"a":1,"b":2.5,"c":true},2.5]', b'[{"a":1,"b":4},4]'] assert result == expected_result @@ -210,25 +210,34 @@ async def test_json_mget(self, glide_client: TGlideClient): result = await json.mget( glide_client, [key1, key2], - [".b.b"], + ".b.b", ) expected_result = [b"2.5", b"4"] assert result == expected_result - # Path doesn't exist + # JSONPath doesn't exist result = await json.mget( glide_client, [key1, key2], - ["$non_existing_path"], + "$non_existing_path", ) expected_result = [b"[]", b"[]"] assert result == expected_result + # Legacy path doesn't exist + result = await json.mget( + glide_client, + [key1, key2], + ".non_existing_path", + ) + expected_result = [None, None] + assert result == expected_result + # Keys don't exist result = await json.mget( glide_client, ["{non_existing_key}1", "{non_existing_key}2"], - ["$a"], + "$a", ) assert result == [None, None] @@ -236,20 +245,11 @@ async def test_json_mget(self, glide_client: TGlideClient): result = await json.mget( glide_client, [key1], - ["$.a"], + "$.a", ) expected_result = [b"[1.0]"] assert result == expected_result - # Value at path isnt an object - result = await json.mget( - glide_client, - [key1, key2], - ["$.e"], - ) - expected_result = [b"[]", b"[]"] - assert result == expected_result - # No path given result = await json.mget( glide_client, From 340cef786e2a00af6374b74cb8efdec0f673310e Mon Sep 17 00:00:00 2001 From: BoazBD Date: Sun, 27 Oct 2024 09:14:41 +0000 Subject: [PATCH 05/10] fix type test fail Signed-off-by: BoazBD --- python/python/tests/tests_server_modules/test_json.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index bb4495665e..a7d0498686 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -230,8 +230,7 @@ async def test_json_mget(self, glide_client: TGlideClient): [key1, key2], ".non_existing_path", ) - expected_result = [None, None] - assert result == expected_result + assert result == [None, None] # Keys don't exist result = await json.mget( From dd5a0b207fbbd53145dc4f4a7cf4e22fe09d47cb Mon Sep 17 00:00:00 2001 From: BoazBD Date: Tue, 12 Nov 2024 07:48:50 +0000 Subject: [PATCH 06/10] update due to multi-slot requests support Signed-off-by: BoazBD --- .../python/glide/async_commands/server_modules/json.py | 9 +++++++-- python/python/tests/tests_server_modules/test_json.py | 6 ++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index 5ca0f2381d..ba2a2469f1 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -213,7 +213,12 @@ async def mget( Retrieves the JSON values at the specified `path` stored at multiple `keys`. Note: - When in cluster mode, the command may route to multiple nodes when `keys` map to different hash slots. + In cluster mode, if keys in `keyValueMap` map to different hash slots, the command + will be split across these slots and executed separately for each. This means the command + is atomic only at the slot level. If one or more slot-specific requests fail, the entire + call will return the first encountered error, even though some requests may have succeeded + while others did not. If this behavior impacts your application logic, consider splitting + the request into sub-requests per slot to ensure atomicity. Args: client (TGlideClient): The Redis client to execute the command. keys (List[TEncodable]): A list of keys for the JSON documents. @@ -244,7 +249,7 @@ async def mget( if path: args.append(path) - return cast(List[Optional[bytes]], await client.custom_command(args)) + return cast(TJsonResponse[Optional[bytes]], await client.custom_command(args)) async def arrappend( diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index 0c084bd5d1..ea5fbeb621 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -165,10 +165,8 @@ async def test_json_get_formatting(self, glide_client: TGlideClient): @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) async def test_json_mget(self, glide_client: TGlideClient): - key = get_random_string(5) - key1 = f"{{{key}}}1" - key2 = f"{{{key}}}2" - # The prefix ensures that both keys hash to the same slot + key1 = get_random_string(5) + key2 = get_random_string(5) json1_value = {"a": 1.0, "b": {"a": 1, "b": 2.5, "c": True}} json2_value = {"a": 3.0, "b": {"a": 1, "b": 4}} From 964fee913c62012b5aafbd26a084b9f2082bad17 Mon Sep 17 00:00:00 2001 From: BoazBD <50696333+BoazBD@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:50:53 +0200 Subject: [PATCH 07/10] Apply suggestions from code review Co-authored-by: Shoham Elias <116083498+shohamazon@users.noreply.github.com> Signed-off-by: BoazBD <50696333+BoazBD@users.noreply.github.com> --- .../async_commands/server_modules/json.py | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index ba2a2469f1..bbc0cef9ea 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -219,30 +219,33 @@ async def mget( call will return the first encountered error, even though some requests may have succeeded while others did not. If this behavior impacts your application logic, consider splitting the request into sub-requests per slot to ensure atomicity. + Args: - client (TGlideClient): The Redis client to execute the command. + client (TGlideClient): The client to execute the command. keys (List[TEncodable]): A list of keys for the JSON documents. path (Optional[TEncodable]): The path within the JSON documents. Default is root `$`. Returns: List[Optional[bytes]]: + List[Optional[bytes]]: For JSONPath (`path` starts with `$`): - Returns a list of byte representations of the values found at the given path for each key. If the path does not exist, - the entry will be an empty array. - For legacy path (`path` starts with `.`): - Returns a string representation of the value at the specified path. If the path does not exist, the entry will be None. - If a key doesn't exist, the corresponding list element will also be `None`. + Returns a list of byte representations of the values found at the given path for each key. + If `path` does not exist within the key, the entry will be an empty array. + For legacy path (`path` doesn't starts with `$`): + Returns a list of byte representations of the values found at the given path for each key. + If `path` does not exist within the key, the entry will be None. + If a key doesn't exist, the corresponding list element will be None. Examples: - >>> from glide import json as redisJson + >>> from glide import json as glideJson >>> import json - >>> json_strs = await redisJson.mget(client, ["doc1", "doc2"], "$") + >>> json_strs = await glideJson.mget(client, ["doc1", "doc2"], "$") >>> [json.loads(js) for js in json_strs] # Parse JSON strings to Python data [[{"a": 1.0, "b": 2}], [{"a": 2.0, "b": {"a": 3.0, "b" : 4.0}}]] # JSON objects retrieved from keys `doc1` and `doc2` - >>> await redisJson.mget(client, ["doc1", "doc2"], "$.a") + >>> await glideJson.mget(client, ["doc1", "doc2"], "$.a") [b"[1.0]", b"[2.0]"] # Returns values at path '$.a' for the JSON documents stored at `doc1` and `doc2`. - >>> await redisJson.mget(client, ["doc1"], "$.non_existing_path") + >>> await glideJson.mget(client, ["doc1"], "$.non_existing_path") [None] # Returns an empty array since the path '$.non_existing_path' does not exist in the JSON document stored at `doc1`. """ args = ["JSON.MGET"] + keys From b9b20cd028dc9f5f667ec6fd5be269302c35b3b1 Mon Sep 17 00:00:00 2001 From: BoazBD Date: Tue, 12 Nov 2024 12:52:29 +0000 Subject: [PATCH 08/10] fix shoham comments Signed-off-by: BoazBD --- .../async_commands/server_modules/json.py | 2 +- .../tests/tests_server_modules/test_json.py | 24 ++++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index bbc0cef9ea..dbbcd70482 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -213,7 +213,7 @@ async def mget( Retrieves the JSON values at the specified `path` stored at multiple `keys`. Note: - In cluster mode, if keys in `keyValueMap` map to different hash slots, the command + In cluster mode, if keys in `keys` map to different hash slots, the command will be split across these slots and executed separately for each. This means the command is atomic only at the slot level. If one or more slot-specific requests fail, the entire call will return the first encountered error, even though some requests may have succeeded diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index ea5fbeb621..6292264a9c 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -178,6 +178,7 @@ async def test_json_mget(self, glide_client: TGlideClient): await json.set(glide_client, key2, "$", OuterJson.dumps(json2_value)) == OK ) + # Test with root JSONPath result = await json.mget( glide_client, [key1, key2], @@ -189,6 +190,7 @@ async def test_json_mget(self, glide_client: TGlideClient): ] assert result == expected_result + # Retrieves the full JSON objects from multiple keys. result = await json.mget( glide_client, [key1, key2], @@ -208,6 +210,7 @@ async def test_json_mget(self, glide_client: TGlideClient): expected_result = [b"[1.0]", b"[3.0]"] assert result == expected_result + # Retrieves the value of the 'b' field for multiple keys. result = await json.mget( glide_client, [key1, key2], @@ -216,6 +219,7 @@ async def test_json_mget(self, glide_client: TGlideClient): expected_result = [b'[{"a":1,"b":2.5,"c":true}]', b'[{"a":1,"b":4}]'] assert result == expected_result + # Retrieves all values of 'b' fields using recursive path for multiple keys result = await json.mget( glide_client, [key1, key2], @@ -224,6 +228,7 @@ async def test_json_mget(self, glide_client: TGlideClient): expected_result = [b'[{"a":1,"b":2.5,"c":true},2.5]', b'[{"a":1,"b":4},4]'] assert result == expected_result + # retrieves the value of the nested 'b.b' field for multiple keys result = await json.mget( glide_client, [key1, key2], @@ -232,6 +237,15 @@ async def test_json_mget(self, glide_client: TGlideClient): expected_result = [b"2.5", b"4"] assert result == expected_result + # Legacy path that exists in only one of the keys + result = await json.mget( + glide_client, + [key1, key2], + ".b.c", + ) + expected_result = [b"true", None] + assert result == expected_result + # JSONPath doesn't exist result = await json.mget( glide_client, @@ -249,7 +263,15 @@ async def test_json_mget(self, glide_client: TGlideClient): ) assert result == [None, None] - # Keys don't exist + # One key doesn't exist + result = await json.mget( + glide_client, + [key1, "{non_existing_key}"], + "$.a", + ) + assert result == [b"[1.0]", None] + + # Both keys don't exist result = await json.mget( glide_client, ["{non_existing_key}1", "{non_existing_key}2"], From 278673e6a1c97d9479b99dd775602a072d937c0e Mon Sep 17 00:00:00 2001 From: BoazBD Date: Tue, 12 Nov 2024 14:07:21 +0000 Subject: [PATCH 09/10] Improve documentation and tests Signed-off-by: BoazBD --- .../async_commands/server_modules/json.py | 1 - .../tests/tests_server_modules/test_json.py | 19 ++++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index dbbcd70482..7ea5e152d0 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -227,7 +227,6 @@ async def mget( Returns: List[Optional[bytes]]: - List[Optional[bytes]]: For JSONPath (`path` starts with `$`): Returns a list of byte representations of the values found at the given path for each key. If `path` does not exist within the key, the entry will be an empty array. diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index 6292264a9c..0f2ddea54d 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -237,6 +237,15 @@ async def test_json_mget(self, glide_client: TGlideClient): expected_result = [b"2.5", b"4"] assert result == expected_result + # JSONPath that exists in only one of the keys + result = await json.mget( + glide_client, + [key1, key2], + "$.b.c", + ) + expected_result = [b"[true]", b"[]"] + assert result == expected_result + # Legacy path that exists in only one of the keys result = await json.mget( glide_client, @@ -263,7 +272,7 @@ async def test_json_mget(self, glide_client: TGlideClient): ) assert result == [None, None] - # One key doesn't exist + # JSONPath one key doesn't exist result = await json.mget( glide_client, [key1, "{non_existing_key}"], @@ -271,6 +280,14 @@ async def test_json_mget(self, glide_client: TGlideClient): ) assert result == [b"[1.0]", None] + # Legacy path one key doesn't exist + result = await json.mget( + glide_client, + [key1, "{non_existing_key}"], + ".a", + ) + assert result == [b"1.0", None] + # Both keys don't exist result = await json.mget( glide_client, From 2f9e62fc6670d043adc0a5e904419a8fc42890c7 Mon Sep 17 00:00:00 2001 From: BoazBD Date: Wed, 13 Nov 2024 07:50:20 +0000 Subject: [PATCH 10/10] make path mandatory Signed-off-by: BoazBD --- .../python/glide/async_commands/server_modules/json.py | 9 +++------ python/python/tests/tests_server_modules/test_json.py | 7 ------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/json.py index 7ea5e152d0..d4c3f8797b 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/json.py @@ -207,7 +207,7 @@ async def get( async def mget( client: TGlideClient, keys: List[TEncodable], - path: Optional[TEncodable] = None, + path: TEncodable, ) -> List[Optional[bytes]]: """ Retrieves the JSON values at the specified `path` stored at multiple `keys`. @@ -223,7 +223,7 @@ async def mget( Args: client (TGlideClient): The client to execute the command. keys (List[TEncodable]): A list of keys for the JSON documents. - path (Optional[TEncodable]): The path within the JSON documents. Default is root `$`. + path (TEncodable): The path within the JSON documents. Returns: List[Optional[bytes]]: @@ -247,10 +247,7 @@ async def mget( >>> await glideJson.mget(client, ["doc1"], "$.non_existing_path") [None] # Returns an empty array since the path '$.non_existing_path' does not exist in the JSON document stored at `doc1`. """ - args = ["JSON.MGET"] + keys - if path: - args.append(path) - + args = ["JSON.MGET"] + keys + [path] return cast(TJsonResponse[Optional[bytes]], await client.custom_command(args)) diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index 0f2ddea54d..eba62faa17 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -305,13 +305,6 @@ async def test_json_mget(self, glide_client: TGlideClient): expected_result = [b"[1.0]"] assert result == expected_result - # No path given - result = await json.mget( - glide_client, - [key1, key2], - ) - assert result == [None] - @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) async def test_json_del(self, glide_client: TGlideClient):