Skip to content

Commit 76fc776

Browse files
gkorlandgkorland
authored
upgrade to redismodule-rs v2 (RedisJSON#964)
* upgrade to redismodule-rs v2 * refactor break KeyValue to its own file (RedisJSON#965) * fix build * fix build remove feature test * support RESP3 * use FormatOptions * add support for multi paths * set redismodule-rs to 2.0 * fix String serializtion * change json.get resutl from map tp array * format code and update deps * add system allocator when running unit test * format code * add initial tests * add more tests * add more tests * fix tests/pytest/requirements.txt * add tests for NUMMULTBY & NUMINCRBY * fix test for cluster * add support for JSON.TYPE * add EXPAND format on json.get * avoid using cutom formatter when not needed * remove bson code * fix test assert on unordered results * update Cargo.lock and add ignore words * revert deps/readies * revert test_resp3 * retrun bson * fix review commentsw
1 parent e6e76ae commit 76fc776

File tree

15 files changed

+444
-104
lines changed

15 files changed

+444
-104
lines changed

‎Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ members = [
99
ijson = "0.1.3"
1010
serde_json = { version="1.0", features = ["unbounded_depth"]}
1111
serde = "1.0"
12+
bson = "0.14"
1213

1314
[workspace.package]
1415
edition = "2021"

‎json_path/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ license.workspace = true
88
[dependencies]
99
pest = "2.1"
1010
pest_derive = "2.1"
11+
bson.workspace = true
1112
serde_json.workspace = true
1213
serde.workspace = true
1314
ijson.workspace = true

‎json_path/json_examples/giveme_every_thing_result.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,3 @@
113113
"red",
114114
19.95
115115
]
116-

‎json_path/tests/array_filter.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
/*
2-
* Copyright Redis Ltd. 2016 - present
3-
* Licensed under your choice of the Redis Source Available License 2.0 (RSALv2) or
4-
* the Server Side Public License v1 (SSPLv1).
5-
*/
6-
1+
/*
2+
* Copyright Redis Ltd. 2016 - present
3+
* Licensed under your choice of the Redis Source Available License 2.0 (RSALv2) or
4+
* the Server Side Public License v1 (SSPLv1).
5+
*/
6+
77
#[macro_use]
88
extern crate serde_json;
99

@@ -150,7 +150,7 @@ fn array_range_only_from_index() {
150150
}
151151

152152
#[test]
153-
fn array_range_only_nagative_end_index() {
153+
fn array_range_only_negative_end_index() {
154154
setup();
155155

156156
select_and_then_compare(

‎redis_json/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ env_logger = "0.10.0"
2020

2121
[dependencies]
2222
bitflags = "1.3"
23-
bson = "0.14"
23+
bson.workspace = true
2424
ijson.workspace = true
2525
serde_json.workspace = true
2626
serde.workspace = true

‎redis_json/src/c_api.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use std::{
1313
os::raw::{c_char, c_void},
1414
};
1515

16+
use crate::formatter::FormatOptions;
1617
use crate::key_value::KeyValue;
1718
use json_path::select_value::{SelectValue, SelectValueType};
1819
use json_path::{compile, create};
@@ -132,7 +133,7 @@ pub fn json_api_get_json<M: Manager>(
132133
str: *mut *mut rawmod::RedisModuleString,
133134
) -> c_int {
134135
let json = unsafe { &*(json.cast::<M::V>()) };
135-
let res = KeyValue::<M::V>::serialize_object(json, None, None, None);
136+
let res = KeyValue::<M::V>::serialize_object(json, &FormatOptions::default());
136137
create_rmstring(ctx, &res, str)
137138
}
138139

@@ -146,7 +147,7 @@ pub fn json_api_get_json_from_iter<M: Manager>(
146147
if iter.pos >= iter.results.len() {
147148
Status::Err as c_int
148149
} else {
149-
let res = KeyValue::<M::V>::serialize_object(&iter.results, None, None, None);
150+
let res = KeyValue::<M::V>::serialize_object(&iter.results, &FormatOptions::default());
150151
create_rmstring(ctx, &res, str);
151152
Status::Ok as c_int
152153
}

‎redis_json/src/commands.rs

Lines changed: 68 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
use crate::error::Error;
8+
use crate::formatter::FormatOptions;
89
use crate::key_value::KeyValue;
910
use crate::manager::err_msg_json_path_doesnt_exist_with_param;
1011
use crate::manager::err_msg_json_path_doesnt_exist_with_param_or;
@@ -79,6 +80,11 @@ impl<'a, V: SelectValue> Serialize for Values<'a, V> {
7980
}
8081
}
8182

83+
fn is_resp3(ctx: &Context) -> bool {
84+
ctx.get_flags()
85+
.contains(redis_module::ContextFlags::FLAGS_RESP3)
86+
}
87+
8288
///
8389
/// JSON.GET <key>
8490
/// [INDENT indentation-string]
@@ -93,23 +99,31 @@ pub fn json_get<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>) -
9399

94100
// Set Capacity to 1 assuming the common case has one path
95101
let mut paths: Vec<Path> = Vec::with_capacity(1);
96-
let mut format = Format::JSON;
97-
let mut indent = None;
98-
let mut space = None;
99-
let mut newline = None;
102+
103+
let mut format_options = FormatOptions {
104+
resp3: is_resp3(ctx),
105+
..Default::default()
106+
};
107+
100108
while let Ok(arg) = args.next_str() {
101109
match arg {
102110
// fast way to consider arg a path by using the max length of all possible subcommands
103111
// See #390 for the comparison of this function with/without this optimization
104112
arg if arg.len() > JSONGET_SUBCOMMANDS_MAXSTRLEN => paths.push(Path::new(arg)),
105-
arg if arg.eq_ignore_ascii_case(CMD_ARG_INDENT) => indent = Some(args.next_str()?),
106-
arg if arg.eq_ignore_ascii_case(CMD_ARG_NEWLINE) => newline = Some(args.next_str()?),
107-
arg if arg.eq_ignore_ascii_case(CMD_ARG_SPACE) => space = Some(args.next_str()?),
108-
// Silently ignore. Compatibility with ReJSON v1.0 which has this option. See #168 TODO add support
109-
arg if arg.eq_ignore_ascii_case(CMD_ARG_NOESCAPE) => continue,
110113
arg if arg.eq_ignore_ascii_case(CMD_ARG_FORMAT) => {
111-
format = Format::from_str(args.next_str()?)?;
114+
format_options.format = Format::from_str(args.next_str()?)?;
112115
}
116+
arg if arg.eq_ignore_ascii_case(CMD_ARG_INDENT) => {
117+
format_options.indent = Some(args.next_str()?)
118+
}
119+
arg if arg.eq_ignore_ascii_case(CMD_ARG_NEWLINE) => {
120+
format_options.newline = Some(args.next_str()?)
121+
}
122+
arg if arg.eq_ignore_ascii_case(CMD_ARG_SPACE) => {
123+
format_options.space = Some(args.next_str()?)
124+
}
125+
// Silently ignore. Compatibility with ReJSON v1.0 which has this option. See #168 TODO add support
126+
arg if arg.eq_ignore_ascii_case(CMD_ARG_NOESCAPE) => continue,
113127
_ => paths.push(Path::new(arg)),
114128
};
115129
}
@@ -121,7 +135,7 @@ pub fn json_get<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>) -
121135

122136
let key = manager.open_key_read(ctx, &key)?;
123137
let value = match key.get_value()? {
124-
Some(doc) => KeyValue::new(doc).to_json(&mut paths, indent, newline, space, format)?,
138+
Some(doc) => KeyValue::new(doc).to_json(&mut paths, &format_options)?,
125139
None => RedisValue::Null,
126140
};
127141

@@ -576,10 +590,15 @@ pub fn json_mget<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>)
576590
let path = Path::new(path.try_as_str()?);
577591
let keys = &args[1..args.len() - 1];
578592

593+
let format_options = FormatOptions {
594+
resp3: is_resp3(ctx),
595+
..Default::default()
596+
};
597+
579598
let to_string =
580-
|doc: &M::V| KeyValue::new(doc).to_string_multi(path.get_path(), None, None, None);
599+
|doc: &M::V| KeyValue::new(doc).to_string_multi(path.get_path(), &format_options);
581600
let to_string_legacy =
582-
|doc: &M::V| KeyValue::new(doc).to_string_single(path.get_path(), None, None, None);
601+
|doc: &M::V| KeyValue::new(doc).to_string_single(path.get_path(), &format_options);
583602
let is_legacy = path.is_legacy();
584603

585604
let results: Result<Vec<RedisValue>, RedisError> = keys
@@ -618,10 +637,17 @@ pub fn json_type<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>)
618637

619638
let key = manager.open_key_read(ctx, &key)?;
620639

621-
if path.is_legacy() {
622-
json_type_legacy::<M>(&key, path.get_path())
640+
let value = if path.is_legacy() {
641+
json_type_legacy::<M>(&key, path.get_path())?
642+
} else {
643+
json_type_impl::<M>(&key, path.get_path())?
644+
};
645+
646+
// Check context flags to see if RESP3 is enabled and return the appropriate result
647+
if is_resp3(ctx) {
648+
Ok(vec![value].into())
623649
} else {
624-
json_type_impl::<M>(&key, path.get_path())
650+
Ok(value)
625651
}
626652
}
627653

@@ -630,7 +656,7 @@ where
630656
M: Manager,
631657
{
632658
let root = redis_key.get_value()?;
633-
let res = match root {
659+
let value = match root {
634660
Some(root) => KeyValue::new(root)
635661
.get_values(path)?
636662
.iter()
@@ -639,7 +665,7 @@ where
639665
.into(),
640666
None => RedisValue::Null,
641667
};
642-
Ok(res)
668+
Ok(value)
643669
}
644670

645671
fn json_type_legacy<M>(redis_key: &M::ReadHolder, path: &str) -> RedisResult
@@ -654,7 +680,6 @@ where
654680
.map_or(RedisValue::Null, |s| s.into())
655681
},
656682
);
657-
658683
Ok(value)
659684
}
660685

@@ -682,10 +707,30 @@ where
682707

683708
let mut redis_key = manager.open_key_write(ctx, key)?;
684709

685-
if path.is_legacy() {
710+
// check context flags to see if RESP3 is enabled
711+
if is_resp3(ctx) {
712+
let res = json_num_op_impl::<M>(&mut redis_key, ctx, path.get_path(), number, op, cmd)?
713+
.drain(..)
714+
.map(|v| {
715+
v.map_or(RedisValue::Null, |v| {
716+
if let Some(i) = v.as_i64() {
717+
RedisValue::Integer(i)
718+
} else {
719+
RedisValue::Float(v.as_f64().unwrap_or_default())
720+
}
721+
})
722+
})
723+
.collect::<Vec<RedisValue>>()
724+
.into();
725+
Ok(res)
726+
} else if path.is_legacy() {
686727
json_num_op_legacy::<M>(&mut redis_key, ctx, path.get_path(), number, op, cmd)
687728
} else {
688-
json_num_op_impl::<M>(&mut redis_key, ctx, path.get_path(), number, op, cmd)
729+
let results = json_num_op_impl::<M>(&mut redis_key, ctx, path.get_path(), number, op, cmd)?;
730+
731+
// Convert to RESP2 format return as one JSON array
732+
let values = to_json_value::<Number>(results, Value::Null);
733+
Ok(KeyValue::<M::V>::serialize_object(&values, &FormatOptions::default()).into())
689734
}
690735
}
691736

@@ -696,7 +741,7 @@ fn json_num_op_impl<M>(
696741
number: &str,
697742
op: NumOp,
698743
cmd: &str,
699-
) -> RedisResult
744+
) -> Result<Vec<Option<Number>>, RedisError>
700745
where
701746
M: Manager,
702747
{
@@ -728,9 +773,7 @@ where
728773
if need_notify {
729774
redis_key.apply_changes(ctx, cmd)?;
730775
}
731-
732-
let res = to_json_value::<Number>(res, Value::Null);
733-
Ok(KeyValue::<M::V>::serialize_object(&res, None, None, None).into())
776+
Ok(res)
734777
}
735778

736779
fn json_num_op_legacy<M>(

‎redis_json/src/formatter.rs

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,38 @@ DEALINGS IN THE SOFTWARE.
3535
use serde_json::ser::Formatter;
3636
use std::io;
3737

38+
use crate::redisjson::Format;
39+
40+
pub struct FormatOptions<'a> {
41+
pub format: Format,
42+
pub indent: Option<&'a str>,
43+
pub space: Option<&'a str>,
44+
pub newline: Option<&'a str>,
45+
pub resp3: bool,
46+
}
47+
48+
impl PartialEq for &FormatOptions<'_> {
49+
fn eq(&self, other: &Self) -> bool {
50+
self.format == other.format
51+
&& self.indent == other.indent
52+
&& self.space == other.space
53+
&& self.newline == other.newline
54+
&& self.resp3 == other.resp3
55+
}
56+
}
57+
58+
impl Default for FormatOptions<'_> {
59+
fn default() -> Self {
60+
Self {
61+
format: Format::JSON,
62+
indent: None,
63+
space: None,
64+
newline: None,
65+
resp3: false,
66+
}
67+
}
68+
}
69+
3870
pub struct RedisJsonFormatter<'a> {
3971
current_indent: usize,
4072
has_value: bool,
@@ -44,17 +76,13 @@ pub struct RedisJsonFormatter<'a> {
4476
}
4577

4678
impl<'a> RedisJsonFormatter<'a> {
47-
pub const fn new(
48-
indent: Option<&'a str>,
49-
space: Option<&'a str>,
50-
newline: Option<&'a str>,
51-
) -> Self {
79+
pub const fn new(format: &'a FormatOptions) -> Self {
5280
RedisJsonFormatter {
5381
current_indent: 0,
5482
has_value: false,
55-
indent,
56-
space,
57-
newline,
83+
indent: format.indent,
84+
space: format.space,
85+
newline: format.newline,
5886
}
5987
}
6088

@@ -177,7 +205,13 @@ mod tests {
177205
#[test]
178206
#[allow(clippy::cognitive_complexity)]
179207
fn test_default_formatter() {
180-
let mut formatter = RedisJsonFormatter::new(None, None, None);
208+
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
209+
format: Format::JSON,
210+
indent: None,
211+
space: None,
212+
newline: None,
213+
resp3: false,
214+
});
181215
let mut writer = vec![];
182216

183217
assert!(matches!(formatter.begin_array(&mut writer), Ok(())));
@@ -235,7 +269,13 @@ mod tests {
235269
#[test]
236270
#[allow(clippy::cognitive_complexity)]
237271
fn test_ident_formatter() {
238-
let mut formatter = RedisJsonFormatter::new(Some("_"), None, None);
272+
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
273+
format: Format::JSON,
274+
indent: Some("_"),
275+
space: None,
276+
newline: None,
277+
resp3: false,
278+
});
239279
let mut writer = vec![];
240280

241281
assert!(matches!(formatter.begin_array(&mut writer), Ok(())));
@@ -293,7 +333,13 @@ mod tests {
293333
#[test]
294334
#[allow(clippy::cognitive_complexity)]
295335
fn test_space_formatter() {
296-
let mut formatter = RedisJsonFormatter::new(None, Some("s"), None);
336+
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
337+
format: Format::JSON,
338+
indent: None,
339+
space: Some("s"),
340+
newline: None,
341+
resp3: false,
342+
});
297343
let mut writer = vec![];
298344

299345
assert!(matches!(formatter.begin_array(&mut writer), Ok(())));
@@ -351,7 +397,13 @@ mod tests {
351397
#[test]
352398
#[allow(clippy::cognitive_complexity)]
353399
fn test_new_line_formatter() {
354-
let mut formatter = RedisJsonFormatter::new(None, None, Some("n"));
400+
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
401+
format: Format::JSON,
402+
indent: None,
403+
space: None,
404+
newline: Some("n"),
405+
resp3: false,
406+
});
355407
let mut writer = vec![];
356408

357409
assert!(matches!(formatter.begin_array(&mut writer), Ok(())));

0 commit comments

Comments
 (0)