From 88b51b11305e742d13214dfe6dd8b69deeafda0a Mon Sep 17 00:00:00 2001 From: Shoham Elias Date: Tue, 26 Mar 2024 15:49:47 +0000 Subject: [PATCH 1/4] Python: adds JSON.TOGGLE command --- CHANGELOG.md | 1 + glide-core/src/client/value_conversion.rs | 69 +++++++++++++++++++ .../async_commands/redis_modules/json.py | 38 +++++++++- python/python/glide/constants.py | 6 +- .../tests/tests_redis_modules/test_json.py | 18 +++++ 5 files changed, 130 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c721856e2f..8538b62fc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ * Python, Node: Added ZRANK command ([#1065](https://github.com/aws/glide-for-redis/pull/1065), [#1149](https://github.com/aws/glide-for-redis/pull/1149)) * Core: Enabled Cluster Mode periodic checks by default ([#1089](https://github.com/aws/glide-for-redis/pull/1089)) * Node: Added Rename command. ([#1124](https://github.com/aws/glide-for-redis/pull/1124)) +* Python: Added JSON.TOGGLE command ([#1184](https://github.com/aws/glide-for-redis/pull/1184)) #### Features diff --git a/glide-core/src/client/value_conversion.rs b/glide-core/src/client/value_conversion.rs index 21bfb3d4b7..d4dc936cbc 100644 --- a/glide-core/src/client/value_conversion.rs +++ b/glide-core/src/client/value_conversion.rs @@ -15,6 +15,7 @@ pub(crate) enum ExpectedReturnType { Set, DoubleOrNull, ZrankReturnType, + JsonToggleReturnType, } pub(crate) fn convert_to_expected_type( @@ -121,6 +122,46 @@ pub(crate) fn convert_to_expected_type( ExpectedReturnType::BulkString => Ok(Value::BulkString( from_owned_redis_value::(value)?.into(), )), + ExpectedReturnType::JsonToggleReturnType => match value { + Value::Array(array) => { + let new_array: Result, _> = array + .into_iter() + .map(|arr| match arr { + Value::Nil => Ok(Value::Nil), + _ => match from_owned_redis_value::(arr.clone()) { + Ok(boolean_value) => Ok(Value::Boolean(boolean_value)), + _ => Err(( + ErrorKind::TypeError, + "Could not convert value to boolean", + format!("(value was {:?})", arr), + ) + .into()), + }, + }) + .collect(); + + match new_array { + Ok(values) => Ok(Value::Array(values)), + Err(err) => Err(err), + } + } + Value::BulkString(s) => match std::str::from_utf8(&s) { + Ok("true") => Ok(Value::Boolean(true)), + Ok("false") => Ok(Value::Boolean(false)), + _ => Err(( + ErrorKind::TypeError, + "Response couldn't be converted to boolean", + format!("(response was {:?})", s), + ) + .into()), + }, + _ => Err(( + ErrorKind::TypeError, + "Response couldn't be converted to Array of bool or null", + format!("(response was {:?})", value), + ) + .into()), + }, } } @@ -204,6 +245,7 @@ pub(crate) fn expected_type_for_cmd(cmd: &Cmd) -> Option { b"SMEMBERS" => Some(ExpectedReturnType::Set), b"ZSCORE" => Some(ExpectedReturnType::DoubleOrNull), b"ZPOPMIN" | b"ZPOPMAX" => Some(ExpectedReturnType::MapOfStringToDouble), + b"JSON.TOGGLE" => Some(ExpectedReturnType::JsonToggleReturnType), _ => None, } } @@ -423,4 +465,31 @@ mod tests { assert!(convert_to_expected_type(Value::Nil, Some(ExpectedReturnType::Double)).is_err()); } + + #[test] + fn test_convert_to_list_of_bool_or_null() { + let array = vec![Value::Nil, Value::Int(0), Value::Int(1)]; + let array_result = convert_to_expected_type( + Value::Array(array), + Some(ExpectedReturnType::JsonToggleReturnType), + ) + .unwrap(); + + let array_result = if let Value::Array(array) = array_result { + array + } else { + panic!("Expected an Array, but got {:?}", array_result); + }; + assert_eq!(array_result.len(), 3); + + assert_eq!(array_result[0], Value::Nil); + assert_eq!(array_result[1], Value::Boolean(false)); + assert_eq!(array_result[2], Value::Boolean(true)); + + assert!(convert_to_expected_type( + Value::Nil, + Some(ExpectedReturnType::JsonToggleReturnType) + ) + .is_err()); + } } diff --git a/python/python/glide/async_commands/redis_modules/json.py b/python/python/glide/async_commands/redis_modules/json.py index fdc8d8178f..73098ab41d 100644 --- a/python/python/glide/async_commands/redis_modules/json.py +++ b/python/python/glide/async_commands/redis_modules/json.py @@ -18,7 +18,7 @@ from typing import List, Optional, Union, cast from glide.async_commands.core import ConditionalChange -from glide.constants import TOK +from glide.constants import TOK, TJsonResponse from glide.protobuf.redis_request_pb2 import RequestType from glide.redis_client import TRedisClient @@ -137,3 +137,39 @@ async def get( args.extend(paths) return cast(str, await client.custom_command(args)) + + +async def toggle( + client: TRedisClient, + key: str, + path: str, +) -> TJsonResponse[bool]: + """ + Toggles a Boolean value stored at the specified `path` within the JSON document stored at `key`. + + See https://redis.io/commands/json.toggle/ for more details. + + Args: + client (TRedisClient): The Redis client to execute the command. + key (str): The key of the JSON document. + path (str): The JSONPath to specify. + + Returns: + TJsonResponse[bool]: Returns an array of boolean replies for each path, with the new value (false or true), or None for JSON values matching the path that are not Boolean. + If `path` doesn't start with `$` (legacy path syntax), returns the new value of the toggled value in `path`. Note that when sending legacy path syntax, the value at `path` must be a boolean value. + + Examples: + >>> from glide import json as redisJson + >>> await json.set(client, "doc", "$", json.dumps({"bool": true , "nested": {"bool": false , "nested": {"bool": 10}}})) + >>> await redisJson.toggle(client, "doc", "$.bool") + [False, True, None] # Indicates successful toggling of the Boolean values at path '$.bool' in the key stored at `doc`. + >>> await redisJson.toggle(client, "doc", "bool") + True # Indicates successful toggling of the Boolean value at path 'bool' in the key stored at `doc`. + >>> await redisJson.get(client, "doc", "$") + "[{\"bool\":true,\"nested\":{\"bool\":true,\"nested\":{\"bool\":10}}}]" # The updated JSON value in the key stored at `doc`. + """ + + return cast( + TJsonResponse[bool], + await client.custom_command(["JSON.TOGGLE", key, path]), + ) diff --git a/python/python/glide/constants.py b/python/python/glide/constants.py index a1c7c58445..d613a90b63 100644 --- a/python/python/glide/constants.py +++ b/python/python/glide/constants.py @@ -1,6 +1,6 @@ # Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0 -from typing import Dict, List, Literal, Set, TypeVar, Union +from typing import Dict, List, Literal, Optional, Set, TypeVar, Union from glide.protobuf.connection_request_pb2 import ConnectionRequest from glide.protobuf.redis_request_pb2 import RedisRequest @@ -27,3 +27,7 @@ # Otherwise, response will be : {Address : response , ... } with type of Dict[str, T]. TClusterResponse = Union[T, Dict[str, T]] TSingleNodeRoute = Union[RandomNode, SlotKeyRoute, SlotIdRoute, ByAddressRoute] +# When specifying legacy path (path doesn't start with `$`), response will be T +# Otherwise, (when specifying JSONPath) , response will be List[Optional[T]]. +# For more information, see: https://redis.io/docs/data-types/json/path/ . +TJsonResponse = Union[T, List[Optional[T]]] diff --git a/python/python/tests/tests_redis_modules/test_json.py b/python/python/tests/tests_redis_modules/test_json.py index 7ddb8cc6d7..121757f331 100644 --- a/python/python/tests/tests_redis_modules/test_json.py +++ b/python/python/tests/tests_redis_modules/test_json.py @@ -8,6 +8,7 @@ from glide.async_commands.redis_modules.json import JsonGetOptions from glide.config import ProtocolVersion from glide.constants import OK +from glide.exceptions import RequestError from glide.redis_client import TRedisClient from tests.test_async_client import get_random_string, parse_info_response @@ -148,3 +149,20 @@ async def test_json_get_formatting(self, redis_client: TRedisClient): '[\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_toggle(self, redis_client: TRedisClient): + key = get_random_string(10) + json_value = {"bool": True, "nested": {"bool": False, "nested": {"bool": 10}}} + assert await json.set(redis_client, key, "$", OuterJson.dumps(json_value)) == OK + + assert await json.toggle(redis_client, key, "$..bool") == [False, True, None] + assert await json.toggle(redis_client, key, "bool") + + assert await json.toggle(redis_client, key, "$.nested") == [None] + with pytest.raises(RequestError): + assert await json.toggle(redis_client, key, "nested") + + with pytest.raises(RequestError): + assert await json.toggle(redis_client, "non_exiting_key", "$") From 41988898d0e59f2715b0e9897092d718a6b46e31 Mon Sep 17 00:00:00 2001 From: Shoham Elias <116083498+shohamazon@users.noreply.github.com> Date: Tue, 26 Mar 2024 20:28:57 +0200 Subject: [PATCH 2/4] Update CHANGELOG.md Co-authored-by: Yury-Fridlyand --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8538b62fc5..5a468c2004 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ * Python, Node: Added ZRANK command ([#1065](https://github.com/aws/glide-for-redis/pull/1065), [#1149](https://github.com/aws/glide-for-redis/pull/1149)) * Core: Enabled Cluster Mode periodic checks by default ([#1089](https://github.com/aws/glide-for-redis/pull/1089)) * Node: Added Rename command. ([#1124](https://github.com/aws/glide-for-redis/pull/1124)) -* Python: Added JSON.TOGGLE command ([#1184](https://github.com/aws/glide-for-redis/pull/1184)) +* Python: Added JSON.TOGGLE command ([#1184](https://github.com/aws/glide-for-redis/pull/1184)) #### Features From f009815ae0451306aaa70564b4d68e452a7ab4e0 Mon Sep 17 00:00:00 2001 From: Shoham Elias <116083498+shohamazon@users.noreply.github.com> Date: Tue, 26 Mar 2024 22:55:11 +0200 Subject: [PATCH 3/4] Update glide-core/src/client/value_conversion.rs --- glide-core/src/client/value_conversion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glide-core/src/client/value_conversion.rs b/glide-core/src/client/value_conversion.rs index d4dc936cbc..b313946f32 100644 --- a/glide-core/src/client/value_conversion.rs +++ b/glide-core/src/client/value_conversion.rs @@ -157,7 +157,7 @@ pub(crate) fn convert_to_expected_type( }, _ => Err(( ErrorKind::TypeError, - "Response couldn't be converted to Array of bool or null", + "Response couldn't be converted to Json Toggle return type", format!("(response was {:?})", value), ) .into()), From 08f3b91a4fc4f69d22a76f0d8811e9144274d1d2 Mon Sep 17 00:00:00 2001 From: Shoham Elias Date: Wed, 27 Mar 2024 10:18:01 +0000 Subject: [PATCH 4/4] fix documentation --- glide-core/src/client/value_conversion.rs | 17 +++++++---------- .../glide/async_commands/redis_modules/json.py | 15 ++++++++++----- python/python/glide/constants.py | 2 +- .../tests/tests_redis_modules/test_json.py | 2 +- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/glide-core/src/client/value_conversion.rs b/glide-core/src/client/value_conversion.rs index b313946f32..46b51ad8f5 100644 --- a/glide-core/src/client/value_conversion.rs +++ b/glide-core/src/client/value_conversion.rs @@ -124,34 +124,31 @@ pub(crate) fn convert_to_expected_type( )), ExpectedReturnType::JsonToggleReturnType => match value { Value::Array(array) => { - let new_array: Result, _> = array + let converted_array: RedisResult> = array .into_iter() - .map(|arr| match arr { + .map(|item| match item { Value::Nil => Ok(Value::Nil), - _ => match from_owned_redis_value::(arr.clone()) { + _ => match from_owned_redis_value::(item.clone()) { Ok(boolean_value) => Ok(Value::Boolean(boolean_value)), _ => Err(( ErrorKind::TypeError, "Could not convert value to boolean", - format!("(value was {:?})", arr), + format!("(value was {:?})", item), ) .into()), }, }) .collect(); - match new_array { - Ok(values) => Ok(Value::Array(values)), - Err(err) => Err(err), - } + converted_array.map(Value::Array) } - Value::BulkString(s) => match std::str::from_utf8(&s) { + Value::BulkString(bytes) => match std::str::from_utf8(&bytes) { Ok("true") => Ok(Value::Boolean(true)), Ok("false") => Ok(Value::Boolean(false)), _ => Err(( ErrorKind::TypeError, "Response couldn't be converted to boolean", - format!("(response was {:?})", s), + format!("(response was {:?})", bytes), ) .into()), }, diff --git a/python/python/glide/async_commands/redis_modules/json.py b/python/python/glide/async_commands/redis_modules/json.py index 73098ab41d..21c29f3efa 100644 --- a/python/python/glide/async_commands/redis_modules/json.py +++ b/python/python/glide/async_commands/redis_modules/json.py @@ -155,18 +155,23 @@ async def toggle( path (str): The JSONPath to specify. Returns: - TJsonResponse[bool]: Returns an array of boolean replies for each path, with the new value (false or true), or None for JSON values matching the path that are not Boolean. - If `path` doesn't start with `$` (legacy path syntax), returns the new value of the toggled value in `path`. Note that when sending legacy path syntax, the value at `path` must be a boolean value. + TJsonResponse[bool]: For JSONPath (`path` starts with `$`), returns a list of boolean replies for every possible path, with the toggled boolean value, + or None for JSON values matching the path that are not boolean. + For legacy path (`path` doesn't starts with `$`), returns the value of the toggled boolean in `path`. + Note that when sending legacy path syntax, If `path` doesn't exist or the value at `path` isn't a boolean, an error is raised. + For more information about the returned type, see `TJsonResponse`. Examples: >>> from glide import json as redisJson - >>> await json.set(client, "doc", "$", json.dumps({"bool": true , "nested": {"bool": false , "nested": {"bool": 10}}})) + >>> import json + >>> await redisJson.set(client, "doc", "$", json.dumps({"bool": True, "nested": {"bool": False, "nested": {"bool": 10}}})) + 'OK' >>> await redisJson.toggle(client, "doc", "$.bool") [False, True, None] # Indicates successful toggling of the Boolean values at path '$.bool' in the key stored at `doc`. >>> await redisJson.toggle(client, "doc", "bool") True # Indicates successful toggling of the Boolean value at path 'bool' in the key stored at `doc`. - >>> await redisJson.get(client, "doc", "$") - "[{\"bool\":true,\"nested\":{\"bool\":true,\"nested\":{\"bool\":10}}}]" # The updated JSON value in the key stored at `doc`. + >>> json.loads(await redisJson.get(client, "doc", "$")) + [{"bool": True, "nested": {"bool": True, "nested": {"bool": 10}}}] # The updated JSON value in the key stored at `doc`. """ return cast( diff --git a/python/python/glide/constants.py b/python/python/glide/constants.py index d613a90b63..54232dd6c5 100644 --- a/python/python/glide/constants.py +++ b/python/python/glide/constants.py @@ -28,6 +28,6 @@ TClusterResponse = Union[T, Dict[str, T]] TSingleNodeRoute = Union[RandomNode, SlotKeyRoute, SlotIdRoute, ByAddressRoute] # When specifying legacy path (path doesn't start with `$`), response will be T -# Otherwise, (when specifying JSONPath) , response will be List[Optional[T]]. +# Otherwise, (when specifying JSONPath), response will be List[Optional[T]]. # For more information, see: https://redis.io/docs/data-types/json/path/ . TJsonResponse = Union[T, List[Optional[T]]] diff --git a/python/python/tests/tests_redis_modules/test_json.py b/python/python/tests/tests_redis_modules/test_json.py index 121757f331..8d04a10ab1 100644 --- a/python/python/tests/tests_redis_modules/test_json.py +++ b/python/python/tests/tests_redis_modules/test_json.py @@ -158,7 +158,7 @@ async def test_json_toggle(self, redis_client: TRedisClient): assert await json.set(redis_client, key, "$", OuterJson.dumps(json_value)) == OK assert await json.toggle(redis_client, key, "$..bool") == [False, True, None] - assert await json.toggle(redis_client, key, "bool") + assert await json.toggle(redis_client, key, "bool") is True assert await json.toggle(redis_client, key, "$.nested") == [None] with pytest.raises(RequestError):