Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: adds JSON.TOGGLE command #1184

Merged
merged 4 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
69 changes: 69 additions & 0 deletions glide-core/src/client/value_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub(crate) enum ExpectedReturnType {
Set,
DoubleOrNull,
ZrankReturnType,
JsonToggleReturnType,
}

pub(crate) fn convert_to_expected_type(
Expand Down Expand Up @@ -121,6 +122,46 @@ pub(crate) fn convert_to_expected_type(
ExpectedReturnType::BulkString => Ok(Value::BulkString(
from_owned_redis_value::<String>(value)?.into(),
)),
ExpectedReturnType::JsonToggleReturnType => match value {
Value::Array(array) => {
let new_array: Result<Vec<_>, _> = array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let new_array: Result<Vec<_>, _> = array
let new_array: RedisResult<Vec<_>> = array

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new_array -> converted_array

.into_iter()
.map(|arr| match arr {
Copy link
Collaborator

@barshaul barshaul Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arr -> item

Value::Nil => Ok(Value::Nil),
_ => match from_owned_redis_value::<bool>(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),
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
match new_array {
Ok(values) => Ok(Value::Array(values)),
Err(err) => Err(err),
}
new_array.map(Value::Array)

}
Value::BulkString(s) => match std::str::from_utf8(&s) {
shohamazon marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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),
)
.into()),
},
_ => Err((
ErrorKind::TypeError,
"Response couldn't be converted to Json Toggle return type",
format!("(response was {:?})", value),
)
.into()),
},
}
}

Expand Down Expand Up @@ -204,6 +245,7 @@ pub(crate) fn expected_type_for_cmd(cmd: &Cmd) -> Option<ExpectedReturnType> {
b"SMEMBERS" => Some(ExpectedReturnType::Set),
b"ZSCORE" => Some(ExpectedReturnType::DoubleOrNull),
b"ZPOPMIN" | b"ZPOPMAX" => Some(ExpectedReturnType::MapOfStringToDouble),
b"JSON.TOGGLE" => Some(ExpectedReturnType::JsonToggleReturnType),
_ => None,
}
}
Expand Down Expand Up @@ -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());
}
}
38 changes: 37 additions & 1 deletion python/python/glide/async_commands/redis_modules/json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"for each path" is confusing as 'path' is a single value

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also refer to TJsonResponse to get more info

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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that when sending legacy path syntax, the value at path must be a boolean value.

instead, write that if X then returns error


Examples:
>>> from glide import json as redisJson
shohamazon marked this conversation as resolved.
Show resolved Hide resolved
shohamazon marked this conversation as resolved.
Show resolved Hide resolved
>>> await json.set(client, "doc", "$", json.dumps({"bool": true , "nested": {"bool": false , "nested": {"bool": 10}}}))
Copy link
Collaborator

@barshaul barshaul Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>> await json.set(client, "doc", "$", json.dumps({"bool": true , "nested": {"bool": false , "nested": {"bool": 10}}}))
>>> 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", "$")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>>> await redisJson.get(client, "doc", "$")
>>> json.loads(await redisJson.get(client, "doc", "$"))

and then output it accordingly

"[{\"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]),
)
6 changes: 5 additions & 1 deletion python/python/glide/constants.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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]].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(when specifying JSONPath) , => (when specifying JSONPath),
(remove redundant space)

# For more information, see: https://redis.io/docs/data-types/json/path/ .
TJsonResponse = Union[T, List[Optional[T]]]
18 changes: 18 additions & 0 deletions python/python/tests/tests_redis_modules/test_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Yury-Fridlyand marked this conversation as resolved.
Show resolved Hide resolved

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", "$")
Loading