Skip to content

Commit 351e834

Browse files
oshadmioshadmi
authored
Merge pull request RedisJSON#1062 from RedisJSON/oshadmi_cp-from-master-for-2.6.4
[2.6] Cherry-pick from master towards 2.6.4
2 parents d8c8d27 + 40bd45f commit 351e834

File tree

11 files changed

+330
-239
lines changed

11 files changed

+330
-239
lines changed

‎Cargo.lock

Lines changed: 133 additions & 128 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎build/docker/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ REDIS_VERSION=7.2-rc2
1010
OSNICK.official=bullseye
1111

1212
INT_BRANCHES=2.6 2.4 2.2 2.0 1.2 1.0
13-
LATEST_BRANCH=2.4
13+
LATEST_BRANCH=2.6
1414
PREVIEW_BRANCH=2.6
1515

1616
ART_DIR=$(ROOT)/bin/artifacts

‎redis_json/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "redis_json"
3-
version = "2.6.99"
3+
version = "2.6.4"
44
authors = ["Guy Korland <guy.korland@redis.com>", "Meir Shpilraien <meir@redis.com>", "Omer Shadmi <omer.shadmi@redis.com>"]
55
edition.workspace = true
66
description = "JSON data type for Redis"

‎redis_json/src/c_api.rs

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

16-
use crate::formatter::FormatOptions;
16+
use crate::formatter::ReplyFormatOptions;
1717
use crate::key_value::KeyValue;
1818
use json_path::select_value::{SelectValue, SelectValueType};
1919
use json_path::{compile, create};
@@ -133,7 +133,7 @@ pub fn json_api_get_json<M: Manager>(
133133
str: *mut *mut rawmod::RedisModuleString,
134134
) -> c_int {
135135
let json = unsafe { &*(json.cast::<M::V>()) };
136-
let res = KeyValue::<M::V>::serialize_object(json, &FormatOptions::default());
136+
let res = KeyValue::<M::V>::serialize_object(json, &ReplyFormatOptions::default());
137137
create_rmstring(ctx, &res, str)
138138
}
139139

@@ -147,7 +147,7 @@ pub fn json_api_get_json_from_iter<M: Manager>(
147147
if iter.pos >= iter.results.len() {
148148
Status::Err as c_int
149149
} else {
150-
let res = KeyValue::<M::V>::serialize_object(&iter.results, &FormatOptions::default());
150+
let res = KeyValue::<M::V>::serialize_object(&iter.results, &ReplyFormatOptions::default());
151151
create_rmstring(ctx, &res, str);
152152
Status::Ok as c_int
153153
}

‎redis_json/src/commands.rs

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@
55
*/
66

77
use crate::error::Error;
8-
use crate::formatter::FormatOptions;
8+
use crate::formatter::ReplyFormatOptions;
99
use crate::key_value::KeyValue;
1010
use crate::manager::err_msg_json_path_doesnt_exist_with_param;
1111
use crate::manager::err_msg_json_path_doesnt_exist_with_param_or;
1212
use crate::manager::{Manager, ReadHolder, UpdateInfo, WriteHolder};
13-
use crate::redisjson::{Format, Path};
13+
use crate::redisjson::{Format, Path, ReplyFormat};
1414
use json_path::select_value::{SelectValue, SelectValueType};
1515
use redis_module::{Context, RedisValue};
1616
use redis_module::{NextArg, RedisError, RedisResult, RedisString, REDIS_OK};
@@ -99,25 +99,35 @@ fn default_path(ctx: &Context) -> &str {
9999
/// [INDENT indentation-string]
100100
/// [NEWLINE line-break-string]
101101
/// [SPACE space-string]
102+
/// [FORMAT {STRING|EXPAND1|EXPAND}] /* default is STRING */
102103
/// [path ...]
103104
///
104-
/// TODO add support for multi path
105105
pub fn json_get<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>) -> RedisResult {
106106
let mut args = args.into_iter().skip(1);
107107
let key = args.next_arg()?;
108108

109109
// Set Capacity to 1 assuming the common case has one path
110110
let mut paths: Vec<Path> = Vec::with_capacity(1);
111111

112-
let mut format_options = FormatOptions::new(is_resp3(ctx));
112+
let mut format_options = ReplyFormatOptions::new(is_resp3(ctx), ReplyFormat::STRING);
113113

114114
while let Ok(arg) = args.next_str() {
115115
match arg {
116116
// fast way to consider arg a path by using the max length of all possible subcommands
117117
// See #390 for the comparison of this function with/without this optimization
118118
arg if arg.len() > JSONGET_SUBCOMMANDS_MAXSTRLEN => paths.push(Path::new(arg)),
119119
arg if arg.eq_ignore_ascii_case(CMD_ARG_FORMAT) => {
120-
format_options.format = Format::from_str(args.next_str()?)?;
120+
if !format_options.resp3 && paths.is_empty() {
121+
return Err(RedisError::Str(
122+
"ERR FORMAT argument is not supported on RESP2",
123+
));
124+
}
125+
// Temporary fix until STRINGS is also supported
126+
let next = args.next_str()?;
127+
if next.eq_ignore_ascii_case("STRINGS") {
128+
return Err(RedisError::Str("ERR wrong reply format"));
129+
}
130+
format_options.format = ReplyFormat::from_str(next)?;
121131
}
122132
arg if arg.eq_ignore_ascii_case(CMD_ARG_INDENT) => {
123133
format_options.indent = Some(args.next_str()?)
@@ -596,7 +606,7 @@ pub fn json_mget<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>)
596606
let path = Path::new(path.try_as_str()?);
597607
let keys = &args[1..args.len() - 1];
598608

599-
let format_options = FormatOptions::new(is_resp3(ctx));
609+
let format_options = ReplyFormatOptions::new(is_resp3(ctx), ReplyFormat::STRING);
600610

601611
// Verify that at least one key exists
602612
if keys.is_empty() {
@@ -736,7 +746,7 @@ where
736746

737747
// Convert to RESP2 format return as one JSON array
738748
let values = to_json_value::<Number>(results, Value::Null);
739-
Ok(KeyValue::<M::V>::serialize_object(&values, &FormatOptions::default()).into())
749+
Ok(KeyValue::<M::V>::serialize_object(&values, &ReplyFormatOptions::default()).into())
740750
}
741751
}
742752

@@ -1368,7 +1378,7 @@ pub fn json_arr_len<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString
13681378

13691379
///
13701380
/// JSON.ARRPOP <key>
1371-
/// [FORMAT {STRING|JSON|EXPAND}]
1381+
/// [FORMAT {STRINGS|EXPAND1|EXPAND}] /* default is STRINGS */
13721382
/// [path [index]]
13731383
///
13741384
pub fn json_arr_pop<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString>) -> RedisResult {
@@ -1377,14 +1387,26 @@ pub fn json_arr_pop<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString
13771387
let key = args.next_arg()?;
13781388

13791389
let is_resp3 = is_resp3(ctx);
1380-
let mut format_options = FormatOptions::new(is_resp3);
1390+
let mut format_options = ReplyFormatOptions::new(is_resp3, ReplyFormat::STRINGS);
13811391

1382-
// Only on RESP3 if the first argument is FORMAT, then the second argument is the format
1383-
// but it's optional so it might be the path
13841392
let path = if let Some(arg) = args.next() {
1385-
if is_resp3 && arg.try_as_str()?.eq_ignore_ascii_case("FORMAT") {
1386-
format_options.format = Format::from_str(args.next_str()?)?;
1387-
args.next()
1393+
if arg.try_as_str()?.eq_ignore_ascii_case(CMD_ARG_FORMAT) {
1394+
if let Ok(next) = args.next_str() {
1395+
format_options.format = ReplyFormat::from_str(next)?;
1396+
if format_options.format == ReplyFormat::STRING {
1397+
// ARRPOP FORMAT STRING is not supported
1398+
return Err(RedisError::Str("ERR wrong reply format"));
1399+
}
1400+
if !format_options.resp3 {
1401+
return Err(RedisError::Str(
1402+
"ERR FORMAT argument is not supported on RESP2",
1403+
));
1404+
}
1405+
args.next()
1406+
} else {
1407+
// If only the FORMAT subcommand is provided, then it's the path
1408+
Some(arg)
1409+
}
13881410
} else {
13891411
// if it's not FORMAT, then it's the path
13901412
Some(arg)
@@ -1409,12 +1431,13 @@ pub fn json_arr_pop<M: Manager>(manager: M, ctx: &Context, args: Vec<RedisString
14091431
(path, index)
14101432
}
14111433
};
1434+
//args.done()?;
14121435

14131436
let mut redis_key = manager.open_key_write(ctx, key)?;
14141437
if path.is_legacy() {
1415-
if format_options.format != Format::STRING {
1438+
if format_options.format != ReplyFormat::STRINGS {
14161439
return Err(RedisError::Str(
1417-
"Legacy paths are supported only with FORMAT STRING",
1440+
"Legacy paths are supported only with FORMAT STRINGS",
14181441
));
14191442
}
14201443

@@ -1429,7 +1452,7 @@ fn json_arr_pop_impl<M>(
14291452
ctx: &Context,
14301453
path: &str,
14311454
index: i64,
1432-
format_options: &FormatOptions,
1455+
format_options: &ReplyFormatOptions,
14331456
) -> RedisResult
14341457
where
14351458
M: Manager,

‎redis_json/src/formatter.rs

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,36 +35,32 @@ DEALINGS IN THE SOFTWARE.
3535
use serde_json::ser::Formatter;
3636
use std::io;
3737

38-
use crate::redisjson::Format;
38+
pub use crate::redisjson::{Format, ReplyFormat};
3939

40-
pub struct FormatOptions<'a> {
41-
pub format: Format,
40+
pub struct ReplyFormatOptions<'a> {
41+
pub format: ReplyFormat,
4242
pub indent: Option<&'a str>,
4343
pub space: Option<&'a str>,
4444
pub newline: Option<&'a str>,
4545
pub resp3: bool,
4646
}
4747

48-
impl FormatOptions<'_> {
49-
/// Creates a new FormatOptions with the default values according to the RESP version
50-
pub fn new(resp3: bool) -> Self {
48+
impl ReplyFormatOptions<'_> {
49+
/// Creates a new FormatOptions
50+
pub fn new(resp3: bool, format: ReplyFormat) -> Self {
5151
Self {
52-
format: if resp3 {
53-
Format::EXPAND
54-
} else {
55-
Format::STRING
56-
},
52+
format,
5753
indent: None,
5854
space: None,
5955
newline: None,
6056
resp3,
6157
}
6258
}
6359

64-
/// Returns true if the format is RESP3 and the format is not STRING format
65-
/// STRING format is fully backward compatible with RESP2
60+
/// Returns true if the format is RESP3 and the format is not STRING/STRINGS format
61+
/// STRING/STRINGS format (depending on the command) is fully backward compatible with RESP2
6662
pub fn is_resp3_reply(&self) -> bool {
67-
self.resp3 && self.format != Format::STRING
63+
self.resp3 && self.format != ReplyFormat::STRING && self.format != ReplyFormat::STRINGS
6864
}
6965

7066
/// Checks if the JSON formatting options are the default ones with no overrides
@@ -73,11 +69,11 @@ impl FormatOptions<'_> {
7369
}
7470
}
7571

76-
impl Default for FormatOptions<'_> {
72+
impl Default for ReplyFormatOptions<'_> {
7773
/// Creates a new FormatOptions with the default values matching RESP2
7874
fn default() -> Self {
7975
Self {
80-
format: Format::STRING,
76+
format: ReplyFormat::STRING,
8177
indent: None,
8278
space: None,
8379
newline: None,
@@ -95,7 +91,7 @@ pub struct RedisJsonFormatter<'a> {
9591
}
9692

9793
impl<'a> RedisJsonFormatter<'a> {
98-
pub const fn new(format: &'a FormatOptions) -> Self {
94+
pub const fn new(format: &'a ReplyFormatOptions) -> Self {
9995
RedisJsonFormatter {
10096
current_indent: 0,
10197
has_value: false,
@@ -224,8 +220,8 @@ mod tests {
224220
#[test]
225221
#[allow(clippy::cognitive_complexity)]
226222
fn test_default_formatter() {
227-
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
228-
format: Format::JSON,
223+
let mut formatter = RedisJsonFormatter::new(&ReplyFormatOptions {
224+
format: ReplyFormat::STRING,
229225
indent: None,
230226
space: None,
231227
newline: None,
@@ -288,8 +284,8 @@ mod tests {
288284
#[test]
289285
#[allow(clippy::cognitive_complexity)]
290286
fn test_ident_formatter() {
291-
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
292-
format: Format::JSON,
287+
let mut formatter = RedisJsonFormatter::new(&ReplyFormatOptions {
288+
format: ReplyFormat::STRING,
293289
indent: Some("_"),
294290
space: None,
295291
newline: None,
@@ -352,8 +348,8 @@ mod tests {
352348
#[test]
353349
#[allow(clippy::cognitive_complexity)]
354350
fn test_space_formatter() {
355-
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
356-
format: Format::JSON,
351+
let mut formatter = RedisJsonFormatter::new(&ReplyFormatOptions {
352+
format: ReplyFormat::STRING,
357353
indent: None,
358354
space: Some("s"),
359355
newline: None,
@@ -416,8 +412,8 @@ mod tests {
416412
#[test]
417413
#[allow(clippy::cognitive_complexity)]
418414
fn test_new_line_formatter() {
419-
let mut formatter = RedisJsonFormatter::new(&FormatOptions {
420-
format: Format::JSON,
415+
let mut formatter = RedisJsonFormatter::new(&ReplyFormatOptions {
416+
format: ReplyFormat::STRING,
421417
indent: None,
422418
space: None,
423419
newline: Some("n"),

‎redis_json/src/ivalue_manager.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,6 @@ impl<'a> Manager for RedisIValueJsonKeyManager<'a> {
632632
Ok(v)
633633
},
634634
),
635-
Format::EXPAND => Err("Unsupported format".into()),
636635
}
637636
}
638637

0 commit comments

Comments
 (0)