Skip to content

Commit fdacfcf

Browse files
gkorlandgkorland
authored
Set FORMAT EXPAND as default (RedisJSON#1021)
* add FORMAT LEGACY as default * rename LEGACY as STRING * set default format as EXPAND
1 parent 4580fb6 commit fdacfcf

File tree

5 files changed

+58
-13
lines changed

5 files changed

+58
-13
lines changed

‎redis_json/src/formatter.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ pub struct FormatOptions<'a> {
4545
pub resp3: bool,
4646
}
4747

48+
impl FormatOptions<'_> {
49+
/// Returns true if the format is RESP3 and the format is not STRING format
50+
/// STRING format is fully backward compatible with RESP2
51+
pub fn is_resp3_reply(&self) -> bool {
52+
self.resp3 && self.format != Format::STRING
53+
}
54+
}
55+
4856
impl PartialEq for &FormatOptions<'_> {
4957
fn eq(&self, other: &Self) -> bool {
5058
self.format == other.format
@@ -58,7 +66,7 @@ impl PartialEq for &FormatOptions<'_> {
5866
impl Default for FormatOptions<'_> {
5967
fn default() -> Self {
6068
Self {
61-
format: Format::JSON,
69+
format: Format::EXPAND,
6270
indent: None,
6371
space: None,
6472
newline: None,

‎redis_json/src/ivalue_manager.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ impl<'a> IValueKeyHolderWrite<'a> {
279279

280280
fn serialize(results: &IValue, format: Format) -> Result<String, Error> {
281281
let res = match format {
282-
Format::JSON => serde_json::to_string(results)?,
282+
Format::JSON | Format::STRING => serde_json::to_string(results)?,
283283
Format::BSON => return Err("ERR Soon to come...".into()), //results.into() as Bson,
284284
Format::EXPAND => return Err("ERR Unknown format specified".into()),
285285
};
@@ -608,7 +608,7 @@ impl<'a> Manager for RedisIValueJsonKeyManager<'a> {
608608

609609
fn from_str(&self, val: &str, format: Format, limit_depth: bool) -> Result<Self::O, Error> {
610610
match format {
611-
Format::JSON => {
611+
Format::JSON | Format::STRING => {
612612
let mut deserializer = serde_json::Deserializer::from_str(val);
613613
if !limit_depth {
614614
deserializer.disable_recursion_limit();

‎redis_json/src/key_value.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ impl<'a, V: SelectValue + 'a> KeyValue<'a, V> {
155155
if let Some(p) = missing_path {
156156
return Err(err_msg_json_path_doesnt_exist_with_param(p.as_str()).into());
157157
}
158-
let res = if format.resp3 {
158+
159+
// If we're using RESP3, we need to convert the HashMap to a RedisValue::Map unless we're using the legacy format
160+
let res = if format.is_resp3_reply() {
159161
let map = temp_doc
160162
.iter()
161163
.map(|(k, v)| {
@@ -197,7 +199,7 @@ impl<'a, V: SelectValue + 'a> KeyValue<'a, V> {
197199
) -> Result<RedisValue, Error> {
198200
let res = if is_legacy {
199201
self.to_string_single(path, format)?.into()
200-
} else if format.resp3 {
202+
} else if format.is_resp3_reply() {
201203
let values = self.get_values(path)?;
202204
Self::values_to_resp3(&values, format)
203205
} else {
@@ -262,7 +264,9 @@ impl<'a, V: SelectValue + 'a> KeyValue<'a, V> {
262264
return Err("ERR Soon to come...".into());
263265
}
264266
let is_legacy = !paths.iter().any(|p| !p.is_legacy());
265-
if format.resp3 {
267+
268+
// If we're using RESP3, we need to reply with an array of values
269+
if format.is_resp3_reply() {
266270
self.to_resp3(paths, format)
267271
} else if paths.len() > 1 {
268272
self.to_json_multi(paths, format, is_legacy)

‎redis_json/src/redisjson.rs

+2
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ pub enum SetOptions {
5858

5959
#[derive(Debug, PartialEq, Eq)]
6060
pub enum Format {
61+
STRING,
6162
JSON,
6263
BSON,
6364
EXPAND,
@@ -67,6 +68,7 @@ impl FromStr for Format {
6768

6869
fn from_str(s: &str) -> Result<Self, Self::Err> {
6970
match s {
71+
"STRING" => Ok(Self::STRING),
7072
"JSON" => Ok(Self::JSON),
7173
"BSON" => Ok(Self::BSON),
7274
"EXPAND" => Ok(Self::EXPAND),

‎tests/pytest/test_resp3.py

+38-7
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class testResp3():
1919
def __init__(self):
2020
self.env = Env(protocol=3)
2121

22-
def test_resp3_set_get(self):
22+
def test_resp3_set_get_json_format(self):
2323
r = self.env
2424
r.skipOnVersionSmaller('7.0')
2525

@@ -29,19 +29,19 @@ def test_resp3_set_get(self):
2929
r.assertOk(r.execute_command('JSON.SET', 'test_resp3', '$', '{"a1":{"b":{"c":true,"d":null}},"a2":{"b":{"c":2}}}'))
3030

3131
# Test JSON.GET RESP3
32-
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', '$'), [['{"a1":{"b":{"c":true,"d":null}},"a2":{"b":{"c":2}}}']])
33-
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', '$..b'), [['{"c":true,"d":null}', '{"c":2}']])
34-
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', '$.a1', '$.a2'), [['{"b":{"c":true,"d":null}}'], ['{"b":{"c":2}}']])
35-
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', '$.a1', '$.a3', '$.a2'), [['{"b":{"c":true,"d":null}}'], [], ['{"b":{"c":2}}']])
36-
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', '$.a3'), [[]])
32+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'JSON', '$'), [['{"a1":{"b":{"c":true,"d":null}},"a2":{"b":{"c":2}}}']])
33+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'JSON', '$..b'), [['{"c":true,"d":null}', '{"c":2}']])
34+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'JSON', '$.a1', '$.a2'), [['{"b":{"c":true,"d":null}}'], ['{"b":{"c":2}}']])
35+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'JSON', '$.a1', '$.a3', '$.a2'), [['{"b":{"c":true,"d":null}}'], [], ['{"b":{"c":2}}']])
36+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'JSON', '$.a3'), [[]])
3737

3838
# TEST JSON.GET with none existent key
3939
r.assertEqual(r.execute_command('JSON.GET', 'test_no_such_key', '$.a3'), None)
4040

4141
# TEST JSON.GET with not a JSON key
4242
r.expect('JSON.GET', 'test_not_JSON', '$.a3').raiseError()
4343

44-
def test_resp3_set_get_expand(self):
44+
def test_resp3_set_get_expand_format(self):
4545
r = self.env
4646
r.skipOnVersionSmaller('7.0')
4747

@@ -57,13 +57,44 @@ def test_resp3_set_get_expand(self):
5757
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'EXPAND','$.a1', '$.a3', '$.a2'), [[{'b': {'c': True, 'd': None}}], [], [{'b': {'c': 2, 'e': [1, True, {'f': None}]}}]])
5858
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'EXPAND','$.a3'), [[]])
5959

60+
# Test JSON.GET RESP3 with default format (EXPAND)
61+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', '$'), [[{'a1': {'b': {'c': True, 'd': None}}, 'a2': {'b': {'e': [1, True, {'f': None}], 'c': 2}}}]])
62+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3','$..b'), [[{'d': None, 'c': True}, {'c': 2, 'e': [1, True, {'f': None}]}]])
63+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3','$.a1', '$.a2'), [[{'b': {'d': None, 'c': True}}], [{'b': {'e': [1, True, {'f': None}], 'c': 2}}]])
64+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3','$.a1', '$.a3', '$.a2'), [[{'b': {'c': True, 'd': None}}], [], [{'b': {'c': 2, 'e': [1, True, {'f': None}]}}]])
65+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3','$.a3'), [[]])
66+
6067
# TEST JSON.GET with none existent key
6168
r.assertEqual(r.execute_command('JSON.GET', 'test_no_such_key', 'FORMAT', 'EXPAND','$.a3'), None)
6269

6370
# TEST JSON.GET with not a JSON key
6471
r.expect('JSON.GET', 'test_not_JSON', 'FORMAT', 'EXPAND','$.a3').raiseError()
6572

6673

74+
def test_resp3_set_get_string_format(self):
75+
r = self.env
76+
r.skipOnVersionSmaller('7.0')
77+
78+
r.assertTrue(r.execute_command('SET', 'test_not_JSON', 'test_not_JSON'))
79+
80+
# Test JSON.SET RESP3
81+
r.assertOk(r.execute_command('JSON.SET', 'test_resp3', '$', '{"a1":{"b":{"c":true,"d":null}},"a2":{"b":{"c":2}}}'))
82+
83+
# Test JSON.GET RESP3
84+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'STRING', '$'), '[{"a1":{"b":{"c":true,"d":null}},"a2":{"b":{"c":2}}}]')
85+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'STRING', '$..b'), '[{"c":true,"d":null},{"c":2}]')
86+
r.assertEqual(json.loads(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'STRING', '$.a1', '$.a2')), {"$.a2":[{"b":{"c":2}}],"$.a1":[{"b":{"c":True,"d":None}}]})
87+
r.assertEqual(json.loads(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'STRING', '.a1', '$.a2')), {"$.a2":[{"b":{"c":2}}],".a1":[{"b":{"c":True,"d":None}}]})
88+
r.assertEqual(json.loads(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'STRING', '$.a1', '$.a3', '$.a2')), {"$.a3":[],"$.a2":[{"b":{"c":2}}],"$.a1":[{"b":{"c":True,"d":None}}]})
89+
r.assertEqual(r.execute_command('JSON.GET', 'test_resp3', 'FORMAT', 'STRING', '$.a3'), '[]')
90+
91+
# TEST JSON.GET with none existent key
92+
r.assertEqual(r.execute_command('JSON.GET', 'test_no_such_key', '$.a3'), None)
93+
94+
# TEST JSON.GET with not a JSON key
95+
r.expect('JSON.GET', 'test_not_JSON', '$.a3').raiseError()
96+
97+
6798
# Test JSON.DEL RESP3
6899
def test_resp_json_del(self):
69100
r = self.env

0 commit comments

Comments
 (0)